[PATCH v1] qemu: monitor: substitute missing model name for host-passthrough

Collin Walling posted 1 patch 2 weeks ago
Test syntax-check failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20200625215846.30757-1-walling@linux.ibm.com
src/qemu/qemu_monitor_json.c | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)

[PATCH v1] qemu: monitor: substitute missing model name for host-passthrough

Posted by Collin Walling 2 weeks ago
When executing the hypervisor-cpu-compare/baseline commands and
the XML file contains a CPU definition using host-passthrough
and no model name, the commands will fail and return an error
message from the QMP response.

Let's fix this by checking for host-passthrough and a missing
model name when converting a CPU definition to a JSON object.
If these conditions are matched, then the JSON object will be
constructed using "host" as the model name.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1850680

Signed-off-by: Collin Walling <walling@linux.ibm.com>
---
 src/qemu/qemu_monitor_json.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 3070c1e6b3..448a3a9356 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -5804,9 +5804,20 @@ qemuMonitorJSONMakeCPUModel(virCPUDefPtr cpu,
 {
     virJSONValuePtr model = virJSONValueNewObject();
     virJSONValuePtr props = NULL;
+    const char *model_name = cpu->model;
     size_t i;
 
-    if (virJSONValueObjectAppendString(model, "name", cpu->model) < 0)
+    if (!model_name) {
+        if (cpu->mode == VIR_CPU_MODE_HOST_PASSTHROUGH) {
+            model_name = "host";
+        } else {
+            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                           _("cpu parameter is missing a model name"));
+            goto error;
+        }
+    }
+
+    if (virJSONValueObjectAppendString(model, "name", model_name) < 0)
         goto error;
 
     if (cpu->nfeatures || !migratable) {
-- 
2.26.2

Re: [PATCH v1] qemu: monitor: substitute missing model name for host-passthrough

Posted by Peter Krempa 1 week ago
On Thu, Jun 25, 2020 at 17:58:46 -0400, Collin Walling wrote:
> When executing the hypervisor-cpu-compare/baseline commands and
> the XML file contains a CPU definition using host-passthrough
> and no model name, the commands will fail and return an error
> message from the QMP response.

This kind of logic does not seem to belong ...

> Let's fix this by checking for host-passthrough and a missing
> model name when converting a CPU definition to a JSON object.
> If these conditions are matched, then the JSON object will be
> constructed using "host" as the model name.
> 
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1850680
> 
> Signed-off-by: Collin Walling <walling@linux.ibm.com>
> ---
>  src/qemu/qemu_monitor_json.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
> index 3070c1e6b3..448a3a9356 100644
> --- a/src/qemu/qemu_monitor_json.c
> +++ b/src/qemu/qemu_monitor_json.c
> @@ -5804,9 +5804,20 @@ qemuMonitorJSONMakeCPUModel(virCPUDefPtr cpu,
>  {
>      virJSONValuePtr model = virJSONValueNewObject();
>      virJSONValuePtr props = NULL;
> +    const char *model_name = cpu->model;
>      size_t i;
>  
> -    if (virJSONValueObjectAppendString(model, "name", cpu->model) < 0)
> +    if (!model_name) {
> +        if (cpu->mode == VIR_CPU_MODE_HOST_PASSTHROUGH) {
> +            model_name = "host";
> +        } else {
> +            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                           _("cpu parameter is missing a model name"));
> +            goto error;
> +        }
> +    }

... to the monitor code, where we try to just deal with the monitor
itself rather than hiding any logic.

But the final word needs to be from Jirka, but he is on holidays until
the end of next week.


> +
> +    if (virJSONValueObjectAppendString(model, "name", model_name) < 0)
>          goto error;
>  
>      if (cpu->nfeatures || !migratable) {
> -- 
> 2.26.2
> 

Re: [PATCH v1] qemu: monitor: substitute missing model name for host-passthrough

Posted by Collin Walling 1 week ago
On 6/26/20 3:22 AM, Peter Krempa wrote:
> On Thu, Jun 25, 2020 at 17:58:46 -0400, Collin Walling wrote:
>> When executing the hypervisor-cpu-compare/baseline commands and
>> the XML file contains a CPU definition using host-passthrough
>> and no model name, the commands will fail and return an error
>> message from the QMP response.
> 
> This kind of logic does not seem to belong ...
> 
>> Let's fix this by checking for host-passthrough and a missing
>> model name when converting a CPU definition to a JSON object.
>> If these conditions are matched, then the JSON object will be
>> constructed using "host" as the model name.
>>
>> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1850680
>>
>> Signed-off-by: Collin Walling <walling@linux.ibm.com>
>> ---
>>  src/qemu/qemu_monitor_json.c | 13 ++++++++++++-
>>  1 file changed, 12 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
>> index 3070c1e6b3..448a3a9356 100644
>> --- a/src/qemu/qemu_monitor_json.c
>> +++ b/src/qemu/qemu_monitor_json.c
>> @@ -5804,9 +5804,20 @@ qemuMonitorJSONMakeCPUModel(virCPUDefPtr cpu,
>>  {
>>      virJSONValuePtr model = virJSONValueNewObject();
>>      virJSONValuePtr props = NULL;
>> +    const char *model_name = cpu->model;
>>      size_t i;
>>  
>> -    if (virJSONValueObjectAppendString(model, "name", cpu->model) < 0)
>> +    if (!model_name) {
>> +        if (cpu->mode == VIR_CPU_MODE_HOST_PASSTHROUGH) {
>> +            model_name = "host";
>> +        } else {
>> +            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>> +                           _("cpu parameter is missing a model name"));
>> +            goto error;
>> +        }
>> +    }
> 
> ... to the monitor code, where we try to just deal with the monitor
> itself rather than hiding any logic.
> 

That's fair. Perhaps it should belong to the qemu driver code after the
CPU XML to a CPU def conversion. I'm also unsure if "host" is an s390
specific thing, or if other archs use it as well.

> But the final word needs to be from Jirka, but he is on holidays until
> the end of next week.
> 
> 
>> +
>> +    if (virJSONValueObjectAppendString(model, "name", model_name) < 0)
>>          goto error;
>>  
>>      if (cpu->nfeatures || !migratable) {
>> -- 
>> 2.26.2
>>
> 


-- 
Regards,
Collin

Stay safe and stay healthy