qemu: command: clean up default cpu formatting

[libvirt] [PATCH 3/3] qemu: command: explicitly error for non-x86 default CPU

Posted by Cole Robinson 7 years, 10 months ago

The code only currently handles writing an x86 default -cpu
argument, and doesn't know anything about other architectures.
Let's make this explicit rather than leaving ex. qemu ppc64 to
throw an error about -cpu qemu64

Signed-off-by: Cole Robinson <crobinso@redhat.com>
---
 src/qemu/qemu_command.c | 25 ++++++++++++++++++-------
 1 file changed, 18 insertions(+), 7 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index aa12479f7..f727e3d30 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -6716,17 +6716,11 @@ qemuBuildCpuCommandLine(virCommandPtr cmd,
     virArch hostarch = virArchFromHost();
     char *cpu = NULL, *cpu_flags = NULL;
     bool hasHwVirt = false;
-    const char *default_model;
     int ret = -1;
     virBuffer cpu_buf = VIR_BUFFER_INITIALIZER;
     virBuffer buf = VIR_BUFFER_INITIALIZER;
     size_t i;
 
-    if (def->os.arch == VIR_ARCH_I686)
-        default_model = "qemu32";
-    else
-        default_model = "qemu64";
-
     if (def->cpu &&
         (def->cpu->mode != VIR_CPU_MODE_CUSTOM || def->cpu->model)) {
         if (qemuBuildCpuModelArgStr(driver, def, &cpu_buf, qemuCaps) < 0)
@@ -6768,7 +6762,7 @@ qemuBuildCpuCommandLine(virCommandPtr cmd,
             ((hostarch == VIR_ARCH_X86_64 &&
               strstr(def->emulator, "kvm")) ||
              strstr(def->emulator, "x86_64"))) {
-            virBufferAdd(&cpu_buf, default_model, -1);
+            virBufferAddLit(&cpu_buf, "qemu32");
         }
     }
 
@@ -6915,6 +6909,23 @@ qemuBuildCpuCommandLine(virCommandPtr cmd,
     cpu_flags = virBufferContentAndReset(&buf);
 
     if (cpu_flags && !cpu) {
+        const char *default_model;
+
+        switch (def->os.arch) {
+        case VIR_ARCH_I686:
+            default_model = "qemu32";
+            break;
+        case VIR_ARCH_X86_64:
+            default_model = "qemu64";
+            break;
+        default:
+            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                           _("CPU flags requested but can't determine "
+                             "default CPU for arch %s"),
+                           virArchToString(def->os.arch));
+            goto cleanup;
+        }
+
         if (VIR_STRDUP(cpu, default_model) < 0)
             goto cleanup;
     }
-- 
2.13.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 3/3] qemu: command: explicitly error for non-x86 default CPU

Posted by Andrea Bolognani 7 years, 10 months ago

On Fri, 2017-07-14 at 19:43 -0400, Cole Robinson wrote:
> The code only currently handles writing an x86 default -cpu
> argument, and doesn't know anything about other architectures.
> Let's make this explicit rather than leaving ex. qemu ppc64 to
> throw an error about -cpu qemu64
> 
> Signed-off-by: Cole Robinson <crobinso@redhat.com>

This seems to also fix

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

I'll review the series early next week.

-- 
Andrea Bolognani / Red Hat / Virtualization

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 3/3] qemu: command: explicitly error for non-x86 default CPU

Posted by Cole Robinson 7 years, 10 months ago

On 07/15/2017 12:40 PM, Andrea Bolognani wrote:
> On Fri, 2017-07-14 at 19:43 -0400, Cole Robinson wrote:
>> The code only currently handles writing an x86 default -cpu
>> argument, and doesn't know anything about other architectures.
>> Let's make this explicit rather than leaving ex. qemu ppc64 to
>> throw an error about -cpu qemu64
>>
>> Signed-off-by: Cole Robinson <crobinso@redhat.com>
> 
> This seems to also fix
> 
>   https://bugzilla.redhat.com/show_bug.cgi?id=1235511
> 
> I'll review the series early next week.
> 

Thanks, but I don't think it will fix pmu=on for ppc64, just improve the error
scenario. We will still need to extend the code to hardcode the default CPU
model for PPC64 (would be nice if qemu had a generic 'default' value to use
here, or some way to get around specifying a cpu model)

- Cole

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 3/3] qemu: command: explicitly error for non-x86 default CPU

Posted by Ján Tomko 7 years, 9 months ago

On Fri, Jul 14, 2017 at 07:43:06PM -0400, Cole Robinson wrote:
>The code only currently handles writing an x86 default -cpu
>argument, and doesn't know anything about other architectures.
>Let's make this explicit rather than leaving ex. qemu ppc64 to
>throw an error about -cpu qemu64
>
>Signed-off-by: Cole Robinson <crobinso@redhat.com>
>---
> src/qemu/qemu_command.c | 25 ++++++++++++++++++-------
> 1 file changed, 18 insertions(+), 7 deletions(-)
>

ACK

Jan
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 3/3] qemu: command: explicitly error for non-x86 default CPU

Posted by Cole Robinson 7 years, 9 months ago

On 07/21/2017 10:25 AM, Ján Tomko wrote:
> On Fri, Jul 14, 2017 at 07:43:06PM -0400, Cole Robinson wrote:
>> The code only currently handles writing an x86 default -cpu
>> argument, and doesn't know anything about other architectures.
>> Let's make this explicit rather than leaving ex. qemu ppc64 to
>> throw an error about -cpu qemu64
>>
>> Signed-off-by: Cole Robinson <crobinso@redhat.com>
>> ---
>> src/qemu/qemu_command.c | 25 ++++++++++++++++++-------
>> 1 file changed, 18 insertions(+), 7 deletions(-)
>>
> 
> ACK
> 
> Jan

Thanks, I pushed patch 2 + 3 now which you reviewed, patch 1 was independent
and has some comments.

- Cole

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list