Re: [libvirt] [RFC PATCH v2 4/4] PowerPC : Bifurcate arch-specific qemu initialization code from generic qemu command line generation.

From ae7764be4454be1900fc3ee0a03bf819d5cd12de Mon Sep 17 00:00:00 2001 From: Prerna Saxena <prerna(a)linux.vnet.ibm.com&gt; Date: Mon, 14 Nov 2011 19:43:26 +0530 Subject: [PATCH 4/4] Separate out arch-specific qemu initialization from generic qemu commandline building code. At present, qemuBuildCommandLine emits many x86-specific options while generating the qemu command line. This patch proposes a framework to add arch-specific handler functions for generating different aspects of command line. At present, it is used to prevent x86-specific qemu options from cluttering the qemu command line for other architectures. Going forward, if newer drivers get defined for any arch, the code for handling these might be added by extending the handler function for that architecture. Signed-off-by: Prerna Saxena <prerna(a)linux.vnet.ibm.com&gt; --- src/qemu/qemu_command.c | 220 ++++++++++++++++++++++++++++++++--------------- src/qemu/qemu_command.h | 20 ++++ 2 files changed, 169 insertions(+), 71 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index b044050..61dabbf 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -107,6 +107,13 @@ VIR_ENUM_IMPL(qemuDomainFSDriver, VIR_DOMAIN_FS_DRIVER_TYPE_LAST, "local", "handle"); +virQemuCommandLineFunction qemuCmdLineX86 = + { .qemuBuildArchFunction = qemuBuildX86CommandLine, + }; + +virQemuCommandLineFunction qemuCmdLinePPC64 = + { .qemuBuildArchFunction = NULL, + }; static void uname_normalize (struct utsname *ut) @@ -3254,6 +3261,8 @@ qemuBuildCommandLine(virConnectPtr conn, bool emitBootindex = false; int usbcontroller = 0; bool usblegacy = false; + char *command_str; + virQemuCommandLineFunctionPtr qemuCmdFunc; uname_normalize(&ut); virUUIDFormat(def->uuid, uuid); @@ -3409,54 +3418,22 @@ qemuBuildCommandLine(virConnectPtr conn, } } - if ((def->os.smbios_mode != VIR_DOMAIN_SMBIOS_NONE) && - (def->os.smbios_mode != VIR_DOMAIN_SMBIOS_EMULATE)) { - virSysinfoDefPtr source = NULL; - bool skip_uuid = false; - - if (!qemuCapsGet(qemuCaps, QEMU_CAPS_SMBIOS_TYPE)) { - qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("the QEMU binary %s does not support smbios settings"), - emulator); - goto error; - } - - /* should we really error out or just warn in those cases ? */ - if (def->os.smbios_mode == VIR_DOMAIN_SMBIOS_HOST) { - if (driver->hostsysinfo == NULL) { - qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("Host SMBIOS information is not available")); - goto error; - } - source = driver->hostsysinfo; - /* Host and guest uuid must differ, by definition of UUID. */ - skip_uuid = true; - } else if (def->os.smbios_mode == VIR_DOMAIN_SMBIOS_SYSINFO) { - if (def->sysinfo == NULL) { - qemuReportError(VIR_ERR_XML_ERROR, - _("Domain '%s' sysinfo are not available"), - def->name); - goto error; - } - source = def->sysinfo; - /* domain_conf guaranteed that system_uuid matches guest uuid. */ - } - if (source != NULL) { - char *smbioscmd; + if (!strcmp(def->os.arch, "i686") || + !strcmp(def->os.arch, "x86_64"))

+ qemuCmdFunc = &qemuCmdLineX86; + else { + if (!strcmp(def->os.arch, "ppc64")) + qemuCmdFunc = &qemuCmdLinePPC64; + }

- smbioscmd = qemuBuildSmbiosBiosStr(source); - if (smbioscmd != NULL) { - virCommandAddArgList(cmd, "-smbios", smbioscmd, NULL); - VIR_FREE(smbioscmd); - } - smbioscmd = qemuBuildSmbiosSystemStr(source, skip_uuid); - if (smbioscmd != NULL) { - virCommandAddArgList(cmd, "-smbios", smbioscmd, NULL); - VIR_FREE(smbioscmd); - } - } + if (qemuCmdFunc->qemuBuildArchFunction) {

+ command_str = (qemuCmdFunc->qemuBuildArchFunction)(conn, driver, def, + qemuCaps); + if (command_str) { + virCommandAddArg(cmd, command_str); + VIR_FREE(command_str); + } } - /* * NB, -nographic *MUST* come before any serial, or monitor * or parallel port flags due to QEMU craziness, where it @@ -3475,25 +3452,6 @@ qemuBuildCommandLine(virConnectPtr conn, "-nodefaults"); /* Disable default guest devices */ } - /* Serial graphics adapter */ - if (def->os.bios.useserial == VIR_DOMAIN_BIOS_USESERIAL_YES) { - if (!qemuCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { - qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("qemu does not support -device")); - goto error; - } - if (!qemuCapsGet(qemuCaps, QEMU_CAPS_SGA)) { - qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("qemu does not support SGA")); - goto error; - } - if (!def->nserials) { - qemuReportError(VIR_ERR_XML_ERROR, "%s", - _("need at least one serial port to use SGA")); - goto error; - } - virCommandAddArgList(cmd, "-device", "sga", NULL); - } if (monitor_chr) { char *chrdev; @@ -3664,9 +3622,6 @@ qemuBuildCommandLine(virConnectPtr conn, if (monitor_json && qemuCapsGet(qemuCaps, QEMU_CAPS_NO_SHUTDOWN)) virCommandAddArg(cmd, "-no-shutdown"); - if (!(def->features & (1 << VIR_DOMAIN_FEATURE_ACPI))) - virCommandAddArg(cmd, "-no-acpi"); - if (!def->os.bootloader) { /* * We prefer using explicit bootindex=N parameters for predictable @@ -4347,8 +4302,10 @@ 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))) + goto error; + virCommandAddArg(cmd, devstr); + VIR_FREE(devstr); } else { virCommandAddArg(cmd, "-serial"); if (!(devstr = qemuBuildChrArgStr(&serial->source, NULL))) @@ -5238,7 +5195,127 @@ qemuBuildCommandLine(virConnectPtr conn, return NULL; } +/* This function separates x86* specific initializations in QEMU command + * line. + */ +char * +qemuBuildX86CommandLine ( virConnectPtr conn, + struct qemud_driver *driver, + virDomainDefPtr def, + virBitmapPtr qemuCaps) +{ + virBuffer opt = VIR_BUFFER_INITIALIZER; + + if ((def->os.smbios_mode != VIR_DOMAIN_SMBIOS_NONE) && + (def->os.smbios_mode != VIR_DOMAIN_SMBIOS_EMULATE)) { + virSysinfoDefPtr source = NULL; + bool skip_uuid = false; + if (!qemuCapsGet(qemuCaps, QEMU_CAPS_SMBIOS_TYPE)) { + qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("the QEMU binary %s does not support smbios settings"), + def->emulator); + goto error; + } + + /* should we really error out or just warn in those cases ? */ + if (def->os.smbios_mode == VIR_DOMAIN_SMBIOS_HOST) { + if (driver->hostsysinfo == NULL) { + qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Host SMBIOS information is not available")); + goto error; + } + source = driver->hostsysinfo; + /* Host and guest uuid must differ, by definition of UUID. */ + skip_uuid = true; + } else if (def->os.smbios_mode == VIR_DOMAIN_SMBIOS_SYSINFO) { + if (def->sysinfo == NULL) { + qemuReportError(VIR_ERR_XML_ERROR, + _("Domain '%s' sysinfo are not available"), + def->name); + goto error; + } + source = def->sysinfo; + /* domain_conf guaranteed that system_uuid matches guest uuid. */ + } + if (source != NULL) { + char *smbioscmd; + + smbioscmd = qemuBuildSmbiosBiosStr(source); + if (smbioscmd != NULL) { + virBufferAsprintf(&opt, ", -smbios %s", smbioscmd); + VIR_FREE(smbioscmd); + } + smbioscmd = qemuBuildSmbiosSystemStr(source, skip_uuid); + if (smbioscmd != NULL) { + virBufferAsprintf(&opt, ", -smbios", smbioscmd, NULL); + VIR_FREE(smbioscmd); + } + } + } + + /* Serial graphics adapter */ + if (def->os.bios.useserial == VIR_DOMAIN_BIOS_USESERIAL_YES) { + if (!qemuCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("qemu does not support -device")); + goto error; + } + if (!qemuCapsGet(qemuCaps, QEMU_CAPS_SGA)) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("qemu does not support SGA")); + goto error; + } + if (!def->nserials) { + qemuReportError(VIR_ERR_XML_ERROR, "%s", + _("need at least one serial port to use SGA")); + goto error; + } + virBufferAddLit(&opt, "-device sga"); + } + + if (!(def->features & (1 << VIR_DOMAIN_FEATURE_ACPI))) + virBufferAddLit(&opt, "-no-acpi"); + + if (virBufferError(&opt)) { + virReportOOMError(); + goto error; + } + + return virBufferContentAndReset(&opt); + + error: + virBufferFreeAndReset(&opt); + return NULL; +}

+/* + * This function generates the correct '-device' string + * for each arch. + */ +char* +qemuBuildChrDeviceStr (virDomainChrDefPtr serial, char *os_arch) +{ + virBuffer cmd = VIR_BUFFER_INITIALIZER; + + if (STREQ(os_arch, "ppc64")) + 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 * optionally prefixed by a list of environment variables. It then tries @@ -6444,7 +6521,8 @@ 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 (!strcmp(def->os.arch, "i686") || !strcmp(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 bfdaff9..9694962 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -40,6 +40,16 @@ # define QEMU_VNC_PORT_MIN 5900 # define QEMU_VNC_PORT_MAX 65535 +struct _virQemuCommandLineFunction { + char * (*qemuBuildArchFunction) (virConnectPtr, + struct qemud_driver *, + virDomainDefPtr, + virBitmapPtr); + /* Arch-specific features, such as SMBIOS, ACPI */ +}; + +typedef struct _virQemuCommandLineFunction virQemuCommandLineFunction; +typedef struct _virQemuCommandLineFunction* virQemuCommandLineFunctionPtr;

@@ -53,6 +63,16 @@ virCommandPtr qemuBuildCommandLine(virConnectPtr conn, enum virVMOperationType vmop) ATTRIBUTE_NONNULL(1); +/* Build Additional X86-specific options on command line */ +char * qemuBuildX86CommandLine(virConnectPtr conn, + struct qemud_driver *driver, + virDomainDefPtr def, + virBitmapPtr qemuCaps);

+ +/* Generate string for arch-specific '-device' parameter */ +char* +qemuBuildChrDeviceStr (virDomainChrDefPtr serial, char *os_arch); + /* With vlan == -1, use netdev syntax, else old hostnet */ char * qemuBuildHostNetStr(virDomainNetDefPtr net, char type_sep,