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

[Guannan Ren]

On 12/12/2012 09:22 PM, Eric Blake wrote:
> On 12/11/2012 08:32 PM, Guannan Ren wrote:
>
>>>> +    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.
>>            Although '-device XXX'  is there but it probably doesn't work
>> for qemu(<1.2)
>>            the following is the comments from  Gerd Hoffmann in v1
>>
>>          "That will not work.  You must check the qemu version.  1.2+ is
>> safe,
>>          IIRC 1.1 will work too.
> Still, I'd rather see us create a capability bit, even if that bit is
> initially set according to a version check in qemu_capabilities.c, and
> use that bit rather than polluting the rest of the qemu code with
> additional version checks.  That way, if someone manages to backport
> fixed device handling even to RHEL qemu 0.12.x, as well as advertise
> that backport, we can set the bit when the backport is detected.  It may
> even be as simple as a RHEL-specific patch that merely blindly sets the
> capability bit at the one place where upstream sets the bit according to
> a version check.  But no matter how we approach it, having a dedicated
> capability bit rather than multiple version checks will make backporting
> much easier.
>

     Agree, I will do that in v3.
     and setting a specific caps flags also make writing new testcase easier
     than checking qemu version directly.

     Guannan