[libvirt] [PATCH 0/5] qemu: Detect KVM usability correctly

There are a few cases in which we're not reporting the correct information through 'virsh capabilities', leading to applications such as libguestfs attempting to launch KVM guests on a host that doesn't actually support them. Our code is also more complicated than it needs to be due to backwards compatibilty reasons that we no longer care about these days, so we can perform some cleanup there; there are further cleanup opportunities in the same area, but some of them are a bit tricky so I'll leave them for a follow-up series. We also get some extra test coverage pretty much for free. Yay! Andrea Bolognani (5): tests: Reuse qemucapabilities data for qemucaps2xml tests: Add more tests to qemucaps2xml qemu: Drop QEMU_CAPS_ENABLE_KVM qemu: Clarify QEMU_CAPS_KVM qemu: Don't check for /dev/kvm presence src/qemu/qemu_capabilities.c | 27 +--- src/qemu/qemu_capabilities.h | 4 +- tests/qemucapabilitiestest.c | 1 + tests/qemucaps2xmldata/all_1.6.0-1.caps | 129 ------------------ .../nodisksnapshot_1.6.0-1.caps | 128 ----------------- .../caps_1.5.3.x86_64.xml} | 12 +- .../caps_1.6.0.x86_64.xml} | 14 +- .../caps_1.7.0.x86_64.xml} | 12 +- .../caps_2.1.1.x86_64.xml} | 12 +- .../caps_2.10.0.aarch64.xml} | 13 +- .../caps_2.10.0.ppc64.xml} | 14 +- .../caps_2.10.0.s390x.xml} | 14 +- .../caps_2.10.0.x86_64.xml} | 12 +- .../caps_2.11.0.s390x.xml} | 14 +- .../caps_2.11.0.x86_64.xml} | 12 +- .../caps_2.12.0.aarch64.xml} | 13 +- .../caps_2.12.0.ppc64.xml} | 14 +- .../caps_2.12.0.s390x.xml} | 14 +- .../caps_2.12.0.x86_64.xml} | 12 +- .../caps_2.4.0.x86_64.xml} | 12 +- .../caps_2.5.0.x86_64.xml} | 12 +- .../caps_2.6.0.aarch64.xml} | 13 +- .../caps_2.6.0.ppc64.xml} | 14 +- .../caps_2.6.0.x86_64.xml} | 12 +- .../caps_2.7.0.s390x.xml} | 14 +- .../caps_2.7.0.x86_64.xml} | 12 +- .../caps_2.8.0.s390x.xml} | 14 +- .../caps_2.8.0.x86_64.xml} | 12 +- .../caps_2.9.0.ppc64.xml} | 14 +- .../caps_2.9.0.s390x.xml} | 14 +- .../caps_2.9.0.x86_64.xml} | 12 +- .../caps_3.0.0.ppc64.xml} | 14 +- .../caps_3.0.0.riscv32.xml} | 13 +- .../caps_3.0.0.riscv64.xml | 25 ++++ .../caps_3.0.0.x86_64.xml} | 12 +- tests/qemucaps2xmltest.c | 63 +++++++-- tests/qemuxml2argvtest.c | 11 +- 37 files changed, 229 insertions(+), 535 deletions(-) delete mode 100644 tests/qemucaps2xmldata/all_1.6.0-1.caps delete mode 100644 tests/qemucaps2xmldata/nodisksnapshot_1.6.0-1.caps copy tests/{qemucaps2xmldata/all_1.6.0-1.xml => qemucaps2xmloutdata/caps_1.5.3.x86_64.xml} (67%) rename tests/{qemucaps2xmldata/nodisksnapshot_1.6.0-1.xml => qemucaps2xmloutdata/caps_1.6.0.x86_64.xml} (60%) copy tests/{qemucaps2xmldata/all_1.6.0-1.xml => qemucaps2xmloutdata/caps_1.7.0.x86_64.xml} (67%) copy tests/{qemucaps2xmldata/all_1.6.0-1.xml => qemucaps2xmloutdata/caps_2.1.1.x86_64.xml} (67%) copy tests/{qemucaps2xmldata/all_1.6.0-1.xml => qemucaps2xmloutdata/caps_2.10.0.aarch64.xml} (62%) copy tests/{qemucaps2xmldata/all_1.6.0-1.xml => qemucaps2xmloutdata/caps_2.10.0.ppc64.xml} (56%) copy tests/{qemucaps2xmldata/all_1.6.0-1.xml => qemucaps2xmloutdata/caps_2.10.0.s390x.xml} (56%) copy tests/{qemucaps2xmldata/all_1.6.0-1.xml => qemucaps2xmloutdata/caps_2.10.0.x86_64.xml} (67%) copy tests/{qemucaps2xmldata/all_1.6.0-1.xml => qemucaps2xmloutdata/caps_2.11.0.s390x.xml} (56%) copy tests/{qemucaps2xmldata/all_1.6.0-1.xml => qemucaps2xmloutdata/caps_2.11.0.x86_64.xml} (67%) copy tests/{qemucaps2xmldata/all_1.6.0-1.xml => qemucaps2xmloutdata/caps_2.12.0.aarch64.xml} (62%) copy tests/{qemucaps2xmldata/all_1.6.0-1.xml => qemucaps2xmloutdata/caps_2.12.0.ppc64.xml} (56%) copy tests/{qemucaps2xmldata/all_1.6.0-1.xml => qemucaps2xmloutdata/caps_2.12.0.s390x.xml} (56%) copy tests/{qemucaps2xmldata/all_1.6.0-1.xml => qemucaps2xmloutdata/caps_2.12.0.x86_64.xml} (67%) copy tests/{qemucaps2xmldata/all_1.6.0-1.xml => qemucaps2xmloutdata/caps_2.4.0.x86_64.xml} (67%) copy tests/{qemucaps2xmldata/all_1.6.0-1.xml => qemucaps2xmloutdata/caps_2.5.0.x86_64.xml} (67%) copy tests/{qemucaps2xmldata/all_1.6.0-1.xml => qemucaps2xmloutdata/caps_2.6.0.aarch64.xml} (62%) copy tests/{qemucaps2xmldata/all_1.6.0-1.xml => qemucaps2xmloutdata/caps_2.6.0.ppc64.xml} (56%) copy tests/{qemucaps2xmldata/all_1.6.0-1.xml => qemucaps2xmloutdata/caps_2.6.0.x86_64.xml} (67%) copy tests/{qemucaps2xmldata/all_1.6.0-1.xml => qemucaps2xmloutdata/caps_2.7.0.s390x.xml} (56%) copy tests/{qemucaps2xmldata/all_1.6.0-1.xml => qemucaps2xmloutdata/caps_2.7.0.x86_64.xml} (67%) copy tests/{qemucaps2xmldata/all_1.6.0-1.xml => qemucaps2xmloutdata/caps_2.8.0.s390x.xml} (56%) copy tests/{qemucaps2xmldata/all_1.6.0-1.xml => qemucaps2xmloutdata/caps_2.8.0.x86_64.xml} (67%) copy tests/{qemucaps2xmldata/all_1.6.0-1.xml => qemucaps2xmloutdata/caps_2.9.0.ppc64.xml} (56%) copy tests/{qemucaps2xmldata/all_1.6.0-1.xml => qemucaps2xmloutdata/caps_2.9.0.s390x.xml} (56%) copy tests/{qemucaps2xmldata/all_1.6.0-1.xml => qemucaps2xmloutdata/caps_2.9.0.x86_64.xml} (67%) copy tests/{qemucaps2xmldata/all_1.6.0-1.xml => qemucaps2xmloutdata/caps_3.0.0.ppc64.xml} (56%) copy tests/{qemucaps2xmldata/all_1.6.0-1.xml => qemucaps2xmloutdata/caps_3.0.0.riscv32.xml} (54%) create mode 100644 tests/qemucaps2xmloutdata/caps_3.0.0.riscv64.xml rename tests/{qemucaps2xmldata/all_1.6.0-1.xml => qemucaps2xmloutdata/caps_3.0.0.x86_64.xml} (67%) -- 2.17.1

