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

[Jiri Denemark]

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.

Jirka