On Tue, Apr 29, 2025 at 12:14:16 +0100, Daniel P. Berrangé via Devel wrote: > From: Daniel P. Berrangé <berra...@redhat.com> > > Allow virCommand to find 'tc' in $PATH. This command is only used > when running privileged in which case both 'bin' and 'sbin' dirs will > be in $PATH, so virFindFileInPath will do the right thing to find it. > > Signed-off-by: Daniel P. Berrangé <berra...@redhat.com> > --- > meson.build | 10 ++---- > src/util/virfirewall.h | 1 + > src/util/virnetdevbandwidth.c | 36 ++++++++++----------- > tests/virnetdevbandwidthtest.c | 58 +++++++++++++++++----------------- > 4 files changed, 50 insertions(+), 55 deletions(-) > > diff --git a/meson.build b/meson.build > index 0515820cdd..4223f5ab86 100644 > --- a/meson.build > +++ b/meson.build > @@ -845,7 +845,7 @@ endforeach > > # optional programs > > -optional_test_programs = [ > +optional_programs = [ > 'augparse', > 'black', > 'flake8', > @@ -853,10 +853,6 @@ optional_test_programs = [ > 'pytest', > ] > > -optional_programs = [ > - 'tc', > -] + optional_test_programs > - > missing_optional_programs = [] > foreach name : optional_programs > prog = find_program(name, required: false, dirs: libvirt_sbin_path) > @@ -865,9 +861,7 @@ foreach name : optional_programs > prog_path = prog.full_path() > else > prog_path = name > - if name in optional_test_programs > - missing_optional_programs += [ name ] > - endif > + missing_optional_programs += [ name ] > endif
I had to dig a bit what happened here. Please mention in the commit message that as there are no optional programs for runtime we're left with optional programs for testing (which are also moaned about in the configure output). > > conf.set_quoted(varname.to_upper(), prog_path) > diff --git a/src/util/virfirewall.h b/src/util/virfirewall.h > index d42e60884b..07391bea67 100644 > --- a/src/util/virfirewall.h > +++ b/src/util/virfirewall.h > @@ -30,6 +30,7 @@ > #define IPTABLES "iptables" > #define IP6TABLES "ip6tables" > #define NFT "nft" > +#define TC "tc" There are just two places referencing the TC macro after this. Do we still need it? src/util/virfirewall.c=VIR_ENUM_IMPL(virFirewallLayerCommand, src/util/virfirewall.c: TC, src/util/virfirewall.c=virFirewallCmdNftablesApply(virFirewall *firewall G_GNUC_UNUSED, src/util/virfirewall.c: cmd = virCommandNew(TC); The rest looks good. If you justify/fix the thing above and mention the build system change in the commit message: Reviewed-by: Peter Krempa <pkre...@redhat.com>
