Re: [libvirt] [PATCH for 1.2.7 8/8] qemu: Implement virConnectGetDomainCapabilities

On 02.07.2014 16:56, Daniel P. Berrange wrote: >On Mon, Jun 30, 2014 at 05:31:51PM +0200, Michal Privoznik wrote: >>So far only information on disks and host devices are exposed in the >>capabilities XML. Well, at least something. Even a new test is >>introduced. The qemu capabilities are stolen from already existing >>qemucapabilities test. There's one tricky point though. Functions that >>checks host's KVM and VFIO capabilities, are impossible to mock >>currently. So in the test, we are setting the capabilities by hand. >> >>Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; >>--- >> src/libvirt_private.syms | 1 + >> src/qemu/qemu_capabilities.c | 90 ++++++++++++++++++ >> src/qemu/qemu_capabilities.h | 4 + >> src/qemu/qemu_driver.c | 101 +++++++++++++++++++++ >> tests/Makefile.am | 5 + >> .../domaincaps-qemu_1.6.50-1.xml | 44 +++++++++ >> tests/domaincapstest.c | 45 +++++++++ >> 7 files changed, 290 insertions(+) >> create mode 100644 tests/domaincapsschemadata/domaincaps-qemu_1.6.50-1.xml >> > > >>+static void >>+virQEMUCapsFillDomainDeviceDiskCaps(virQEMUCapsPtr qemuCaps, >>+ virDomainCapsDeviceDiskPtr disk) >>+{ >>+ disk->device.supported = true; >>+ /* QEMU supports all of these */ >>+ VIR_DOMAIN_CAPS_ENUM_SET(disk->diskDevice, >>+ VIR_DOMAIN_DISK_DEVICE_DISK, >>+ VIR_DOMAIN_DISK_DEVICE_CDROM, >>+ VIR_DOMAIN_DISK_DEVICE_FLOPPY); >>+ >>+ if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_BLK_SG_IO)) >>+ VIR_DOMAIN_CAPS_ENUM_SET(disk->diskDevice, VIR_DOMAIN_DISK_DEVICE_LUN); >>+ >>+ VIR_DOMAIN_CAPS_ENUM_SET(disk->bus, >>+ VIR_DOMAIN_DISK_BUS_IDE, >>+ VIR_DOMAIN_DISK_BUS_FDC, >>+ VIR_DOMAIN_DISK_BUS_SCSI, >>+ VIR_DOMAIN_DISK_BUS_VIRTIO, >>+ VIR_DOMAIN_DISK_BUS_SD); > >I have a feeling that 'SD' is not supported in all QEMU's we claim to >work with, though perhaps we've never checked this before when >building the CLI args. Well, we don't. I haven't found any code that checks for 'SD' in qemu driver. I can remove it until the time we have a resolution. > >>+ >>+ if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_USB_STORAGE)) >>+ VIR_DOMAIN_CAPS_ENUM_SET(disk->bus, VIR_DOMAIN_DISK_BUS_USB); >>+} >>+ >>+ > >>+ <hostdev supported='yes'> >>+ <enum name='mode'/> > >Hmm, so that's claiming we don't support any values for >the mode attribute, but we support subsys. Ouch, yeah. The problem is, I was not setting the correct struct member. Here's the fix: index 9fc58ff..6ed85a9 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -3603,7 +3603,7 @@ virQEMUCapsFillDomainDeviceHostdevCaps(virQEMUCapsPtr qemuCaps, hostdev->device.supported = true; /* VIR_DOMAIN_HOSTDEV_MODE_CAPABILITIES is for containers only */ - VIR_DOMAIN_CAPS_ENUM_SET(hostdev->subsysType, + VIR_DOMAIN_CAPS_ENUM_SET(hostdev->mode, VIR_DOMAIN_HOSTDEV_MODE_SUBSYS); VIR_DOMAIN_CAPS_ENUM_SET(hostdev->startupPolicy, diff --git a/tests/domaincapsschemadata/domaincaps-qemu_1.6.50-1.xml b/tests/domaincapsschemadata/domaincaps-qemu_1.6.50-1.xml index 562e2f4..b7d9c26 100644 --- a/tests/domaincapsschemadata/domaincaps-qemu_1.6.50-1.xml +++ b/tests/domaincapsschemadata/domaincaps-qemu_1.6.50-1.xml @@ -21,7 +21,9 @@ </enum> </disk> <hostdev supported='yes'> - <enum name='mode'/> + <enum name='mode'> + <value>subsystem</value> + </enum> <enum name='startupPolicy'> <value>default</value> <value>mandatory</value>