[libvirt] [PATCH v2 3/4] qemu: use newer -device video device in qemu commandline

[Eric Blake]

On 12/11/2012 07:14 AM, Guannan Ren wrote:
> '-device VGA' maps to '-vga std'
> '-device cirrus-vga' maps to '-vga cirrus'
> '-device qxl-vga' maps to '-vga qxl'
>              (there is also '-device qxl' for secondary devices)
> '-device vmware-svga' maps to '-vga vmware'
> 
> For qemu(>=1.2), we can use -device to replace -vga for video
> device. For the primary video device, the patch trys to use 0x2
> slot for matching old qemu. If the 0x2 slot is allocated already,
> the addr property could help for using any available slot.
> For qemu(< 1.2), we keep using -vga for primary device.
> ---
>  src/qemu/qemu_command.c  | 132 ++++++++++++++++++++++++++++++++++++-----------
>  src/qemu/qemu_command.h  |   4 +-
>  tests/qemuxml2argvtest.c |   9 ++--
>  3 files changed, 110 insertions(+), 35 deletions(-)
> 
>  {
>      size_t i, j;
>      bool reservedIDE = false;
>      bool reservedUSB = false;
>      int function;
> +    bool qemu1dot2plus = qemuCapsGetVersion(caps) >= 1002000;

Eww.  Version string comparison is wrong, as it is feasible that someone
could backport the improved command line to older qemu.  We should
instead be going off of whether specific '-device XXX' is supported;
probably by checking for QEMU_CAPS_DEVICE_QXL.

> @@ -6427,22 +6477,42 @@ qemuBuildCommandLine(virConnectPtr conn,
>              goto error;
>      }
>      if (def->nvideos > 0) {
> -        if (qemuCapsGet(caps, QEMU_CAPS_VGA)) {
> -            if (def->videos[0]->type == VIR_DOMAIN_VIDEO_TYPE_XEN) {
> +        int primaryVideoType = def->videos[0]->type;
> +        if (qemuCapsGetVersion(caps) >= 1002000 &&

Again, version comparison feels wrong.  What are you really gating on,
whether qemu is new enough to support primary video on a non-default
address?  If so, set up the right capability bit for that.

> +             ((primaryVideoType == VIR_DOMAIN_VIDEO_TYPE_VGA &&
> +                 qemuCapsGet(caps, QEMU_CAPS_DEVICE_VGA)) ||
> +             (primaryVideoType == VIR_DOMAIN_VIDEO_TYPE_CIRRUS &&
> +                 qemuCapsGet(caps, QEMU_CAPS_DEVICE_CIRRUS_VGA)) ||
> +             (primaryVideoType == VIR_DOMAIN_VIDEO_TYPE_VMVGA &&
> +                 qemuCapsGet(caps, QEMU_CAPS_DEVICE_VMWARE_SVGA)) ||
> +             (primaryVideoType == VIR_DOMAIN_VIDEO_TYPE_QXL &&
> +                 qemuCapsGet(caps, QEMU_CAPS_DEVICE_QXL_VGA)))

Or is the existence of these four device capability bits sufficient?

> +++ b/tests/qemuxml2argvtest.c
> @@ -574,7 +574,8 @@ mymain(void)
>              QEMU_CAPS_VGA, QEMU_CAPS_VGA_NONE);
>      DO_TEST("graphics-spice",
>              QEMU_CAPS_VGA, QEMU_CAPS_VGA_QXL,
> -            QEMU_CAPS_DEVICE, QEMU_CAPS_SPICE);
> +            QEMU_CAPS_DEVICE, QEMU_CAPS_SPICE,
> +            QEMU_CAPS_DEVICE_QXL);

Where are the tests for the new command line possible with newer qemu?

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 619 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20121211/7732fd23/attachment-0001.sig>