Re: [libvirt] [RFC v3 PATCH 4/5] PowerPC : Clean up qemuBuildCommandLine to remove x86-specific assumptions from generic code.

On 11/29/2011 10:01 AM, Prerna Saxena wrote: > From: Prerna Saxena&lt;prerna(a)linux.vnet.ibm.com&gt; > Date: Mon, 21 Nov 2011 18:20:42 +0530 > Subject: [PATCH 4/5] Clean up qemuBuildCommandLine to remove x86-specific > assumptions from generic code. > > This implements the minimal set of changes needed in libvirt to launch a > PowerPC-KVM based guest. > It removes x86-specific assumptions about choice of serial driver backend > from generic qemu guest commandline generation code. > It also restricts the ACPI capability to be available for an x86 or > x86_64 domain. > This is not a complete solution -- it still does not guarantee libvirt > the capability to flag non-supported options in guest XML. (Eg, an ACPI > specification in a PowerPC guest XML will still get processed, even > though qemu-system-ppc64 does not support it while qemu-system-x86_64 > does.) > This drawback exists because libvirt falls back on qemu to query > supported > features, and qemu '-h' blindly lists all capabilities -- irrespective > of whether they are available while emulating a given architecture or > not. > The long-term solution would be for qemu to list out capabilities based > on architecture and platform -- so that libvirt can cleanly make out what > devices are supported on an arch (say 'ppc64') and platform (say, > 'mac99'). > > Signed-off-by: Prerna Saxena&lt;prerna(a)linux.vnet.ibm.com&gt; > --- > src/qemu/qemu_command.c | 39 ++++++++++++++++++++++++++++++++++++--- > src/qemu/qemu_command.h | 6 ++++++ > 2 files changed, 42 insertions(+), 3 deletions(-) > > diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c > index 85b34bb..57b25d6 100644 > --- a/src/qemu/qemu_command.c > +++ b/src/qemu/qemu_command.c > @@ -4550,8 +4550,11 @@ qemuBuildCommandLine(virConnectPtr conn, > VIR_FREE(devstr); > > virCommandAddArg(cmd, "-device"); > - virCommandAddArgFormat(cmd, "isa-serial,chardev=char%s,id=%s", > - serial->info.alias, serial->info.alias); > + if (!(devstr = qemuBuildChrDeviceStr(serial, def->os.arch, > + def->os.machine))) > + goto error; > + virCommandAddArg(cmd, devstr); > + VIR_FREE(devstr); > } else { > virCommandAddArg(cmd, "-serial"); > if (!(devstr = qemuBuildChrArgStr(&serial->source, NULL))) > @@ -5438,6 +5441,34 @@ qemuBuildCommandLine(virConnectPtr conn, > return NULL; > } > > +/* This function generates the correct '-device' string for character > + * devices of each architecture. > + */ > +char * > +qemuBuildChrDeviceStr (virDomainChrDefPtr serial, > + char *os_arch, > + char *machine) > +{ > + virBuffer cmd = VIR_BUFFER_INITIALIZER; > + > + if (STREQ(os_arch, "ppc64")&& STREQ(machine, "pseries")) > + virBufferAsprintf(&cmd, "spapr-vty,chardev=char%s", > + serial->info.alias); > + else > + virBufferAsprintf(&cmd, "isa-serial,chardev=char%s,id=%s", > + serial->info.alias, serial->info.alias); > + > + if (virBufferError(&cmd)) { > + virReportOOMError(); > + goto error; > + } > + > + return virBufferContentAndReset(&cmd); > + > + error: > + virBufferFreeAndReset(&cmd); > + return NULL; > +} > > /* > * This method takes a string representing a QEMU command line ARGV set > @@ -6649,7 +6680,9 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr > caps, > def->maxvcpus = 1; > def->vcpus = 1; > def->clock.offset = VIR_DOMAIN_CLOCK_OFFSET_UTC; > - def->features = (1<< VIR_DOMAIN_FEATURE_ACPI) > + > + if (STREQ(def->os.arch, "i686")||STREQ(def->os.arch, "x86_64")) > + def->features = (1<< VIR_DOMAIN_FEATURE_ACPI) > /*| (1<< VIR_DOMAIN_FEATURE_APIC)*/; > def->onReboot = VIR_DOMAIN_LIFECYCLE_RESTART; > def->onCrash = VIR_DOMAIN_LIFECYCLE_DESTROY; > diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h > index dbe2fb2..1fe0394 100644 > --- a/src/qemu/qemu_command.h > +++ b/src/qemu/qemu_command.h > @@ -53,6 +53,12 @@ virCommandPtr qemuBuildCommandLine(virConnectPtr conn, > enum virNetDevVPortProfileOp vmop) > ATTRIBUTE_NONNULL(1); > > +/* Generate string for arch-specific '-device' parameter */ > +char * > +qemuBuildChrDeviceStr (virDomainChrDefPtr serial, > + char *os_arch, > + char *machine); Remove the space before '('.

> + > /* With vlan == -1, use netdev syntax, else old hostnet */ > char * qemuBuildHostNetStr(virDomainNetDefPtr net, > char type_sep,