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

[Prerna Saxena]

On 11/29/2011 10:27 PM, Stefan Berger wrote:
> On 11/29/2011 10:01 AM, Prerna Saxena wrote:
>> From: Prerna Saxena<prerna at linux.vnet.ibm.com>
>> 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<prerna at linux.vnet.ibm.com>
>> ---
>> 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 '('.

Thanks for pointing this out.. it had escaped 'make syntax-check' too !
I'll fix this.
>> +
>> /* With vlan == -1, use netdev syntax, else old hostnet */
>> char * qemuBuildHostNetStr(virDomainNetDefPtr net,
>> char type_sep,
>


-- 
Prerna Saxena

Linux Technology Centre,
IBM Systems and Technology Lab,
Bangalore, India