While qemucaps2xml has a meager two test cases to its name, we have plenty of data from qemucapabilities which is taken from actual QEMU binaries, covers pretty much all supported QEMU versions and architectures and is even in the right format already! Rewrite qemucaps2xml so that it uses qemucapabilities data as input. Right now we have a single test case, but we're going to add a lot more next. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- tests/qemucaps2xmldata/all_1.6.0-1.caps | 129 ------------------ .../nodisksnapshot_1.6.0-1.caps | 128 ----------------- .../nodisksnapshot_1.6.0-1.xml | 32 ----- .../caps_1.6.0.x86_64.xml} | 12 +- tests/qemucaps2xmltest.c | 33 +++-- 5 files changed, 23 insertions(+), 311 deletions(-) delete mode 100644 tests/qemucaps2xmldata/all_1.6.0-1.caps delete mode 100644 tests/qemucaps2xmldata/nodisksnapshot_1.6.0-1.caps delete mode 100644 tests/qemucaps2xmldata/nodisksnapshot_1.6.0-1.xml rename tests/{qemucaps2xmldata/all_1.6.0-1.xml => qemucaps2xmloutdata/caps_1.6.0.x86_64.xml} (67%) diff --git a/tests/qemucaps2xmldata/all_1.6.0-1.caps b/tests/qemucaps2xmldata/all_1.6.0-1.caps deleted file mode 100644 index d39d0bebbf..0000000000 --- a/tests/qemucaps2xmldata/all_1.6.0-1.caps +++ /dev/null @@ -1,129 +0,0 @@ - <qemuCaps> - <flag name='mem-path'/> - <flag name='drive-serial'/> - <flag name='chardev'/> - <flag name='enable-kvm'/> - <flag name='monitor-json'/> - <flag name='balloon'/> - <flag name='device'/> - <flag name='sdl'/> - <flag name='smp-topology'/> - <flag name='netdev'/> - <flag name='rtc'/> - <flag name='vhost-net'/> - <flag name='no-hpet'/> - <flag name='no-kvm-pit'/> - <flag name='pci-configfd'/> - <flag name='nodefconfig'/> - <flag name='boot-menu'/> - <flag name='fsdev'/> - <flag name='name-process'/> - <flag name='drive-readonly'/> - <flag name='smbios-type'/> - <flag name='vga-qxl'/> - <flag name='spice'/> - <flag name='vga-none'/> - <flag name='boot-index'/> - <flag name='hda-duplex'/> - <flag name='drive-aio'/> - <flag name='pci-multibus'/> - <flag name='pci-bootindex'/> - <flag name='ccid-emulated'/> - <flag name='ccid-passthru'/> - <flag name='chardev-spicevmc'/> - <flag name='virtio-tx-alg'/> - <flag name='device-qxl-vga'/> - <flag name='pci-multifunction'/> - <flag name='virtio-blk-pci.ioeventfd'/> - <flag name='sga'/> - <flag name='virtio-blk-pci.event_idx'/> - <flag name='virtio-net-pci.event_idx'/> - <flag name='cache-directsync'/> - <flag name='piix3-usb-uhci'/> - <flag name='piix4-usb-uhci'/> - <flag name='usb-ehci'/> - <flag name='ich9-usb-ehci1'/> - <flag name='vt82c686b-usb-uhci'/> - <flag name='pci-ohci'/> - <flag name='usb-hub'/> - <flag name='no-shutdown'/> - <flag name='cache-unsafe'/> - <flag name='rombar'/> - <flag name='ich9-ahci'/> - <flag name='no-acpi'/> - <flag name='fsdev-readonly'/> - <flag name='virtio-blk-pci.scsi'/> - <flag name='blk-sg-io'/> - <flag name='drive-copy-on-read'/> - <flag name='cpu-host'/> - <flag name='fsdev-writeout'/> - <flag name='drive-iotune'/> - <flag name='system_wakeup'/> - <flag name='scsi-disk.channel'/> - <flag name='scsi-block'/> - <flag name='transaction'/> - <flag name='block-job-async'/> - <flag name='scsi-cd'/> - <flag name='ide-cd'/> - <flag name='no-user-config'/> - <flag name='hda-micro'/> - <flag name='dump-guest-memory'/> - <flag name='nec-usb-xhci'/> - <flag name='balloon-event'/> - <flag name='bridge'/> - <flag name='lsi'/> - <flag name='virtio-scsi-pci'/> - <flag name='blockio'/> - <flag name='disable-s3'/> - <flag name='disable-s4'/> - <flag name='ide-drive.wwn'/> - <flag name='scsi-disk.wwn'/> - <flag name='seccomp-sandbox'/> - <flag name='dump-guest-core'/> - <flag name='seamless-migration'/> - <flag name='block-commit'/> - <flag name='vnc'/> - <flag name='drive-mirror'/> - <flag name='usb-host.bootindex'/> - <flag name='blockdev-snapshot-sync'/> - <flag name='qxl'/> - <flag name='VGA'/> - <flag name='cirrus-vga'/> - <flag name='vmware-svga'/> - <flag name='device-video-primary'/> - <flag name='usb-serial'/> - <flag name='usb-net'/> - <flag name='add-fd'/> - <flag name='nbd-server'/> - <flag name='virtio-rng'/> - <flag name='rng-random'/> - <flag name='rng-egd'/> - <flag name='dtb'/> - <flag name='megasas'/> - <flag name='ipv6-migration'/> - <flag name='machine-opt'/> - <flag name='machine-usb-opt'/> - <flag name='pci-bridge'/> - <flag name='vfio-pci'/> - <flag name='vfio-pci.bootindex'/> - <flag name='scsi-generic'/> - <flag name='scsi-generic.bootindex'/> - <flag name='mem-merge'/> - <flag name='vnc-websocket'/> - <flag name='mlock'/> - <flag name='vnc-share-policy'/> - <flag name='device-del-event'/> - <flag name='dmi-to-pci-bridge'/> - <flag name='i440fx-pci-hole64-size'/> - <flag name='q35-pci-hole64-size'/> - <flag name='usb-storage'/> - <flag name='usb-storage.removable'/> - <flag name='virtio-mmio'/> - <flag name='ich9-intel-hda'/> - <flag name='kvm-pit-lost-tick-policy'/> - <flag name='boot-strict'/> - <flag name='pvpanic'/> - <flag name='reboot-timeout'/> - <flag name='enable-fips'/> - <flag name='name-guest'/> - </qemuCaps> diff --git a/tests/qemucaps2xmldata/nodisksnapshot_1.6.0-1.caps b/tests/qemucaps2xmldata/nodisksnapshot_1.6.0-1.caps deleted file mode 100644 index 5a0372c917..0000000000 --- a/tests/qemucaps2xmldata/nodisksnapshot_1.6.0-1.caps +++ /dev/null @@ -1,128 +0,0 @@ - <qemuCaps> - <flag name='mem-path'/> - <flag name='drive-serial'/> - <flag name='chardev'/> - <flag name='enable-kvm'/> - <flag name='monitor-json'/> - <flag name='balloon'/> - <flag name='device'/> - <flag name='sdl'/> - <flag name='smp-topology'/> - <flag name='netdev'/> - <flag name='rtc'/> - <flag name='vhost-net'/> - <flag name='no-hpet'/> - <flag name='no-kvm-pit'/> - <flag name='pci-configfd'/> - <flag name='nodefconfig'/> - <flag name='boot-menu'/> - <flag name='fsdev'/> - <flag name='name-process'/> - <flag name='drive-readonly'/> - <flag name='smbios-type'/> - <flag name='vga-qxl'/> - <flag name='spice'/> - <flag name='vga-none'/> - <flag name='boot-index'/> - <flag name='hda-duplex'/> - <flag name='drive-aio'/> - <flag name='pci-multibus'/> - <flag name='pci-bootindex'/> - <flag name='ccid-emulated'/> - <flag name='ccid-passthru'/> - <flag name='chardev-spicevmc'/> - <flag name='virtio-tx-alg'/> - <flag name='device-qxl-vga'/> - <flag name='pci-multifunction'/> - <flag name='virtio-blk-pci.ioeventfd'/> - <flag name='sga'/> - <flag name='virtio-blk-pci.event_idx'/> - <flag name='virtio-net-pci.event_idx'/> - <flag name='cache-directsync'/> - <flag name='piix3-usb-uhci'/> - <flag name='piix4-usb-uhci'/> - <flag name='usb-ehci'/> - <flag name='ich9-usb-ehci1'/> - <flag name='vt82c686b-usb-uhci'/> - <flag name='pci-ohci'/> - <flag name='usb-hub'/> - <flag name='no-shutdown'/> - <flag name='cache-unsafe'/> - <flag name='rombar'/> - <flag name='ich9-ahci'/> - <flag name='no-acpi'/> - <flag name='fsdev-readonly'/> - <flag name='virtio-blk-pci.scsi'/> - <flag name='blk-sg-io'/> - <flag name='drive-copy-on-read'/> - <flag name='cpu-host'/> - <flag name='fsdev-writeout'/> - <flag name='drive-iotune'/> - <flag name='system_wakeup'/> - <flag name='scsi-disk.channel'/> - <flag name='scsi-block'/> - <flag name='transaction'/> - <flag name='block-job-async'/> - <flag name='scsi-cd'/> - <flag name='ide-cd'/> - <flag name='no-user-config'/> - <flag name='hda-micro'/> - <flag name='dump-guest-memory'/> - <flag name='nec-usb-xhci'/> - <flag name='balloon-event'/> - <flag name='bridge'/> - <flag name='lsi'/> - <flag name='virtio-scsi-pci'/> - <flag name='blockio'/> - <flag name='disable-s3'/> - <flag name='disable-s4'/> - <flag name='ide-drive.wwn'/> - <flag name='scsi-disk.wwn'/> - <flag name='seccomp-sandbox'/> - <flag name='dump-guest-core'/> - <flag name='seamless-migration'/> - <flag name='block-commit'/> - <flag name='vnc'/> - <flag name='drive-mirror'/> - <flag name='usb-host.bootindex'/> - <flag name='qxl'/> - <flag name='VGA'/> - <flag name='cirrus-vga'/> - <flag name='vmware-svga'/> - <flag name='device-video-primary'/> - <flag name='usb-serial'/> - <flag name='usb-net'/> - <flag name='add-fd'/> - <flag name='nbd-server'/> - <flag name='virtio-rng'/> - <flag name='rng-random'/> - <flag name='rng-egd'/> - <flag name='dtb'/> - <flag name='megasas'/> - <flag name='ipv6-migration'/> - <flag name='machine-opt'/> - <flag name='machine-usb-opt'/> - <flag name='pci-bridge'/> - <flag name='vfio-pci'/> - <flag name='vfio-pci.bootindex'/> - <flag name='scsi-generic'/> - <flag name='scsi-generic.bootindex'/> - <flag name='mem-merge'/> - <flag name='vnc-websocket'/> - <flag name='mlock'/> - <flag name='vnc-share-policy'/> - <flag name='device-del-event'/> - <flag name='dmi-to-pci-bridge'/> - <flag name='i440fx-pci-hole64-size'/> - <flag name='q35-pci-hole64-size'/> - <flag name='usb-storage'/> - <flag name='usb-storage.removable'/> - <flag name='virtio-mmio'/> - <flag name='ich9-intel-hda'/> - <flag name='kvm-pit-lost-tick-policy'/> - <flag name='boot-strict'/> - <flag name='pvpanic'/> - <flag name='reboot-timeout'/> - <flag name='enable-fips'/> - <flag name='name-guest'/> - </qemuCaps> diff --git a/tests/qemucaps2xmldata/nodisksnapshot_1.6.0-1.xml b/tests/qemucaps2xmldata/nodisksnapshot_1.6.0-1.xml deleted file mode 100644 index 981344e6fd..0000000000 --- a/tests/qemucaps2xmldata/nodisksnapshot_1.6.0-1.xml +++ /dev/null @@ -1,32 +0,0 @@ -<capabilities> - - <host> - <cpu> - <arch>i686</arch> - </cpu> - <power_management/> - <iommu support='no'/> - </host> - - <guest> - <os_type>hvm</os_type> - <arch name='i686'> - <wordsize>32</wordsize> - <emulator>/usr/bin/qemu-system-i386</emulator> - <domain type='qemu'/> - <domain type='kvm'> - <emulator>/usr/bin/qemu-system-i386</emulator> - </domain> - </arch> - <features> - <cpuselection/> - <deviceboot/> - <disksnapshot default='off' toggle='no'/> - <acpi default='on' toggle='yes'/> - <apic default='on' toggle='no'/> - <pae/> - <nonpae/> - </features> - </guest> - -</capabilities> diff --git a/tests/qemucaps2xmldata/all_1.6.0-1.xml b/tests/qemucaps2xmloutdata/caps_1.6.0.x86_64.xml similarity index 67% rename from tests/qemucaps2xmldata/all_1.6.0-1.xml rename to tests/qemucaps2xmloutdata/caps_1.6.0.x86_64.xml index efe86b9a12..b58f54fefd 100644 --- a/tests/qemucaps2xmldata/all_1.6.0-1.xml +++ b/tests/qemucaps2xmloutdata/caps_1.6.0.x86_64.xml @@ -2,7 +2,7 @@ <host> <cpu> - <arch>i686</arch> + <arch>x86_64</arch> </cpu> <power_management/> <iommu support='no'/> @@ -10,12 +10,12 @@ <guest> <os_type>hvm</os_type> - <arch name='i686'> - <wordsize>32</wordsize> - <emulator>/usr/bin/qemu-system-i386</emulator> + <arch name='x86_64'> + <wordsize>64</wordsize> + <emulator>/usr/bin/qemu-system-x86_64</emulator> <domain type='qemu'/> <domain type='kvm'> - <emulator>/usr/bin/qemu-system-i386</emulator> + <emulator>/usr/bin/qemu-system-x86_64</emulator> </domain> </arch> <features> @@ -24,8 +24,6 @@ <disksnapshot default='on' toggle='no'/> <acpi default='on' toggle='yes'/> <apic default='on' toggle='no'/> - <pae/> - <nonpae/> </features> </guest> diff --git a/tests/qemucaps2xmltest.c b/tests/qemucaps2xmltest.c index 5b9152b04d..65dd97c0a4 100644 --- a/tests/qemucaps2xmltest.c +++ b/tests/qemucaps2xmltest.c @@ -32,7 +32,7 @@ typedef struct _testQemuData testQemuData; typedef testQemuData *testQemuDataPtr; struct _testQemuData { const char *base; - virArch guestarch; + const char *archName; }; static virQEMUCapsPtr @@ -87,23 +87,28 @@ testGetCaps(char *capsData, const testQemuData *data) { virQEMUCapsPtr qemuCaps = NULL; virCapsPtr caps = NULL; + virArch arch = virArchFromString(data->archName); + char *binary = NULL; + + if (virAsprintf(&binary, "/usr/bin/qemu-system-%s", data->archName) < 0) + goto error; if ((qemuCaps = testQemuGetCaps(capsData)) == NULL) { fprintf(stderr, "failed to parse qemu capabilities flags"); goto error; } - if ((caps = virCapabilitiesNew(data->guestarch, false, false)) == NULL) { + if ((caps = virCapabilitiesNew(arch, false, false)) == NULL) { fprintf(stderr, "failed to create the fake capabilities"); goto error; } if (virQEMUCapsInitGuestFromBinary(caps, - "/usr/bin/qemu-system-i386", + binary, qemuCaps, NULL, NULL, - data->guestarch) < 0) { + arch) < 0) { fprintf(stderr, "failed to create the capabilities from qemu"); goto error; } @@ -114,6 +119,7 @@ testGetCaps(char *capsData, const testQemuData *data) error: virObjectUnref(qemuCaps); virObjectUnref(caps); + VIR_FREE(binary); return NULL; } @@ -127,12 +133,12 @@ testQemuCapsXML(const void *opaque) char *capsXml = NULL; virCapsPtr capsProvided = NULL; - if (virAsprintf(&xmlFile, "%s/qemucaps2xmldata/%s.xml", - abs_srcdir, data->base) < 0) + if (virAsprintf(&xmlFile, "%s/qemucaps2xmloutdata/%s.%s.xml", + abs_srcdir, data->base, data->archName) < 0) goto cleanup; - if (virAsprintf(&capsFile, "%s/qemucaps2xmldata/%s.caps", - abs_srcdir, data->base) < 0) + if (virAsprintf(&capsFile, "%s/qemucapabilitiesdata/%s.%s.xml", + abs_srcdir, data->base, data->archName) < 0) goto cleanup; if (virTestLoadFile(capsFile, &capsData) < 0) @@ -175,16 +181,13 @@ mymain(void) virEventRegisterDefaultImpl(); -#define DO_TEST_FULL(name, guest) \ +#define DO_TEST(arch, name) \ + data.archName = arch; \ data.base = name; \ - data.guestarch = guest; \ - if (virTestRun(name, testQemuCapsXML, &data) < 0) \ + if (virTestRun(name "(" arch ")", testQemuCapsXML, &data) < 0) \ ret = -1 -#define DO_TEST(name) DO_TEST_FULL(name, VIR_ARCH_I686) - - DO_TEST("all_1.6.0-1"); - DO_TEST("nodisksnapshot_1.6.0-1"); + DO_TEST("x86_64", "caps_1.6.0"); return (ret == 0) ? EXIT_SUCCESS : EXIT_FAILURE; } -- 2.17.1

