[libvirt] [PATCH] qemu-capabilities: Fix query-cpu-model-expansion on s390 with older kernel

[Jason J. Herne]

On 01/13/2017 09:11 AM, Jiri Denemark wrote:
> On Fri, Jan 13, 2017 at 11:04:21 +0100, Jiri Denemark wrote:
>> On Thu, Jan 12, 2017 at 11:18:11 -0500, Collin L. Walling wrote:
>>> When running on s390 with a kernel that does not support cpu model checking and
>>> with a Qemu new enough to support query-cpu-model-expansion, the gathering of qemu
>>> capabilities will fail. Qemu responds to the query-cpu-model-expansion qmp
>>> command with an error because the needed kernel ioct does not exist. When this
>>> happens a guest cannot even be defined due to missing qemu capabilities data.
>>>
>>> This patch fixes the problem by silently ignoring generic errors stemming from
>>> calls to query-cpu-model-expansion.
>>>
>>> Reported-by: Farhan Ali <alifm at linux.vnet.ibm.com>
>>> Signed-off-by: Collin L. Walling <walling at linux.vnet.ibm.com>
>>> Signed-off-by: Jason J. Herne <jjherne at linux.vnet.ibm.com>
>>> ---
>>>  src/qemu/qemu_monitor_json.c | 9 +++++++++
>>>  1 file changed, 9 insertions(+)
>>>
>>> diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
>>> index e767437..1662749 100644
>>> --- a/src/qemu/qemu_monitor_json.c
>>> +++ b/src/qemu/qemu_monitor_json.c
>>> @@ -5041,6 +5041,15 @@ qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr mon,
>>>      if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0)
>>>          goto cleanup;
>>>
>>> +    /* Some QEMU architectures have the query-cpu-model-expansion
>>> +     * command, but return 'GenericError' instead of simply omitting
>>> +     * the command entirely.
>>> +     */
>>
>> Hmm, this comment says something a bit different than the commit
>> message. I hope the issue described by this comment was fixed in QEMU
>> within the same release the query-cpu-model-expansion command was
>> introduced. But we need to fix this nevertheless to address what the
>> commit message describes.
>>
>>> +    if (qemuMonitorJSONHasError(reply, "GenericError")) {
>>> +        ret = 0;
>>> +        goto cleanup;
>>> +    }
>>> +
>>>      if (qemuMonitorJSONCheckError(cmd, reply) < 0)
>>>          goto cleanup;
>>
>> However, we need to do a little bit more than just ignoring this error.
>> I'll send a v2 soon.
>
> No I won't send the v2. I was wrong. I thought we should clear the
> QEMU_CAPS_QUERY_CPU_MODEL_EXPANSION capability when we detect the
> command is not actually working, but we can't do that since the
> existence of this command serves as an indicator of other CPU
> configuration capabilities which cannot be probed directly. Thus just
> ignoring non-working query-cpu-model-expansion command is the right
> thing to do.
>
> But I suggest to change the comment to something like the following:
>
>     /* Even though query-cpu-model-expansion is advertised by
>      * query-commands it may just return GenericError if it is not
>      * implemented for the requested guest architecture or it is not
>      * supported in the host environment.
>      */
>
> ACK to this patch and please let me know if you agree with the modified
> comment.
>

Jiri, yes, I agree with the comment change. Thanks for taking a look at 
this.

-- 
-- Jason J. Herne (jjherne at linux.vnet.ibm.com)