On Mon, Jan 23, 2012 at 14:11:08 +0100, Paolo Bonzini wrote:
> The qemu32 CPU model is chosen based on the <os arch=...> name when
> creating the QEMU command line. Reflect the kvm32/kvm64/qemu32/qemu64
> CPU models in the <os> element when doing the opposite transformation.
> To do this, we need to not look at def->os.arch until after the
> command-line has been parsed.
>
> At the same time, avoid creating an empty <cpu> element when the QEMU
> command-line specifies the default CPU model for the guest arch.
>
> Signed-off-by: Paolo Bonzini <pbonzini@xxxxxxxxxx>
> ---
> src/qemu/qemu_command.c | 70 +++++++++++++++++++++++++++++++----------------
> 1 files changed, 46 insertions(+), 24 deletions(-)
>
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index aaccf62..7c4460e 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
...
> @@ -6831,6 +6832,21 @@ qemuParseCommandLineCPU(virDomainDefPtr dom,
> }
> } while ((p = next));
>
> + if (model) {
> + if (STREQ(model, "qemu32") || STREQ(model, "kvm32")) {
> + dom->os.arch = strdup("i686");
> + VIR_FREE(model);
> + } else if (STREQ(model, "qemu64") || STREQ(model, "kvm64")) {
> + dom->os.arch = strdup("x86_64");
> + VIR_FREE(model);
> + } else {
> + if (!cpu && !(cpu = qemuInitGuestCPU(dom)))
> + goto error;
> +
> + cpu->model = model;
> + }
> + }
> +
I think we could just set cpu->model even if the model used in qemu command
line was {qemu,kvm}{32,64}. That would probably mean we need to add some of
the models in cpu_map.xml, though.
> return 0;
>
> syntax:
...
> @@ -7542,6 +7544,26 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr caps,
> }
> }
>
> + if (!def->os.arch) {
> + if (STRPREFIX(def->emulator, "qemu"))
> + path = def->emulator;
> + else
> + path = strstr(def->emulator, "qemu");
> + if (path &&
> + STRPREFIX(path, "qemu-system-"))
> + def->os.arch = strdup(path + strlen("qemu-system-"));
> + else
> + def->os.arch = strdup("i686");
> + if (!def->os.arch)
> + goto no_memory;
> + }
> +
> + if (STREQ(def->os.arch, "i686")||STREQ(def->os.arch, "x86_64"))
> + def->features = (1 << VIR_DOMAIN_FEATURE_ACPI)
> + /*| (1 << VIR_DOMAIN_FEATURE_APIC)*/;
I know you were just copy&pasting, but while doing that, you could also make
this last statement a bit more readable and add spaces around "||" and either
use {} or remove the last line (the comment).
> +
> + def->features &= ~disabled_features;
> +
> #undef WANT_VALUE
> if (def->ndisks > 0 && ceph_args) {
> char *hosts, *port, *saveptr = NULL, *token;
Otherwise the patch looks good.
Jirka
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list