More specifically, everything that's tested by qemucapabilities now goes through qemucaps2xml as well. Ideally we'll rewrite both so that listing all test cases is unnecessary and they get picked up automatically by listing the contents of the input directory instead, but that's a refactor for another day :) Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- tests/qemucapabilitiestest.c | 1 + .../qemucaps2xmloutdata/caps_1.5.3.x86_64.xml | 30 +++++++++++++++++++ .../qemucaps2xmloutdata/caps_1.7.0.x86_64.xml | 30 +++++++++++++++++++ .../qemucaps2xmloutdata/caps_2.1.1.x86_64.xml | 30 +++++++++++++++++++ .../caps_2.10.0.aarch64.xml | 29 ++++++++++++++++++ .../qemucaps2xmloutdata/caps_2.10.0.ppc64.xml | 28 +++++++++++++++++ .../qemucaps2xmloutdata/caps_2.10.0.s390x.xml | 28 +++++++++++++++++ .../caps_2.10.0.x86_64.xml | 30 +++++++++++++++++++ .../qemucaps2xmloutdata/caps_2.11.0.s390x.xml | 28 +++++++++++++++++ .../caps_2.11.0.x86_64.xml | 30 +++++++++++++++++++ .../caps_2.12.0.aarch64.xml | 29 ++++++++++++++++++ .../qemucaps2xmloutdata/caps_2.12.0.ppc64.xml | 28 +++++++++++++++++ .../qemucaps2xmloutdata/caps_2.12.0.s390x.xml | 28 +++++++++++++++++ .../caps_2.12.0.x86_64.xml | 30 +++++++++++++++++++ .../qemucaps2xmloutdata/caps_2.4.0.x86_64.xml | 30 +++++++++++++++++++ .../qemucaps2xmloutdata/caps_2.5.0.x86_64.xml | 30 +++++++++++++++++++ .../caps_2.6.0.aarch64.xml | 29 ++++++++++++++++++ .../qemucaps2xmloutdata/caps_2.6.0.ppc64.xml | 28 +++++++++++++++++ .../qemucaps2xmloutdata/caps_2.6.0.x86_64.xml | 30 +++++++++++++++++++ .../qemucaps2xmloutdata/caps_2.7.0.s390x.xml | 28 +++++++++++++++++ .../qemucaps2xmloutdata/caps_2.7.0.x86_64.xml | 30 +++++++++++++++++++ .../qemucaps2xmloutdata/caps_2.8.0.s390x.xml | 28 +++++++++++++++++ .../qemucaps2xmloutdata/caps_2.8.0.x86_64.xml | 30 +++++++++++++++++++ .../qemucaps2xmloutdata/caps_2.9.0.ppc64.xml | 28 +++++++++++++++++ .../qemucaps2xmloutdata/caps_2.9.0.s390x.xml | 28 +++++++++++++++++ .../qemucaps2xmloutdata/caps_2.9.0.x86_64.xml | 30 +++++++++++++++++++ .../qemucaps2xmloutdata/caps_3.0.0.ppc64.xml | 28 +++++++++++++++++ .../caps_3.0.0.riscv32.xml | 25 ++++++++++++++++ .../caps_3.0.0.riscv64.xml | 25 ++++++++++++++++ .../qemucaps2xmloutdata/caps_3.0.0.x86_64.xml | 30 +++++++++++++++++++ tests/qemucaps2xmltest.c | 30 +++++++++++++++++++ 31 files changed, 866 insertions(+) create mode 100644 tests/qemucaps2xmloutdata/caps_1.5.3.x86_64.xml create mode 100644 tests/qemucaps2xmloutdata/caps_1.7.0.x86_64.xml create mode 100644 tests/qemucaps2xmloutdata/caps_2.1.1.x86_64.xml create mode 100644 tests/qemucaps2xmloutdata/caps_2.10.0.aarch64.xml create mode 100644 tests/qemucaps2xmloutdata/caps_2.10.0.ppc64.xml create mode 100644 tests/qemucaps2xmloutdata/caps_2.10.0.s390x.xml create mode 100644 tests/qemucaps2xmloutdata/caps_2.10.0.x86_64.xml create mode 100644 tests/qemucaps2xmloutdata/caps_2.11.0.s390x.xml create mode 100644 tests/qemucaps2xmloutdata/caps_2.11.0.x86_64.xml create mode 100644 tests/qemucaps2xmloutdata/caps_2.12.0.aarch64.xml create mode 100644 tests/qemucaps2xmloutdata/caps_2.12.0.ppc64.xml create mode 100644 tests/qemucaps2xmloutdata/caps_2.12.0.s390x.xml create mode 100644 tests/qemucaps2xmloutdata/caps_2.12.0.x86_64.xml create mode 100644 tests/qemucaps2xmloutdata/caps_2.4.0.x86_64.xml create mode 100644 tests/qemucaps2xmloutdata/caps_2.5.0.x86_64.xml create mode 100644 tests/qemucaps2xmloutdata/caps_2.6.0.aarch64.xml create mode 100644 tests/qemucaps2xmloutdata/caps_2.6.0.ppc64.xml create mode 100644 tests/qemucaps2xmloutdata/caps_2.6.0.x86_64.xml create mode 100644 tests/qemucaps2xmloutdata/caps_2.7.0.s390x.xml create mode 100644 tests/qemucaps2xmloutdata/caps_2.7.0.x86_64.xml create mode 100644 tests/qemucaps2xmloutdata/caps_2.8.0.s390x.xml create mode 100644 tests/qemucaps2xmloutdata/caps_2.8.0.x86_64.xml create mode 100644 tests/qemucaps2xmloutdata/caps_2.9.0.ppc64.xml create mode 100644 tests/qemucaps2xmloutdata/caps_2.9.0.s390x.xml create mode 100644 tests/qemucaps2xmloutdata/caps_2.9.0.x86_64.xml create mode 100644 tests/qemucaps2xmloutdata/caps_3.0.0.ppc64.xml create mode 100644 tests/qemucaps2xmloutdata/caps_3.0.0.riscv32.xml create mode 100644 tests/qemucaps2xmloutdata/caps_3.0.0.riscv64.xml create mode 100644 tests/qemucaps2xmloutdata/caps_3.0.0.x86_64.xml diff --git a/tests/qemucapabilitiestest.c b/tests/qemucapabilitiestest.c index e53023b3b9..498876e383 100644 --- a/tests/qemucapabilitiestest.c +++ b/tests/qemucapabilitiestest.c @@ -163,6 +163,7 @@ mymain(void) ret = -1; \ } while (0) + /* Keep this in sync with qemucaps2xmltest */ DO_TEST("x86_64", "caps_1.5.3"); DO_TEST("x86_64", "caps_1.6.0"); DO_TEST("x86_64", "caps_1.7.0"); diff --git a/tests/qemucaps2xmloutdata/caps_1.5.3.x86_64.xml b/tests/qemucaps2xmloutdata/caps_1.5.3.x86_64.xml new file mode 100644 index 0000000000..b58f54fefd --- /dev/null +++ b/tests/qemucaps2xmloutdata/caps_1.5.3.x86_64.xml @@ -0,0 +1,30 @@ +<capabilities> + + <host> + <cpu> + <arch>x86_64</arch> + </cpu> + <power_management/> + <iommu support='no'/> + </host> + + <guest> + <os_type>hvm</os_type> + <arch name='x86_64'> + <wordsize>64</wordsize> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + <domain type='qemu'/> + <domain type='kvm'> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + </domain> + </arch> + <features> + <cpuselection/> + <deviceboot/> + <disksnapshot default='on' toggle='no'/> + <acpi default='on' toggle='yes'/> + <apic default='on' toggle='no'/> + </features> + </guest> + +</capabilities> diff --git a/tests/qemucaps2xmloutdata/caps_1.7.0.x86_64.xml b/tests/qemucaps2xmloutdata/caps_1.7.0.x86_64.xml new file mode 100644 index 0000000000..b58f54fefd --- /dev/null +++ b/tests/qemucaps2xmloutdata/caps_1.7.0.x86_64.xml @@ -0,0 +1,30 @@ +<capabilities> + + <host> + <cpu> + <arch>x86_64</arch> + </cpu> + <power_management/> + <iommu support='no'/> + </host> + + <guest> + <os_type>hvm</os_type> + <arch name='x86_64'> + <wordsize>64</wordsize> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + <domain type='qemu'/> + <domain type='kvm'> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + </domain> + </arch> + <features> + <cpuselection/> + <deviceboot/> + <disksnapshot default='on' toggle='no'/> + <acpi default='on' toggle='yes'/> + <apic default='on' toggle='no'/> + </features> + </guest> + +</capabilities> diff --git a/tests/qemucaps2xmloutdata/caps_2.1.1.x86_64.xml b/tests/qemucaps2xmloutdata/caps_2.1.1.x86_64.xml new file mode 100644 index 0000000000..b58f54fefd --- /dev/null +++ b/tests/qemucaps2xmloutdata/caps_2.1.1.x86_64.xml @@ -0,0 +1,30 @@ +<capabilities> + + <host> + <cpu> + <arch>x86_64</arch> + </cpu> + <power_management/> + <iommu support='no'/> + </host> + + <guest> + <os_type>hvm</os_type> + <arch name='x86_64'> + <wordsize>64</wordsize> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + <domain type='qemu'/> + <domain type='kvm'> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + </domain> + </arch> + <features> + <cpuselection/> + <deviceboot/> + <disksnapshot default='on' toggle='no'/> + <acpi default='on' toggle='yes'/> + <apic default='on' toggle='no'/> + </features> + </guest> + +</capabilities> diff --git a/tests/qemucaps2xmloutdata/caps_2.10.0.aarch64.xml b/tests/qemucaps2xmloutdata/caps_2.10.0.aarch64.xml new file mode 100644 index 0000000000..a879d67df3 --- /dev/null +++ b/tests/qemucaps2xmloutdata/caps_2.10.0.aarch64.xml @@ -0,0 +1,29 @@ +<capabilities> + + <host> + <cpu> + <arch>aarch64</arch> + </cpu> + <power_management/> + <iommu support='no'/> + </host> + + <guest> + <os_type>hvm</os_type> + <arch name='aarch64'> + <wordsize>64</wordsize> + <emulator>/usr/bin/qemu-system-aarch64</emulator> + <domain type='qemu'/> + <domain type='kvm'> + <emulator>/usr/bin/qemu-system-aarch64</emulator> + </domain> + </arch> + <features> + <cpuselection/> + <deviceboot/> + <disksnapshot default='on' toggle='no'/> + <acpi default='on' toggle='yes'/> + </features> + </guest> + +</capabilities> diff --git a/tests/qemucaps2xmloutdata/caps_2.10.0.ppc64.xml b/tests/qemucaps2xmloutdata/caps_2.10.0.ppc64.xml new file mode 100644 index 0000000000..74eaf3ba0e --- /dev/null +++ b/tests/qemucaps2xmloutdata/caps_2.10.0.ppc64.xml @@ -0,0 +1,28 @@ +<capabilities> + + <host> + <cpu> + <arch>ppc64</arch> + </cpu> + <power_management/> + <iommu support='no'/> + </host> + + <guest> + <os_type>hvm</os_type> + <arch name='ppc64'> + <wordsize>64</wordsize> + <emulator>/usr/bin/qemu-system-ppc64</emulator> + <domain type='qemu'/> + <domain type='kvm'> + <emulator>/usr/bin/qemu-system-ppc64</emulator> + </domain> + </arch> + <features> + <cpuselection/> + <deviceboot/> + <disksnapshot default='on' toggle='no'/> + </features> + </guest> + +</capabilities> diff --git a/tests/qemucaps2xmloutdata/caps_2.10.0.s390x.xml b/tests/qemucaps2xmloutdata/caps_2.10.0.s390x.xml new file mode 100644 index 0000000000..20ef995d62 --- /dev/null +++ b/tests/qemucaps2xmloutdata/caps_2.10.0.s390x.xml @@ -0,0 +1,28 @@ +<capabilities> + + <host> + <cpu> + <arch>s390x</arch> + </cpu> + <power_management/> + <iommu support='no'/> + </host> + + <guest> + <os_type>hvm</os_type> + <arch name='s390x'> + <wordsize>64</wordsize> + <emulator>/usr/bin/qemu-system-s390x</emulator> + <domain type='qemu'/> + <domain type='kvm'> + <emulator>/usr/bin/qemu-system-s390x</emulator> + </domain> + </arch> + <features> + <cpuselection/> + <deviceboot/> + <disksnapshot default='on' toggle='no'/> + </features> + </guest> + +</capabilities> diff --git a/tests/qemucaps2xmloutdata/caps_2.10.0.x86_64.xml b/tests/qemucaps2xmloutdata/caps_2.10.0.x86_64.xml new file mode 100644 index 0000000000..b58f54fefd --- /dev/null +++ b/tests/qemucaps2xmloutdata/caps_2.10.0.x86_64.xml @@ -0,0 +1,30 @@ +<capabilities> + + <host> + <cpu> + <arch>x86_64</arch> + </cpu> + <power_management/> + <iommu support='no'/> + </host> + + <guest> + <os_type>hvm</os_type> + <arch name='x86_64'> + <wordsize>64</wordsize> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + <domain type='qemu'/> + <domain type='kvm'> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + </domain> + </arch> + <features> + <cpuselection/> + <deviceboot/> + <disksnapshot default='on' toggle='no'/> + <acpi default='on' toggle='yes'/> + <apic default='on' toggle='no'/> + </features> + </guest> + +</capabilities> diff --git a/tests/qemucaps2xmloutdata/caps_2.11.0.s390x.xml b/tests/qemucaps2xmloutdata/caps_2.11.0.s390x.xml new file mode 100644 index 0000000000..20ef995d62 --- /dev/null +++ b/tests/qemucaps2xmloutdata/caps_2.11.0.s390x.xml @@ -0,0 +1,28 @@ +<capabilities> + + <host> + <cpu> + <arch>s390x</arch> + </cpu> + <power_management/> + <iommu support='no'/> + </host> + + <guest> + <os_type>hvm</os_type> + <arch name='s390x'> + <wordsize>64</wordsize> + <emulator>/usr/bin/qemu-system-s390x</emulator> + <domain type='qemu'/> + <domain type='kvm'> + <emulator>/usr/bin/qemu-system-s390x</emulator> + </domain> + </arch> + <features> + <cpuselection/> + <deviceboot/> + <disksnapshot default='on' toggle='no'/> + </features> + </guest> + +</capabilities> diff --git a/tests/qemucaps2xmloutdata/caps_2.11.0.x86_64.xml b/tests/qemucaps2xmloutdata/caps_2.11.0.x86_64.xml new file mode 100644 index 0000000000..b58f54fefd --- /dev/null +++ b/tests/qemucaps2xmloutdata/caps_2.11.0.x86_64.xml @@ -0,0 +1,30 @@ +<capabilities> + + <host> + <cpu> + <arch>x86_64</arch> + </cpu> + <power_management/> + <iommu support='no'/> + </host> + + <guest> + <os_type>hvm</os_type> + <arch name='x86_64'> + <wordsize>64</wordsize> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + <domain type='qemu'/> + <domain type='kvm'> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + </domain> + </arch> + <features> + <cpuselection/> + <deviceboot/> + <disksnapshot default='on' toggle='no'/> + <acpi default='on' toggle='yes'/> + <apic default='on' toggle='no'/> + </features> + </guest> + +</capabilities> diff --git a/tests/qemucaps2xmloutdata/caps_2.12.0.aarch64.xml b/tests/qemucaps2xmloutdata/caps_2.12.0.aarch64.xml new file mode 100644 index 0000000000..a879d67df3 --- /dev/null +++ b/tests/qemucaps2xmloutdata/caps_2.12.0.aarch64.xml @@ -0,0 +1,29 @@ +<capabilities> + + <host> + <cpu> + <arch>aarch64</arch> + </cpu> + <power_management/> + <iommu support='no'/> + </host> + + <guest> + <os_type>hvm</os_type> + <arch name='aarch64'> + <wordsize>64</wordsize> + <emulator>/usr/bin/qemu-system-aarch64</emulator> + <domain type='qemu'/> + <domain type='kvm'> + <emulator>/usr/bin/qemu-system-aarch64</emulator> + </domain> + </arch> + <features> + <cpuselection/> + <deviceboot/> + <disksnapshot default='on' toggle='no'/> + <acpi default='on' toggle='yes'/> + </features> + </guest> + +</capabilities> diff --git a/tests/qemucaps2xmloutdata/caps_2.12.0.ppc64.xml b/tests/qemucaps2xmloutdata/caps_2.12.0.ppc64.xml new file mode 100644 index 0000000000..74eaf3ba0e --- /dev/null +++ b/tests/qemucaps2xmloutdata/caps_2.12.0.ppc64.xml @@ -0,0 +1,28 @@ +<capabilities> + + <host> + <cpu> + <arch>ppc64</arch> + </cpu> + <power_management/> + <iommu support='no'/> + </host> + + <guest> + <os_type>hvm</os_type> + <arch name='ppc64'> + <wordsize>64</wordsize> + <emulator>/usr/bin/qemu-system-ppc64</emulator> + <domain type='qemu'/> + <domain type='kvm'> + <emulator>/usr/bin/qemu-system-ppc64</emulator> + </domain> + </arch> + <features> + <cpuselection/> + <deviceboot/> + <disksnapshot default='on' toggle='no'/> + </features> + </guest> + +</capabilities> diff --git a/tests/qemucaps2xmloutdata/caps_2.12.0.s390x.xml b/tests/qemucaps2xmloutdata/caps_2.12.0.s390x.xml new file mode 100644 index 0000000000..20ef995d62 --- /dev/null +++ b/tests/qemucaps2xmloutdata/caps_2.12.0.s390x.xml @@ -0,0 +1,28 @@ +<capabilities> + + <host> + <cpu> + <arch>s390x</arch> + </cpu> + <power_management/> + <iommu support='no'/> + </host> + + <guest> + <os_type>hvm</os_type> + <arch name='s390x'> + <wordsize>64</wordsize> + <emulator>/usr/bin/qemu-system-s390x</emulator> + <domain type='qemu'/> + <domain type='kvm'> + <emulator>/usr/bin/qemu-system-s390x</emulator> + </domain> + </arch> + <features> + <cpuselection/> + <deviceboot/> + <disksnapshot default='on' toggle='no'/> + </features> + </guest> + +</capabilities> diff --git a/tests/qemucaps2xmloutdata/caps_2.12.0.x86_64.xml b/tests/qemucaps2xmloutdata/caps_2.12.0.x86_64.xml new file mode 100644 index 0000000000..b58f54fefd --- /dev/null +++ b/tests/qemucaps2xmloutdata/caps_2.12.0.x86_64.xml @@ -0,0 +1,30 @@ +<capabilities> + + <host> + <cpu> + <arch>x86_64</arch> + </cpu> + <power_management/> + <iommu support='no'/> + </host> + + <guest> + <os_type>hvm</os_type> + <arch name='x86_64'> + <wordsize>64</wordsize> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + <domain type='qemu'/> + <domain type='kvm'> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + </domain> + </arch> + <features> + <cpuselection/> + <deviceboot/> + <disksnapshot default='on' toggle='no'/> + <acpi default='on' toggle='yes'/> + <apic default='on' toggle='no'/> + </features> + </guest> + +</capabilities> diff --git a/tests/qemucaps2xmloutdata/caps_2.4.0.x86_64.xml b/tests/qemucaps2xmloutdata/caps_2.4.0.x86_64.xml new file mode 100644 index 0000000000..b58f54fefd --- /dev/null +++ b/tests/qemucaps2xmloutdata/caps_2.4.0.x86_64.xml @@ -0,0 +1,30 @@ +<capabilities> + + <host> + <cpu> + <arch>x86_64</arch> + </cpu> + <power_management/> + <iommu support='no'/> + </host> + + <guest> + <os_type>hvm</os_type> + <arch name='x86_64'> + <wordsize>64</wordsize> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + <domain type='qemu'/> + <domain type='kvm'> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + </domain> + </arch> + <features> + <cpuselection/> + <deviceboot/> + <disksnapshot default='on' toggle='no'/> + <acpi default='on' toggle='yes'/> + <apic default='on' toggle='no'/> + </features> + </guest> + +</capabilities> diff --git a/tests/qemucaps2xmloutdata/caps_2.5.0.x86_64.xml b/tests/qemucaps2xmloutdata/caps_2.5.0.x86_64.xml new file mode 100644 index 0000000000..b58f54fefd --- /dev/null +++ b/tests/qemucaps2xmloutdata/caps_2.5.0.x86_64.xml @@ -0,0 +1,30 @@ +<capabilities> + + <host> + <cpu> + <arch>x86_64</arch> + </cpu> + <power_management/> + <iommu support='no'/> + </host> + + <guest> + <os_type>hvm</os_type> + <arch name='x86_64'> + <wordsize>64</wordsize> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + <domain type='qemu'/> + <domain type='kvm'> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + </domain> + </arch> + <features> + <cpuselection/> + <deviceboot/> + <disksnapshot default='on' toggle='no'/> + <acpi default='on' toggle='yes'/> + <apic default='on' toggle='no'/> + </features> + </guest> + +</capabilities> diff --git a/tests/qemucaps2xmloutdata/caps_2.6.0.aarch64.xml b/tests/qemucaps2xmloutdata/caps_2.6.0.aarch64.xml new file mode 100644 index 0000000000..a879d67df3 --- /dev/null +++ b/tests/qemucaps2xmloutdata/caps_2.6.0.aarch64.xml @@ -0,0 +1,29 @@ +<capabilities> + + <host> + <cpu> + <arch>aarch64</arch> + </cpu> + <power_management/> + <iommu support='no'/> + </host> + + <guest> + <os_type>hvm</os_type> + <arch name='aarch64'> + <wordsize>64</wordsize> + <emulator>/usr/bin/qemu-system-aarch64</emulator> + <domain type='qemu'/> + <domain type='kvm'> + <emulator>/usr/bin/qemu-system-aarch64</emulator> + </domain> + </arch> + <features> + <cpuselection/> + <deviceboot/> + <disksnapshot default='on' toggle='no'/> + <acpi default='on' toggle='yes'/> + </features> + </guest> + +</capabilities> diff --git a/tests/qemucaps2xmloutdata/caps_2.6.0.ppc64.xml b/tests/qemucaps2xmloutdata/caps_2.6.0.ppc64.xml new file mode 100644 index 0000000000..74eaf3ba0e --- /dev/null +++ b/tests/qemucaps2xmloutdata/caps_2.6.0.ppc64.xml @@ -0,0 +1,28 @@ +<capabilities> + + <host> + <cpu> + <arch>ppc64</arch> + </cpu> + <power_management/> + <iommu support='no'/> + </host> + + <guest> + <os_type>hvm</os_type> + <arch name='ppc64'> + <wordsize>64</wordsize> + <emulator>/usr/bin/qemu-system-ppc64</emulator> + <domain type='qemu'/> + <domain type='kvm'> + <emulator>/usr/bin/qemu-system-ppc64</emulator> + </domain> + </arch> + <features> + <cpuselection/> + <deviceboot/> + <disksnapshot default='on' toggle='no'/> + </features> + </guest> + +</capabilities> diff --git a/tests/qemucaps2xmloutdata/caps_2.6.0.x86_64.xml b/tests/qemucaps2xmloutdata/caps_2.6.0.x86_64.xml new file mode 100644 index 0000000000..b58f54fefd --- /dev/null +++ b/tests/qemucaps2xmloutdata/caps_2.6.0.x86_64.xml @@ -0,0 +1,30 @@ +<capabilities> + + <host> + <cpu> + <arch>x86_64</arch> + </cpu> + <power_management/> + <iommu support='no'/> + </host> + + <guest> + <os_type>hvm</os_type> + <arch name='x86_64'> + <wordsize>64</wordsize> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + <domain type='qemu'/> + <domain type='kvm'> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + </domain> + </arch> + <features> + <cpuselection/> + <deviceboot/> + <disksnapshot default='on' toggle='no'/> + <acpi default='on' toggle='yes'/> + <apic default='on' toggle='no'/> + </features> + </guest> + +</capabilities> diff --git a/tests/qemucaps2xmloutdata/caps_2.7.0.s390x.xml b/tests/qemucaps2xmloutdata/caps_2.7.0.s390x.xml new file mode 100644 index 0000000000..20ef995d62 --- /dev/null +++ b/tests/qemucaps2xmloutdata/caps_2.7.0.s390x.xml @@ -0,0 +1,28 @@ +<capabilities> + + <host> + <cpu> + <arch>s390x</arch> + </cpu> + <power_management/> + <iommu support='no'/> + </host> + + <guest> + <os_type>hvm</os_type> + <arch name='s390x'> + <wordsize>64</wordsize> + <emulator>/usr/bin/qemu-system-s390x</emulator> + <domain type='qemu'/> + <domain type='kvm'> + <emulator>/usr/bin/qemu-system-s390x</emulator> + </domain> + </arch> + <features> + <cpuselection/> + <deviceboot/> + <disksnapshot default='on' toggle='no'/> + </features> + </guest> + +</capabilities> diff --git a/tests/qemucaps2xmloutdata/caps_2.7.0.x86_64.xml b/tests/qemucaps2xmloutdata/caps_2.7.0.x86_64.xml new file mode 100644 index 0000000000..b58f54fefd --- /dev/null +++ b/tests/qemucaps2xmloutdata/caps_2.7.0.x86_64.xml @@ -0,0 +1,30 @@ +<capabilities> + + <host> + <cpu> + <arch>x86_64</arch> + </cpu> + <power_management/> + <iommu support='no'/> + </host> + + <guest> + <os_type>hvm</os_type> + <arch name='x86_64'> + <wordsize>64</wordsize> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + <domain type='qemu'/> + <domain type='kvm'> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + </domain> + </arch> + <features> + <cpuselection/> + <deviceboot/> + <disksnapshot default='on' toggle='no'/> + <acpi default='on' toggle='yes'/> + <apic default='on' toggle='no'/> + </features> + </guest> + +</capabilities> diff --git a/tests/qemucaps2xmloutdata/caps_2.8.0.s390x.xml b/tests/qemucaps2xmloutdata/caps_2.8.0.s390x.xml new file mode 100644 index 0000000000..20ef995d62 --- /dev/null +++ b/tests/qemucaps2xmloutdata/caps_2.8.0.s390x.xml @@ -0,0 +1,28 @@ +<capabilities> + + <host> + <cpu> + <arch>s390x</arch> + </cpu> + <power_management/> + <iommu support='no'/> + </host> + + <guest> + <os_type>hvm</os_type> + <arch name='s390x'> + <wordsize>64</wordsize> + <emulator>/usr/bin/qemu-system-s390x</emulator> + <domain type='qemu'/> + <domain type='kvm'> + <emulator>/usr/bin/qemu-system-s390x</emulator> + </domain> + </arch> + <features> + <cpuselection/> + <deviceboot/> + <disksnapshot default='on' toggle='no'/> + </features> + </guest> + +</capabilities> diff --git a/tests/qemucaps2xmloutdata/caps_2.8.0.x86_64.xml b/tests/qemucaps2xmloutdata/caps_2.8.0.x86_64.xml new file mode 100644 index 0000000000..b58f54fefd --- /dev/null +++ b/tests/qemucaps2xmloutdata/caps_2.8.0.x86_64.xml @@ -0,0 +1,30 @@ +<capabilities> + + <host> + <cpu> + <arch>x86_64</arch> + </cpu> + <power_management/> + <iommu support='no'/> + </host> + + <guest> + <os_type>hvm</os_type> + <arch name='x86_64'> + <wordsize>64</wordsize> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + <domain type='qemu'/> + <domain type='kvm'> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + </domain> + </arch> + <features> + <cpuselection/> + <deviceboot/> + <disksnapshot default='on' toggle='no'/> + <acpi default='on' toggle='yes'/> + <apic default='on' toggle='no'/> + </features> + </guest> + +</capabilities> diff --git a/tests/qemucaps2xmloutdata/caps_2.9.0.ppc64.xml b/tests/qemucaps2xmloutdata/caps_2.9.0.ppc64.xml new file mode 100644 index 0000000000..74eaf3ba0e --- /dev/null +++ b/tests/qemucaps2xmloutdata/caps_2.9.0.ppc64.xml @@ -0,0 +1,28 @@ +<capabilities> + + <host> + <cpu> + <arch>ppc64</arch> + </cpu> + <power_management/> + <iommu support='no'/> + </host> + + <guest> + <os_type>hvm</os_type> + <arch name='ppc64'> + <wordsize>64</wordsize> + <emulator>/usr/bin/qemu-system-ppc64</emulator> + <domain type='qemu'/> + <domain type='kvm'> + <emulator>/usr/bin/qemu-system-ppc64</emulator> + </domain> + </arch> + <features> + <cpuselection/> + <deviceboot/> + <disksnapshot default='on' toggle='no'/> + </features> + </guest> + +</capabilities> diff --git a/tests/qemucaps2xmloutdata/caps_2.9.0.s390x.xml b/tests/qemucaps2xmloutdata/caps_2.9.0.s390x.xml new file mode 100644 index 0000000000..20ef995d62 --- /dev/null +++ b/tests/qemucaps2xmloutdata/caps_2.9.0.s390x.xml @@ -0,0 +1,28 @@ +<capabilities> + + <host> + <cpu> + <arch>s390x</arch> + </cpu> + <power_management/> + <iommu support='no'/> + </host> + + <guest> + <os_type>hvm</os_type> + <arch name='s390x'> + <wordsize>64</wordsize> + <emulator>/usr/bin/qemu-system-s390x</emulator> + <domain type='qemu'/> + <domain type='kvm'> + <emulator>/usr/bin/qemu-system-s390x</emulator> + </domain> + </arch> + <features> + <cpuselection/> + <deviceboot/> + <disksnapshot default='on' toggle='no'/> + </features> + </guest> + +</capabilities> diff --git a/tests/qemucaps2xmloutdata/caps_2.9.0.x86_64.xml b/tests/qemucaps2xmloutdata/caps_2.9.0.x86_64.xml new file mode 100644 index 0000000000..b58f54fefd --- /dev/null +++ b/tests/qemucaps2xmloutdata/caps_2.9.0.x86_64.xml @@ -0,0 +1,30 @@ +<capabilities> + + <host> + <cpu> + <arch>x86_64</arch> + </cpu> + <power_management/> + <iommu support='no'/> + </host> + + <guest> + <os_type>hvm</os_type> + <arch name='x86_64'> + <wordsize>64</wordsize> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + <domain type='qemu'/> + <domain type='kvm'> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + </domain> + </arch> + <features> + <cpuselection/> + <deviceboot/> + <disksnapshot default='on' toggle='no'/> + <acpi default='on' toggle='yes'/> + <apic default='on' toggle='no'/> + </features> + </guest> + +</capabilities> diff --git a/tests/qemucaps2xmloutdata/caps_3.0.0.ppc64.xml b/tests/qemucaps2xmloutdata/caps_3.0.0.ppc64.xml new file mode 100644 index 0000000000..74eaf3ba0e --- /dev/null +++ b/tests/qemucaps2xmloutdata/caps_3.0.0.ppc64.xml @@ -0,0 +1,28 @@ +<capabilities> + + <host> + <cpu> + <arch>ppc64</arch> + </cpu> + <power_management/> + <iommu support='no'/> + </host> + + <guest> + <os_type>hvm</os_type> + <arch name='ppc64'> + <wordsize>64</wordsize> + <emulator>/usr/bin/qemu-system-ppc64</emulator> + <domain type='qemu'/> + <domain type='kvm'> + <emulator>/usr/bin/qemu-system-ppc64</emulator> + </domain> + </arch> + <features> + <cpuselection/> + <deviceboot/> + <disksnapshot default='on' toggle='no'/> + </features> + </guest> + +</capabilities> diff --git a/tests/qemucaps2xmloutdata/caps_3.0.0.riscv32.xml b/tests/qemucaps2xmloutdata/caps_3.0.0.riscv32.xml new file mode 100644 index 0000000000..63c374da7f --- /dev/null +++ b/tests/qemucaps2xmloutdata/caps_3.0.0.riscv32.xml @@ -0,0 +1,25 @@ +<capabilities> + + <host> + <cpu> + <arch>riscv32</arch> + </cpu> + <power_management/> + <iommu support='no'/> + </host> + + <guest> + <os_type>hvm</os_type> + <arch name='riscv32'> + <wordsize>32</wordsize> + <emulator>/usr/bin/qemu-system-riscv32</emulator> + <domain type='qemu'/> + </arch> + <features> + <cpuselection/> + <deviceboot/> + <disksnapshot default='on' toggle='no'/> + </features> + </guest> + +</capabilities> diff --git a/tests/qemucaps2xmloutdata/caps_3.0.0.riscv64.xml b/tests/qemucaps2xmloutdata/caps_3.0.0.riscv64.xml new file mode 100644 index 0000000000..09b7eb7f2f --- /dev/null +++ b/tests/qemucaps2xmloutdata/caps_3.0.0.riscv64.xml @@ -0,0 +1,25 @@ +<capabilities> + + <host> + <cpu> + <arch>riscv64</arch> + </cpu> + <power_management/> + <iommu support='no'/> + </host> + + <guest> + <os_type>hvm</os_type> + <arch name='riscv64'> + <wordsize>64</wordsize> + <emulator>/usr/bin/qemu-system-riscv64</emulator> + <domain type='qemu'/> + </arch> + <features> + <cpuselection/> + <deviceboot/> + <disksnapshot default='on' toggle='no'/> + </features> + </guest> + +</capabilities> diff --git a/tests/qemucaps2xmloutdata/caps_3.0.0.x86_64.xml b/tests/qemucaps2xmloutdata/caps_3.0.0.x86_64.xml new file mode 100644 index 0000000000..b58f54fefd --- /dev/null +++ b/tests/qemucaps2xmloutdata/caps_3.0.0.x86_64.xml @@ -0,0 +1,30 @@ +<capabilities> + + <host> + <cpu> + <arch>x86_64</arch> + </cpu> + <power_management/> + <iommu support='no'/> + </host> + + <guest> + <os_type>hvm</os_type> + <arch name='x86_64'> + <wordsize>64</wordsize> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + <domain type='qemu'/> + <domain type='kvm'> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + </domain> + </arch> + <features> + <cpuselection/> + <deviceboot/> + <disksnapshot default='on' toggle='no'/> + <acpi default='on' toggle='yes'/> + <apic default='on' toggle='no'/> + </features> + </guest> + +</capabilities> diff --git a/tests/qemucaps2xmltest.c b/tests/qemucaps2xmltest.c index 65dd97c0a4..e765a03b73 100644 --- a/tests/qemucaps2xmltest.c +++ b/tests/qemucaps2xmltest.c @@ -187,7 +187,37 @@ mymain(void) if (virTestRun(name "(" arch ")", testQemuCapsXML, &data) < 0) \ ret = -1 + /* Keep this in sync with qemucapabilitiestest */ + DO_TEST("x86_64", "caps_1.5.3"); DO_TEST("x86_64", "caps_1.6.0"); + DO_TEST("x86_64", "caps_1.7.0"); + DO_TEST("x86_64", "caps_2.1.1"); + DO_TEST("x86_64", "caps_2.4.0"); + DO_TEST("x86_64", "caps_2.5.0"); + DO_TEST("x86_64", "caps_2.6.0"); + DO_TEST("x86_64", "caps_2.7.0"); + DO_TEST("x86_64", "caps_2.8.0"); + DO_TEST("x86_64", "caps_2.9.0"); + DO_TEST("x86_64", "caps_2.10.0"); + DO_TEST("x86_64", "caps_2.11.0"); + DO_TEST("x86_64", "caps_2.12.0"); + DO_TEST("x86_64", "caps_3.0.0"); + DO_TEST("aarch64", "caps_2.6.0"); + DO_TEST("aarch64", "caps_2.10.0"); + DO_TEST("aarch64", "caps_2.12.0"); + DO_TEST("ppc64", "caps_2.6.0"); + DO_TEST("ppc64", "caps_2.9.0"); + DO_TEST("ppc64", "caps_2.10.0"); + DO_TEST("ppc64", "caps_2.12.0"); + DO_TEST("ppc64", "caps_3.0.0"); + DO_TEST("s390x", "caps_2.7.0"); + DO_TEST("s390x", "caps_2.8.0"); + DO_TEST("s390x", "caps_2.9.0"); + DO_TEST("s390x", "caps_2.10.0"); + DO_TEST("s390x", "caps_2.11.0"); + DO_TEST("s390x", "caps_2.12.0"); + DO_TEST("riscv32", "caps_3.0.0"); + DO_TEST("riscv64", "caps_3.0.0"); return (ret == 0) ? EXIT_SUCCESS : EXIT_FAILURE; } -- 2.17.1

