[libvirt] [PATCH 2/2] Add support for QEMU -add-fd support detection

[Corey Bryant]

On 01/31/2013 09:06 AM, Eric Blake wrote:
> On 01/31/2013 04:46 AM, Stefan Berger wrote:
>> On 01/30/2013 11:21 PM, Eric Blake wrote:
>>> From: Stefan Berger <stefanb at linux.vnet.ibm.com>
>>>
>>> Add support for QEMU -add-fd command line parameter detection.
>>> This intentionally rejects qemu 1.2, where 'add-fd' QMP did
>>> not allow full control of set ids, and where there was no command
>>> line counterpart, but accepts qemu 1.3.
>>>
>>> Signed-off-by: Eric Blake <eblake at redhat.com>
>>> ---
>
>>> +        else if (STREQ(name, "add-fd"))
>>> +            qemuCapsSet(caps, QEMU_CAPS_ADD_FD);
>>
>> This checks for QMP support -> version 1.2.
>
> Yes, but then...
>
>>
>>>            VIR_FREE(name);
>>>        }
>>>        VIR_FREE(commands);
>>>
>>> +    /* QMP add-fd was introduced in 1.2, but did not support
>>> +     * management control of set numbering, and did not have a
>>> +     * counterpart -add-fd command line option.  We require the
>>> +     * add-fd features from 1.3 or later.  */
>>> +    if (qemuCapsGet(caps, QEMU_CAPS_ADD_FD)) {
>>> +        int fd = open("/dev/null", O_RDONLY);
>>> +        if (fd < 0) {
>>> +            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>>> +                           _("unable to probe for add-fd"));
>>> +            return -1;
>>> +        }
>>> +        if (qemuMonitorAddFd(mon, 0, fd, "/dev/null") < 0)
>>> +            qemuCapsClear(caps, QEMU_CAPS_ADD_FD);
>>> +        VIR_FORCE_CLOSE(fd);
>>> +    }
>>> +
>>
>> Aren't you only detecting version 1.2 with this?
>
> ...this code is what checks for 1.3.  In 1.2, the qemuMonitorAddFd()
> call fails with an error, so we clear the bit back out.  Only 1.3 allows
> libvirt to specify its own set number, because that was one of the
> changes qemu had to make in order to add the -add-fd command-line option.
>

This approach looks fine for verifying the availability of 1.3 support
(assuming no fd sets were introduced prior to this point).

-- 
Regards,
Corey Bryant

>>
>> I thought we would need something along the lines of this example here:
>>
>>      if (version >= 9000)
>>          qemuCapsSet(caps, QEMU_CAPS_VNC_COLON);
>
> That was my original idea, but a version number check is more fragile
> (it doesn't backport well); anywhere that we can do a feature check
> instead of a version check, we make backporting easier.
>
>
>
> --
> libvir-list mailing list
> libvir-list at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
>