[libvirt PATCH v2 00/21] cleanup meson checks for runtime binaries
Recent attempt to add a lot of meson options to specify different runtime paths motivated me enough to cleanup this from meson. Changes in v2: - split and rework patch 16/17 to address review comments - added a new patch to cleanup libvirt.spec.in file Pavel Hrdina (21): bridge_driver: fix comment about dnsmasqCaps virdnsmasq: drop unused dnsmasqCapsNewFromFile function virdnsmasq: drop unused dnsmasqCapsRefresh function virdnsmasq: remove binaryPath argument from dnsmasqCapsNewFromBuffer virdnsmasq: remove binaryPath argument from dnsmasqCapsNewFromBinary virfirewall: use virFindFileInPath instead of virFileIsExecutable tests: introduce virfirewallmock tests: use virfirewallmock instead of hasNetfilterTools virnetdev: move virNetDevSetRootQDisc to virnetdevbandwidth tests: testutilsqemu: move virFindFileInPath into domaincapsmock meson: don't check collie as program for sheepdog bhyvexml2argvtest: use virCommandToStringFull to strip command path storage: use virFindFileInPath to validate presence of mkfs virfile: introduce virFindFileInPathFull() qemu_conf: use virFindFileInPathFull for runtime binaries meson: drop check for runtime binary dependencies meson: move iscsiadm check into storage_iscsi condition meson: stop setting runtime binaries defines during compilation meson: use runtime binaries to only resolve features with "auto" value meson: optional_programs should be used only for building libvirt libvirt.spec: drop no longer required build dependencies libvirt.spec.in | 31 --- meson.build | 214 ++++++------------ src/bhyve/bhyve_command.c | 4 + src/libvirt_private.syms | 6 +- src/locking/lock_driver_lockd.c | 12 +- src/network/bridge_driver.c | 8 +- src/node_device/node_device_driver.c | 2 + src/qemu/qemu_conf.c | 23 +- src/qemu/qemu_domain.c | 3 +- src/storage/storage_backend_fs.c | 24 +- src/storage/storage_backend_logical.c | 13 ++ src/storage/storage_backend_sheepdog.c | 2 + src/storage/storage_backend_zfs.c | 3 + src/storage/storage_util.c | 2 + src/storage/storage_util.h | 6 + src/util/virdnsmasq.c | 56 +---- src/util/virdnsmasq.h | 8 +- src/util/virfile.c | 16 +- src/util/virfile.h | 6 +- src/util/virfirewall.c | 4 +- src/util/virfirewall.h | 4 + src/util/viriscsi.h | 2 + src/util/virkmod.h | 3 + src/util/virnetdev.c | 46 ---- src/util/virnetdev.h | 4 - src/util/virnetdevbandwidth.c | 50 ++++ src/util/virnetdevbandwidth.h | 6 + src/util/virnetdevip.c | 2 + src/util/virnetdevmidonet.c | 2 + src/util/virnetdevopenvswitch.c | 2 + src/util/virnuma.c | 1 + src/util/virsysinfo.c | 1 + src/util/virutil.c | 2 + .../bhyvexml2argv-acpiapic.args | 2 +- .../bhyvexml2argv-acpiapic.ldargs | 2 +- ...ml2argv-addr-isa-controller-on-slot-1.args | 2 +- ...2argv-addr-isa-controller-on-slot-1.ldargs | 2 +- ...l2argv-addr-isa-controller-on-slot-31.args | 2 +- ...argv-addr-isa-controller-on-slot-31.ldargs | 2 +- ...xml2argv-addr-more-than-32-sata-disks.args | 2 +- ...l2argv-addr-more-than-32-sata-disks.ldargs | 2 +- ...hyvexml2argv-addr-multiple-sata-disks.args | 2 +- ...vexml2argv-addr-multiple-sata-disks.ldargs | 2 +- ...vexml2argv-addr-multiple-virtio-disks.args | 2 +- ...xml2argv-addr-multiple-virtio-disks.ldargs | 2 +- ...rgv-addr-no32devs-multiple-sata-disks.args | 2 +- ...v-addr-no32devs-multiple-sata-disks.ldargs | 2 +- ...l2argv-addr-no32devs-single-sata-disk.args | 2 +- ...argv-addr-no32devs-single-sata-disk.ldargs | 2 +- ...rgv-addr-non-isa-controller-on-slot-1.args | 2 +- .../bhyvexml2argv-addr-single-sata-disk.args | 2 +- ...bhyvexml2argv-addr-single-sata-disk.ldargs | 2 +- ...bhyvexml2argv-addr-single-virtio-disk.args | 2 +- ...yvexml2argv-addr-single-virtio-disk.ldargs | 2 +- .../bhyvexml2argvdata/bhyvexml2argv-base.args | 2 +- .../bhyvexml2argv-base.ldargs | 2 +- .../bhyvexml2argv-bhyveload-bootorder.args | 2 +- .../bhyvexml2argv-bhyveload-bootorder.ldargs | 2 +- .../bhyvexml2argv-bhyveload-bootorder1.args | 2 +- .../bhyvexml2argv-bhyveload-bootorder1.ldargs | 2 +- .../bhyvexml2argv-bhyveload-bootorder3.args | 2 +- .../bhyvexml2argv-bhyveload-bootorder3.ldargs | 2 +- .../bhyvexml2argv-bhyveload-explicitargs.args | 2 +- ...hyvexml2argv-bhyveload-explicitargs.ldargs | 2 +- .../bhyvexml2argv-commandline.args | 2 +- .../bhyvexml2argv-commandline.ldargs | 2 +- ...gv-console-master-slave-not-specified.args | 2 +- ...-console-master-slave-not-specified.ldargs | 2 +- .../bhyvexml2argv-console.args | 2 +- .../bhyvexml2argv-console.ldargs | 2 +- .../bhyvexml2argv-cputopology.args | 2 +- .../bhyvexml2argv-cputopology.ldargs | 2 +- .../bhyvexml2argv-custom-loader.args | 2 +- .../bhyvexml2argv-custom-loader.ldargs | 2 +- .../bhyvexml2argv-disk-cdrom-grub.args | 2 +- .../bhyvexml2argv-disk-cdrom-grub.ldargs | 2 +- .../bhyvexml2argv-disk-cdrom.args | 2 +- .../bhyvexml2argv-disk-cdrom.ldargs | 2 +- .../bhyvexml2argv-disk-virtio.args | 2 +- .../bhyvexml2argv-disk-virtio.ldargs | 2 +- .../bhyvexml2argv-firmware-efi.args | 2 +- .../bhyvexml2argv-fs-9p-readonly.args | 2 +- .../bhyvexml2argv-fs-9p-readonly.ldargs | 2 +- .../bhyvexml2argv-fs-9p.args | 2 +- .../bhyvexml2argv-fs-9p.ldargs | 2 +- .../bhyvexml2argv-grub-bootorder.args | 2 +- .../bhyvexml2argv-grub-bootorder.ldargs | 2 +- .../bhyvexml2argv-grub-bootorder2.args | 2 +- .../bhyvexml2argv-grub-bootorder2.ldargs | 2 +- .../bhyvexml2argv-grub-defaults.args | 2 +- .../bhyvexml2argv-grub-defaults.ldargs | 2 +- .../bhyvexml2argv-input-xhci-tablet.args | 2 +- .../bhyvexml2argv-input-xhci-tablet.ldargs | 2 +- .../bhyvexml2argv-isa-controller.args | 2 +- .../bhyvexml2argv-isa-controller.ldargs | 2 +- .../bhyvexml2argv-localtime.args | 2 +- .../bhyvexml2argv-localtime.ldargs | 2 +- .../bhyvexml2argv-macaddr.args | 2 +- .../bhyvexml2argv-macaddr.ldargs | 2 +- .../bhyvexml2argvdata/bhyvexml2argv-msrs.args | 2 +- .../bhyvexml2argv-msrs.ldargs | 2 +- .../bhyvexml2argv-net-e1000.args | 2 +- .../bhyvexml2argv-net-e1000.ldargs | 2 +- .../bhyvexml2argv-serial-grub-nocons.args | 2 +- .../bhyvexml2argv-serial-grub-nocons.ldargs | 2 +- .../bhyvexml2argv-serial-grub.args | 2 +- .../bhyvexml2argv-serial-grub.ldargs | 2 +- .../bhyvexml2argv-serial.args | 2 +- .../bhyvexml2argv-serial.ldargs | 2 +- .../bhyvexml2argv-sound.args | 2 +- .../bhyvexml2argv-sound.ldargs | 2 +- .../bhyvexml2argvdata/bhyvexml2argv-uefi.args | 2 +- .../bhyvexml2argv-vnc-autoport.args | 2 +- .../bhyvexml2argv-vnc-password.args | 2 +- .../bhyvexml2argv-vnc-resolution.args | 2 +- .../bhyvexml2argv-vnc-vgaconf-io.args | 2 +- .../bhyvexml2argv-vnc-vgaconf-off.args | 2 +- .../bhyvexml2argv-vnc-vgaconf-on.args | 2 +- .../bhyvexml2argvdata/bhyvexml2argv-vnc.args | 2 +- .../bhyvexml2argv-wired.args | 2 +- .../bhyvexml2argv-wired.ldargs | 2 +- tests/bhyvexml2argvtest.c | 4 +- tests/domaincapsmock.c | 17 ++ tests/meson.build | 1 + tests/networkxml2conftest.c | 6 +- tests/networkxml2firewalltest.c | 16 +- tests/nwfilterebiptablestest.c | 15 +- tests/nwfilterxml2firewalltest.c | 14 +- tests/qemuxml2argvmock.c | 5 +- tests/testutilsqemu.c | 15 -- tests/virfirewallmock.c | 35 +++ tests/virfirewalltest.c | 15 +- 132 files changed, 403 insertions(+), 484 deletions(-) create mode 100644 tests/virfirewallmock.c -- 2.30.2
We will never call dnsmasqCapsRefresh() so reflect what actually happens. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/network/bridge_driver.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 1172dbbc18..706a557ee8 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -748,7 +748,7 @@ networkStateInitialize(bool privileged, false, getpid())) < 0) goto error; - /* if this fails now, it will be retried later with dnsmasqCapsRefresh() */ + /* if this fails now, it will be retried later with networkDnsmasqCapsRefresh() */ network_driver->dnsmasqCaps = dnsmasqCapsNewFromBinary(DNSMASQ); if (!(network_driver->networks = virNetworkObjListNew())) -- 2.30.2
On Mon, Apr 19, 2021 at 07:14:04PM +0200, Pavel Hrdina wrote:
We will never call dnsmasqCapsRefresh() so reflect what actually happens.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/network/bridge_driver.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
Instead of removing binaryPath let's drop the function completely as it is not used anywhere. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/libvirt_private.syms | 1 - src/util/virdnsmasq.c | 30 ------------------------------ src/util/virdnsmasq.h | 2 -- 3 files changed, 33 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index fe3b443171..d60d15a43e 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2091,7 +2091,6 @@ dnsmasqCapsGetBinaryPath; dnsmasqCapsGetVersion; dnsmasqCapsNewFromBinary; dnsmasqCapsNewFromBuffer; -dnsmasqCapsNewFromFile; dnsmasqCapsRefresh; dnsmasqContextFree; dnsmasqContextNew; diff --git a/src/util/virdnsmasq.c b/src/util/virdnsmasq.c index 3db718c1d9..57f7ef67f1 100644 --- a/src/util/virdnsmasq.c +++ b/src/util/virdnsmasq.c @@ -673,21 +673,6 @@ dnsmasqCapsSetFromBuffer(dnsmasqCaps *caps, const char *buf) } -static int -dnsmasqCapsSetFromFile(dnsmasqCaps *caps, const char *path) -{ - int ret = -1; - g_autofree char *buf = NULL; - - if (virFileReadAll(path, 1024 * 1024, &buf) < 0) - goto cleanup; - - ret = dnsmasqCapsSetFromBuffer(caps, buf); - - cleanup: - return ret; -} - static int dnsmasqCapsRefreshInternal(dnsmasqCaps *caps, bool force) { @@ -773,21 +758,6 @@ dnsmasqCapsNewFromBuffer(const char *buf, const char *binaryPath) return caps; } -dnsmasqCaps * -dnsmasqCapsNewFromFile(const char *dataPath, const char *binaryPath) -{ - dnsmasqCaps *caps = dnsmasqCapsNewEmpty(binaryPath); - - if (!caps) - return NULL; - - if (dnsmasqCapsSetFromFile(caps, dataPath) < 0) { - virObjectUnref(caps); - return NULL; - } - return caps; -} - dnsmasqCaps * dnsmasqCapsNewFromBinary(const char *binaryPath) { diff --git a/src/util/virdnsmasq.h b/src/util/virdnsmasq.h index 6cdc619d8f..09e6164dc0 100644 --- a/src/util/virdnsmasq.h +++ b/src/util/virdnsmasq.h @@ -101,8 +101,6 @@ int dnsmasqReload(pid_t pid); dnsmasqCaps *dnsmasqCapsNewFromBuffer(const char *buf, const char *binaryPath); -dnsmasqCaps *dnsmasqCapsNewFromFile(const char *dataPath, - const char *binaryPath); dnsmasqCaps *dnsmasqCapsNewFromBinary(const char *binaryPath); int dnsmasqCapsRefresh(dnsmasqCaps **caps, const char *binaryPath); bool dnsmasqCapsGet(dnsmasqCaps *caps, dnsmasqCapsFlags flag); -- 2.30.2
On Mon, Apr 19, 2021 at 07:14:05PM +0200, Pavel Hrdina wrote:
Instead of removing binaryPath let's drop the function completely as it is not used anywhere.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/libvirt_private.syms | 1 - src/util/virdnsmasq.c | 30 ------------------------------ src/util/virdnsmasq.h | 2 -- 3 files changed, 33 deletions(-)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
Instead of removing binaryPath let's drop the function completely as it is not used anywhere. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/libvirt_private.syms | 1 - src/util/virdnsmasq.c | 17 ----------------- src/util/virdnsmasq.h | 1 - 3 files changed, 19 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index d60d15a43e..34e22d37d6 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2091,7 +2091,6 @@ dnsmasqCapsGetBinaryPath; dnsmasqCapsGetVersion; dnsmasqCapsNewFromBinary; dnsmasqCapsNewFromBuffer; -dnsmasqCapsRefresh; dnsmasqContextFree; dnsmasqContextNew; dnsmasqDelete; diff --git a/src/util/virdnsmasq.c b/src/util/virdnsmasq.c index 57f7ef67f1..89f33c22a8 100644 --- a/src/util/virdnsmasq.c +++ b/src/util/virdnsmasq.c @@ -773,23 +773,6 @@ dnsmasqCapsNewFromBinary(const char *binaryPath) return caps; } -/** dnsmasqCapsRefresh: - * - * Refresh an existing caps object if the binary has changed. If - * there isn't yet a caps object (if it's NULL), create a new one. - * - * Returns 0 on success, -1 on failure - */ -int -dnsmasqCapsRefresh(dnsmasqCaps **caps, const char *binaryPath) -{ - if (!*caps) { - *caps = dnsmasqCapsNewFromBinary(binaryPath); - return *caps ? 0 : -1; - } - return dnsmasqCapsRefreshInternal(*caps, false); -} - const char * dnsmasqCapsGetBinaryPath(dnsmasqCaps *caps) { diff --git a/src/util/virdnsmasq.h b/src/util/virdnsmasq.h index 09e6164dc0..8b9474f774 100644 --- a/src/util/virdnsmasq.h +++ b/src/util/virdnsmasq.h @@ -102,7 +102,6 @@ int dnsmasqReload(pid_t pid); dnsmasqCaps *dnsmasqCapsNewFromBuffer(const char *buf, const char *binaryPath); dnsmasqCaps *dnsmasqCapsNewFromBinary(const char *binaryPath); -int dnsmasqCapsRefresh(dnsmasqCaps **caps, const char *binaryPath); bool dnsmasqCapsGet(dnsmasqCaps *caps, dnsmasqCapsFlags flag); const char *dnsmasqCapsGetBinaryPath(dnsmasqCaps *caps); unsigned long dnsmasqCapsGetVersion(dnsmasqCaps *caps); -- 2.30.2
On Mon, Apr 19, 2021 at 07:14:06PM +0200, Pavel Hrdina wrote:
Instead of removing binaryPath let's drop the function completely as it is not used anywhere.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/libvirt_private.syms | 1 - src/util/virdnsmasq.c | 17 ----------------- src/util/virdnsmasq.h | 1 - 3 files changed, 19 deletions(-)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
We always pass DNSMASQ so there is no need for the argument at all. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/util/virdnsmasq.c | 4 ++-- src/util/virdnsmasq.h | 3 +-- tests/networkxml2conftest.c | 6 +++--- 3 files changed, 6 insertions(+), 7 deletions(-) diff --git a/src/util/virdnsmasq.c b/src/util/virdnsmasq.c index 89f33c22a8..82a26e83e9 100644 --- a/src/util/virdnsmasq.c +++ b/src/util/virdnsmasq.c @@ -744,9 +744,9 @@ dnsmasqCapsNewEmpty(const char *binaryPath) } dnsmasqCaps * -dnsmasqCapsNewFromBuffer(const char *buf, const char *binaryPath) +dnsmasqCapsNewFromBuffer(const char *buf) { - dnsmasqCaps *caps = dnsmasqCapsNewEmpty(binaryPath); + dnsmasqCaps *caps = dnsmasqCapsNewEmpty(DNSMASQ); if (!caps) return NULL; diff --git a/src/util/virdnsmasq.h b/src/util/virdnsmasq.h index 8b9474f774..cf6d50fe38 100644 --- a/src/util/virdnsmasq.h +++ b/src/util/virdnsmasq.h @@ -99,8 +99,7 @@ int dnsmasqSave(const dnsmasqContext *ctx); int dnsmasqDelete(const dnsmasqContext *ctx); int dnsmasqReload(pid_t pid); -dnsmasqCaps *dnsmasqCapsNewFromBuffer(const char *buf, - const char *binaryPath); +dnsmasqCaps *dnsmasqCapsNewFromBuffer(const char *buf); dnsmasqCaps *dnsmasqCapsNewFromBinary(const char *binaryPath); bool dnsmasqCapsGet(dnsmasqCaps *caps, dnsmasqCapsFlags flag); const char *dnsmasqCapsGetBinaryPath(dnsmasqCaps *caps); diff --git a/tests/networkxml2conftest.c b/tests/networkxml2conftest.c index 402244ea4a..7444390b77 100644 --- a/tests/networkxml2conftest.c +++ b/tests/networkxml2conftest.c @@ -123,11 +123,11 @@ mymain(void) { int ret = 0; dnsmasqCaps *restricted - = dnsmasqCapsNewFromBuffer("Dnsmasq version 2.48", DNSMASQ); + = dnsmasqCapsNewFromBuffer("Dnsmasq version 2.48"); dnsmasqCaps *full - = dnsmasqCapsNewFromBuffer("Dnsmasq version 2.63\n--bind-dynamic", DNSMASQ); + = dnsmasqCapsNewFromBuffer("Dnsmasq version 2.63\n--bind-dynamic"); dnsmasqCaps *dhcpv6 - = dnsmasqCapsNewFromBuffer("Dnsmasq version 2.64\n--bind-dynamic", DNSMASQ); + = dnsmasqCapsNewFromBuffer("Dnsmasq version 2.64\n--bind-dynamic"); #define DO_TEST(xname, xcaps) \ do { \ -- 2.30.2
On Mon, Apr 19, 2021 at 07:14:07PM +0200, Pavel Hrdina wrote:
We always pass DNSMASQ so there is no need for the argument at all.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/util/virdnsmasq.c | 4 ++-- src/util/virdnsmasq.h | 3 +-- tests/networkxml2conftest.c | 6 +++--- 3 files changed, 6 insertions(+), 7 deletions(-)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
We always pass DNSMASQ so there is no need for the argument at all. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/network/bridge_driver.c | 4 ++-- src/util/virdnsmasq.c | 4 ++-- src/util/virdnsmasq.h | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 706a557ee8..ee3f9dab0a 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -130,7 +130,7 @@ networkDnsmasqCapsRefresh(virNetworkDriverState *driver) { dnsmasqCaps *caps; - if (!(caps = dnsmasqCapsNewFromBinary(DNSMASQ))) + if (!(caps = dnsmasqCapsNewFromBinary())) return -1; networkDriverLock(driver); @@ -749,7 +749,7 @@ networkStateInitialize(bool privileged, goto error; /* if this fails now, it will be retried later with networkDnsmasqCapsRefresh() */ - network_driver->dnsmasqCaps = dnsmasqCapsNewFromBinary(DNSMASQ); + network_driver->dnsmasqCaps = dnsmasqCapsNewFromBinary(); if (!(network_driver->networks = virNetworkObjListNew())) goto error; diff --git a/src/util/virdnsmasq.c b/src/util/virdnsmasq.c index 82a26e83e9..f2f606913f 100644 --- a/src/util/virdnsmasq.c +++ b/src/util/virdnsmasq.c @@ -759,9 +759,9 @@ dnsmasqCapsNewFromBuffer(const char *buf) } dnsmasqCaps * -dnsmasqCapsNewFromBinary(const char *binaryPath) +dnsmasqCapsNewFromBinary(void) { - dnsmasqCaps *caps = dnsmasqCapsNewEmpty(binaryPath); + dnsmasqCaps *caps = dnsmasqCapsNewEmpty(DNSMASQ); if (!caps) return NULL; diff --git a/src/util/virdnsmasq.h b/src/util/virdnsmasq.h index cf6d50fe38..ee9839cd25 100644 --- a/src/util/virdnsmasq.h +++ b/src/util/virdnsmasq.h @@ -100,7 +100,7 @@ int dnsmasqDelete(const dnsmasqContext *ctx); int dnsmasqReload(pid_t pid); dnsmasqCaps *dnsmasqCapsNewFromBuffer(const char *buf); -dnsmasqCaps *dnsmasqCapsNewFromBinary(const char *binaryPath); +dnsmasqCaps *dnsmasqCapsNewFromBinary(void); bool dnsmasqCapsGet(dnsmasqCaps *caps, dnsmasqCapsFlags flag); const char *dnsmasqCapsGetBinaryPath(dnsmasqCaps *caps); unsigned long dnsmasqCapsGetVersion(dnsmasqCaps *caps); -- 2.30.2
On Mon, Apr 19, 2021 at 07:14:08PM +0200, Pavel Hrdina wrote:
We always pass DNSMASQ so there is no need for the argument at all.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/network/bridge_driver.c | 4 ++-- src/util/virdnsmasq.c | 4 ++-- src/util/virdnsmasq.h | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
Following patches will make this change necessary as we will stop detecting the full path during compile time. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/util/virfirewall.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/util/virfirewall.c b/src/util/virfirewall.c index 78aa7e580e..e79fe52ac8 100644 --- a/src/util/virfirewall.c +++ b/src/util/virfirewall.c @@ -104,7 +104,9 @@ virFirewallValidateBackend(virFirewallBackend backend) size_t i; for (i = 0; i < G_N_ELEMENTS(commands); i++) { - if (!virFileIsExecutable(commands[i])) { + g_autofree char *path = virFindFileInPath(commands[i]); + + if (!path) { virReportSystemError(errno, _("%s not available, firewall backend will not function"), commands[i]); -- 2.30.2
On Mon, Apr 19, 2021 at 07:14:09PM +0200, Pavel Hrdina wrote:
Following patches will make this change necessary as we will stop detecting the full path during compile time.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/util/virfirewall.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
This will allow us to run tests using firewall on hosts where the mocked binaries are not available/installed instead of skipping these tests. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/util/virfile.h | 3 ++- tests/meson.build | 1 + tests/virfirewallmock.c | 34 ++++++++++++++++++++++++++++++++++ 3 files changed, 37 insertions(+), 1 deletion(-) create mode 100644 tests/virfirewallmock.c diff --git a/src/util/virfile.h b/src/util/virfile.h index a02779f0ec..b9f7b1766f 100644 --- a/src/util/virfile.h +++ b/src/util/virfile.h @@ -184,7 +184,8 @@ int virFileResolveAllLinks(const char *linkpath, int virFileIsLink(const char *linkpath) ATTRIBUTE_NONNULL(1) G_GNUC_WARN_UNUSED_RESULT; -char *virFindFileInPath(const char *file); +char *virFindFileInPath(const char *file) + G_GNUC_NO_INLINE; char *virFileFindResource(const char *filename, const char *builddir, diff --git a/tests/meson.build b/tests/meson.build index 3f66861332..05c3e90195 100644 --- a/tests/meson.build +++ b/tests/meson.build @@ -60,6 +60,7 @@ mock_libs = [ { 'name': 'shunload', 'sources': [ 'shunloadhelper.c' ] }, { 'name': 'vircgroupmock' }, { 'name': 'virfilecachemock' }, + { 'name': 'virfirewallmock' }, { 'name': 'virgdbusmock' }, { 'name': 'virhostcpumock' }, { 'name': 'virhostdevmock' }, diff --git a/tests/virfirewallmock.c b/tests/virfirewallmock.c new file mode 100644 index 0000000000..6b096701c9 --- /dev/null +++ b/tests/virfirewallmock.c @@ -0,0 +1,34 @@ +/* + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + */ + +#include <config.h> + +#include "internal.h" +#include "virfile.h" + +char * +virFindFileInPath(const char *file) +{ + if (file && + (g_strrstr(file, "ebtables") || + g_strrstr(file, "iptables") || + g_strrstr(file, "ip6tables"))) { + return g_strdup(file); + } + + /* We should not need any other binaries so return NULL. */ + return NULL; +} -- 2.30.2
On Mon, Apr 19, 2021 at 07:14:10PM +0200, Pavel Hrdina wrote:
This will allow us to run tests using firewall on hosts where the mocked binaries are not available/installed instead of skipping these tests.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/util/virfile.h | 3 ++- tests/meson.build | 1 + tests/virfirewallmock.c | 34 ++++++++++++++++++++++++++++++++++ 3 files changed, 37 insertions(+), 1 deletion(-) create mode 100644 tests/virfirewallmock.c
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
diff --git a/src/util/virfile.h b/src/util/virfile.h index a02779f0ec..b9f7b1766f 100644 --- a/src/util/virfile.h +++ b/src/util/virfile.h @@ -184,7 +184,8 @@ int virFileResolveAllLinks(const char *linkpath, int virFileIsLink(const char *linkpath) ATTRIBUTE_NONNULL(1) G_GNUC_WARN_UNUSED_RESULT;
-char *virFindFileInPath(const char *file); +char *virFindFileInPath(const char *file) + G_GNUC_NO_INLINE;
Hmm, pre-existing bug - we have virFindFileinPath mocke in the testutilsqemu.c file already. Our mock test case doesn't report this bug since we're only validating functions present in a file ending in mock.c Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
Instead of checking for specific error that the binaries are not available mock the virFindFileInPath function. This way we don't have to skip these tests on host where the binaries are missing. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- tests/networkxml2firewalltest.c | 16 ++-------------- tests/nwfilterebiptablestest.c | 15 +-------------- tests/nwfilterxml2firewalltest.c | 14 +------------- tests/virfirewalltest.c | 15 ++------------- 4 files changed, 6 insertions(+), 54 deletions(-) diff --git a/tests/networkxml2firewalltest.c b/tests/networkxml2firewalltest.c index 80ba510d14..91336a0c55 100644 --- a/tests/networkxml2firewalltest.c +++ b/tests/networkxml2firewalltest.c @@ -152,14 +152,6 @@ testCompareXMLToIPTablesHelper(const void *data) return result; } -static bool -hasNetfilterTools(void) -{ - return virFileIsExecutable(IPTABLES_PATH) && - virFileIsExecutable(IP6TABLES_PATH) && - virFileIsExecutable(EBTABLES_PATH); -} - static int mymain(void) @@ -179,11 +171,6 @@ mymain(void) } while (0) if (virFirewallSetBackend(VIR_FIREWALL_BACKEND_DIRECT) < 0) { - if (!hasNetfilterTools()) { - fprintf(stderr, "iptables/ip6tables/ebtables tools not present"); - return EXIT_AM_SKIP; - } - return EXIT_FAILURE; } @@ -203,7 +190,8 @@ mymain(void) return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } -VIR_TEST_MAIN_PRELOAD(mymain, VIR_TEST_MOCK("virgdbus")) +VIR_TEST_MAIN_PRELOAD(mymain, VIR_TEST_MOCK("virgdbus"), + VIR_TEST_MOCK("virfirewall")) #else /* ! defined (__linux__) */ diff --git a/tests/nwfilterebiptablestest.c b/tests/nwfilterebiptablestest.c index 5f504cbe64..65b63959fb 100644 --- a/tests/nwfilterebiptablestest.c +++ b/tests/nwfilterebiptablestest.c @@ -482,14 +482,6 @@ testNWFilterEBIPTablesApplyDropAllRules(const void *opaque G_GNUC_UNUSED) return ret; } -static bool -hasNetfilterTools(void) -{ - return virFileIsExecutable(IPTABLES_PATH) && - virFileIsExecutable(IP6TABLES_PATH) && - virFileIsExecutable(EBTABLES_PATH); -} - static int mymain(void) @@ -497,11 +489,6 @@ mymain(void) int ret = 0; if (virFirewallSetBackend(VIR_FIREWALL_BACKEND_DIRECT) < 0) { - if (!hasNetfilterTools()) { - fprintf(stderr, "iptables/ip6tables/ebtables tools not present"); - return EXIT_AM_SKIP; - } - return EXIT_FAILURE; } @@ -543,4 +530,4 @@ mymain(void) return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } -VIR_TEST_MAIN(mymain) +VIR_TEST_MAIN_PRELOAD(mymain, VIR_TEST_MOCK("virfirewall")) diff --git a/tests/nwfilterxml2firewalltest.c b/tests/nwfilterxml2firewalltest.c index 560372b802..6709cc15fd 100644 --- a/tests/nwfilterxml2firewalltest.c +++ b/tests/nwfilterxml2firewalltest.c @@ -431,14 +431,6 @@ testCompareXMLToIPTablesHelper(const void *data) return result; } -static bool -hasNetfilterTools(void) -{ - return virFileIsExecutable(IPTABLES_PATH) && - virFileIsExecutable(IP6TABLES_PATH) && - virFileIsExecutable(EBTABLES_PATH); -} - static int mymain(void) @@ -456,10 +448,6 @@ mymain(void) } while (0) if (virFirewallSetBackend(VIR_FIREWALL_BACKEND_DIRECT) < 0) { - if (!hasNetfilterTools()) { - fprintf(stderr, "iptables/ip6tables/ebtables tools not present"); - return EXIT_AM_SKIP; - } return EXIT_FAILURE; } @@ -506,7 +494,7 @@ mymain(void) return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } -VIR_TEST_MAIN(mymain) +VIR_TEST_MAIN_PRELOAD(mymain, VIR_TEST_MOCK("virfirewall")) #else /* ! defined (__linux__) */ diff --git a/tests/virfirewalltest.c b/tests/virfirewalltest.c index 8e65fa8524..ac039f3bbf 100644 --- a/tests/virfirewalltest.c +++ b/tests/virfirewalltest.c @@ -1034,24 +1034,12 @@ testFirewallQuery(const void *opaque G_GNUC_UNUSED) return ret; } -static bool -hasNetfilterTools(void) -{ - return virFileIsExecutable(IPTABLES_PATH) && - virFileIsExecutable(IP6TABLES_PATH) && - virFileIsExecutable(EBTABLES_PATH); -} static int mymain(void) { int ret = 0; - if (!hasNetfilterTools()) { - fprintf(stderr, "iptables/ip6tables/ebtables tools not present"); - return EXIT_AM_SKIP; - } - # define RUN_TEST_DIRECT(name, method) \ do { \ struct testFirewallData data; \ @@ -1100,7 +1088,8 @@ mymain(void) return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } -VIR_TEST_MAIN_PRELOAD(mymain, VIR_TEST_MOCK("virgdbus")) +VIR_TEST_MAIN_PRELOAD(mymain, VIR_TEST_MOCK("virgdbus"), + VIR_TEST_MOCK("virfirewall")) #else /* ! defined (__linux__) */ -- 2.30.2
On Mon, Apr 19, 2021 at 07:14:11PM +0200, Pavel Hrdina wrote:
Instead of checking for specific error that the binaries are not available mock the virFindFileInPath function. This way we don't have to skip these tests on host where the binaries are missing.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- tests/networkxml2firewalltest.c | 16 ++-------------- tests/nwfilterebiptablestest.c | 15 +-------------- tests/nwfilterxml2firewalltest.c | 14 +------------- tests/virfirewalltest.c | 15 ++------------- 4 files changed, 6 insertions(+), 54 deletions(-)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
The function in question uses "tc" binary so virnetdevbandwidth feels like better place for it. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/libvirt_private.syms | 2 +- src/qemu/qemu_domain.c | 3 ++- src/util/virnetdev.c | 46 -------------------------------- src/util/virnetdev.h | 4 --- src/util/virnetdevbandwidth.c | 50 +++++++++++++++++++++++++++++++++++ src/util/virnetdevbandwidth.h | 4 +++ tests/qemuxml2argvmock.c | 5 ++-- 7 files changed, 60 insertions(+), 54 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 34e22d37d6..abd3dc4bd1 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2714,7 +2714,6 @@ virNetDevSetOnline; virNetDevSetPromiscuous; virNetDevSetRcvAllMulti; virNetDevSetRcvMulti; -virNetDevSetRootQDisc; virNetDevSetupControl; virNetDevSysfsFile; virNetDevValidateConfig; @@ -2728,6 +2727,7 @@ virNetDevBandwidthEqual; virNetDevBandwidthFree; virNetDevBandwidthPlug; virNetDevBandwidthSet; +virNetDevBandwidthSetRootQDisc; virNetDevBandwidthUnplug; virNetDevBandwidthUpdateFilter; virNetDevBandwidthUpdateRate; diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 2c8ea39840..6e3e3555c7 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -50,6 +50,7 @@ #include "domain_event.h" #include "domain_validate.h" #include "virtime.h" +#include "virnetdevbandwidth.h" #include "virnetdevopenvswitch.h" #include "virstoragefile.h" #include "storage_source.h" @@ -11571,7 +11572,7 @@ qemuDomainInterfaceSetDefaultQDisc(virQEMUDriver *driver, actualType == VIR_DOMAIN_NET_TYPE_NETWORK || actualType == VIR_DOMAIN_NET_TYPE_BRIDGE || actualType == VIR_DOMAIN_NET_TYPE_DIRECT) { - if (virNetDevSetRootQDisc(net->ifname, "noqueue") < 0) + if (virNetDevBandwidthSetRootQDisc(net->ifname, "noqueue") < 0) return -1; } diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c index 86abdc2e5b..1870f3f875 100644 --- a/src/util/virnetdev.c +++ b/src/util/virnetdev.c @@ -3507,52 +3507,6 @@ virNetDevRunEthernetScript(const char *ifname, const char *script) } -/** - * virNetDevSetRootQDisc: - * @ifname: the interface name - * @qdisc: queueing discipline to set - * - * For given interface @ifname set its root queueing discipline - * to @qdisc. This can be used to replace the default qdisc - * (usually pfifo_fast or whatever is set in - * /proc/sys/net/core/default_qdisc) with different qdisc. - * - * Returns: 0 on success, - * -1 if failed to exec tc (with error reported) - * -2 if tc failed (with no error reported) - */ -int -virNetDevSetRootQDisc(const char *ifname, - const char *qdisc) -{ - g_autoptr(virCommand) cmd = NULL; - g_autofree char *outbuf = NULL; - g_autofree char *errbuf = NULL; - int status; - - /* Ideally, we would have a netlink implementation and just - * call it here. But honestly, I tried and failed miserably. - * Fallback to spawning tc. */ - cmd = virCommandNewArgList(TC, "qdisc", "add", "dev", ifname, - "root", "handle", "0:", qdisc, - NULL); - - virCommandAddEnvString(cmd, "LC_ALL=C"); - virCommandSetOutputBuffer(cmd, &outbuf); - virCommandSetErrorBuffer(cmd, &errbuf); - - if (virCommandRun(cmd, &status) < 0) - return -1; - - if (status != 0) { - VIR_DEBUG("Setting qdisc failed: output='%s' err='%s'", outbuf, errbuf); - return -2; - } - - return 0; -} - - /** * virNetDevReserveName: * @name: name of an existing network device diff --git a/src/util/virnetdev.h b/src/util/virnetdev.h index 1f03d88293..b694f4ac35 100644 --- a/src/util/virnetdev.h +++ b/src/util/virnetdev.h @@ -333,10 +333,6 @@ int virNetDevSysfsFile(char **pf_sysfs_device_link, int virNetDevRunEthernetScript(const char *ifname, const char *script) G_GNUC_NO_INLINE; -int virNetDevSetRootQDisc(const char *ifname, - const char *qdisc) - G_GNUC_NO_INLINE; - int virNetDevVFInterfaceStats(virPCIDeviceAddress *vfAddr, virDomainInterfaceStatsPtr stats) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); diff --git a/src/util/virnetdevbandwidth.c b/src/util/virnetdevbandwidth.c index 0fc802b174..75fc5607ad 100644 --- a/src/util/virnetdevbandwidth.c +++ b/src/util/virnetdevbandwidth.c @@ -23,11 +23,14 @@ #include "vircommand.h" #include "viralloc.h" #include "virerror.h" +#include "virlog.h" #include "virstring.h" #include "virutil.h" #define VIR_FROM_THIS VIR_FROM_NONE +VIR_LOG_INIT("util.netdevbandwidth"); + void virNetDevBandwidthFree(virNetDevBandwidth *def) { @@ -749,3 +752,50 @@ virNetDevBandwidthUpdateFilter(const char *ifname, VIR_FREE(class_id); return ret; } + + + +/** + * virNetDevBandwidthSetRootQDisc: + * @ifname: the interface name + * @qdisc: queueing discipline to set + * + * For given interface @ifname set its root queueing discipline + * to @qdisc. This can be used to replace the default qdisc + * (usually pfifo_fast or whatever is set in + * /proc/sys/net/core/default_qdisc) with different qdisc. + * + * Returns: 0 on success, + * -1 if failed to exec tc (with error reported) + * -2 if tc failed (with no error reported) + */ +int +virNetDevBandwidthSetRootQDisc(const char *ifname, + const char *qdisc) +{ + g_autoptr(virCommand) cmd = NULL; + g_autofree char *outbuf = NULL; + g_autofree char *errbuf = NULL; + int status; + + /* Ideally, we would have a netlink implementation and just + * call it here. But honestly, I tried and failed miserably. + * Fallback to spawning tc. */ + cmd = virCommandNewArgList(TC, "qdisc", "add", "dev", ifname, + "root", "handle", "0:", qdisc, + NULL); + + virCommandAddEnvString(cmd, "LC_ALL=C"); + virCommandSetOutputBuffer(cmd, &outbuf); + virCommandSetErrorBuffer(cmd, &errbuf); + + if (virCommandRun(cmd, &status) < 0) + return -1; + + if (status != 0) { + VIR_DEBUG("Setting qdisc failed: output='%s' err='%s'", outbuf, errbuf); + return -2; + } + + return 0; +} diff --git a/src/util/virnetdevbandwidth.h b/src/util/virnetdevbandwidth.h index 35e26b66f7..3d520721f6 100644 --- a/src/util/virnetdevbandwidth.h +++ b/src/util/virnetdevbandwidth.h @@ -74,3 +74,7 @@ int virNetDevBandwidthUpdateFilter(const char *ifname, unsigned int id) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) G_GNUC_WARN_UNUSED_RESULT; + +int virNetDevBandwidthSetRootQDisc(const char *ifname, + const char *qdisc) + G_GNUC_NO_INLINE; diff --git a/tests/qemuxml2argvmock.c b/tests/qemuxml2argvmock.c index 9d812bfc2e..77a0814c08 100644 --- a/tests/qemuxml2argvmock.c +++ b/tests/qemuxml2argvmock.c @@ -25,6 +25,7 @@ #include "virmock.h" #include "virlog.h" #include "virnetdev.h" +#include "virnetdevbandwidth.h" #include "virnetdevip.h" #include "virnetdevtap.h" #include "virnetdevopenvswitch.h" @@ -277,8 +278,8 @@ qemuBuildTPMOpenBackendFDs(const char *tpmdev G_GNUC_UNUSED, int -virNetDevSetRootQDisc(const char *ifname G_GNUC_UNUSED, - const char *qdisc G_GNUC_UNUSED) +virNetDevBandwidthSetRootQDisc(const char *ifname G_GNUC_UNUSED, + const char *qdisc G_GNUC_UNUSED) { return 0; } -- 2.30.2
On Mon, Apr 19, 2021 at 07:14:12PM +0200, Pavel Hrdina wrote:
The function in question uses "tc" binary so virnetdevbandwidth feels like better place for it.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/libvirt_private.syms | 2 +- src/qemu/qemu_domain.c | 3 ++- src/util/virnetdev.c | 46 -------------------------------- src/util/virnetdev.h | 4 --- src/util/virnetdevbandwidth.c | 50 +++++++++++++++++++++++++++++++++++ src/util/virnetdevbandwidth.h | 4 +++ tests/qemuxml2argvmock.c | 5 ++-- 7 files changed, 60 insertions(+), 54 deletions(-)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
Having the function on mock library reflect more closely what we usually do in tests. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- tests/domaincapsmock.c | 16 ++++++++++++++++ tests/testutilsqemu.c | 15 --------------- 2 files changed, 16 insertions(+), 15 deletions(-) diff --git a/tests/domaincapsmock.c b/tests/domaincapsmock.c index d81a898dc0..73690f0b9e 100644 --- a/tests/domaincapsmock.c +++ b/tests/domaincapsmock.c @@ -16,6 +16,7 @@ #include <config.h> +#include "virfile.h" #include "virhostcpu.h" #ifdef WITH_LIBXL # include "libxl/libxl_capabilities.h" @@ -40,3 +41,18 @@ virHostCPUGetMicrocodeVersion(virArch hostArch G_GNUC_UNUSED) { return 0; } + +char * +virFindFileInPath(const char *file) +{ + if (g_str_has_prefix(file, "qemu-system") || + g_str_equal(file, "qemu-kvm")) { + return g_strdup_printf("/usr/bin/%s", file); + } + + /* Nothing in tests should be relying on real files + * in host OS, so we return NULL to try to force + * an error in such a case + */ + return NULL; +} diff --git a/tests/testutilsqemu.c b/tests/testutilsqemu.c index 7451929807..0a2af5036e 100644 --- a/tests/testutilsqemu.c +++ b/tests/testutilsqemu.c @@ -117,21 +117,6 @@ static const char *qemu_default_ram_id[VIR_ARCH_LAST] = { [VIR_ARCH_SPARC] = "sun4m.ram", }; -char * -virFindFileInPath(const char *file) -{ - if (g_str_has_prefix(file, "qemu-system") || - g_str_equal(file, "qemu-kvm")) { - return g_strdup_printf("/usr/bin/%s", file); - } - - /* Nothing in tests should be relying on real files - * in host OS, so we return NULL to try to force - * an error in such a case - */ - return NULL; -} - virCapsHostNUMA * virCapabilitiesHostNUMANewHost(void) -- 2.30.2
On Mon, Apr 19, 2021 at 07:14:13PM +0200, Pavel Hrdina wrote:
Having the function on mock library reflect more closely what we usually do in tests.
Yes & no - having it in testutilsqemu.c would make it available to all QEMU tests, while domaincapsmock.c only makes it available from those loading that mock. Using testutilsqemu.c might be simpler if we need to expand what it mocks, as we have one place to put all the hacks. It gets confusing if we have 2 mocks both overriding the same function. This isn't a NACK - just saying that I don't think the commit message is a clear justification for the move. Guess I'd like an explanation of why it should be restricted to just tests using domaincapsmock vs any of those linking to testutilsqemu
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- tests/domaincapsmock.c | 16 ++++++++++++++++ tests/testutilsqemu.c | 15 --------------- 2 files changed, 16 insertions(+), 15 deletions(-)
diff --git a/tests/domaincapsmock.c b/tests/domaincapsmock.c index d81a898dc0..73690f0b9e 100644 --- a/tests/domaincapsmock.c +++ b/tests/domaincapsmock.c @@ -16,6 +16,7 @@
#include <config.h>
+#include "virfile.h" #include "virhostcpu.h" #ifdef WITH_LIBXL # include "libxl/libxl_capabilities.h" @@ -40,3 +41,18 @@ virHostCPUGetMicrocodeVersion(virArch hostArch G_GNUC_UNUSED) { return 0; } + +char * +virFindFileInPath(const char *file) +{ + if (g_str_has_prefix(file, "qemu-system") || + g_str_equal(file, "qemu-kvm")) { + return g_strdup_printf("/usr/bin/%s", file); + } + + /* Nothing in tests should be relying on real files + * in host OS, so we return NULL to try to force + * an error in such a case + */ + return NULL; +} diff --git a/tests/testutilsqemu.c b/tests/testutilsqemu.c index 7451929807..0a2af5036e 100644 --- a/tests/testutilsqemu.c +++ b/tests/testutilsqemu.c @@ -117,21 +117,6 @@ static const char *qemu_default_ram_id[VIR_ARCH_LAST] = { [VIR_ARCH_SPARC] = "sun4m.ram", };
-char * -virFindFileInPath(const char *file) -{ - if (g_str_has_prefix(file, "qemu-system") || - g_str_equal(file, "qemu-kvm")) { - return g_strdup_printf("/usr/bin/%s", file); - } - - /* Nothing in tests should be relying on real files - * in host OS, so we return NULL to try to force - * an error in such a case - */ - return NULL; -} -
virCapsHostNUMA * virCapabilitiesHostNUMANewHost(void) -- 2.30.2
Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On Tue, Apr 20, 2021 at 11:11:54AM +0100, Daniel P. Berrangé wrote:
On Mon, Apr 19, 2021 at 07:14:13PM +0200, Pavel Hrdina wrote:
Having the function on mock library reflect more closely what we usually do in tests.
Yes & no - having it in testutilsqemu.c would make it available to all QEMU tests, while domaincapsmock.c only makes it available from those loading that mock.
Using testutilsqemu.c might be simpler if we need to expand what it mocks, as we have one place to put all the hacks. It gets confusing if we have 2 mocks both overriding the same function.
This isn't a NACK - just saying that I don't think the commit message is a clear justification for the move. Guess I'd like an explanation of why it should be restricted to just tests using domaincapsmock vs any of those linking to testutilsqemu
I don't have any explanation why it should be restricted. My main motivation was to have it clearly separated in a mock library. The alternative I had in mind was to introduce a mock library that would be used for all tests but that is for different patch series. I'll drop this patch as it is not necessary for this series. Pavel
Upstream sheepdog changed collie to dog back in 2013 in version 0.7.0. Looking into repology that version is no longer used by any distribution supported by libvirt. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- meson.build | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/meson.build b/meson.build index cd88e26f9a..837955de37 100644 --- a/meson.build +++ b/meson.build @@ -1927,7 +1927,7 @@ if conf.has('WITH_LIBVIRTD') endif if not get_option('storage_sheepdog').disabled() - sheepdogcli_prog = find_program(['collie', 'dog'], required: get_option('storage_sheepdog'), dirs: libvirt_sbin_path) + sheepdogcli_prog = find_program('dog', required: get_option('storage_sheepdog'), dirs: libvirt_sbin_path) if sheepdogcli_prog.found() use_storage = true -- 2.30.2
On Mon, Apr 19, 2021 at 07:14:14PM +0200, Pavel Hrdina wrote:
Upstream sheepdog changed collie to dog back in 2013 in version 0.7.0. Looking into repology that version is no longer used by any distribution supported by libvirt.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- meson.build | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
Currently the tests would fail if the bhyve commands are installed in different path then /usr/bin. Strip the command path to not depend on the host environment. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- tests/bhyvexml2argvdata/bhyvexml2argv-acpiapic.args | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-acpiapic.ldargs | 2 +- .../bhyvexml2argv-addr-isa-controller-on-slot-1.args | 2 +- .../bhyvexml2argv-addr-isa-controller-on-slot-1.ldargs | 2 +- .../bhyvexml2argv-addr-isa-controller-on-slot-31.args | 2 +- .../bhyvexml2argv-addr-isa-controller-on-slot-31.ldargs | 2 +- .../bhyvexml2argv-addr-more-than-32-sata-disks.args | 2 +- .../bhyvexml2argv-addr-more-than-32-sata-disks.ldargs | 2 +- .../bhyvexml2argv-addr-multiple-sata-disks.args | 2 +- .../bhyvexml2argv-addr-multiple-sata-disks.ldargs | 2 +- .../bhyvexml2argv-addr-multiple-virtio-disks.args | 2 +- .../bhyvexml2argv-addr-multiple-virtio-disks.ldargs | 2 +- .../bhyvexml2argv-addr-no32devs-multiple-sata-disks.args | 2 +- .../bhyvexml2argv-addr-no32devs-multiple-sata-disks.ldargs | 2 +- .../bhyvexml2argv-addr-no32devs-single-sata-disk.args | 2 +- .../bhyvexml2argv-addr-no32devs-single-sata-disk.ldargs | 2 +- .../bhyvexml2argv-addr-non-isa-controller-on-slot-1.args | 2 +- .../bhyvexml2argv-addr-single-sata-disk.args | 2 +- .../bhyvexml2argv-addr-single-sata-disk.ldargs | 2 +- .../bhyvexml2argv-addr-single-virtio-disk.args | 2 +- .../bhyvexml2argv-addr-single-virtio-disk.ldargs | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-base.args | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-base.ldargs | 2 +- .../bhyvexml2argvdata/bhyvexml2argv-bhyveload-bootorder.args | 2 +- .../bhyvexml2argv-bhyveload-bootorder.ldargs | 2 +- .../bhyvexml2argvdata/bhyvexml2argv-bhyveload-bootorder1.args | 2 +- .../bhyvexml2argv-bhyveload-bootorder1.ldargs | 2 +- .../bhyvexml2argvdata/bhyvexml2argv-bhyveload-bootorder3.args | 2 +- .../bhyvexml2argv-bhyveload-bootorder3.ldargs | 2 +- .../bhyvexml2argv-bhyveload-explicitargs.args | 2 +- .../bhyvexml2argv-bhyveload-explicitargs.ldargs | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-commandline.args | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-commandline.ldargs | 2 +- .../bhyvexml2argv-console-master-slave-not-specified.args | 2 +- .../bhyvexml2argv-console-master-slave-not-specified.ldargs | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-console.args | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-console.ldargs | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-cputopology.args | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-cputopology.ldargs | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-custom-loader.args | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-custom-loader.ldargs | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-disk-cdrom-grub.args | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-disk-cdrom-grub.ldargs | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-disk-cdrom.args | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-disk-cdrom.ldargs | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-disk-virtio.args | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-disk-virtio.ldargs | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-firmware-efi.args | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-fs-9p-readonly.args | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-fs-9p-readonly.ldargs | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-fs-9p.args | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-fs-9p.ldargs | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-grub-bootorder.args | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-grub-bootorder.ldargs | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-grub-bootorder2.args | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-grub-bootorder2.ldargs | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-grub-defaults.args | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-grub-defaults.ldargs | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-input-xhci-tablet.args | 2 +- .../bhyvexml2argvdata/bhyvexml2argv-input-xhci-tablet.ldargs | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.args | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.ldargs | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-localtime.args | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-localtime.ldargs | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-macaddr.args | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-macaddr.ldargs | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-msrs.args | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-msrs.ldargs | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-net-e1000.args | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-net-e1000.ldargs | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-serial-grub-nocons.args | 2 +- .../bhyvexml2argvdata/bhyvexml2argv-serial-grub-nocons.ldargs | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-serial-grub.args | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-serial-grub.ldargs | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-serial.args | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-serial.ldargs | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-sound.args | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-sound.ldargs | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-uefi.args | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-vnc-autoport.args | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-vnc-password.args | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-vnc-resolution.args | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-vnc-vgaconf-io.args | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-vnc-vgaconf-off.args | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-vnc-vgaconf-on.args | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-vnc.args | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-wired.args | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-wired.ldargs | 2 +- tests/bhyvexml2argvtest.c | 4 ++-- 89 files changed, 90 insertions(+), 90 deletions(-) diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-acpiapic.args b/tests/bhyvexml2argvdata/bhyvexml2argv-acpiapic.args index 001e44147e..146e3c6eee 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-acpiapic.args +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-acpiapic.args @@ -1,4 +1,4 @@ -/usr/sbin/bhyve \ +bhyve \ -c 1 \ -m 214 \ -A \ diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-acpiapic.ldargs b/tests/bhyvexml2argvdata/bhyvexml2argv-acpiapic.ldargs index 91f9658137..5905f4b3e6 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-acpiapic.ldargs +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-acpiapic.ldargs @@ -1,4 +1,4 @@ -/usr/sbin/bhyveload \ +bhyveload \ -m 214 \ -d /tmp/freebsd.img \ bhyve diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.args b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.args index 498455275a..735a084ac7 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.args +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.args @@ -1,4 +1,4 @@ -/usr/sbin/bhyve \ +bhyve \ -c 1 \ -m 214 \ -u \ diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.ldargs b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.ldargs index 91f9658137..5905f4b3e6 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.ldargs +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.ldargs @@ -1,4 +1,4 @@ -/usr/sbin/bhyveload \ +bhyveload \ -m 214 \ -d /tmp/freebsd.img \ bhyve diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.args b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.args index 70c44932c8..0846afafd0 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.args +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.args @@ -1,4 +1,4 @@ -/usr/sbin/bhyve \ +bhyve \ -c 1 \ -m 214 \ -u \ diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.ldargs b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.ldargs index 91f9658137..5905f4b3e6 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.ldargs +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.ldargs @@ -1,4 +1,4 @@ -/usr/sbin/bhyveload \ +bhyveload \ -m 214 \ -d /tmp/freebsd.img \ bhyve diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-addr-more-than-32-sata-disks.args b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-more-than-32-sata-disks.args index bd987c86aa..51434ed1df 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-addr-more-than-32-sata-disks.args +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-more-than-32-sata-disks.args @@ -1,4 +1,4 @@ -/usr/sbin/bhyve \ +bhyve \ -c 1 \ -m 214 \ -u \ diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-addr-more-than-32-sata-disks.ldargs b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-more-than-32-sata-disks.ldargs index ec3f482ca8..4257ffccee 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-addr-more-than-32-sata-disks.ldargs +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-more-than-32-sata-disks.ldargs @@ -1,4 +1,4 @@ -/usr/sbin/bhyveload \ +bhyveload \ -m 214 \ -d /tmp/freebsd1.img \ bhyve diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-addr-multiple-sata-disks.args b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-multiple-sata-disks.args index 0bd1957e96..976d800f47 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-addr-multiple-sata-disks.args +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-multiple-sata-disks.args @@ -1,4 +1,4 @@ -/usr/sbin/bhyve \ +bhyve \ -c 1 \ -m 214 \ -u \ diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-addr-multiple-sata-disks.ldargs b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-multiple-sata-disks.ldargs index ec3f482ca8..4257ffccee 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-addr-multiple-sata-disks.ldargs +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-multiple-sata-disks.ldargs @@ -1,4 +1,4 @@ -/usr/sbin/bhyveload \ +bhyveload \ -m 214 \ -d /tmp/freebsd1.img \ bhyve diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-addr-multiple-virtio-disks.args b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-multiple-virtio-disks.args index 73b1e2ad57..47b9ca094f 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-addr-multiple-virtio-disks.args +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-multiple-virtio-disks.args @@ -1,4 +1,4 @@ -/usr/sbin/bhyve \ +bhyve \ -c 1 \ -m 214 \ -u \ diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-addr-multiple-virtio-disks.ldargs b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-multiple-virtio-disks.ldargs index 91f9658137..5905f4b3e6 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-addr-multiple-virtio-disks.ldargs +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-multiple-virtio-disks.ldargs @@ -1,4 +1,4 @@ -/usr/sbin/bhyveload \ +bhyveload \ -m 214 \ -d /tmp/freebsd.img \ bhyve diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-addr-no32devs-multiple-sata-disks.args b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-no32devs-multiple-sata-disks.args index 821cc58287..2bb2689c79 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-addr-no32devs-multiple-sata-disks.args +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-no32devs-multiple-sata-disks.args @@ -1,4 +1,4 @@ -/usr/sbin/bhyve \ +bhyve \ -c 1 \ -m 214 \ -u \ diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-addr-no32devs-multiple-sata-disks.ldargs b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-no32devs-multiple-sata-disks.ldargs index ec3f482ca8..4257ffccee 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-addr-no32devs-multiple-sata-disks.ldargs +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-no32devs-multiple-sata-disks.ldargs @@ -1,4 +1,4 @@ -/usr/sbin/bhyveload \ +bhyveload \ -m 214 \ -d /tmp/freebsd1.img \ bhyve diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-addr-no32devs-single-sata-disk.args b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-no32devs-single-sata-disk.args index 9923c4cd85..153a1d5035 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-addr-no32devs-single-sata-disk.args +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-no32devs-single-sata-disk.args @@ -1,4 +1,4 @@ -/usr/sbin/bhyve \ +bhyve \ -c 1 \ -m 214 \ -u \ diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-addr-no32devs-single-sata-disk.ldargs b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-no32devs-single-sata-disk.ldargs index 91f9658137..5905f4b3e6 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-addr-no32devs-single-sata-disk.ldargs +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-no32devs-single-sata-disk.ldargs @@ -1,4 +1,4 @@ -/usr/sbin/bhyveload \ +bhyveload \ -m 214 \ -d /tmp/freebsd.img \ bhyve diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-addr-non-isa-controller-on-slot-1.args b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-non-isa-controller-on-slot-1.args index 12859f7d56..8517c0d06a 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-addr-non-isa-controller-on-slot-1.args +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-non-isa-controller-on-slot-1.args @@ -1,4 +1,4 @@ -/usr/sbin/bhyve \ +bhyve \ -c 1 \ -m 214 \ -u \ diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-addr-single-sata-disk.args b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-single-sata-disk.args index edcb45a0ae..507e0be668 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-addr-single-sata-disk.args +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-single-sata-disk.args @@ -1,4 +1,4 @@ -/usr/sbin/bhyve \ +bhyve \ -c 1 \ -m 214 \ -u \ diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-addr-single-sata-disk.ldargs b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-single-sata-disk.ldargs index 91f9658137..5905f4b3e6 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-addr-single-sata-disk.ldargs +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-single-sata-disk.ldargs @@ -1,4 +1,4 @@ -/usr/sbin/bhyveload \ +bhyveload \ -m 214 \ -d /tmp/freebsd.img \ bhyve diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-addr-single-virtio-disk.args b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-single-virtio-disk.args index 3ee9614733..9c00e02000 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-addr-single-virtio-disk.args +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-single-virtio-disk.args @@ -1,4 +1,4 @@ -/usr/sbin/bhyve \ +bhyve \ -c 1 \ -m 214 \ -u \ diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-addr-single-virtio-disk.ldargs b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-single-virtio-disk.ldargs index 91f9658137..5905f4b3e6 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-addr-single-virtio-disk.ldargs +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-single-virtio-disk.ldargs @@ -1,4 +1,4 @@ -/usr/sbin/bhyveload \ +bhyveload \ -m 214 \ -d /tmp/freebsd.img \ bhyve diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-base.args b/tests/bhyvexml2argvdata/bhyvexml2argv-base.args index edcb45a0ae..507e0be668 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-base.args +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-base.args @@ -1,4 +1,4 @@ -/usr/sbin/bhyve \ +bhyve \ -c 1 \ -m 214 \ -u \ diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-base.ldargs b/tests/bhyvexml2argvdata/bhyvexml2argv-base.ldargs index 91f9658137..5905f4b3e6 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-base.ldargs +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-base.ldargs @@ -1,4 +1,4 @@ -/usr/sbin/bhyveload \ +bhyveload \ -m 214 \ -d /tmp/freebsd.img \ bhyve diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-bootorder.args b/tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-bootorder.args index f790f746a9..a6df2a673e 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-bootorder.args +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-bootorder.args @@ -1,4 +1,4 @@ -/usr/sbin/bhyve \ +bhyve \ -c 1 \ -m 214 \ -u \ diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-bootorder.ldargs b/tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-bootorder.ldargs index b31d65cd06..25cfbc4cc3 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-bootorder.ldargs +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-bootorder.ldargs @@ -1,4 +1,4 @@ -/usr/sbin/bhyveload \ +bhyveload \ -m 214 \ -d /tmp/cdrom.iso \ bhyve diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-bootorder1.args b/tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-bootorder1.args index e621edbf0e..f10304cc9f 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-bootorder1.args +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-bootorder1.args @@ -1,4 +1,4 @@ -/usr/sbin/bhyve \ +bhyve \ -c 1 \ -m 214 \ -u \ diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-bootorder1.ldargs b/tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-bootorder1.ldargs index 91f9658137..5905f4b3e6 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-bootorder1.ldargs +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-bootorder1.ldargs @@ -1,4 +1,4 @@ -/usr/sbin/bhyveload \ +bhyveload \ -m 214 \ -d /tmp/freebsd.img \ bhyve diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-bootorder3.args b/tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-bootorder3.args index eedc36d35e..96e05c6c9c 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-bootorder3.args +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-bootorder3.args @@ -1,4 +1,4 @@ -/usr/sbin/bhyve \ +bhyve \ -c 1 \ -m 214 \ -u \ diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-bootorder3.ldargs b/tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-bootorder3.ldargs index b31d65cd06..25cfbc4cc3 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-bootorder3.ldargs +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-bootorder3.ldargs @@ -1,4 +1,4 @@ -/usr/sbin/bhyveload \ +bhyveload \ -m 214 \ -d /tmp/cdrom.iso \ bhyve diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-explicitargs.args b/tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-explicitargs.args index 1c5aa009e8..514ec78c82 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-explicitargs.args +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-explicitargs.args @@ -1,4 +1,4 @@ -/usr/sbin/bhyve \ +bhyve \ -c 1 \ -m 214 \ -u \ diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-explicitargs.ldargs b/tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-explicitargs.ldargs index 73871acad0..265475b7dc 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-explicitargs.ldargs +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-explicitargs.ldargs @@ -1,4 +1,4 @@ -/usr/sbin/bhyveload \ +bhyveload \ -X \ -Y \ -Z diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-commandline.args b/tests/bhyvexml2argvdata/bhyvexml2argv-commandline.args index 153639c370..cfbc57bd8c 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-commandline.args +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-commandline.args @@ -1,4 +1,4 @@ -/usr/sbin/bhyve \ +bhyve \ -c 1 \ -m 214 \ -u \ diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-commandline.ldargs b/tests/bhyvexml2argvdata/bhyvexml2argv-commandline.ldargs index 91f9658137..5905f4b3e6 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-commandline.ldargs +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-commandline.ldargs @@ -1,4 +1,4 @@ -/usr/sbin/bhyveload \ +bhyveload \ -m 214 \ -d /tmp/freebsd.img \ bhyve diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-console-master-slave-not-specified.args b/tests/bhyvexml2argvdata/bhyvexml2argv-console-master-slave-not-specified.args index 4d7cb11999..5d0c5c2987 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-console-master-slave-not-specified.args +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-console-master-slave-not-specified.args @@ -1,4 +1,4 @@ -/usr/sbin/bhyve \ +bhyve \ -c 1 \ -m 214 \ -u \ diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-console-master-slave-not-specified.ldargs b/tests/bhyvexml2argvdata/bhyvexml2argv-console-master-slave-not-specified.ldargs index 91f9658137..5905f4b3e6 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-console-master-slave-not-specified.ldargs +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-console-master-slave-not-specified.ldargs @@ -1,4 +1,4 @@ -/usr/sbin/bhyveload \ +bhyveload \ -m 214 \ -d /tmp/freebsd.img \ bhyve diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-console.args b/tests/bhyvexml2argvdata/bhyvexml2argv-console.args index 8c6cb78c9f..712dee9a2b 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-console.args +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-console.args @@ -1,4 +1,4 @@ -/usr/sbin/bhyve \ +bhyve \ -c 1 \ -m 214 \ -u \ diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-console.ldargs b/tests/bhyvexml2argvdata/bhyvexml2argv-console.ldargs index 91f9658137..5905f4b3e6 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-console.ldargs +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-console.ldargs @@ -1,4 +1,4 @@ -/usr/sbin/bhyveload \ +bhyveload \ -m 214 \ -d /tmp/freebsd.img \ bhyve diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-cputopology.args b/tests/bhyvexml2argvdata/bhyvexml2argv-cputopology.args index 2c88b650b5..d7a232fcb1 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-cputopology.args +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-cputopology.args @@ -1,4 +1,4 @@ -/usr/sbin/bhyve \ +bhyve \ -c cpus=2,sockets=1,cores=2,threads=1 \ -m 214 \ -u \ diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-cputopology.ldargs b/tests/bhyvexml2argvdata/bhyvexml2argv-cputopology.ldargs index 91f9658137..5905f4b3e6 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-cputopology.ldargs +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-cputopology.ldargs @@ -1,4 +1,4 @@ -/usr/sbin/bhyveload \ +bhyveload \ -m 214 \ -d /tmp/freebsd.img \ bhyve diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-custom-loader.args b/tests/bhyvexml2argvdata/bhyvexml2argv-custom-loader.args index aca701c3bf..a8570b3ca2 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-custom-loader.args +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-custom-loader.args @@ -1,4 +1,4 @@ -/usr/sbin/bhyve \ +bhyve \ -c 1 \ -m 214 \ -u \ diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-custom-loader.ldargs b/tests/bhyvexml2argvdata/bhyvexml2argv-custom-loader.ldargs index 359ad90a2c..ca4bbaa987 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-custom-loader.ldargs +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-custom-loader.ldargs @@ -1,4 +1,4 @@ -/fizz_buzz_bazz \ +fizz_buzz_bazz \ -X \ -Y \ -Z diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-disk-cdrom-grub.args b/tests/bhyvexml2argvdata/bhyvexml2argv-disk-cdrom-grub.args index 8f336d7ef2..f54ebbc8b3 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-disk-cdrom-grub.args +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-disk-cdrom-grub.args @@ -1,4 +1,4 @@ -/usr/sbin/bhyve \ +bhyve \ -c 1 \ -m 214 \ -u \ diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-disk-cdrom-grub.ldargs b/tests/bhyvexml2argvdata/bhyvexml2argv-disk-cdrom-grub.ldargs index 85990fc60b..627a07bdfd 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-disk-cdrom-grub.ldargs +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-disk-cdrom-grub.ldargs @@ -1,4 +1,4 @@ -/usr/local/sbin/grub-bhyve \ +grub-bhyve \ --root cd \ --device-map '<device.map>' \ --memory 214 \ diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-disk-cdrom.args b/tests/bhyvexml2argvdata/bhyvexml2argv-disk-cdrom.args index fd36515efc..ba6ddeffbe 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-disk-cdrom.args +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-disk-cdrom.args @@ -1,4 +1,4 @@ -/usr/sbin/bhyve \ +bhyve \ -c 1 \ -m 214 \ -u \ diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-disk-cdrom.ldargs b/tests/bhyvexml2argvdata/bhyvexml2argv-disk-cdrom.ldargs index b31d65cd06..25cfbc4cc3 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-disk-cdrom.ldargs +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-disk-cdrom.ldargs @@ -1,4 +1,4 @@ -/usr/sbin/bhyveload \ +bhyveload \ -m 214 \ -d /tmp/cdrom.iso \ bhyve diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-disk-virtio.args b/tests/bhyvexml2argvdata/bhyvexml2argv-disk-virtio.args index 3ee9614733..9c00e02000 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-disk-virtio.args +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-disk-virtio.args @@ -1,4 +1,4 @@ -/usr/sbin/bhyve \ +bhyve \ -c 1 \ -m 214 \ -u \ diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-disk-virtio.ldargs b/tests/bhyvexml2argvdata/bhyvexml2argv-disk-virtio.ldargs index 91f9658137..5905f4b3e6 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-disk-virtio.ldargs +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-disk-virtio.ldargs @@ -1,4 +1,4 @@ -/usr/sbin/bhyveload \ +bhyveload \ -m 214 \ -d /tmp/freebsd.img \ bhyve diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-firmware-efi.args b/tests/bhyvexml2argvdata/bhyvexml2argv-firmware-efi.args index b343469d19..c87dc7d779 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-firmware-efi.args +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-firmware-efi.args @@ -1,4 +1,4 @@ -/usr/sbin/bhyve \ +bhyve \ -c 1 \ -m 214 \ -u \ diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-fs-9p-readonly.args b/tests/bhyvexml2argvdata/bhyvexml2argv-fs-9p-readonly.args index de986b4eb6..c321d4d76d 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-fs-9p-readonly.args +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-fs-9p-readonly.args @@ -1,4 +1,4 @@ -/usr/sbin/bhyve \ +bhyve \ -c 1 \ -m 214 \ -u \ diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-fs-9p-readonly.ldargs b/tests/bhyvexml2argvdata/bhyvexml2argv-fs-9p-readonly.ldargs index 91f9658137..5905f4b3e6 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-fs-9p-readonly.ldargs +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-fs-9p-readonly.ldargs @@ -1,4 +1,4 @@ -/usr/sbin/bhyveload \ +bhyveload \ -m 214 \ -d /tmp/freebsd.img \ bhyve diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-fs-9p.args b/tests/bhyvexml2argvdata/bhyvexml2argv-fs-9p.args index 8943ae211f..0f02d5be4e 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-fs-9p.args +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-fs-9p.args @@ -1,4 +1,4 @@ -/usr/sbin/bhyve \ +bhyve \ -c 1 \ -m 214 \ -u \ diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-fs-9p.ldargs b/tests/bhyvexml2argvdata/bhyvexml2argv-fs-9p.ldargs index 91f9658137..5905f4b3e6 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-fs-9p.ldargs +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-fs-9p.ldargs @@ -1,4 +1,4 @@ -/usr/sbin/bhyveload \ +bhyveload \ -m 214 \ -d /tmp/freebsd.img \ bhyve diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-grub-bootorder.args b/tests/bhyvexml2argvdata/bhyvexml2argv-grub-bootorder.args index 1c400e1a7c..9ed28162ac 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-grub-bootorder.args +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-grub-bootorder.args @@ -1,4 +1,4 @@ -/usr/sbin/bhyve \ +bhyve \ -c 1 \ -m 214 \ -u \ diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-grub-bootorder.ldargs b/tests/bhyvexml2argvdata/bhyvexml2argv-grub-bootorder.ldargs index 9a50cf6537..56e2f27cfc 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-grub-bootorder.ldargs +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-grub-bootorder.ldargs @@ -1,4 +1,4 @@ -/usr/local/sbin/grub-bhyve \ +grub-bhyve \ --root hd0,msdos1 \ --device-map '<device.map>' \ --memory 214 \ diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-grub-bootorder2.args b/tests/bhyvexml2argvdata/bhyvexml2argv-grub-bootorder2.args index a09338d004..7ed87b2578 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-grub-bootorder2.args +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-grub-bootorder2.args @@ -1,4 +1,4 @@ -/usr/sbin/bhyve \ +bhyve \ -c 1 \ -m 214 \ -u \ diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-grub-bootorder2.ldargs b/tests/bhyvexml2argvdata/bhyvexml2argv-grub-bootorder2.ldargs index 9a50cf6537..56e2f27cfc 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-grub-bootorder2.ldargs +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-grub-bootorder2.ldargs @@ -1,4 +1,4 @@ -/usr/local/sbin/grub-bhyve \ +grub-bhyve \ --root hd0,msdos1 \ --device-map '<device.map>' \ --memory 214 \ diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-grub-defaults.args b/tests/bhyvexml2argvdata/bhyvexml2argv-grub-defaults.args index 91c06e3e3a..5a6ae06256 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-grub-defaults.args +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-grub-defaults.args @@ -1,4 +1,4 @@ -/usr/sbin/bhyve \ +bhyve \ -c 1 \ -m 214 \ -u \ diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-grub-defaults.ldargs b/tests/bhyvexml2argvdata/bhyvexml2argv-grub-defaults.ldargs index 9a50cf6537..56e2f27cfc 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-grub-defaults.ldargs +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-grub-defaults.ldargs @@ -1,4 +1,4 @@ -/usr/local/sbin/grub-bhyve \ +grub-bhyve \ --root hd0,msdos1 \ --device-map '<device.map>' \ --memory 214 \ diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-input-xhci-tablet.args b/tests/bhyvexml2argvdata/bhyvexml2argv-input-xhci-tablet.args index dfe1334edd..cb64d19aff 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-input-xhci-tablet.args +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-input-xhci-tablet.args @@ -1,4 +1,4 @@ -/usr/sbin/bhyve \ +bhyve \ -c 1 \ -m 214 \ -u \ diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-input-xhci-tablet.ldargs b/tests/bhyvexml2argvdata/bhyvexml2argv-input-xhci-tablet.ldargs index 91f9658137..5905f4b3e6 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-input-xhci-tablet.ldargs +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-input-xhci-tablet.ldargs @@ -1,4 +1,4 @@ -/usr/sbin/bhyveload \ +bhyveload \ -m 214 \ -d /tmp/freebsd.img \ bhyve diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.args b/tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.args index 498455275a..735a084ac7 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.args +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.args @@ -1,4 +1,4 @@ -/usr/sbin/bhyve \ +bhyve \ -c 1 \ -m 214 \ -u \ diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.ldargs b/tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.ldargs index 91f9658137..5905f4b3e6 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.ldargs +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.ldargs @@ -1,4 +1,4 @@ -/usr/sbin/bhyveload \ +bhyveload \ -m 214 \ -d /tmp/freebsd.img \ bhyve diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-localtime.args b/tests/bhyvexml2argvdata/bhyvexml2argv-localtime.args index e4f66080b9..c06ffceba1 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-localtime.args +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-localtime.args @@ -1,4 +1,4 @@ -/usr/sbin/bhyve \ +bhyve \ -c 1 \ -m 214 \ -H \ diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-localtime.ldargs b/tests/bhyvexml2argvdata/bhyvexml2argv-localtime.ldargs index 91f9658137..5905f4b3e6 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-localtime.ldargs +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-localtime.ldargs @@ -1,4 +1,4 @@ -/usr/sbin/bhyveload \ +bhyveload \ -m 214 \ -d /tmp/freebsd.img \ bhyve diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-macaddr.args b/tests/bhyvexml2argvdata/bhyvexml2argv-macaddr.args index c17f216cb6..1c737b66e7 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-macaddr.args +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-macaddr.args @@ -1,4 +1,4 @@ -/usr/sbin/bhyve \ +bhyve \ -c 1 \ -m 214 \ -u \ diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-macaddr.ldargs b/tests/bhyvexml2argvdata/bhyvexml2argv-macaddr.ldargs index 91f9658137..5905f4b3e6 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-macaddr.ldargs +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-macaddr.ldargs @@ -1,4 +1,4 @@ -/usr/sbin/bhyveload \ +bhyveload \ -m 214 \ -d /tmp/freebsd.img \ bhyve diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-msrs.args b/tests/bhyvexml2argvdata/bhyvexml2argv-msrs.args index 4b0e28ba41..60e5a60db2 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-msrs.args +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-msrs.args @@ -1,4 +1,4 @@ -/usr/sbin/bhyve \ +bhyve \ -c 1 \ -m 214 \ -w \ diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-msrs.ldargs b/tests/bhyvexml2argvdata/bhyvexml2argv-msrs.ldargs index 91f9658137..5905f4b3e6 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-msrs.ldargs +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-msrs.ldargs @@ -1,4 +1,4 @@ -/usr/sbin/bhyveload \ +bhyveload \ -m 214 \ -d /tmp/freebsd.img \ bhyve diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-net-e1000.args b/tests/bhyvexml2argvdata/bhyvexml2argv-net-e1000.args index f6f45bdf43..370929a232 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-net-e1000.args +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-net-e1000.args @@ -1,4 +1,4 @@ -/usr/sbin/bhyve \ +bhyve \ -c 1 \ -m 214 \ -u \ diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-net-e1000.ldargs b/tests/bhyvexml2argvdata/bhyvexml2argv-net-e1000.ldargs index 91f9658137..5905f4b3e6 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-net-e1000.ldargs +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-net-e1000.ldargs @@ -1,4 +1,4 @@ -/usr/sbin/bhyveload \ +bhyveload \ -m 214 \ -d /tmp/freebsd.img \ bhyve diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-serial-grub-nocons.args b/tests/bhyvexml2argvdata/bhyvexml2argv-serial-grub-nocons.args index 050452cfa2..f9ec58793d 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-serial-grub-nocons.args +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-serial-grub-nocons.args @@ -1,4 +1,4 @@ -/usr/sbin/bhyve \ +bhyve \ -c 1 \ -m 214 \ -u \ diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-serial-grub-nocons.ldargs b/tests/bhyvexml2argvdata/bhyvexml2argv-serial-grub-nocons.ldargs index 9a50cf6537..56e2f27cfc 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-serial-grub-nocons.ldargs +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-serial-grub-nocons.ldargs @@ -1,4 +1,4 @@ -/usr/local/sbin/grub-bhyve \ +grub-bhyve \ --root hd0,msdos1 \ --device-map '<device.map>' \ --memory 214 \ diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-serial-grub.args b/tests/bhyvexml2argvdata/bhyvexml2argv-serial-grub.args index f51fa0834c..330f791b85 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-serial-grub.args +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-serial-grub.args @@ -1,4 +1,4 @@ -/usr/sbin/bhyve \ +bhyve \ -c 1 \ -m 214 \ -u \ diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-serial-grub.ldargs b/tests/bhyvexml2argvdata/bhyvexml2argv-serial-grub.ldargs index 10f8d92e6c..c9d59cb0b8 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-serial-grub.ldargs +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-serial-grub.ldargs @@ -1,4 +1,4 @@ -/usr/local/sbin/grub-bhyve \ +grub-bhyve \ --root hd0,msdos1 \ --device-map '<device.map>' \ --memory 214 \ diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-serial.args b/tests/bhyvexml2argvdata/bhyvexml2argv-serial.args index df12646a88..06ba1ae513 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-serial.args +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-serial.args @@ -1,4 +1,4 @@ -/usr/sbin/bhyve \ +bhyve \ -c 1 \ -m 214 \ -u \ diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-serial.ldargs b/tests/bhyvexml2argvdata/bhyvexml2argv-serial.ldargs index 91f9658137..5905f4b3e6 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-serial.ldargs +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-serial.ldargs @@ -1,4 +1,4 @@ -/usr/sbin/bhyveload \ +bhyveload \ -m 214 \ -d /tmp/freebsd.img \ bhyve diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-sound.args b/tests/bhyvexml2argvdata/bhyvexml2argv-sound.args index 8a99690ae5..cb61b89ee5 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-sound.args +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-sound.args @@ -1,4 +1,4 @@ -/usr/sbin/bhyve \ +bhyve \ -c 1 \ -m 214 \ -u \ diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-sound.ldargs b/tests/bhyvexml2argvdata/bhyvexml2argv-sound.ldargs index 91f9658137..5905f4b3e6 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-sound.ldargs +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-sound.ldargs @@ -1,4 +1,4 @@ -/usr/sbin/bhyveload \ +bhyveload \ -m 214 \ -d /tmp/freebsd.img \ bhyve diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-uefi.args b/tests/bhyvexml2argvdata/bhyvexml2argv-uefi.args index 22a21065db..479712967d 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-uefi.args +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-uefi.args @@ -1,4 +1,4 @@ -/usr/sbin/bhyve \ +bhyve \ -c 1 \ -m 214 \ -u \ diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-vnc-autoport.args b/tests/bhyvexml2argvdata/bhyvexml2argv-vnc-autoport.args index 8b692c1877..3991f9fc16 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-vnc-autoport.args +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-vnc-autoport.args @@ -1,4 +1,4 @@ -/usr/sbin/bhyve \ +bhyve \ -c 1 \ -m 214 \ -u \ diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-vnc-password.args b/tests/bhyvexml2argvdata/bhyvexml2argv-vnc-password.args index 682b03b2e0..dfd3b0a285 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-vnc-password.args +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-vnc-password.args @@ -1,4 +1,4 @@ -/usr/sbin/bhyve \ +bhyve \ -c 1 \ -m 214 \ -u \ diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-vnc-resolution.args b/tests/bhyvexml2argvdata/bhyvexml2argv-vnc-resolution.args index 4b4d62a68c..52cae37f76 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-vnc-resolution.args +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-vnc-resolution.args @@ -1,4 +1,4 @@ -/usr/sbin/bhyve \ +bhyve \ -c 1 \ -m 214 \ -u \ diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-vnc-vgaconf-io.args b/tests/bhyvexml2argvdata/bhyvexml2argv-vnc-vgaconf-io.args index 960b2dfe5e..80e2bd7223 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-vnc-vgaconf-io.args +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-vnc-vgaconf-io.args @@ -1,4 +1,4 @@ -/usr/sbin/bhyve \ +bhyve \ -c 1 \ -m 214 \ -u \ diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-vnc-vgaconf-off.args b/tests/bhyvexml2argvdata/bhyvexml2argv-vnc-vgaconf-off.args index 0639161ad0..1975fdc4d7 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-vnc-vgaconf-off.args +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-vnc-vgaconf-off.args @@ -1,4 +1,4 @@ -/usr/sbin/bhyve \ +bhyve \ -c 1 \ -m 214 \ -u \ diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-vnc-vgaconf-on.args b/tests/bhyvexml2argvdata/bhyvexml2argv-vnc-vgaconf-on.args index caa9b0e9a6..e62dfff239 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-vnc-vgaconf-on.args +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-vnc-vgaconf-on.args @@ -1,4 +1,4 @@ -/usr/sbin/bhyve \ +bhyve \ -c 1 \ -m 214 \ -u \ diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-vnc.args b/tests/bhyvexml2argvdata/bhyvexml2argv-vnc.args index 0301aa666e..049e65227d 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-vnc.args +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-vnc.args @@ -1,4 +1,4 @@ -/usr/sbin/bhyve \ +bhyve \ -c 1 \ -m 214 \ -u \ diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-wired.args b/tests/bhyvexml2argvdata/bhyvexml2argv-wired.args index f2c415ece5..b6b0a18529 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-wired.args +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-wired.args @@ -1,4 +1,4 @@ -/usr/sbin/bhyve \ +bhyve \ -c 1 \ -m 214 \ -S \ diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-wired.ldargs b/tests/bhyvexml2argvdata/bhyvexml2argv-wired.ldargs index 91f9658137..5905f4b3e6 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-wired.ldargs +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-wired.ldargs @@ -1,4 +1,4 @@ -/usr/sbin/bhyveload \ +bhyveload \ -m 214 \ -d /tmp/freebsd.img \ bhyve diff --git a/tests/bhyvexml2argvtest.c b/tests/bhyvexml2argvtest.c index c4fe4ef17a..cdaa32f65c 100644 --- a/tests/bhyvexml2argvtest.c +++ b/tests/bhyvexml2argvtest.c @@ -86,13 +86,13 @@ static int testCompareXMLToArgvFiles(const char *xml, goto out; } - if (!(actualargv = virCommandToString(cmd, true))) + if (!(actualargv = virCommandToStringFull(cmd, true, true))) goto out; if (actualdm != NULL) virTrimSpaces(actualdm, NULL); - if (!(actualld = virCommandToString(ldcmd, true))) + if (!(actualld = virCommandToStringFull(ldcmd, true, true))) goto out; if (virTestCompareToFileFull(actualargv, cmdline, false) < 0) -- 2.30.2
On Mon, Apr 19, 2021 at 07:14:15PM +0200, Pavel Hrdina wrote:
Currently the tests would fail if the bhyve commands are installed in different path then /usr/bin. Strip the command path to not depend on the host environment.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- tests/bhyvexml2argvdata/bhyvexml2argv-acpiapic.args | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-acpiapic.ldargs | 2 +- .../bhyvexml2argv-addr-isa-controller-on-slot-1.args | 2 +- .../bhyvexml2argv-addr-isa-controller-on-slot-1.ldargs | 2 +- .../bhyvexml2argv-addr-isa-controller-on-slot-31.args | 2 +- .../bhyvexml2argv-addr-isa-controller-on-slot-31.ldargs | 2 +- .../bhyvexml2argv-addr-more-than-32-sata-disks.args | 2 +- .../bhyvexml2argv-addr-more-than-32-sata-disks.ldargs | 2 +- .../bhyvexml2argv-addr-multiple-sata-disks.args | 2 +- .../bhyvexml2argv-addr-multiple-sata-disks.ldargs | 2 +- .../bhyvexml2argv-addr-multiple-virtio-disks.args | 2 +- .../bhyvexml2argv-addr-multiple-virtio-disks.ldargs | 2 +- .../bhyvexml2argv-addr-no32devs-multiple-sata-disks.args | 2 +- .../bhyvexml2argv-addr-no32devs-multiple-sata-disks.ldargs | 2 +- .../bhyvexml2argv-addr-no32devs-single-sata-disk.args | 2 +- .../bhyvexml2argv-addr-no32devs-single-sata-disk.ldargs | 2 +- .../bhyvexml2argv-addr-non-isa-controller-on-slot-1.args | 2 +- .../bhyvexml2argv-addr-single-sata-disk.args | 2 +- .../bhyvexml2argv-addr-single-sata-disk.ldargs | 2 +- .../bhyvexml2argv-addr-single-virtio-disk.args | 2 +- .../bhyvexml2argv-addr-single-virtio-disk.ldargs | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-base.args | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-base.ldargs | 2 +- .../bhyvexml2argvdata/bhyvexml2argv-bhyveload-bootorder.args | 2 +- .../bhyvexml2argv-bhyveload-bootorder.ldargs | 2 +- .../bhyvexml2argvdata/bhyvexml2argv-bhyveload-bootorder1.args | 2 +- .../bhyvexml2argv-bhyveload-bootorder1.ldargs | 2 +- .../bhyvexml2argvdata/bhyvexml2argv-bhyveload-bootorder3.args | 2 +- .../bhyvexml2argv-bhyveload-bootorder3.ldargs | 2 +- .../bhyvexml2argv-bhyveload-explicitargs.args | 2 +- .../bhyvexml2argv-bhyveload-explicitargs.ldargs | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-commandline.args | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-commandline.ldargs | 2 +- .../bhyvexml2argv-console-master-slave-not-specified.args | 2 +- .../bhyvexml2argv-console-master-slave-not-specified.ldargs | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-console.args | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-console.ldargs | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-cputopology.args | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-cputopology.ldargs | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-custom-loader.args | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-custom-loader.ldargs | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-disk-cdrom-grub.args | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-disk-cdrom-grub.ldargs | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-disk-cdrom.args | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-disk-cdrom.ldargs | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-disk-virtio.args | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-disk-virtio.ldargs | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-firmware-efi.args | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-fs-9p-readonly.args | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-fs-9p-readonly.ldargs | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-fs-9p.args | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-fs-9p.ldargs | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-grub-bootorder.args | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-grub-bootorder.ldargs | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-grub-bootorder2.args | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-grub-bootorder2.ldargs | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-grub-defaults.args | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-grub-defaults.ldargs | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-input-xhci-tablet.args | 2 +- .../bhyvexml2argvdata/bhyvexml2argv-input-xhci-tablet.ldargs | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.args | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.ldargs | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-localtime.args | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-localtime.ldargs | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-macaddr.args | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-macaddr.ldargs | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-msrs.args | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-msrs.ldargs | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-net-e1000.args | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-net-e1000.ldargs | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-serial-grub-nocons.args | 2 +- .../bhyvexml2argvdata/bhyvexml2argv-serial-grub-nocons.ldargs | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-serial-grub.args | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-serial-grub.ldargs | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-serial.args | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-serial.ldargs | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-sound.args | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-sound.ldargs | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-uefi.args | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-vnc-autoport.args | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-vnc-password.args | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-vnc-resolution.args | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-vnc-vgaconf-io.args | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-vnc-vgaconf-off.args | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-vnc-vgaconf-on.args | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-vnc.args | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-wired.args | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-wired.ldargs | 2 +- tests/bhyvexml2argvtest.c | 4 ++-- 89 files changed, 90 insertions(+), 90 deletions(-)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
Future patch will remove MKFS define as we will no longer check it during compilation. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/storage/storage_backend_fs.c | 24 +++++++++--------------- 1 file changed, 9 insertions(+), 15 deletions(-) diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index bb93d64bbe..b0f8070c6c 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -397,13 +397,20 @@ virStorageBackendFileSystemCheck(virStoragePoolObj *pool, return 0; } -/* some platforms don't support mkfs */ -#ifdef MKFS static int virStorageBackendExecuteMKFS(const char *device, const char *format) { g_autoptr(virCommand) cmd = NULL; + g_autofree char *mkfs = virFindFileInPath(MKFS); + + if (!mkfs) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("mkfs is not supported on this platform: " + "Failed to make filesystem of type '%s' on device '%s'"), + format, device); + return -1; + } cmd = virCommandNewArgList(MKFS, "-t", format, NULL); @@ -426,19 +433,6 @@ virStorageBackendExecuteMKFS(const char *device, return 0; } -#else /* #ifdef MKFS */ -static int -virStorageBackendExecuteMKFS(const char *device G_GNUC_UNUSED, - const char *format G_GNUC_UNUSED) -{ - virReportError(VIR_ERR_INTERNAL_ERROR, - _("mkfs is not supported on this platform: " - "Failed to make filesystem of " - "type '%s' on device '%s'"), - format, device); - return -1; -} -#endif /* #ifdef MKFS */ static int virStorageBackendMakeFileSystem(virStoragePoolObj *pool, -- 2.30.2
On Mon, Apr 19, 2021 at 07:14:16PM +0200, Pavel Hrdina wrote:
Future patch will remove MKFS define as we will no longer check it during compilation.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/storage/storage_backend_fs.c | 24 +++++++++--------------- 1 file changed, 9 insertions(+), 15 deletions(-)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index bb93d64bbe..b0f8070c6c 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -397,13 +397,20 @@ virStorageBackendFileSystemCheck(virStoragePoolObj *pool, return 0; }
-/* some platforms don't support mkfs */ -#ifdef MKFS static int virStorageBackendExecuteMKFS(const char *device, const char *format) { g_autoptr(virCommand) cmd = NULL; + g_autofree char *mkfs = virFindFileInPath(MKFS); + + if (!mkfs) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("mkfs is not supported on this platform: "
s/supported/available/ since it may simply be uninstalled now.
+ "Failed to make filesystem of type '%s' on device '%s'"), + format, device); + return -1; + }
Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On Tue, Apr 20, 2021 at 02:44:53PM +0100, Daniel P. Berrangé wrote:
On Mon, Apr 19, 2021 at 07:14:16PM +0200, Pavel Hrdina wrote:
Future patch will remove MKFS define as we will no longer check it during compilation.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/storage/storage_backend_fs.c | 24 +++++++++--------------- 1 file changed, 9 insertions(+), 15 deletions(-)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Thanks for the review. I've pushed the cleanup patches and will post v3 without the virFindFileInPath move from testutilsqemu.c. Pavel
Extend virFindFileInPath to search in custom extra paths as well. Some binaries that libvirt needs are not usually in $PATH so we need to have a way to look for these as well. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/libvirt_private.syms | 2 +- src/util/virfile.c | 16 ++++++++++++++-- src/util/virfile.h | 5 ++++- tests/domaincapsmock.c | 3 ++- tests/virfirewallmock.c | 3 ++- 5 files changed, 23 insertions(+), 6 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index abd3dc4bd1..717686ddee 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2254,7 +2254,7 @@ virFileWrapperFdClose; virFileWrapperFdFree; virFileWrapperFdNew; virFileWriteStr; -virFindFileInPath; +virFindFileInPathFull; # util/virfilecache.h diff --git a/src/util/virfile.c b/src/util/virfile.c index 03a7725dd3..7922fda2e5 100644 --- a/src/util/virfile.c +++ b/src/util/virfile.c @@ -1654,13 +1654,15 @@ virFileIsLink(const char *linkpath) } /* - * Finds a requested executable file in the PATH env. e.g.: + * Finds a requested executable file in the paths provided by @extra_paths + * argument or in PATH env. e.g.: * "qemu-img" will return "/usr/bin/qemu-img" * * You must free the result */ char * -virFindFileInPath(const char *file) +virFindFileInPathFull(const char *file, + const GStrv extra_paths) { const char *origpath = NULL; g_auto(GStrv) paths = NULL; @@ -1692,6 +1694,16 @@ virFindFileInPath(const char *file) return abspath; } + /* First search in paths provided by caller. + */ + if (extra_paths) { + for (pathiter = extra_paths; *pathiter; pathiter++) { + g_autofree char *fullpath = g_strdup_printf("%s/%s", *pathiter, file); + if (virFileIsExecutable(fullpath)) + return g_steal_pointer(&fullpath); + } + } + /* copy PATH env so we can tweak it */ origpath = getenv("PATH"); if (!origpath) diff --git a/src/util/virfile.h b/src/util/virfile.h index b9f7b1766f..eee27c2efc 100644 --- a/src/util/virfile.h +++ b/src/util/virfile.h @@ -184,9 +184,12 @@ int virFileResolveAllLinks(const char *linkpath, int virFileIsLink(const char *linkpath) ATTRIBUTE_NONNULL(1) G_GNUC_WARN_UNUSED_RESULT; -char *virFindFileInPath(const char *file) +char *virFindFileInPathFull(const char *file, + const GStrv extra_paths) G_GNUC_NO_INLINE; +#define virFindFileInPath(file) virFindFileInPathFull(file, NULL) + char *virFileFindResource(const char *filename, const char *builddir, const char *installdir) diff --git a/tests/domaincapsmock.c b/tests/domaincapsmock.c index 73690f0b9e..4bef2b72d0 100644 --- a/tests/domaincapsmock.c +++ b/tests/domaincapsmock.c @@ -43,7 +43,8 @@ virHostCPUGetMicrocodeVersion(virArch hostArch G_GNUC_UNUSED) } char * -virFindFileInPath(const char *file) +virFindFileInPathFull(const char *file, + const GStrv extra_args G_GNUC_UNUSED) { if (g_str_has_prefix(file, "qemu-system") || g_str_equal(file, "qemu-kvm")) { diff --git a/tests/virfirewallmock.c b/tests/virfirewallmock.c index 6b096701c9..a047e56bd9 100644 --- a/tests/virfirewallmock.c +++ b/tests/virfirewallmock.c @@ -20,7 +20,8 @@ #include "virfile.h" char * -virFindFileInPath(const char *file) +virFindFileInPathFull(const char *file, + const GStrv extra_args G_GNUC_UNUSED) { if (file && (g_strrstr(file, "ebtables") || -- 2.30.2
Following patches will stop detecting the full path during compilation so we will need to do it here. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_conf.c | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 916a3d36ee..437b3ce2be 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -109,6 +109,12 @@ virQEMUDriverConfig *virQEMUDriverConfigNew(bool privileged, const char *root) { g_autoptr(virQEMUDriverConfig) cfg = NULL; + const char *qemu_extra_paths[] = { + "/usr/libexec", + "/usr/lib/qemu", + "/usr/lib", + NULL, + }; if (virQEMUConfigInitialize() < 0) return NULL; @@ -267,10 +273,14 @@ virQEMUDriverConfig *virQEMUDriverConfigNew(bool privileged, return NULL; } - cfg->bridgeHelperName = g_strdup(QEMU_BRIDGE_HELPER); - cfg->prHelperName = g_strdup(QEMU_PR_HELPER); - cfg->slirpHelperName = g_strdup(QEMU_SLIRP_HELPER); - cfg->dbusDaemonName = g_strdup(QEMU_DBUS_DAEMON); + cfg->bridgeHelperName = virFindFileInPathFull(QEMU_BRIDGE_HELPER, + (GStrv) qemu_extra_paths); + cfg->prHelperName = virFindFileInPathFull(QEMU_PR_HELPER, + (GStrv) qemu_extra_paths); + cfg->slirpHelperName = virFindFileInPathFull(QEMU_SLIRP_HELPER, + (GStrv) qemu_extra_paths); + cfg->dbusDaemonName = virFindFileInPathFull(QEMU_DBUS_DAEMON, + (GStrv) qemu_extra_paths); cfg->securityDefaultConfined = true; cfg->securityRequireConfined = false; -- 2.30.2
These binaries are used only during runtime so technically there is no need to check for them while compiling libvirt. Usually the location is the same while compiling and running but it may not be true. In addition they are not strictly required to compile the code so this way developers don't have to install it or create fake binaries in order to compile the code. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- meson.build | 63 ---------------------------- src/network/bridge_driver.c | 2 + src/node_device/node_device_driver.c | 2 + src/qemu/qemu_conf.c | 5 +++ src/storage/storage_util.c | 2 + src/util/virdnsmasq.c | 1 + src/util/virfirewall.h | 4 ++ src/util/virkmod.h | 3 ++ src/util/virnetdevbandwidth.h | 2 + src/util/virnetdevip.c | 2 + src/util/virnetdevmidonet.c | 2 + src/util/virnetdevopenvswitch.c | 2 + src/util/virsysinfo.c | 1 + src/util/virutil.c | 2 + 14 files changed, 30 insertions(+), 63 deletions(-) diff --git a/meson.build b/meson.build index 837955de37..fc8fc4f252 100644 --- a/meson.build +++ b/meson.build @@ -926,24 +926,9 @@ endforeach optional_programs = [ 'augparse', - 'dmidecode', - 'dnsmasq', - 'ebtables', 'flake8', - 'ip', - 'ip6tables', - 'iptables', 'iscsiadm', - 'mdevctl', - 'mm-ctl', - 'modprobe', - 'ovs-vsctl', 'pdwtags', - 'radvd', - 'rmmod', - 'scrub', - 'tc', - 'udevadm', ] foreach name : optional_programs @@ -1706,54 +1691,6 @@ if not get_option('driver_qemu').disabled() endif conf.set_quoted('QEMU_USER', qemu_user) conf.set_quoted('QEMU_GROUP', qemu_group) - - qemu_bridge_prog = find_program( - 'qemu-bridge-helper', - dirs: [ '/usr/libexec', '/usr/lib/qemu', '/usr/lib' ], - required: false - ) - if qemu_bridge_prog.found() - qemu_bridge_path = qemu_bridge_prog.path() - else - qemu_bridge_path = '/usr/libexec/qemu-bridge-helper' - endif - conf.set_quoted('QEMU_BRIDGE_HELPER', qemu_bridge_path) - - qemu_pr_prog = find_program( - 'qemu-pr-helper', - dirs: [ '/usr/bin', '/usr/libexec' ], - required: false - ) - if qemu_pr_prog.found() - qemu_pr_path = qemu_pr_prog.path() - else - qemu_pr_path = '/usr/bin/qemu-pr-helper' - endif - conf.set_quoted('QEMU_PR_HELPER', qemu_pr_path) - - qemu_slirp_prog = find_program( - 'slirp-helper', - dirs: [ '/usr/bin', '/usr/libexec' ], - required: false - ) - if qemu_slirp_prog.found() - qemu_slirp_path = qemu_slirp_prog.path() - else - qemu_slirp_path = '/usr/bin/slirp-helper' - endif - conf.set_quoted('QEMU_SLIRP_HELPER', qemu_slirp_path) - - qemu_dbus_daemon_prog = find_program( - 'dbus-daemon', - dirs: [ '/usr/bin', '/usr/libexec' ], - required: false - ) - if qemu_dbus_daemon_prog.found() - qemu_dbus_daemon_path = qemu_dbus_daemon_prog.path() - else - qemu_dbus_daemon_path = '/usr/bin/dbus-daemon' - endif - conf.set_quoted('QEMU_DBUS_DAEMON', qemu_dbus_daemon_path) endif endif diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index ee3f9dab0a..8e1a42b119 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -74,6 +74,8 @@ #define VIR_FROM_THIS VIR_FROM_NETWORK #define MAX_BRIDGE_ID 256 +#define RADVD "radvd" + static virMutex bridgeNameValidateMutex = VIR_MUTEX_INITIALIZER; /** diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c index b0cc63ed42..bf70083f2a 100644 --- a/src/node_device/node_device_driver.c +++ b/src/node_device/node_device_driver.c @@ -48,6 +48,8 @@ VIR_LOG_INIT("node_device.node_device_driver"); +#define MDEVCTL "mdevctl" + virNodeDeviceDriverState *driver; diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 437b3ce2be..d228504d8c 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -55,6 +55,11 @@ VIR_LOG_INIT("qemu.qemu_conf"); +#define QEMU_BRIDGE_HELPER "qemu-bridge-helper" +#define QEMU_PR_HELPER "qemu-pr-helper" +#define QEMU_SLIRP_HELPER "slirp-helper" +#define QEMU_DBUS_DAEMON "dbus-daemon" + /* These are only defaults, they can be changed now in qemu.conf and * explicitly specified port is checked against these two (makes * sense to limit the values). diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c index 7efadc2197..c225f2ab6b 100644 --- a/src/storage/storage_util.c +++ b/src/storage/storage_util.c @@ -84,6 +84,8 @@ VIR_LOG_INIT("storage.storage_util"); # define S_IRWXUGO (S_IRWXU | S_IRWXG | S_IRWXO) #endif +#define SCRUB "scrub" + /* virStorageBackendNamespaceInit: * @poolType: virStoragePoolType * @xmlns: Storage Pool specific namespace callback methods diff --git a/src/util/virdnsmasq.c b/src/util/virdnsmasq.c index f2f606913f..b666c2f9d3 100644 --- a/src/util/virdnsmasq.c +++ b/src/util/virdnsmasq.c @@ -48,6 +48,7 @@ VIR_LOG_INIT("util.dnsmasq"); #define DNSMASQ_HOSTSFILE_SUFFIX "hostsfile" #define DNSMASQ_ADDNHOSTSFILE_SUFFIX "addnhosts" +#define DNSMASQ "dnsmasq" static void dhcphostFreeContent(dnsmasqDhcpHost *host) diff --git a/src/util/virfirewall.h b/src/util/virfirewall.h index 169d99fe2b..a3954c5ed0 100644 --- a/src/util/virfirewall.h +++ b/src/util/virfirewall.h @@ -22,6 +22,10 @@ #include "internal.h" +#define EBTABLES_PATH "ebtables" +#define IPTABLES_PATH "iptables" +#define IP6TABLES_PATH "ip6tables" + typedef struct _virFirewall virFirewall; typedef struct _virFirewallRule virFirewallRule; diff --git a/src/util/virkmod.h b/src/util/virkmod.h index bb043d4876..58311909eb 100644 --- a/src/util/virkmod.h +++ b/src/util/virkmod.h @@ -23,6 +23,9 @@ #include "internal.h" +#define MODPROBE "modprobe" +#define RMMOD "rmmod" + char *virKModLoad(const char *) ATTRIBUTE_NONNULL(1); char *virKModUnload(const char *) diff --git a/src/util/virnetdevbandwidth.h b/src/util/virnetdevbandwidth.h index 3d520721f6..6c91bd0f33 100644 --- a/src/util/virnetdevbandwidth.h +++ b/src/util/virnetdevbandwidth.h @@ -21,6 +21,8 @@ #include "internal.h" #include "virmacaddr.h" +#define TC "tc" + typedef struct _virNetDevBandwidthRate virNetDevBandwidthRate; struct _virNetDevBandwidthRate { unsigned long long average; /* kbytes/s */ diff --git a/src/util/virnetdevip.c b/src/util/virnetdevip.c index 0ce8f5b536..f4cf6ac39f 100644 --- a/src/util/virnetdevip.c +++ b/src/util/virnetdevip.c @@ -50,6 +50,8 @@ #define VIR_FROM_THIS VIR_FROM_NONE +#define IP_PATH "ip" + VIR_LOG_INIT("util.netdevip"); #if defined(WITH_LIBNL) diff --git a/src/util/virnetdevmidonet.c b/src/util/virnetdevmidonet.c index 9061f1516f..735607e5df 100644 --- a/src/util/virnetdevmidonet.c +++ b/src/util/virnetdevmidonet.c @@ -26,6 +26,8 @@ #define VIR_FROM_THIS VIR_FROM_NONE +#define MM_CTL "mm-ctl" + /** * virNetDevMidonetBindPort: * @ifname: the network interface name diff --git a/src/util/virnetdevopenvswitch.c b/src/util/virnetdevopenvswitch.c index 21ee4bdd42..a05128dbca 100644 --- a/src/util/virnetdevopenvswitch.c +++ b/src/util/virnetdevopenvswitch.c @@ -35,6 +35,8 @@ VIR_LOG_INIT("util.netdevopenvswitch"); +#define OVS_VSCTL "ovs-vsctl" + /* * Set openvswitch default timeout */ diff --git a/src/util/virsysinfo.c b/src/util/virsysinfo.c index af9e03c5ac..8bbda9ea69 100644 --- a/src/util/virsysinfo.c +++ b/src/util/virsysinfo.c @@ -53,6 +53,7 @@ static const char *sysinfoCpuinfo = "/proc/cpuinfo"; #define SYSINFO sysinfoSysinfo #define CPUINFO sysinfoCpuinfo #define CPUINFO_FILE_LEN (1024*1024) /* 1MB limit for /proc/cpuinfo file */ +#define DMIDECODE "dmidecode" void diff --git a/src/util/virutil.c b/src/util/virutil.c index 3f49a469e5..b7a9808f6a 100644 --- a/src/util/virutil.c +++ b/src/util/virutil.c @@ -57,6 +57,8 @@ #include "virutil.h" #include "virsocket.h" +#define UDEVADM "udevadm" + G_STATIC_ASSERT(sizeof(gid_t) <= sizeof(unsigned int) && sizeof(uid_t) <= sizeof(unsigned int)); -- 2.30.2
This requires to define the binary name in code because we compile src/util/viriscsi.c unconditionally. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- meson.build | 22 ++++++++++++++++------ src/util/viriscsi.h | 2 ++ 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/meson.build b/meson.build index fc8fc4f252..8c71e803bf 100644 --- a/meson.build +++ b/meson.build @@ -927,7 +927,6 @@ endforeach optional_programs = [ 'augparse', 'flake8', - 'iscsiadm', 'pdwtags', ] @@ -1802,11 +1801,22 @@ if conf.has('WITH_LIBVIRTD') error('Need glusterfs (libgfapi) for gluster storage driver') endif - if not get_option('storage_iscsi').disabled() and iscsiadm_prog.found() - use_storage = true - conf.set('WITH_STORAGE_ISCSI', 1) - elif get_option('storage_iscsi').enabled() - error('We need iscsiadm for iSCSI storage driver') + if not get_option('storage_iscsi').disabled() + iscsi_enable = true + iscsiadm_prog = find_program('iscsiadm', required: false, dirs: libvirt_sbin_path) + + if not iscsiadm_prog.found() + if get_option('storage_iscsi').enabled() + error('We need iscsiadm for iSCSI storage driver') + else + iscsi_enable = false + endif + endif + + if iscsi_enable + use_storage = true + conf.set('WITH_STORAGE_ISCSI', 1) + endif endif if not get_option('storage_iscsi_direct').disabled() and libiscsi_dep.found() diff --git a/src/util/viriscsi.h b/src/util/viriscsi.h index 6d17d139eb..fa7ff280c2 100644 --- a/src/util/viriscsi.h +++ b/src/util/viriscsi.h @@ -23,6 +23,8 @@ #include "internal.h" +#define ISCSIADM "iscsiadm" + char * virISCSIGetSession(const char *devpath, bool probe) -- 2.30.2
Technically the location of these binaries may be different when compiling libvirt or running it. This will allow users to change $PATH to use different binaries as well. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- meson.build | 41 ++++---------------------- src/bhyve/bhyve_command.c | 4 +++ src/locking/lock_driver_lockd.c | 12 ++------ src/storage/storage_backend_logical.c | 13 ++++++++ src/storage/storage_backend_sheepdog.c | 2 ++ src/storage/storage_backend_zfs.c | 3 ++ src/storage/storage_util.h | 6 ++++ src/util/virnuma.c | 1 + 8 files changed, 36 insertions(+), 46 deletions(-) diff --git a/meson.build b/meson.build index 8c71e803bf..5d8eb7f95c 100644 --- a/meson.build +++ b/meson.build @@ -1074,9 +1074,7 @@ libparted_version = '1.8.0' libparted_dep = dependency('libparted', version: '>=' + libparted_version, required: false) if libparted_dep.found() parted_prog = find_program('parted', required: false, dirs: libvirt_sbin_path) - if parted_prog.found() - conf.set_quoted('PARTED', parted_prog.path()) - else + if not parted_prog.found() libparted_dep = dependency('', required: false) endif endif @@ -1488,9 +1486,6 @@ if not get_option('driver_bhyve').disabled() and host_machine.system() == 'freeb if bhyve_prog.found() and bhyvectl_prog.found() and bhyveload_prog.found() conf.set('WITH_BHYVE', 1) - conf.set_quoted('BHYVE', bhyve_prog.path()) - conf.set_quoted('BHYVECTL', bhyvectl_prog.path()) - conf.set_quoted('BHYVELOAD', bhyveload_prog.path()) endif elif get_option('driver_bhyve').enabled() error('The bhyve driver cannot be enabled') @@ -1779,18 +1774,7 @@ if conf.has('WITH_LIBVIRTD') if fs_enable use_storage = true - conf.set('WITH_STORAGE_FS', 1) - conf.set_quoted('MOUNT', mount_prog.path()) - conf.set_quoted('UMOUNT', umount_prog.path()) - conf.set_quoted('MKFS', mkfs_prog.path()) - - showmount_prog = find_program('showmount', required: false, dirs: libvirt_sbin_path) - showmount_path = '' - if showmount_prog.found() - showmount_path = showmount_prog.path() - endif - conf.set_quoted('SHOWMOUNT', showmount_path) endif endif @@ -1835,11 +1819,8 @@ if conf.has('WITH_LIBVIRTD') 'pvs', 'vgs', 'lvs', ] foreach name : lvm_progs - set_variable( - '@0@_prog'.format(name), - find_program(name, required: get_option('storage_lvm'), dirs: libvirt_sbin_path) - ) - if not get_variable('@0@_prog'.format(name)).found() + lvm_prog = find_program(name, required: get_option('storage_lvm'), dirs: libvirt_sbin_path) + if not lvm_prog.found() lvm_enable = false endif endforeach @@ -1847,10 +1828,6 @@ if conf.has('WITH_LIBVIRTD') if lvm_enable use_storage = true conf.set('WITH_STORAGE_LVM', 1) - - foreach name : lvm_progs - conf.set_quoted(name.to_upper(), get_variable('@0@_prog'.format(name)).path()) - endforeach endif endif @@ -1879,7 +1856,6 @@ if conf.has('WITH_LIBVIRTD') if sheepdogcli_prog.found() use_storage = true conf.set('WITH_STORAGE_SHEEPDOG', 1) - conf.set_quoted('SHEEPDOGCLI', sheepdogcli_prog.path()) endif endif @@ -1902,11 +1878,8 @@ if conf.has('WITH_LIBVIRTD') if not get_option('storage_zfs').disabled() zfs_enable = true foreach name : ['zfs', 'zpool'] - set_variable( - '@0@_prog'.format(name), - find_program(name, required: get_option('storage_zfs'), dirs: libvirt_sbin_path) - ) - if not get_variable('@0@_prog'.format(name)).found() + zfs_prog = find_program(name, required: get_option('storage_zfs'), dirs: libvirt_sbin_path) + if not zfs_prog.found() zfs_enable = false endif endforeach @@ -1914,9 +1887,6 @@ if conf.has('WITH_LIBVIRTD') if zfs_enable use_storage = true conf.set('WITH_STORAGE_ZFS', 1) - foreach name : ['zfs', 'zpool'] - conf.set_quoted(name.to_upper(), get_variable('@0@_prog'.format(name)).path()) - endforeach endif endif endif @@ -2030,7 +2000,6 @@ if not get_option('numad').disabled() and numactl_dep.found() numad_prog = find_program('numad', required: get_option('numad'), dirs: libvirt_sbin_path) if numad_prog.found() conf.set('WITH_NUMAD', 1) - conf.set_quoted('NUMAD', numad_prog.path()) endif elif get_option('numad').enabled() error('You must have numactl enabled for numad support.') diff --git a/src/bhyve/bhyve_command.c b/src/bhyve/bhyve_command.c index f8e0ce5123..061138c8ec 100644 --- a/src/bhyve/bhyve_command.c +++ b/src/bhyve/bhyve_command.c @@ -37,6 +37,10 @@ #define VIR_FROM_THIS VIR_FROM_BHYVE +#define BHYVE "bhyve" +#define BHYVECTL "bhyvectl" +#define BHYVELOAD "bhyveload" + VIR_LOG_INIT("bhyve.bhyve_command"); static int diff --git a/src/locking/lock_driver_lockd.c b/src/locking/lock_driver_lockd.c index 823b918db3..ea787bf87d 100644 --- a/src/locking/lock_driver_lockd.c +++ b/src/locking/lock_driver_lockd.c @@ -454,7 +454,8 @@ static int virLockManagerLockDaemonNew(virLockManager *lock, } -#ifdef LVS +#define LVS "lvs" + static int virLockManagerGetLVMKey(const char *path, char **key) @@ -508,15 +509,6 @@ virLockManagerGetLVMKey(const char *path, return ret; } -#else -static int -virLockManagerGetLVMKey(const char *path, - char **key G_GNUC_UNUSED) -{ - virReportSystemError(ENOSYS, _("Unable to get LVM key for %s"), path); - return -1; -} -#endif static int virLockManagerLockDaemonAddResource(virLockManager *lock, diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c index ed8e47d880..4bbb585d07 100644 --- a/src/storage/storage_backend_logical.c +++ b/src/storage/storage_backend_logical.c @@ -40,6 +40,19 @@ VIR_LOG_INIT("storage.storage_backend_logical"); +#define PVCREATE "pvcreate" +#define PVREMOVE "pvremove" +#define PVS "pvs" +#define VGCREATE "vgcreate" +#define VGREMOVE "vgremove" +#define VGCHANGE "vgchange" +#define VGSCAN "vgscan" +#define VGS "vgs" +#define LVCREATE "lvcreate" +#define LVREMOVE "lvremove" +#define LVCHANGE "lvchange" +#define LVS "lvs" + static int virStorageBackendLogicalSetActive(virStoragePoolObj *pool, diff --git a/src/storage/storage_backend_sheepdog.c b/src/storage/storage_backend_sheepdog.c index 6490dfae52..beb6445e1e 100644 --- a/src/storage/storage_backend_sheepdog.c +++ b/src/storage/storage_backend_sheepdog.c @@ -35,6 +35,8 @@ #define VIR_FROM_THIS VIR_FROM_STORAGE +#define SHEEPDOGCLI "dog" + static int virStorageBackendSheepdogRefreshVol(virStoragePoolObj *pool, virStorageVolDef *vol); diff --git a/src/storage/storage_backend_zfs.c b/src/storage/storage_backend_zfs.c index 6fabeade11..2a5d74357d 100644 --- a/src/storage/storage_backend_zfs.c +++ b/src/storage/storage_backend_zfs.c @@ -33,6 +33,9 @@ VIR_LOG_INIT("storage.storage_backend_zfs"); +#define ZFS "zfs" +#define ZPOOL "zpool" + /* * Some common flags of zfs and zpool commands we use: * -H -- don't print headers and separate fields by tab diff --git a/src/storage/storage_util.h b/src/storage/storage_util.h index aa3c25e9fc..487345ff22 100644 --- a/src/storage/storage_util.h +++ b/src/storage/storage_util.h @@ -25,6 +25,12 @@ #include "storage_driver.h" #include "storage_backend.h" +#define PARTED "parted" +#define MOUNT "mount" +#define UMOUNT "umount" +#define MKFS "mkfs" +#define SHOWMOUNT "showmount" + /* Storage Pool Namespace options to share w/ storage_backend_fs.c and * the virStorageBackendFileSystemMountCmd method */ typedef struct _virStoragePoolFSMountOptionsDef virStoragePoolFSMountOptionsDef; diff --git a/src/util/virnuma.c b/src/util/virnuma.c index 31f65d8902..d3cdc3ba53 100644 --- a/src/util/virnuma.c +++ b/src/util/virnuma.c @@ -52,6 +52,7 @@ VIR_LOG_INIT("util.numa"); +#define NUMAD "numad" #if WITH_NUMAD char * -- 2.30.2
This way meson will try to do the right thing by default but we will allow users to change this behavior by using -Dname=enabled. This comes with two benefits compared to the previous behavior: - no need to install the binaries if developers would like to check that the code compiles correctly - package maintainers can drop some build dependencies Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- meson.build | 102 ++++++++++++++++++++++++++++++++-------------------- 1 file changed, 63 insertions(+), 39 deletions(-) diff --git a/meson.build b/meson.build index 5d8eb7f95c..a7d402cd31 100644 --- a/meson.build +++ b/meson.build @@ -1072,12 +1072,6 @@ endif libparted_version = '1.8.0' libparted_dep = dependency('libparted', version: '>=' + libparted_version, required: false) -if libparted_dep.found() - parted_prog = find_program('parted', required: false, dirs: libvirt_sbin_path) - if not parted_prog.found() - libparted_dep = dependency('', required: false) - endif -endif libpcap_version = '1.5.0' if not get_option('libpcap').disabled() @@ -1480,11 +1474,17 @@ if not get_option('driver_libvirtd').disabled() endif if not get_option('driver_bhyve').disabled() and host_machine.system() == 'freebsd' - bhyve_prog = find_program('bhyve', required: get_option('driver_bhyve')) - bhyvectl_prog = find_program('bhyvectl', required: get_option('driver_bhyve')) - bhyveload_prog = find_program('bhyveload', required: get_option('driver_bhyve')) + bhyve_enable = true - if bhyve_prog.found() and bhyvectl_prog.found() and bhyveload_prog.found() + if get_option('driver_bhyve').auto() + bhyve_prog = find_program('bhyve', required: false) + bhyvectl_prog = find_program('bhyvectl', required: false) + bhyveload_prog = find_program('bhyveload', required: false) + + bhyve_enable = bhyve_prog.found() and bhyvectl_prog.found() and bhyveload_prog.found() + endif + + if bhyve_enable conf.set('WITH_BHYVE', 1) endif elif get_option('driver_bhyve').enabled() @@ -1740,8 +1740,17 @@ if conf.has('WITH_LIBVIRTD') endif if not get_option('storage_disk').disabled() and devmapper_dep.found() and libparted_dep.found() - use_storage = true - conf.set('WITH_STORAGE_DISK', 1) + disk_enable = true + + if get_option('storage_disk').auto() + parted_prog = find_program('parted', required: false, dirs: libvirt_sbin_path) + disk_enable = parted_prog.found() + endif + + if disk_enable + use_storage = true + conf.set('WITH_STORAGE_DISK', 1) + endif elif get_option('storage_disk').enabled() error('You must install libparted and libdevmapper to compile libvirt with disk storage driver') endif @@ -1762,10 +1771,10 @@ if conf.has('WITH_LIBVIRTD') endif endif - if fs_enable - mount_prog = find_program('mount', required: get_option('storage_fs'), dirs: libvirt_sbin_path) - umount_prog = find_program('umount', required: get_option('storage_fs'), dirs: libvirt_sbin_path) - mkfs_prog = find_program('mkfs', required: get_option('storage_fs'), dirs: libvirt_sbin_path) + if fs_enable and get_option('storage_fs').auto() + mount_prog = find_program('mount', required: false, dirs: libvirt_sbin_path) + umount_prog = find_program('umount', required: false, dirs: libvirt_sbin_path) + mkfs_prog = find_program('mkfs', required: false, dirs: libvirt_sbin_path) if not mount_prog.found() or not umount_prog.found() or not mkfs_prog.found() fs_enable = false @@ -1787,14 +1796,11 @@ if conf.has('WITH_LIBVIRTD') if not get_option('storage_iscsi').disabled() iscsi_enable = true - iscsiadm_prog = find_program('iscsiadm', required: false, dirs: libvirt_sbin_path) - if not iscsiadm_prog.found() - if get_option('storage_iscsi').enabled() - error('We need iscsiadm for iSCSI storage driver') - else - iscsi_enable = false - endif + if get_option('storage_iscsi').auto() + iscsiadm_prog = find_program('iscsiadm', required: false, dirs: libvirt_sbin_path) + + iscsi_enable = iscsiadm_prog.found() endif if iscsi_enable @@ -1818,12 +1824,15 @@ if conf.has('WITH_LIBVIRTD') 'lvchange', 'vgchange', 'vgscan', 'pvs', 'vgs', 'lvs', ] - foreach name : lvm_progs - lvm_prog = find_program(name, required: get_option('storage_lvm'), dirs: libvirt_sbin_path) - if not lvm_prog.found() - lvm_enable = false - endif - endforeach + + if get_option('storage_lvm').auto() + foreach name : lvm_progs + lvm_prog = find_program(name, required: false, dirs: libvirt_sbin_path) + if not lvm_prog.found() + lvm_enable = false + endif + endforeach + endif if lvm_enable use_storage = true @@ -1851,9 +1860,15 @@ if conf.has('WITH_LIBVIRTD') endif if not get_option('storage_sheepdog').disabled() - sheepdogcli_prog = find_program('dog', required: get_option('storage_sheepdog'), dirs: libvirt_sbin_path) + sheepdog_enable = true - if sheepdogcli_prog.found() + if get_option('storage_sheepdog').auto() + sheepdogcli_prog = find_program('dog', required: false, dirs: libvirt_sbin_path) + + sheepdog_enable = sheepdogcli_prog.found() + endif + + if sheepdog_enable use_storage = true conf.set('WITH_STORAGE_SHEEPDOG', 1) endif @@ -1877,12 +1892,15 @@ if conf.has('WITH_LIBVIRTD') if not get_option('storage_zfs').disabled() zfs_enable = true - foreach name : ['zfs', 'zpool'] - zfs_prog = find_program(name, required: get_option('storage_zfs'), dirs: libvirt_sbin_path) - if not zfs_prog.found() - zfs_enable = false - endif - endforeach + + if get_option('storage_zfs').auto() + foreach name : ['zfs', 'zpool'] + zfs_prog = find_program(name, required: false, dirs: libvirt_sbin_path) + if not zfs_prog.found() + zfs_enable = false + endif + endforeach + endif if zfs_enable use_storage = true @@ -1997,8 +2015,14 @@ if not get_option('nss').disabled() endif if not get_option('numad').disabled() and numactl_dep.found() - numad_prog = find_program('numad', required: get_option('numad'), dirs: libvirt_sbin_path) - if numad_prog.found() + numad_enable = true + + if get_option('numad').auto() + numad_prog = find_program('numad', required: false, dirs: libvirt_sbin_path) + numad_enable = numad_prog.found() + endif + + if numad_enable conf.set('WITH_NUMAD', 1) endif elif get_option('numad').enabled() -- 2.30.2
Drop code that creates defines with program paths and update the comment to reflect current usage of optional_programs. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- meson.build | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/meson.build b/meson.build index a7d402cd31..d93f2ff469 100644 --- a/meson.build +++ b/meson.build @@ -922,7 +922,7 @@ foreach item : required_programs_groups endforeach -# optional programs +# optional programs used while building libvirt optional_programs = [ 'augparse', @@ -933,14 +933,6 @@ optional_programs = [ foreach name : optional_programs prog = find_program(name, required: false, dirs: libvirt_sbin_path) varname = name.underscorify() - if prog.found() - prog_path = prog.path() - else - prog_path = name - endif - - conf.set_quoted(varname.to_upper(), prog_path) - conf.set_quoted('@0@_PATH'.format(varname.to_upper()), prog_path) set_variable('@0@_prog'.format(varname), prog) endforeach -- 2.30.2
These are no longer required to build libvirt as they are used during compilation only by meson to detect if some "auto" features should be enabled or not but in spec file we explicitly enable/disable all libvirt features. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- libvirt.spec.in | 31 ------------------------------- 1 file changed, 31 deletions(-) diff --git a/libvirt.spec.in b/libvirt.spec.in index f9af330186..7c661b6a82 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -299,10 +299,6 @@ BuildRequires: sanlock-devel >= 2.4 BuildRequires: libpcap-devel >= 1.5.0 BuildRequires: libnl3-devel BuildRequires: libselinux-devel -BuildRequires: dnsmasq >= 2.41 -BuildRequires: iptables -BuildRequires: radvd -BuildRequires: ebtables BuildRequires: module-init-tools BuildRequires: cyrus-sasl-devel BuildRequires: polkit >= 0.112 @@ -311,13 +307,7 @@ BuildRequires: util-linux %if %{with_qemu} # For managing ACLs BuildRequires: libacl-devel -# From QEMU RPMs -BuildRequires: /usr/bin/qemu-img %endif -# For LVM drivers -BuildRequires: lvm2 -# For pool type=iscsi -BuildRequires: iscsi-initiator-utils %if %{with_storage_iscsi_direct} # For pool type=iscsi-direct BuildRequires: libiscsi-devel @@ -341,15 +331,6 @@ BuildRequires: librbd1-devel BuildRequires: glusterfs-api-devel >= 3.4.1 BuildRequires: glusterfs-devel >= 3.4.1 %endif -%if %{with_storage_sheepdog} -BuildRequires: sheepdog -%endif -%if %{with_storage_zfs} -# Support any conforming implementation of zfs. On stock Fedora -# this is zfs-fuse, but could be zfsonlinux upstream RPMs -BuildRequires: /sbin/zfs -BuildRequires: /sbin/zpool -%endif %if %{with_numactl} # For QEMU/LXC numa info BuildRequires: numactl-devel @@ -374,21 +355,9 @@ BuildRequires: audit-libs-devel # we need /usr/sbin/dtrace BuildRequires: systemtap-sdt-devel -# For mount/umount in FS driver -BuildRequires: util-linux -# For showmount in FS driver (netfs discovery) -BuildRequires: nfs-utils - # Fedora build root suckage BuildRequires: gawk -# For storage wiping with different algorithms -BuildRequires: scrub - -%if %{with_numad} -BuildRequires: numad -%endif - %if %{with_wireshark} BuildRequires: wireshark-devel %endif -- 2.30.2
On Mon, Apr 19, 2021 at 1:14 PM Pavel Hrdina <phrdina@redhat.com> wrote:
Recent attempt to add a lot of meson options to specify different runtime paths motivated me enough to cleanup this from meson.
Changes in v2: - split and rework patch 16/17 to address review comments - added a new patch to cleanup libvirt.spec.in file
Pavel Hrdina (21): bridge_driver: fix comment about dnsmasqCaps virdnsmasq: drop unused dnsmasqCapsNewFromFile function virdnsmasq: drop unused dnsmasqCapsRefresh function virdnsmasq: remove binaryPath argument from dnsmasqCapsNewFromBuffer virdnsmasq: remove binaryPath argument from dnsmasqCapsNewFromBinary virfirewall: use virFindFileInPath instead of virFileIsExecutable tests: introduce virfirewallmock tests: use virfirewallmock instead of hasNetfilterTools virnetdev: move virNetDevSetRootQDisc to virnetdevbandwidth tests: testutilsqemu: move virFindFileInPath into domaincapsmock meson: don't check collie as program for sheepdog bhyvexml2argvtest: use virCommandToStringFull to strip command path storage: use virFindFileInPath to validate presence of mkfs virfile: introduce virFindFileInPathFull() qemu_conf: use virFindFileInPathFull for runtime binaries meson: drop check for runtime binary dependencies meson: move iscsiadm check into storage_iscsi condition meson: stop setting runtime binaries defines during compilation meson: use runtime binaries to only resolve features with "auto" value meson: optional_programs should be used only for building libvirt libvirt.spec: drop no longer required build dependencies
libvirt.spec.in | 31 --- meson.build | 214 ++++++------------ src/bhyve/bhyve_command.c | 4 + src/libvirt_private.syms | 6 +- src/locking/lock_driver_lockd.c | 12 +- src/network/bridge_driver.c | 8 +- src/node_device/node_device_driver.c | 2 + src/qemu/qemu_conf.c | 23 +- src/qemu/qemu_domain.c | 3 +- src/storage/storage_backend_fs.c | 24 +- src/storage/storage_backend_logical.c | 13 ++ src/storage/storage_backend_sheepdog.c | 2 + src/storage/storage_backend_zfs.c | 3 + src/storage/storage_util.c | 2 + src/storage/storage_util.h | 6 + src/util/virdnsmasq.c | 56 +---- src/util/virdnsmasq.h | 8 +- src/util/virfile.c | 16 +- src/util/virfile.h | 6 +- src/util/virfirewall.c | 4 +- src/util/virfirewall.h | 4 + src/util/viriscsi.h | 2 + src/util/virkmod.h | 3 + src/util/virnetdev.c | 46 ---- src/util/virnetdev.h | 4 - src/util/virnetdevbandwidth.c | 50 ++++ src/util/virnetdevbandwidth.h | 6 + src/util/virnetdevip.c | 2 + src/util/virnetdevmidonet.c | 2 + src/util/virnetdevopenvswitch.c | 2 + src/util/virnuma.c | 1 + src/util/virsysinfo.c | 1 + src/util/virutil.c | 2 + .../bhyvexml2argv-acpiapic.args | 2 +- .../bhyvexml2argv-acpiapic.ldargs | 2 +- ...ml2argv-addr-isa-controller-on-slot-1.args | 2 +- ...2argv-addr-isa-controller-on-slot-1.ldargs | 2 +- ...l2argv-addr-isa-controller-on-slot-31.args | 2 +- ...argv-addr-isa-controller-on-slot-31.ldargs | 2 +- ...xml2argv-addr-more-than-32-sata-disks.args | 2 +- ...l2argv-addr-more-than-32-sata-disks.ldargs | 2 +- ...hyvexml2argv-addr-multiple-sata-disks.args | 2 +- ...vexml2argv-addr-multiple-sata-disks.ldargs | 2 +- ...vexml2argv-addr-multiple-virtio-disks.args | 2 +- ...xml2argv-addr-multiple-virtio-disks.ldargs | 2 +- ...rgv-addr-no32devs-multiple-sata-disks.args | 2 +- ...v-addr-no32devs-multiple-sata-disks.ldargs | 2 +- ...l2argv-addr-no32devs-single-sata-disk.args | 2 +- ...argv-addr-no32devs-single-sata-disk.ldargs | 2 +- ...rgv-addr-non-isa-controller-on-slot-1.args | 2 +- .../bhyvexml2argv-addr-single-sata-disk.args | 2 +- ...bhyvexml2argv-addr-single-sata-disk.ldargs | 2 +- ...bhyvexml2argv-addr-single-virtio-disk.args | 2 +- ...yvexml2argv-addr-single-virtio-disk.ldargs | 2 +- .../bhyvexml2argvdata/bhyvexml2argv-base.args | 2 +- .../bhyvexml2argv-base.ldargs | 2 +- .../bhyvexml2argv-bhyveload-bootorder.args | 2 +- .../bhyvexml2argv-bhyveload-bootorder.ldargs | 2 +- .../bhyvexml2argv-bhyveload-bootorder1.args | 2 +- .../bhyvexml2argv-bhyveload-bootorder1.ldargs | 2 +- .../bhyvexml2argv-bhyveload-bootorder3.args | 2 +- .../bhyvexml2argv-bhyveload-bootorder3.ldargs | 2 +- .../bhyvexml2argv-bhyveload-explicitargs.args | 2 +- ...hyvexml2argv-bhyveload-explicitargs.ldargs | 2 +- .../bhyvexml2argv-commandline.args | 2 +- .../bhyvexml2argv-commandline.ldargs | 2 +- ...gv-console-master-slave-not-specified.args | 2 +- ...-console-master-slave-not-specified.ldargs | 2 +- .../bhyvexml2argv-console.args | 2 +- .../bhyvexml2argv-console.ldargs | 2 +- .../bhyvexml2argv-cputopology.args | 2 +- .../bhyvexml2argv-cputopology.ldargs | 2 +- .../bhyvexml2argv-custom-loader.args | 2 +- .../bhyvexml2argv-custom-loader.ldargs | 2 +- .../bhyvexml2argv-disk-cdrom-grub.args | 2 +- .../bhyvexml2argv-disk-cdrom-grub.ldargs | 2 +- .../bhyvexml2argv-disk-cdrom.args | 2 +- .../bhyvexml2argv-disk-cdrom.ldargs | 2 +- .../bhyvexml2argv-disk-virtio.args | 2 +- .../bhyvexml2argv-disk-virtio.ldargs | 2 +- .../bhyvexml2argv-firmware-efi.args | 2 +- .../bhyvexml2argv-fs-9p-readonly.args | 2 +- .../bhyvexml2argv-fs-9p-readonly.ldargs | 2 +- .../bhyvexml2argv-fs-9p.args | 2 +- .../bhyvexml2argv-fs-9p.ldargs | 2 +- .../bhyvexml2argv-grub-bootorder.args | 2 +- .../bhyvexml2argv-grub-bootorder.ldargs | 2 +- .../bhyvexml2argv-grub-bootorder2.args | 2 +- .../bhyvexml2argv-grub-bootorder2.ldargs | 2 +- .../bhyvexml2argv-grub-defaults.args | 2 +- .../bhyvexml2argv-grub-defaults.ldargs | 2 +- .../bhyvexml2argv-input-xhci-tablet.args | 2 +- .../bhyvexml2argv-input-xhci-tablet.ldargs | 2 +- .../bhyvexml2argv-isa-controller.args | 2 +- .../bhyvexml2argv-isa-controller.ldargs | 2 +- .../bhyvexml2argv-localtime.args | 2 +- .../bhyvexml2argv-localtime.ldargs | 2 +- .../bhyvexml2argv-macaddr.args | 2 +- .../bhyvexml2argv-macaddr.ldargs | 2 +- .../bhyvexml2argvdata/bhyvexml2argv-msrs.args | 2 +- .../bhyvexml2argv-msrs.ldargs | 2 +- .../bhyvexml2argv-net-e1000.args | 2 +- .../bhyvexml2argv-net-e1000.ldargs | 2 +- .../bhyvexml2argv-serial-grub-nocons.args | 2 +- .../bhyvexml2argv-serial-grub-nocons.ldargs | 2 +- .../bhyvexml2argv-serial-grub.args | 2 +- .../bhyvexml2argv-serial-grub.ldargs | 2 +- .../bhyvexml2argv-serial.args | 2 +- .../bhyvexml2argv-serial.ldargs | 2 +- .../bhyvexml2argv-sound.args | 2 +- .../bhyvexml2argv-sound.ldargs | 2 +- .../bhyvexml2argvdata/bhyvexml2argv-uefi.args | 2 +- .../bhyvexml2argv-vnc-autoport.args | 2 +- .../bhyvexml2argv-vnc-password.args | 2 +- .../bhyvexml2argv-vnc-resolution.args | 2 +- .../bhyvexml2argv-vnc-vgaconf-io.args | 2 +- .../bhyvexml2argv-vnc-vgaconf-off.args | 2 +- .../bhyvexml2argv-vnc-vgaconf-on.args | 2 +- .../bhyvexml2argvdata/bhyvexml2argv-vnc.args | 2 +- .../bhyvexml2argv-wired.args | 2 +- .../bhyvexml2argv-wired.ldargs | 2 +- tests/bhyvexml2argvtest.c | 4 +- tests/domaincapsmock.c | 17 ++ tests/meson.build | 1 + tests/networkxml2conftest.c | 6 +- tests/networkxml2firewalltest.c | 16 +- tests/nwfilterebiptablestest.c | 15 +- tests/nwfilterxml2firewalltest.c | 14 +- tests/qemuxml2argvmock.c | 5 +- tests/testutilsqemu.c | 15 -- tests/virfirewallmock.c | 35 +++ tests/virfirewalltest.c | 15 +- 132 files changed, 403 insertions(+), 484 deletions(-) create mode 100644 tests/virfirewallmock.c
-- 2.30.2
In case my reply on the v1 thread is missed, I'll say it again here, since it still applies here. So, I think you *shouldn't* do it that way. The problem with doing this is that we can wind up with mismatched capabilities and non-functional libvirt build based on the assumption that things will just "be there" with no check that they will actually be there. In other words, runtime executable dependencies should be treated *exactly* as they are now, because we have no other avenue to guarantee that things work for a given installation. The idea that binaries would randomly get replaced because of differences in system and session could also lead to other unexpected issues, but I think those are considerably less likely. I don't think Daniel's idea of this being supported is a good one though. It's remarkably easy to accidentally break things that way, and since GNOME Boxes is a very prominent user of the session connection, I'd rather not see that get broken by accident. To make it clear, I still NACK the series, because I think this is fundamentally a bad idea. -- 真実はいつも一つ！/ Always, there's only one truth!
On Tue, Apr 20, 2021 at 05:30:09AM -0400, Neal Gompa wrote:
On Mon, Apr 19, 2021 at 1:14 PM Pavel Hrdina <phrdina@redhat.com> wrote:
Recent attempt to add a lot of meson options to specify different runtime paths motivated me enough to cleanup this from meson.
Changes in v2: - split and rework patch 16/17 to address review comments - added a new patch to cleanup libvirt.spec.in file
Pavel Hrdina (21): bridge_driver: fix comment about dnsmasqCaps virdnsmasq: drop unused dnsmasqCapsNewFromFile function virdnsmasq: drop unused dnsmasqCapsRefresh function virdnsmasq: remove binaryPath argument from dnsmasqCapsNewFromBuffer virdnsmasq: remove binaryPath argument from dnsmasqCapsNewFromBinary virfirewall: use virFindFileInPath instead of virFileIsExecutable tests: introduce virfirewallmock tests: use virfirewallmock instead of hasNetfilterTools virnetdev: move virNetDevSetRootQDisc to virnetdevbandwidth tests: testutilsqemu: move virFindFileInPath into domaincapsmock meson: don't check collie as program for sheepdog bhyvexml2argvtest: use virCommandToStringFull to strip command path storage: use virFindFileInPath to validate presence of mkfs virfile: introduce virFindFileInPathFull() qemu_conf: use virFindFileInPathFull for runtime binaries meson: drop check for runtime binary dependencies meson: move iscsiadm check into storage_iscsi condition meson: stop setting runtime binaries defines during compilation meson: use runtime binaries to only resolve features with "auto" value meson: optional_programs should be used only for building libvirt libvirt.spec: drop no longer required build dependencies
libvirt.spec.in | 31 --- meson.build | 214 ++++++------------ src/bhyve/bhyve_command.c | 4 + src/libvirt_private.syms | 6 +- src/locking/lock_driver_lockd.c | 12 +- src/network/bridge_driver.c | 8 +- src/node_device/node_device_driver.c | 2 + src/qemu/qemu_conf.c | 23 +- src/qemu/qemu_domain.c | 3 +- src/storage/storage_backend_fs.c | 24 +- src/storage/storage_backend_logical.c | 13 ++ src/storage/storage_backend_sheepdog.c | 2 + src/storage/storage_backend_zfs.c | 3 + src/storage/storage_util.c | 2 + src/storage/storage_util.h | 6 + src/util/virdnsmasq.c | 56 +---- src/util/virdnsmasq.h | 8 +- src/util/virfile.c | 16 +- src/util/virfile.h | 6 +- src/util/virfirewall.c | 4 +- src/util/virfirewall.h | 4 + src/util/viriscsi.h | 2 + src/util/virkmod.h | 3 + src/util/virnetdev.c | 46 ---- src/util/virnetdev.h | 4 - src/util/virnetdevbandwidth.c | 50 ++++ src/util/virnetdevbandwidth.h | 6 + src/util/virnetdevip.c | 2 + src/util/virnetdevmidonet.c | 2 + src/util/virnetdevopenvswitch.c | 2 + src/util/virnuma.c | 1 + src/util/virsysinfo.c | 1 + src/util/virutil.c | 2 + .../bhyvexml2argv-acpiapic.args | 2 +- .../bhyvexml2argv-acpiapic.ldargs | 2 +- ...ml2argv-addr-isa-controller-on-slot-1.args | 2 +- ...2argv-addr-isa-controller-on-slot-1.ldargs | 2 +- ...l2argv-addr-isa-controller-on-slot-31.args | 2 +- ...argv-addr-isa-controller-on-slot-31.ldargs | 2 +- ...xml2argv-addr-more-than-32-sata-disks.args | 2 +- ...l2argv-addr-more-than-32-sata-disks.ldargs | 2 +- ...hyvexml2argv-addr-multiple-sata-disks.args | 2 +- ...vexml2argv-addr-multiple-sata-disks.ldargs | 2 +- ...vexml2argv-addr-multiple-virtio-disks.args | 2 +- ...xml2argv-addr-multiple-virtio-disks.ldargs | 2 +- ...rgv-addr-no32devs-multiple-sata-disks.args | 2 +- ...v-addr-no32devs-multiple-sata-disks.ldargs | 2 +- ...l2argv-addr-no32devs-single-sata-disk.args | 2 +- ...argv-addr-no32devs-single-sata-disk.ldargs | 2 +- ...rgv-addr-non-isa-controller-on-slot-1.args | 2 +- .../bhyvexml2argv-addr-single-sata-disk.args | 2 +- ...bhyvexml2argv-addr-single-sata-disk.ldargs | 2 +- ...bhyvexml2argv-addr-single-virtio-disk.args | 2 +- ...yvexml2argv-addr-single-virtio-disk.ldargs | 2 +- .../bhyvexml2argvdata/bhyvexml2argv-base.args | 2 +- .../bhyvexml2argv-base.ldargs | 2 +- .../bhyvexml2argv-bhyveload-bootorder.args | 2 +- .../bhyvexml2argv-bhyveload-bootorder.ldargs | 2 +- .../bhyvexml2argv-bhyveload-bootorder1.args | 2 +- .../bhyvexml2argv-bhyveload-bootorder1.ldargs | 2 +- .../bhyvexml2argv-bhyveload-bootorder3.args | 2 +- .../bhyvexml2argv-bhyveload-bootorder3.ldargs | 2 +- .../bhyvexml2argv-bhyveload-explicitargs.args | 2 +- ...hyvexml2argv-bhyveload-explicitargs.ldargs | 2 +- .../bhyvexml2argv-commandline.args | 2 +- .../bhyvexml2argv-commandline.ldargs | 2 +- ...gv-console-master-slave-not-specified.args | 2 +- ...-console-master-slave-not-specified.ldargs | 2 +- .../bhyvexml2argv-console.args | 2 +- .../bhyvexml2argv-console.ldargs | 2 +- .../bhyvexml2argv-cputopology.args | 2 +- .../bhyvexml2argv-cputopology.ldargs | 2 +- .../bhyvexml2argv-custom-loader.args | 2 +- .../bhyvexml2argv-custom-loader.ldargs | 2 +- .../bhyvexml2argv-disk-cdrom-grub.args | 2 +- .../bhyvexml2argv-disk-cdrom-grub.ldargs | 2 +- .../bhyvexml2argv-disk-cdrom.args | 2 +- .../bhyvexml2argv-disk-cdrom.ldargs | 2 +- .../bhyvexml2argv-disk-virtio.args | 2 +- .../bhyvexml2argv-disk-virtio.ldargs | 2 +- .../bhyvexml2argv-firmware-efi.args | 2 +- .../bhyvexml2argv-fs-9p-readonly.args | 2 +- .../bhyvexml2argv-fs-9p-readonly.ldargs | 2 +- .../bhyvexml2argv-fs-9p.args | 2 +- .../bhyvexml2argv-fs-9p.ldargs | 2 +- .../bhyvexml2argv-grub-bootorder.args | 2 +- .../bhyvexml2argv-grub-bootorder.ldargs | 2 +- .../bhyvexml2argv-grub-bootorder2.args | 2 +- .../bhyvexml2argv-grub-bootorder2.ldargs | 2 +- .../bhyvexml2argv-grub-defaults.args | 2 +- .../bhyvexml2argv-grub-defaults.ldargs | 2 +- .../bhyvexml2argv-input-xhci-tablet.args | 2 +- .../bhyvexml2argv-input-xhci-tablet.ldargs | 2 +- .../bhyvexml2argv-isa-controller.args | 2 +- .../bhyvexml2argv-isa-controller.ldargs | 2 +- .../bhyvexml2argv-localtime.args | 2 +- .../bhyvexml2argv-localtime.ldargs | 2 +- .../bhyvexml2argv-macaddr.args | 2 +- .../bhyvexml2argv-macaddr.ldargs | 2 +- .../bhyvexml2argvdata/bhyvexml2argv-msrs.args | 2 +- .../bhyvexml2argv-msrs.ldargs | 2 +- .../bhyvexml2argv-net-e1000.args | 2 +- .../bhyvexml2argv-net-e1000.ldargs | 2 +- .../bhyvexml2argv-serial-grub-nocons.args | 2 +- .../bhyvexml2argv-serial-grub-nocons.ldargs | 2 +- .../bhyvexml2argv-serial-grub.args | 2 +- .../bhyvexml2argv-serial-grub.ldargs | 2 +- .../bhyvexml2argv-serial.args | 2 +- .../bhyvexml2argv-serial.ldargs | 2 +- .../bhyvexml2argv-sound.args | 2 +- .../bhyvexml2argv-sound.ldargs | 2 +- .../bhyvexml2argvdata/bhyvexml2argv-uefi.args | 2 +- .../bhyvexml2argv-vnc-autoport.args | 2 +- .../bhyvexml2argv-vnc-password.args | 2 +- .../bhyvexml2argv-vnc-resolution.args | 2 +- .../bhyvexml2argv-vnc-vgaconf-io.args | 2 +- .../bhyvexml2argv-vnc-vgaconf-off.args | 2 +- .../bhyvexml2argv-vnc-vgaconf-on.args | 2 +- .../bhyvexml2argvdata/bhyvexml2argv-vnc.args | 2 +- .../bhyvexml2argv-wired.args | 2 +- .../bhyvexml2argv-wired.ldargs | 2 +- tests/bhyvexml2argvtest.c | 4 +- tests/domaincapsmock.c | 17 ++ tests/meson.build | 1 + tests/networkxml2conftest.c | 6 +- tests/networkxml2firewalltest.c | 16 +- tests/nwfilterebiptablestest.c | 15 +- tests/nwfilterxml2firewalltest.c | 14 +- tests/qemuxml2argvmock.c | 5 +- tests/testutilsqemu.c | 15 -- tests/virfirewallmock.c | 35 +++ tests/virfirewalltest.c | 15 +- 132 files changed, 403 insertions(+), 484 deletions(-) create mode 100644 tests/virfirewallmock.c
-- 2.30.2
In case my reply on the v1 thread is missed, I'll say it again here, since it still applies here.
So, I think you *shouldn't* do it that way. The problem with doing this is that we can wind up with mismatched capabilities and non-functional libvirt build based on the assumption that things will just "be there" with no check that they will actually be there.
In other words, runtime executable dependencies should be treated *exactly* as they are now, because we have no other avenue to guarantee that things work for a given installation.
I disagree, let me explain why using several possible scenarios: 1) libvirt is compiled by package maintainers to be shipped in some distribution In this case it is responsibility of the package maintainer to ensure that the build dependencies and runtime dependencies are correctly defined. Users usually don't have an option to remove any dependency without removing the dependent package as well so the assumption that things will just "be there" is correct. 2) libvirt is compiled directly by user from git/tarball In this situation there is nothing ensuring the correct dependencies are installed for compilation or runtime so everything can breake with or without this series. For example without this series users can easily install the necessary binaries, compile libvirt and uninstall the binaries again which would result in non-functional libvirt features using these binaries. Another point for this series is that by default (keeping all libvirt defined meson features set to "auto") will behave exactly the same as before this series. The difference comes only once user explicitly enables some libvirt feature without required binaries. The following list of binaries was always optional and if missing in the system only the name was defined so libvirt would always figure out the full path during runtime: dmidecode dnsmasq ebtables ip ip6tables iptables mdevctl mm-ctl modprobe ovs-vsctl radvd rmmod scrub tc udevadm qemu-bridge-helper qemu-pr-helper slirp-helper dbus-daemon showmount Before this series if these are available during compilation libvirt would set the full path to be used during runtime but if they are not available it would silently use only the names. After the series it will always use only the names. So the possibility to end up with non-functional libvirt is the same as libvirt assumes that things will just "be there" if they are not during compilation. For remaining binaries affected by this series we change the behavior to not error out if they are missing but as I've already pointed out we keep the default behavior to not enable the libvirt features requiring these binaries. bhyve, bhyveclt, bhyveload (driver_bhyve) parted (storage_disk) mount, umount, mkfs (storage_fs) lvm aliases (storage_lvm) dog (storage_sheepdog) zfs, zpool (storage_zfs) numad (numad) Here we will use the same behavior as the previous list of optional binaries.
The idea that binaries would randomly get replaced because of differences in system and session could also lead to other unexpected issues, but I think those are considerably less likely. I don't think Daniel's idea of this being supported is a good one though. It's remarkably easy to accidentally break things that way, and since GNOME Boxes is a very prominent user of the session connection, I'd rather not see that get broken by accident.
Looking at the list of binaries that will be affected by this series I don't see any that would be used by GNOME Boxes.
To make it clear, I still NACK the series, because I think this is fundamentally a bad idea.
It was already pointed out that the series have other patches fixing issues not related to the binary path detection, NACKing the whole series is ignoring the fact. To summarize it I don't see any strong reason why this series shouldn't be accepted. Pavel
On Tue, Apr 20, 2021 at 12:50:51PM +0200, Pavel Hrdina wrote:
On Tue, Apr 20, 2021 at 05:30:09AM -0400, Neal Gompa wrote:
On Mon, Apr 19, 2021 at 1:14 PM Pavel Hrdina <phrdina@redhat.com> wrote:
Recent attempt to add a lot of meson options to specify different runtime paths motivated me enough to cleanup this from meson.
Changes in v2: - split and rework patch 16/17 to address review comments - added a new patch to cleanup libvirt.spec.in file
Pavel Hrdina (21): bridge_driver: fix comment about dnsmasqCaps virdnsmasq: drop unused dnsmasqCapsNewFromFile function virdnsmasq: drop unused dnsmasqCapsRefresh function virdnsmasq: remove binaryPath argument from dnsmasqCapsNewFromBuffer virdnsmasq: remove binaryPath argument from dnsmasqCapsNewFromBinary virfirewall: use virFindFileInPath instead of virFileIsExecutable tests: introduce virfirewallmock tests: use virfirewallmock instead of hasNetfilterTools virnetdev: move virNetDevSetRootQDisc to virnetdevbandwidth tests: testutilsqemu: move virFindFileInPath into domaincapsmock meson: don't check collie as program for sheepdog bhyvexml2argvtest: use virCommandToStringFull to strip command path storage: use virFindFileInPath to validate presence of mkfs virfile: introduce virFindFileInPathFull() qemu_conf: use virFindFileInPathFull for runtime binaries meson: drop check for runtime binary dependencies meson: move iscsiadm check into storage_iscsi condition meson: stop setting runtime binaries defines during compilation meson: use runtime binaries to only resolve features with "auto" value meson: optional_programs should be used only for building libvirt libvirt.spec: drop no longer required build dependencies
-- 2.30.2
In case my reply on the v1 thread is missed, I'll say it again here, since it still applies here.
So, I think you *shouldn't* do it that way. The problem with doing this is that we can wind up with mismatched capabilities and non-functional libvirt build based on the assumption that things will just "be there" with no check that they will actually be there.
In other words, runtime executable dependencies should be treated *exactly* as they are now, because we have no other avenue to guarantee that things work for a given installation.
I disagree, let me explain why using several possible scenarios:
1) libvirt is compiled by package maintainers to be shipped in some distribution
In this case it is responsibility of the package maintainer to ensure that the build dependencies and runtime dependencies are correctly defined. Users usually don't have an option to remove any dependency without removing the dependent package as well so the assumption that things will just "be there" is correct.
2) libvirt is compiled directly by user from git/tarball
In this situation there is nothing ensuring the correct dependencies are installed for compilation or runtime so everything can breake with or without this series. For example without this series users can easily install the necessary binaries, compile libvirt and uninstall the binaries again which would result in non-functional libvirt features using these binaries.
Another point for this series is that by default (keeping all libvirt defined meson features set to "auto") will behave exactly the same as before this series. The difference comes only once user explicitly enables some libvirt feature without required binaries.
The following list of binaries was always optional and if missing in the system only the name was defined so libvirt would always figure out the full path during runtime:
dmidecode dnsmasq ebtables ip ip6tables iptables mdevctl mm-ctl modprobe ovs-vsctl radvd rmmod scrub tc udevadm
qemu-bridge-helper qemu-pr-helper slirp-helper dbus-daemon
showmount
Before this series if these are available during compilation libvirt would set the full path to be used during runtime but if they are not available it would silently use only the names. After the series it will always use only the names. So the possibility to end up with non-functional libvirt is the same as libvirt assumes that things will just "be there" if they are not during compilation.
For remaining binaries affected by this series we change the behavior to not error out if they are missing but as I've already pointed out we keep the default behavior to not enable the libvirt features requiring these binaries.
bhyve, bhyveclt, bhyveload (driver_bhyve)
parted (storage_disk) mount, umount, mkfs (storage_fs) lvm aliases (storage_lvm) dog (storage_sheepdog) zfs, zpool (storage_zfs)
numad (numad)
Here we will use the same behavior as the previous list of optional binaries.
I would note that over the years users have fairly frequent hit problems where they didn't have a binary present during build, but installed it later, and then complained to us that libvirt didn't use it. This is why alot of the binaries you list in the previous set we changed to be optional at configure time, relying on $PATH at runtime.
The idea that binaries would randomly get replaced because of differences in system and session could also lead to other unexpected issues, but I think those are considerably less likely. I don't think Daniel's idea of this being supported is a good one though. It's remarkably easy to accidentally break things that way, and since GNOME Boxes is a very prominent user of the session connection, I'd rather not see that get broken by accident.
Looking at the list of binaries that will be affected by this series I don't see any that would be used by GNOME Boxes.
Even if it does though, I don't think this is a reason not to do the change. If an unprivileged user is replacing system binaries then it is their responsiblity if their replacements break stuff. I think this is not likely to be a significant problem in the real world though. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On Mon, 2021-04-19 at 19:14 +0200, Pavel Hrdina wrote:
Recent attempt to add a lot of meson options to specify different runtime paths motivated me enough to cleanup this from meson.
Changes in v2: - split and rework patch 16/17 to address review comments - added a new patch to cleanup libvirt.spec.in file
Overall I like this change, if nothing else because it finally cleans up the mess where some optional programs would be detected at build time while others would be detected at runtime, with no apparent reason for going one way rather than the other. The only concern I have, which is one I have expressed already in the past, is that it will now be harder for users and packagers alike to figure out that they don't have a certain optional program installed: AFAICT, after your patches the only way would be to try using each feature and look out for errors. Can we still check for the availability of these commands at build time and simply report their presence? I'm thinking of something like optional_runtime_programs = [ 'dmidecode', 'dnsmasq', ... ] which would be reported in the Meson summary as Optional runtime programs: dmidecode: YES dnsmasq: NO ... Then, for each of these commands, we'd also create a macro definition in meson-config.h like #define PROGRAM_DMIDECODE "dmidecode" #define PROGRAM_DNSMASQ "dnsmasq" ... and use those in the various C functions instead of the ad-hoc, private defines that are currently sprinkled throughout the code. Note that said macro would *not* be defined conditionally based on whether the optional program was found at build time: the intention is simply to have a single location where information about all these optional runtime programs are stored, instead of forcing users and packagers to go hunting for them across all of libvirt, potentially missing a bunch in the process. What do you think? -- Andrea Bolognani / Red Hat / Virtualization
On Tue, Apr 20, 2021 at 03:05:26PM +0200, Andrea Bolognani wrote:
On Mon, 2021-04-19 at 19:14 +0200, Pavel Hrdina wrote:
Recent attempt to add a lot of meson options to specify different runtime paths motivated me enough to cleanup this from meson.
Changes in v2: - split and rework patch 16/17 to address review comments - added a new patch to cleanup libvirt.spec.in file
Overall I like this change, if nothing else because it finally cleans up the mess where some optional programs would be detected at build time while others would be detected at runtime, with no apparent reason for going one way rather than the other.
The only concern I have, which is one I have expressed already in the past, is that it will now be harder for users and packagers alike to figure out that they don't have a certain optional program installed: AFAICT, after your patches the only way would be to try using each feature and look out for errors.
I'd say that running "meson" and looking at its output is a poor way for users to learn what the pre-requisites are, both before and after this change. Ideally we should document the list of packages we depend on. We have it sorta documented in the many docker files. I wonder if we should include the list of mapping names from libvirt-ci as an explicit doc item in README.md or somewhere else that could be useful. Or just tell tem to look at the dockerfiles. Even if their distro isn't covered, it'll be enough info for them to get most of the way there.
Note that said macro would *not* be defined conditionally based on whether the optional program was found at build time: the intention is simply to have a single location where information about all these optional runtime programs are stored, instead of forcing users and packagers to go hunting for them across all of libvirt, potentially missing a bunch in the process.
What do you think?
-- Andrea Bolognani / Red Hat / Virtualization
Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On Tue, Apr 20, 2021 at 02:14:59PM +0100, Daniel P. Berrangé wrote:
On Tue, Apr 20, 2021 at 03:05:26PM +0200, Andrea Bolognani wrote:
On Mon, 2021-04-19 at 19:14 +0200, Pavel Hrdina wrote:
Recent attempt to add a lot of meson options to specify different runtime paths motivated me enough to cleanup this from meson.
Changes in v2: - split and rework patch 16/17 to address review comments - added a new patch to cleanup libvirt.spec.in file
Overall I like this change, if nothing else because it finally cleans up the mess where some optional programs would be detected at build time while others would be detected at runtime, with no apparent reason for going one way rather than the other.
The only concern I have, which is one I have expressed already in the past, is that it will now be harder for users and packagers alike to figure out that they don't have a certain optional program installed: AFAICT, after your patches the only way would be to try using each feature and look out for errors.
I'd say that running "meson" and looking at its output is a poor way for users to learn what the pre-requisites are, both before and after this change.
Ideally we should document the list of packages we depend on. We have it sorta documented in the many docker files.
I wonder if we should include the list of mapping names from libvirt-ci as an explicit doc item in README.md or somewhere else that could be useful. Or just tell tem to look at the dockerfiles. Even if their distro isn't covered, it'll be enough info for them to get most of the way there.
Agreed, instead of having it in meson I would also prefer having it documented somewhere. Ideally we should not document only what optional programs we use but every single dependency we use ideally with links to upstream projects and possibly examples of downstream packages which is as Dan pointed out stored in libvirt-ci. Pavel
Note that said macro would *not* be defined conditionally based on whether the optional program was found at build time: the intention is simply to have a single location where information about all these optional runtime programs are stored, instead of forcing users and packagers to go hunting for them across all of libvirt, potentially missing a bunch in the process.
What do you think?
-- Andrea Bolognani / Red Hat / Virtualization
Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On Tue, 2021-04-20 at 15:20 +0200, Pavel Hrdina wrote:
On Tue, Apr 20, 2021 at 02:14:59PM +0100, Daniel P. Berrangé wrote:
On Tue, Apr 20, 2021 at 03:05:26PM +0200, Andrea Bolognani wrote:
The only concern I have, which is one I have expressed already in the past, is that it will now be harder for users and packagers alike to figure out that they don't have a certain optional program installed: AFAICT, after your patches the only way would be to try using each feature and look out for errors.
I'd say that running "meson" and looking at its output is a poor way for users to learn what the pre-requisites are, both before and after this change.
Ideally we should document the list of packages we depend on. We have it sorta documented in the many docker files.
I wonder if we should include the list of mapping names from libvirt-ci as an explicit doc item in README.md or somewhere else that could be useful. Or just tell tem to look at the dockerfiles. Even if their distro isn't covered, it'll be enough info for them to get most of the way there.
Agreed, instead of having it in meson I would also prefer having it documented somewhere. Ideally we should not document only what optional programs we use but every single dependency we use ideally with links to upstream projects and possibly examples of downstream packages which is as Dan pointed out stored in libvirt-ci.
Documentation is great and all, but the problem with it is that it's extremely easy for it to become outdated as new optional runtime dependencies are added; the advantage of having this list stored in Meson is that the person who's adding a new optional runtime dependency is likely to look around for how the existing ones are handled, and thus automatically do the right thing. The point about libvirt-ci storing all the dependencies is a good one, but note that we only keep track of *build time* dependencies for projects: that is, once e.g. dnsmasq is no longer required to be available in the build environment in order to produce a fully-featured libvirt build, we'll simply stop including it. This will happen with almost all other optional runtime dependencies too, so really the mappings that are maintained as part of the libvirt-ci project doesn't solve the issue of keeping track of optional runtime dependencies at all. -- Andrea Bolognani / Red Hat / Virtualization
On Tue, Apr 20, 2021 at 03:36:55PM +0200, Andrea Bolognani wrote:
On Tue, 2021-04-20 at 15:20 +0200, Pavel Hrdina wrote:
On Tue, Apr 20, 2021 at 02:14:59PM +0100, Daniel P. Berrangé wrote:
On Tue, Apr 20, 2021 at 03:05:26PM +0200, Andrea Bolognani wrote:
The only concern I have, which is one I have expressed already in the past, is that it will now be harder for users and packagers alike to figure out that they don't have a certain optional program installed: AFAICT, after your patches the only way would be to try using each feature and look out for errors.
I'd say that running "meson" and looking at its output is a poor way for users to learn what the pre-requisites are, both before and after this change.
Ideally we should document the list of packages we depend on. We have it sorta documented in the many docker files.
I wonder if we should include the list of mapping names from libvirt-ci as an explicit doc item in README.md or somewhere else that could be useful. Or just tell tem to look at the dockerfiles. Even if their distro isn't covered, it'll be enough info for them to get most of the way there.
Agreed, instead of having it in meson I would also prefer having it documented somewhere. Ideally we should not document only what optional programs we use but every single dependency we use ideally with links to upstream projects and possibly examples of downstream packages which is as Dan pointed out stored in libvirt-ci.
Documentation is great and all, but the problem with it is that it's extremely easy for it to become outdated as new optional runtime dependencies are added; the advantage of having this list stored in Meson is that the person who's adding a new optional runtime dependency is likely to look around for how the existing ones are handled, and thus automatically do the right thing.
I would say the same applies to the documentation. If the person who's adding a new dependency will "git grep" for any already existing binary to see what needs to be done they will find the documentation as well. At the same time there is no guarantee that having it in meson would make sure that we will not miss any new dependency if it's only optional. The only way to make sure the list is not outdated in meson or in documentation is to add a check which would compare the list with what we actually have in the code, like for example we do for po/POTFILES.in to ensure that every source code file using translation is listed there. I don't see this as a strong argument for having the list in meson.
The point about libvirt-ci storing all the dependencies is a good one, but note that we only keep track of *build time* dependencies for projects: that is, once e.g. dnsmasq is no longer required to be available in the build environment in order to produce a fully-featured libvirt build, we'll simply stop including it. This will happen with almost all other optional runtime dependencies too, so really the mappings that are maintained as part of the libvirt-ci project doesn't solve the issue of keeping track of optional runtime dependencies at all.
Good point, but as a starting point libvirt-ci can be used to get at least build dependencies. Pavel
participants (4)
-
Andrea Bolognani -
Daniel P. Berrangé -
Neal Gompa -
Pavel Hrdina