It was already available in 1.5.0. Moreover, we're not even formatting it on the QEMU command line, ever: we just use it as part of some logic that decides whether KVM support should be advertised, and as it turns out that logic is actually buggy and dropping this capability fixes it. https://bugzilla.redhat.com/show_bug.cgi?id=1628469 Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/qemu/qemu_capabilities.c | 3 --- src/qemu/qemu_capabilities.h | 2 +- tests/qemuxml2argvtest.c | 11 +++++------ 3 files changed, 6 insertions(+), 10 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index e04a3d775f..469f0283f5 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -836,7 +836,6 @@ virQEMUCapsInitGuestFromBinary(virCapsPtr caps, if (virFileExists("/dev/kvm") && (virQEMUCapsGet(qemubinCaps, QEMU_CAPS_KVM) || - virQEMUCapsGet(qemubinCaps, QEMU_CAPS_ENABLE_KVM) || kvmbin)) haskvm = true; @@ -2640,7 +2639,6 @@ virQEMUCapsProbeQMPKVMState(virQEMUCapsPtr qemuCaps, virQEMUCapsClear(qemuCaps, QEMU_CAPS_KVM); } else if (!enabled) { virQEMUCapsClear(qemuCaps, QEMU_CAPS_KVM); - virQEMUCapsSet(qemuCaps, QEMU_CAPS_ENABLE_KVM); } return 0; @@ -3938,7 +3936,6 @@ virQEMUCapsIsValid(void *data, priv->runUid, priv->runGid) == 0; if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_KVM) && - virQEMUCapsGet(qemuCaps, QEMU_CAPS_ENABLE_KVM) && kvmUsable) { VIR_DEBUG("KVM was not enabled when probing '%s', " "but it should be usable now", diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index a0134493aa..f00d790542 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -79,7 +79,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for syntax-check */ X_QEMU_CAPS_XEN_DOMID, /* -xen-domid */ X_QEMU_CAPS_MIGRATE_QEMU_UNIX, /* qemu migration via unix sockets */ X_QEMU_CAPS_CHARDEV, /* Is the new -chardev arg available */ - QEMU_CAPS_ENABLE_KVM, /* -enable-kvm flag */ + X_QEMU_CAPS_ENABLE_KVM, /* -enable-kvm flag */ X_QEMU_CAPS_MONITOR_JSON, /* JSON mode for monitor */ /* 25 */ diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 3d84cb346a..20052575ab 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -965,16 +965,15 @@ mymain(void) DO_TEST("clock-france", NONE); DO_TEST("clock-hpet-off", NONE); DO_TEST("clock-catchup", QEMU_CAPS_KVM_PIT_TICK_POLICY); - DO_TEST("cpu-kvmclock", QEMU_CAPS_ENABLE_KVM); - DO_TEST("cpu-host-kvmclock", QEMU_CAPS_ENABLE_KVM); + DO_TEST("cpu-kvmclock", NONE); + DO_TEST("cpu-host-kvmclock", NONE); DO_TEST("kvmclock", QEMU_CAPS_KVM); DO_TEST("clock-timer-hyperv-rtc", QEMU_CAPS_KVM); - DO_TEST("cpu-eoi-disabled", QEMU_CAPS_ENABLE_KVM); - DO_TEST("cpu-eoi-enabled", QEMU_CAPS_ENABLE_KVM); + DO_TEST("cpu-eoi-disabled", NONE); + DO_TEST("cpu-eoi-enabled", NONE); DO_TEST("controller-order", QEMU_CAPS_KVM, - QEMU_CAPS_ENABLE_KVM, QEMU_CAPS_PIIX3_USB_UHCI, QEMU_CAPS_CCID_PASSTHRU, QEMU_CAPS_SPICE, @@ -986,7 +985,7 @@ mymain(void) DO_TEST("eoi-enabled", NONE); DO_TEST("pv-spinlock-disabled", NONE); DO_TEST("pv-spinlock-enabled", NONE); - DO_TEST("kvmclock+eoi-disabled", QEMU_CAPS_ENABLE_KVM); + DO_TEST("kvmclock+eoi-disabled", NONE); DO_TEST("hyperv", NONE); DO_TEST("hyperv-off", NONE); -- 2.17.1

