| Message ID | 6a8b5ec69544b9f2582b8b38e3bab1127da6358d.1500075702.git.crobinso@redhat.com |
|---|---|
| State | Accepted |
| Commit | 64665fa82f605a5e9aaafd4327fd7cf53751b1bd |
| Headers | show |
| Series | qemu: command: clean up default cpu formatting | expand |
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.
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
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
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
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; }
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(-) -- 2.13.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list