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

[Eduardo Habkost]

On Fri, Jan 13, 2017 at 03:11:54PM +0100, 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.
>      */

If the command is not implemented for the architecture, it should
be already omitted from query-commands. However, if the host
environment doesn't support CPU models, I expect the commands to
fail only for the "host" CPU model, but to be still possible to
expand the other CPU models.

Expansion of static CPU models, specifically, should be always
possible and should never be affected by the host environment.

-- 
Eduardo