This capability is documented as having one meaning (whether KVM is enabled by default) but is actually assigned two other meanings over its life: whether the query-kvm QMP command is available at first, and later on whether KVM is usable / was used during probing. Since the query-kvm QMP command was available in 1.5.0, we can avoid probing for it; additionally, we can simplify the logic by setting the flag when it applies instead of initially setting it and then clearing it when it doesn't. The flag's description is also updated to reflect reality. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/qemu/qemu_capabilities.c | 19 ++----------------- src/qemu/qemu_capabilities.h | 2 +- 2 files changed, 3 insertions(+), 18 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 469f0283f5..348b478159 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -1004,7 +1004,6 @@ struct virQEMUCapsStringFlags virQEMUCapsCommands[] = { { "block-stream", QEMU_CAPS_BLOCKJOB_ASYNC }, { "dump-guest-memory", QEMU_CAPS_DUMP_GUEST_MEMORY }, { "query-spice", QEMU_CAPS_SPICE }, - { "query-kvm", QEMU_CAPS_KVM }, { "block-commit", QEMU_CAPS_BLOCK_COMMIT }, { "query-vnc", QEMU_CAPS_VNC }, { "drive-mirror", QEMU_CAPS_DRIVE_MIRROR }, @@ -2621,25 +2620,11 @@ virQEMUCapsProbeQMPKVMState(virQEMUCapsPtr qemuCaps, bool enabled = false; bool present = false; - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_KVM)) - return 0; - if (qemuMonitorGetKVMState(mon, &enabled, &present) < 0) return -1; - /* The QEMU_CAPS_KVM flag was initially set according to the QEMU - * reporting the recognition of 'query-kvm' QMP command. That merely - * indicates existence of the command though, not whether KVM support - * is actually available, nor whether it is enabled by default. - * - * If it is not present we need to clear the flag, and if it is - * not enabled by default we need to change the flag. - */ - if (!present) { - virQEMUCapsClear(qemuCaps, QEMU_CAPS_KVM); - } else if (!enabled) { - virQEMUCapsClear(qemuCaps, QEMU_CAPS_KVM); - } + if (present && enabled) + virQEMUCapsSet(qemuCaps, QEMU_CAPS_KVM); return 0; } diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index f00d790542..e671f74ebb 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -65,7 +65,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for syntax-check */ X_QEMU_CAPS_MIGRATE_QEMU_TCP, /* have qemu tcp migration */ X_QEMU_CAPS_MIGRATE_QEMU_EXEC, /* have qemu exec migration */ X_QEMU_CAPS_DRIVE_CACHE_V2, /* cache= flag wanting new v2 values */ - QEMU_CAPS_KVM, /* Whether KVM is enabled by default */ + QEMU_CAPS_KVM, /* Whether KVM is usable / was used during probing */ X_QEMU_CAPS_DRIVE_FORMAT, /* Is -drive format= avail */ /* 15 */ -- 2.17.1

