Re: [libvirt PATCH v2 00/21] cleanup meson checks for runtime binaries

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Apr 19, 2021 at 1:14 PM Pavel Hrdina <phrdina@xxxxxxxxxx> 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!





[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux