[libvirt] [PATCH] qemu-capabilities: Fix query-cpu-model-expansion on s390 with older kernel
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@linux.vnet.ibm.com> Signed-off-by: Collin L. Walling <walling@linux.vnet.ibm.com> Signed-off-by: Jason J. Herne <jjherne@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. + */ + if (qemuMonitorJSONHasError(reply, "GenericError")) { + ret = 0; + goto cleanup; + } + if (qemuMonitorJSONCheckError(cmd, reply) < 0) goto cleanup; -- 2.7.4
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@linux.vnet.ibm.com> Signed-off-by: Collin L. Walling <walling@linux.vnet.ibm.com> Signed-off-by: Jason J. Herne <jjherne@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. Jirka
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@linux.vnet.ibm.com> Signed-off-by: Collin L. Walling <walling@linux.vnet.ibm.com> Signed-off-by: Jason J. Herne <jjherne@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.
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?
However, we need to do a little bit more than just ignoring this error. I'll send a v2 soon.
-- -- Jason J. Herne (jjherne@linux.vnet.ibm.com)
On Fri, Jan 13, 2017 at 09:06:52AM -0500, Jason J. Herne wrote:
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@linux.vnet.ibm.com> Signed-off-by: Collin L. Walling <walling@linux.vnet.ibm.com> Signed-off-by: Jason J. Herne <jjherne@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.
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?
I'm not sure, probably unregistering the command after KVM is initialized is too late. Even if today QMP is available only after the accelerator is already initialized, we might want to delay accelerator initialization in the future (so the accelerator could be configured using QMP commands). Also, I'm not sure when/how exactly the query-cpu-model-expansion call fails. If the ioctl isn't available, shouldn't query-cpu-model-expansion fail only when querying "host", but let the other CPU models to be queried? In this case, unregistering the command doesn't seem to be the right solution. Probably omitting "host" (or flagging it as unavailable?) on query-cpu-model-definitions would be better. -- Eduardo
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@linux.vnet.ibm.com> Signed-off-by: Collin L. Walling <walling@linux.vnet.ibm.com> Signed-off-by: Jason J. Herne <jjherne@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
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@linux.vnet.ibm.com> Signed-off-by: Collin L. Walling <walling@linux.vnet.ibm.com> Signed-off-by: Jason J. Herne <jjherne@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@linux.vnet.ibm.com)
On Fri, Jan 13, 2017 at 09:20:40 -0500, Jason J. Herne wrote:
On 01/13/2017 09:11 AM, Jiri Denemark wrote:
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.
OK, I pushed this with the modified comment. Thanks. Jirka
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@linux.vnet.ibm.com> Signed-off-by: Collin L. Walling <walling@linux.vnet.ibm.com> Signed-off-by: Jason J. Herne <jjherne@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
participants (4)
-
Collin L. Walling -
Eduardo Habkost -
Jason J. Herne -
Jiri Denemark