The file being present doesn't necessarily mean anything these days, as it's created independently of whether the kvm module has been loaded; moreover, we're already gathering all the information we need through QMP, so poking the filesystem at all is entirely unnecessary. [1] https://github.com/systemd/systemd/commit/d35d6249d5a7ed3228 Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/qemu/qemu_capabilities.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 348b478159..3d99c95a6a 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -834,9 +834,8 @@ virQEMUCapsInitGuestFromBinary(virCapsPtr caps, if (!binary) return 0; - if (virFileExists("/dev/kvm") && - (virQEMUCapsGet(qemubinCaps, QEMU_CAPS_KVM) || - kvmbin)) + if (virQEMUCapsGet(qemubinCaps, QEMU_CAPS_KVM) || + kvmbin) haskvm = true; if (virQEMUCapsGetMachineTypesCaps(qemubinCaps, &nmachines, &machines) < 0) -- 2.17.1

On Fri, Sep 14, 2018 at 10:35:58 +0200, Andrea Bolognani wrote:
There are a few cases in which we're not reporting the correct information through 'virsh capabilities', leading to applications such as libguestfs attempting to launch KVM guests on a host that doesn't actually support them.
Our code is also more complicated than it needs to be due to backwards compatibilty reasons that we no longer care about these days, so we can perform some cleanup there; there are further cleanup opportunities in the same area, but some of them are a bit tricky so I'll leave them for a follow-up series.
We also get some extra test coverage pretty much for free. Yay!
So this makes things cleaner and fixes KVM reporting in capabilities. But we also check /dev/kvm in virQEMUCapsIsValid. Which means we may reprobe QEMU everytime in case the kvm module is not loaded (but /dev/kvm exists) and we will never catch the case when kvm module was loaded originally, but the admin unloads it while libvirtd is running. Although this series doesn't make it any worse.
qemu: Drop QEMU_CAPS_ENABLE_KVM
After removing this capability it looks like virQEMUCapsIsValid will always invalidate every single non-native QEMU binary since kvm could not clearly be used when probing and /dev/kvm is present. Shouldn't we add a check which makes sure we don't go this far for non-native binaries? Jirka

On Fri, 2018-09-14 at 15:01 +0200, Jiri Denemark wrote:
On Fri, Sep 14, 2018 at 10:35:58 +0200, Andrea Bolognani wrote:
There are a few cases in which we're not reporting the correct information through 'virsh capabilities', leading to applications such as libguestfs attempting to launch KVM guests on a host that doesn't actually support them.
Our code is also more complicated than it needs to be due to backwards compatibilty reasons that we no longer care about these days, so we can perform some cleanup there; there are further cleanup opportunities in the same area, but some of them are a bit tricky so I'll leave them for a follow-up series.
We also get some extra test coverage pretty much for free. Yay!
So this makes things cleaner and fixes KVM reporting in capabilities.
But we also check /dev/kvm in virQEMUCapsIsValid. Which means we may reprobe QEMU everytime in case the kvm module is not loaded (but /dev/kvm exists)
We can try poking harder at /dev/kvm, eg. open() it, run some ioctl()s on it or whatever is necessary to figure out whether the device is actually functional rather than just showing up. However, that would mean duplicating some work that QEMU can already do for us, so I'm not too fond of it. It also feels a bit too ad-hoc. How ridiculous would it be to invalidate capabilities whenever the daemon is restarted? That might strike a somewhat reasonable balance between requiring the admin to delete a bunch of internal files after configuring the system to load the KVM module and re-probing QEMU all the time.
and we will never catch the case when kvm module was loaded originally, but the admin unloads it while libvirtd is running. Although this series doesn't make it any worse.
Is there a concrete use case for that? It doesn't look like a situation that we should really concern ourselves with.
qemu: Drop QEMU_CAPS_ENABLE_KVM
After removing this capability it looks like virQEMUCapsIsValid will always invalidate every single non-native QEMU binary since kvm could not clearly be used when probing and /dev/kvm is present. Shouldn't we add a check which makes sure we don't go this far for non-native binaries?
We can use virQEMUCapsGuestIsNative() and skip all KVM-related checks for non-native binaries. Does that sound okay? -- Andrea Bolognani / Red Hat / Virtualization

