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(a)linux.vnet.ibm.com&gt; Signed-off-by: Collin L. Walling <walling(a)linux.vnet.ibm.com&gt; Signed-off-by: Jason J. Herne <jjherne(a)linux.vnet.ibm.com&gt; --- 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. + */

+ if (qemuMonitorJSONHasError(reply, "GenericError")) { + ret = 0; + goto cleanup; + } + if (qemuMonitorJSONCheckError(cmd, reply) < 0) goto cleanup;

On 01/13/2017 05:04 AM, 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(a)linux.vnet.ibm.com&gt; > > Signed-off-by: Collin L. Walling <walling(a)linux.vnet.ibm.com&gt; > > Signed-off-by: Jason J. Herne <jjherne(a)linux.vnet.ibm.com&gt; > > --- > > 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. > The issue still exists in Qemu. I can work up a patch to handle it. My first idea is to simply test that the ioctl exists and functions at Qemu initialization and deregister the query-cpu-model-expansion command in the event that the ioctl does not exist or fails. Thoughts?

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(a)linux.vnet.ibm.com&gt; >> Signed-off-by: Collin L. Walling <walling(a)linux.vnet.ibm.com&gt; >> Signed-off-by: Jason J. Herne <jjherne(a)linux.vnet.ibm.com&gt; >> --- >> 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.

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(a)linux.vnet.ibm.com&gt; > > Signed-off-by: Collin L. Walling <walling(a)linux.vnet.ibm.com&gt; > > Signed-off-by: Jason J. Herne <jjherne(a)linux.vnet.ibm.com&gt; > > --- > > 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. */