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

[Daniel P. Berrange]

On Thu, Jul 03, 2014 at 11:55:44AM +0200, Michal Privoznik wrote:
> 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 at redhat.com>
> >>---
> >>  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>

ACK

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|