On Fri, Sep 14, 2018 at 15:36:42 +0200, Andrea Bolognani wrote:
On Fri, 2018-09-14 at 15:01 +0200, Jiri Denemark wrote:
On Fri, Sep 14, 2018 at 10:35:58 +0200, Andrea Bolognani wrote:
There are a few cases in which we're not reporting the correct information through 'virsh capabilities', leading to applications such as libguestfs attempting to launch KVM guests on a host that doesn't actually support them.
Our code is also more complicated than it needs to be due to backwards compatibilty reasons that we no longer care about these days, so we can perform some cleanup there; there are further cleanup opportunities in the same area, but some of them are a bit tricky so I'll leave them for a follow-up series.
We also get some extra test coverage pretty much for free. Yay!
So this makes things cleaner and fixes KVM reporting in capabilities.
But we also check /dev/kvm in virQEMUCapsIsValid. Which means we may reprobe QEMU everytime in case the kvm module is not loaded (but /dev/kvm exists)
We can try poking harder at /dev/kvm, eg. open() it, run some ioctl()s on it or whatever is necessary to figure out whether the device is actually functional rather than just showing up.
However, that would mean duplicating some work that QEMU can already do for us, so I'm not too fond of it. It also feels a bit too ad-hoc.
I don't know. I think we can keep the code as is for now since this series does not make it any worse :-)
How ridiculous would it be to invalidate capabilities whenever the daemon is restarted? That might strike a somewhat reasonable balance between requiring the admin to delete a bunch of internal files after configuring the system to load the KVM module and re-probing QEMU all the time.
This would effectively revert the addition of on-disk cache, which was introduced with a very good reason. Starting libvirtd with a lot of QEMU binaries would take a very long time without the persistent cache. Also libguestfs running libvirt in session mode would get crazy about it. In other words, invalidating capabilities cache whenever libvirtd is started is not acceptable.
and we will never catch the case when kvm module was loaded originally, but the admin unloads it while libvirtd is running. Although this series doesn't make it any worse.
Is there a concrete use case for that? It doesn't look like a situation that we should really concern ourselves with.
qemu: Drop QEMU_CAPS_ENABLE_KVM
After removing this capability it looks like virQEMUCapsIsValid will always invalidate every single non-native QEMU binary since kvm could not clearly be used when probing and /dev/kvm is present. Shouldn't we add a check which makes sure we don't go this far for non-native binaries?
We can use virQEMUCapsGuestIsNative() and skip all KVM-related checks for non-native binaries. Does that sound okay?
Yeah, I think that should work. Jirka

On Fri, 2018-09-14 at 16:35 +0200, Jiri Denemark wrote:
On Fri, Sep 14, 2018 at 15:36:42 +0200, Andrea Bolognani wrote:
How ridiculous would it be to invalidate capabilities whenever the daemon is restarted? That might strike a somewhat reasonable balance between requiring the admin to delete a bunch of internal files after configuring the system to load the KVM module and re-probing QEMU all the time.
This would effectively revert the addition of on-disk cache, which was introduced with a very good reason. Starting libvirtd with a lot of QEMU binaries would take a very long time without the persistent cache. Also libguestfs running libvirt in session mode would get crazy about it. In other words, invalidating capabilities cache whenever libvirtd is started is not acceptable.
Fair enough! [...]
After removing this capability it looks like virQEMUCapsIsValid will always invalidate every single non-native QEMU binary since kvm could not clearly be used when probing and /dev/kvm is present. Shouldn't we add a check which makes sure we don't go this far for non-native binaries?
We can use virQEMUCapsGuestIsNative() and skip all KVM-related checks for non-native binaries. Does that sound okay?
Yeah, I think that should work.
I just posted a 6/5 that does just that. -- Andrea Bolognani / Red Hat / Virtualization

A side effect of recent changes is that we would always try to regenerate the capabilities cache for non-native QEMU binaries based on /dev/kvm availability, which is of course complete nonsense. Make sure that doesn't happen. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- A better spot would be between 3/5 and 4/5, but the order doesn't make any difference functionality-wise so 6/5 it is for the sake of mailing list archives :) src/qemu/qemu_capabilities.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 3d99c95a6a..04c2adcfb5 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -3916,6 +3916,14 @@ virQEMUCapsIsValid(void *data, return false; } + if (!virQEMUCapsGuestIsNative(priv->hostArch, qemuCaps->arch)) { + VIR_DEBUG("Guest arch (%s) is not native to host arch (%s), " + "skipping KVM-related checks", + virArchToString(qemuCaps->arch), + virArchToString(priv->hostArch)); + return true; + } + kvmUsable = virFileAccessibleAs("/dev/kvm", R_OK | W_OK, priv->runUid, priv->runGid) == 0; -- 2.17.1

On Mon, Sep 17, 2018 at 13:22:59 +0200, Andrea Bolognani wrote:
A side effect of recent changes is that we would always try to regenerate the capabilities cache for non-native QEMU binaries based on /dev/kvm availability, which is of course complete nonsense. Make sure that doesn't happen.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- A better spot would be between 3/5 and 4/5, but the order doesn't make any difference functionality-wise so 6/5 it is for the sake of mailing list archives :)
You could have used 3.5/5, it wouldn't be the first patch with similar numbering :-) Series Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
participants (2)
-
Andrea Bolognani
-
Jiri Denemark