[libvirt] [PATCH v5 1/2] Add NVRAM device
From: Li Zhang <zhlcindy@linux.vnet.ibm.com> For pSeries guest in QEMU, NVRAM is one kind of spapr-vio device. Users are allowed to specify spapr-vio devices'address. But NVRAM is not supported in libvirt. So this patch is to add NVRAM device to allow users to specify its address. In QEMU, NVRAM device's address is specified by "-global spapr-nvram.reg=xxxxx". In libvirt, XML file is defined as the following: <nvram> <address type='spapr-vio' reg='0x3000'/> </nvram> Signed-off-by: Li Zhang <zhlcindy@linux.vnet.ibm.com> --- v5 -> v4: * Add NVRAM capability suggested by Osier Yang. * Define macros for VIO device address. * Define qemuBuildNVRAMDevStr as static func. * Correct several error messages. * Add xml2xml test case v4 -> v3: * Seperate NVRAM definition from qemu command line parser. v3 -> v2: * Add NVRAM in domaincommon.rng and formatdomain.xml.in suggested by Daniel P.Berrange * Add NVRAM test cases suggested by Daniel P.Berrange * Remove nvram allocation when it is not specified suggested by Daniel P.Berrange * Add several error reports suggested by Daniel P.Berrange v2 -> v1: * Add NVRAM parameters parsing in qemuParseCommandLine docs/formatdomain.html.in | 35 +++++++++++++++++++ docs/schemas/domaincommon.rng | 10 ++++++ src/conf/domain_conf.c | 81 +++++++++++++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 10 ++++++ 4 files changed, 136 insertions(+) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 0cc56d9..4a7a66f 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -4502,6 +4502,41 @@ qemu-kvm -net nic,model=? /dev/null </dd> </dl> + <h4><a name="elementsNVRAM">NVRAM device</a></h4> + <p> + One NVRAM device is always added to pSeries guests on PPC64. + And on PPC64, NVRAM devices' address type are VIO which + allows users to change.<code>nvram</code> element in XML file + is provided to specify its address. + Currently, libvirt only considers configuration for pSeries guests. + </p> + <p> + Example: usage of NVRAM configuration + </p> +<pre> + ... + <devices> + <nvram> + <address type='spapr-vio' reg='0x3000'/> + </nvram> + </devices> + ... +</pre> + <dl> + <dt><code>spapr-vio</code></dt> + <dd> + <p> + VIO device address type, this is only for PPC64. + </p> + </dd> + <dt><code>reg</code></dt> + <dd> + <p> + Devices' address + </p> + </dd> + </dl> + <h3><a name="seclabel">Security label</a></h3> <p> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 3976b82..86ef540 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -2793,6 +2793,13 @@ </optional> </element> </define> + <define name="nvram"> + <element name="nvram"> + <optional> + <ref name="address"/> + </optional> + </element> + </define> <define name="memballoon"> <element name="memballoon"> <attribute name="model"> @@ -3280,6 +3287,9 @@ <optional> <ref name="memballoon"/> </optional> + <optional> + <ref name="nvram"/> + </optional> </interleave> </element> </define> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 1643f30..acaec20 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -194,6 +194,7 @@ VIR_ENUM_IMPL(virDomainDevice, VIR_DOMAIN_DEVICE_LAST, "smartcard", "chr", "memballoon", + "nvram", "rng") VIR_ENUM_IMPL(virDomainDeviceAddress, VIR_DOMAIN_DEVICE_ADDRESS_TYPE_LAST, @@ -1560,6 +1561,16 @@ void virDomainMemballoonDefFree(virDomainMemballoonDefPtr def) VIR_FREE(def); } +void virDomainNVRAMDefFree(virDomainNVRAMDefPtr def) +{ + if (!def) + return; + + virDomainDeviceInfoClear(&def->info); + + VIR_FREE(def); +} + void virDomainWatchdogDefFree(virDomainWatchdogDefPtr def) { if (!def) @@ -1743,6 +1754,7 @@ void virDomainDeviceDefFree(virDomainDeviceDefPtr def) case VIR_DOMAIN_DEVICE_SMARTCARD: case VIR_DOMAIN_DEVICE_CHR: case VIR_DOMAIN_DEVICE_MEMBALLOON: + case VIR_DOMAIN_DEVICE_NVRAM: case VIR_DOMAIN_DEVICE_LAST: break; } @@ -1951,6 +1963,7 @@ void virDomainDefFree(virDomainDefPtr def) virDomainWatchdogDefFree(def->watchdog); virDomainMemballoonDefFree(def->memballoon); + virDomainNVRAMDefFree(def->nvram); for (i = 0; i < def->nseclabels; i++) virSecurityLabelDefFree(def->seclabels[i]); @@ -2519,6 +2532,12 @@ virDomainDeviceInfoIterateInternal(virDomainDefPtr def, if (cb(def, &device, &def->rng->info, opaque) < 0) return -1; } + if (def->nvram) { + device.type = VIR_DOMAIN_DEVICE_NVRAM; + device.data.nvram = def->nvram; + if (cb(def, &device, &def->nvram->info, opaque) < 0) + return -1; + } device.type = VIR_DOMAIN_DEVICE_HUB; for (i = 0; i < def->nhubs ; i++) { device.data.hub = def->hubs[i]; @@ -2550,6 +2569,7 @@ virDomainDeviceInfoIterateInternal(virDomainDefPtr def, case VIR_DOMAIN_DEVICE_SMARTCARD: case VIR_DOMAIN_DEVICE_CHR: case VIR_DOMAIN_DEVICE_MEMBALLOON: + case VIR_DOMAIN_DEVICE_NVRAM: case VIR_DOMAIN_DEVICE_LAST: case VIR_DOMAIN_DEVICE_RNG: break; @@ -8130,6 +8150,28 @@ error: goto cleanup; } +static virDomainNVRAMDefPtr +virDomainNVRAMDefParseXML(const xmlNodePtr node, + unsigned int flags) +{ + virDomainNVRAMDefPtr def; + + if (VIR_ALLOC(def) < 0) { + virReportOOMError(); + return NULL; + } + + if (virDomainDeviceInfoParseXML(node, NULL, &def->info, flags) < 0) + goto error; + + return def; + +error: + virDomainNVRAMDefFree(def); + def = NULL; + return def; +} + static virSysinfoDefPtr virSysinfoParseXML(const xmlNodePtr node, xmlXPathContextPtr ctxt) @@ -11304,6 +11346,26 @@ virDomainDefParseXML(xmlDocPtr xml, } VIR_FREE(nodes); + def->nvram = NULL; + if ((n = virXPathNodeSet("./devices/nvram", ctxt, &nodes)) < 0) { + goto error; + } + + if (n > 1) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("only a single nvram device is supported")); + goto error; + } + + if (n > 0) { + virDomainNVRAMDefPtr nvram = + virDomainNVRAMDefParseXML(nodes[0], flags); + if (!nvram) + goto error; + def->nvram = nvram; + VIR_FREE(nodes); + } + /* analysis of the hub devices */ if ((n = virXPathNodeSet("./devices/hub", ctxt, &nodes)) < 0) { goto error; @@ -14391,6 +14453,21 @@ virDomainMemballoonDefFormat(virBufferPtr buf, } static int +virDomainNVRAMDefFormat(virBufferPtr buf, + virDomainNVRAMDefPtr def, + unsigned int flags) +{ + virBufferAddLit(buf, " <nvram>\n"); + if (virDomainDeviceInfoIsSet(&def->info, flags) && + virDomainDeviceInfoFormat(buf, &def->info, flags) < 0) + return -1; + + virBufferAddLit(buf, " </nvram>\n"); + + return 0; +} + +static int virDomainSysinfoDefFormat(virBufferPtr buf, virSysinfoDefPtr def) { @@ -15720,6 +15797,9 @@ virDomainDefFormatInternal(virDomainDefPtr def, if (def->rng) virDomainRNGDefFormat(buf, def->rng, flags); + if (def->nvram) + virDomainNVRAMDefFormat(buf, def->nvram, flags); + virBufferAddLit(buf, " </devices>\n"); virBufferAdjustIndent(buf, 2); @@ -17002,6 +17082,7 @@ virDomainDeviceDefCopy(virDomainDeviceDefPtr src, case VIR_DOMAIN_DEVICE_SMARTCARD: case VIR_DOMAIN_DEVICE_CHR: case VIR_DOMAIN_DEVICE_MEMBALLOON: + case VIR_DOMAIN_DEVICE_NVRAM: case VIR_DOMAIN_DEVICE_LAST: virReportError(VIR_ERR_INTERNAL_ERROR, _("Copying definition of '%d' type " diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index f1f01fa..502629a 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -109,6 +109,9 @@ typedef virDomainChrDef *virDomainChrDefPtr; typedef struct _virDomainMemballoonDef virDomainMemballoonDef; typedef virDomainMemballoonDef *virDomainMemballoonDefPtr; +typedef struct _virDomainNVRAMDef virDomainNVRAMDef; +typedef virDomainNVRAMDef *virDomainNVRAMDefPtr; + typedef struct _virDomainSnapshotObj virDomainSnapshotObj; typedef virDomainSnapshotObj *virDomainSnapshotObjPtr; @@ -137,6 +140,7 @@ typedef enum { VIR_DOMAIN_DEVICE_SMARTCARD, VIR_DOMAIN_DEVICE_CHR, VIR_DOMAIN_DEVICE_MEMBALLOON, + VIR_DOMAIN_DEVICE_NVRAM, VIR_DOMAIN_DEVICE_RNG, VIR_DOMAIN_DEVICE_LAST @@ -163,6 +167,7 @@ struct _virDomainDeviceDef { virDomainSmartcardDefPtr smartcard; virDomainChrDefPtr chr; virDomainMemballoonDefPtr memballoon; + virDomainNVRAMDefPtr nvram; virDomainRNGDefPtr rng; } data; }; @@ -1469,6 +1474,9 @@ struct _virDomainMemballoonDef { virDomainDeviceInfo info; }; +struct _virDomainNVRAMDef { + virDomainDeviceInfo info; +}; enum virDomainSmbiosMode { VIR_DOMAIN_SMBIOS_NONE = 0, @@ -1916,6 +1924,7 @@ struct _virDomainDef { /* Only 1 */ virDomainWatchdogDefPtr watchdog; virDomainMemballoonDefPtr memballoon; + virDomainNVRAMDefPtr nvram; virDomainTPMDefPtr tpm; virCPUDefPtr cpu; virSysinfoDefPtr sysinfo; @@ -2076,6 +2085,7 @@ int virDomainChrSourceDefCopy(virDomainChrSourceDefPtr src, void virDomainSoundCodecDefFree(virDomainSoundCodecDefPtr def); void virDomainSoundDefFree(virDomainSoundDefPtr def); void virDomainMemballoonDefFree(virDomainMemballoonDefPtr def); +void virDomainNVRAMDefFree(virDomainNVRAMDefPtr def); void virDomainWatchdogDefFree(virDomainWatchdogDefPtr def); void virDomainVideoDefFree(virDomainVideoDefPtr def); virDomainHostdevDefPtr virDomainHostdevDefAlloc(void); -- 1.8.1.4
From: Li Zhang <zhlcindy@linux.vnet.ibm.com> This patch is to add command line builder and parser for NVRAM device, and add test cases. Signed-off-by: Li Zhang <zhlcindy@linux.vnet.ibm.com> --- v5 -> v4: * Fix one memory leakage suggested by Eric Blake * Fix several code style suggested by Eric Blake src/qemu/qemu_capabilities.c | 3 ++ src/qemu/qemu_capabilities.h | 2 + src/qemu/qemu_command.c | 88 ++++++++++++++++++++++++++++++++++++++++++-- tests/qemuargv2xmltest.c | 2 + tests/qemuxml2argvtest.c | 1 + tests/qemuxml2xmltest.c | 2 + 6 files changed, 95 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index ef291c0..1d54477 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -220,6 +220,8 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, "machine-usb-opt", "tpm-passthrough", "tpm-tis", + + "nvram", /* 140 */ ); struct _virQEMUCaps { @@ -1347,6 +1349,7 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = { { "virtio-rng-ccw", QEMU_CAPS_DEVICE_VIRTIO_RNG }, { "rng-random", QEMU_CAPS_OBJECT_RNG_RANDOM }, { "rng-egd", QEMU_CAPS_OBJECT_RNG_EGD }, + { "spapr-nvram", QEMU_CAPS_DEVICE_NVRAM }, }; static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsVirtioBlk[] = { diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 4e76799..85f47c4 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -179,6 +179,8 @@ enum virQEMUCapsFlags { QEMU_CAPS_DEVICE_TPM_PASSTHROUGH = 138, /* -tpmdev passthrough */ QEMU_CAPS_DEVICE_TPM_TIS = 139, /* -device tpm_tis */ + QEMU_CAPS_DEVICE_NVRAM = 140, /*-global spapr-nvram.reg=xxxx*/ + QEMU_CAPS_LAST, /* this must always be the last item */ }; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 009d42d..41b8d78 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -53,6 +53,10 @@ #define VIR_FROM_THIS VIR_FROM_QEMU +#define VIO_ADDR_NET 0x1000ul +#define VIO_ADDR_SCSI 0x2000ul +#define VIO_ADDR_SERIAL 0x30000000ul +#define VIO_ADDR_NVRAM 0x3000ul VIR_ENUM_DECL(virDomainDiskQEMUBus) VIR_ENUM_IMPL(virDomainDiskQEMUBus, VIR_DOMAIN_DISK_BUS_LAST, @@ -1148,7 +1152,7 @@ int qemuDomainAssignSpaprVIOAddresses(virDomainDefPtr def, STREQ(def->nets[i]->model, "spapr-vlan")) def->nets[i]->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO; if (qemuAssignSpaprVIOAddress(def, &def->nets[i]->info, - 0x1000ul) < 0) + VIO_ADDR_NET) < 0) goto cleanup; } @@ -1163,7 +1167,7 @@ int qemuDomainAssignSpaprVIOAddresses(virDomainDefPtr def, def->controllers[i]->type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI) def->controllers[i]->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO; if (qemuAssignSpaprVIOAddress(def, &def->controllers[i]->info, - 0x2000ul) < 0) + VIO_ADDR_SCSI) < 0) goto cleanup; } @@ -1173,7 +1177,16 @@ int qemuDomainAssignSpaprVIOAddresses(virDomainDefPtr def, STREQ(def->os.machine, "pseries")) def->serials[i]->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO; if (qemuAssignSpaprVIOAddress(def, &def->serials[i]->info, - 0x30000000ul) < 0) + VIO_ADDR_SERIAL) < 0) + goto cleanup; + } + + if (def->nvram) { + if (def->os.arch == VIR_ARCH_PPC64 && + STREQ(def->os.machine, "pseries")) + def->nvram->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO; + if (qemuAssignSpaprVIOAddress(def, &def->nvram->info, + VIO_ADDR_NVRAM) < 0) goto cleanup; } @@ -3969,6 +3982,32 @@ error: return NULL; } +static char * +qemuBuildNVRAMDevStr(virDomainNVRAMDefPtr dev) +{ + virBuffer buf = VIR_BUFFER_INITIALIZER; + + if (dev->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO && + dev->info.addr.spaprvio.has_reg) { + virBufferAsprintf(&buf, "spapr-nvram.reg=0x%llx", + dev->info.addr.spaprvio.reg); + } else { + virReportError(VIR_ERR_XML_ERROR, + "%s", _("NVRAM address only can be spaprvio currently.\n")); + goto error; + } + + if (virBufferError(&buf)) { + virReportOOMError(); + goto error; + } + + return virBufferContentAndReset(&buf); + +error: + virBufferFreeAndReset(&buf); + return NULL; +} char * qemuBuildUSBInputDevStr(virDomainInputDefPtr dev, @@ -7776,6 +7815,30 @@ qemuBuildCommandLine(virConnectPtr conn, goto error; } + if (def->nvram) { + if (def->os.arch == VIR_ARCH_PPC64 && + STREQ(def->os.machine, "pseries")) { + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_NVRAM)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + "%s", _("NVRAM device is not available " + " with this QEMU binary")); + goto error; + } + + char *optstr; + virCommandAddArg(cmd, "-global"); + optstr = qemuBuildNVRAMDevStr(def->nvram); + if (!optstr) + goto error; + if (optstr) + virCommandAddArg(cmd, optstr); + VIR_FREE(optstr); + } else { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("NVRAM device is only supported for PPC64")); + goto error; + } + } if (snapshot) virCommandAddArgList(cmd, "-loadvm", snapshot->def->name, NULL); @@ -9884,6 +9947,25 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr qemuCaps, goto error; } + } else if (STREQ(arg, "-global") && + STRPREFIX(progargv[i + 1], "spapr-nvram.reg=")) { + + WANT_VALUE(); + + if (VIR_ALLOC(def->nvram) < 0) + goto no_memory; + + def->nvram->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO; + def->nvram->info.addr.spaprvio.has_reg = true; + + val += strlen("spapr-nvram.reg="); + if (virStrToLong_ull(val, NULL, 16, + &def->nvram->info.addr.spaprvio.reg) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot parse nvram's address:" + "'%s'"), val); + goto error; + } } else if (STREQ(arg, "-S")) { /* ignore, always added by libvirt */ } else { diff --git a/tests/qemuargv2xmltest.c b/tests/qemuargv2xmltest.c index ee6c7a9..9f1bb24 100644 --- a/tests/qemuargv2xmltest.c +++ b/tests/qemuargv2xmltest.c @@ -244,6 +244,8 @@ mymain(void) DO_TEST("hyperv"); + DO_TEST("pseries-nvram"); + DO_TEST_FULL("restore-v1", 0, "stdio"); DO_TEST_FULL("restore-v2", 0, "stdio"); DO_TEST_FULL("restore-v2", 0, "exec:cat"); diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 4bf13f0..1c21a63 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -907,6 +907,7 @@ mymain(void) QEMU_CAPS_CHARDEV, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG); DO_TEST_ERROR("pseries-vio-address-clash", QEMU_CAPS_DRIVE, QEMU_CAPS_CHARDEV, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG); + DO_TEST("pseries-nvram", QEMU_CAPS_DEVICE_NVRAM); DO_TEST("disk-ide-drive-split", QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_IDE_CD); diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 7434190..1d10bf2 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -259,6 +259,8 @@ mymain(void) DO_TEST("virtio-rng-random"); DO_TEST("virtio-rng-egd"); + DO_TEST("pseries-nvram"); + /* These tests generate different XML */ DO_TEST_DIFFERENT("balloon-device-auto"); DO_TEST_DIFFERENT("channel-virtio-auto"); -- 1.8.1.4
On 19/04/13 16:37, Li Zhang wrote:
From: Li Zhang <zhlcindy@linux.vnet.ibm.com>
This patch is to add command line builder and parser for NVRAM device, and add test cases.
Signed-off-by: Li Zhang <zhlcindy@linux.vnet.ibm.com> --- v5 -> v4: * Fix one memory leakage suggested by Eric Blake * Fix several code style suggested by Eric Blake
src/qemu/qemu_capabilities.c | 3 ++ src/qemu/qemu_capabilities.h | 2 + src/qemu/qemu_command.c | 88 ++++++++++++++++++++++++++++++++++++++++++-- tests/qemuargv2xmltest.c | 2 + tests/qemuxml2argvtest.c | 1 + tests/qemuxml2xmltest.c | 2 + 6 files changed, 95 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index ef291c0..1d54477 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -220,6 +220,8 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, "machine-usb-opt", "tpm-passthrough", "tpm-tis", + + "nvram", /* 140 */ );
struct _virQEMUCaps { @@ -1347,6 +1349,7 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = { { "virtio-rng-ccw", QEMU_CAPS_DEVICE_VIRTIO_RNG }, { "rng-random", QEMU_CAPS_OBJECT_RNG_RANDOM }, { "rng-egd", QEMU_CAPS_OBJECT_RNG_EGD }, + { "spapr-nvram", QEMU_CAPS_DEVICE_NVRAM }, };
static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsVirtioBlk[] = { diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 4e76799..85f47c4 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -179,6 +179,8 @@ enum virQEMUCapsFlags { QEMU_CAPS_DEVICE_TPM_PASSTHROUGH = 138, /* -tpmdev passthrough */ QEMU_CAPS_DEVICE_TPM_TIS = 139, /* -device tpm_tis */
+ QEMU_CAPS_DEVICE_NVRAM = 140, /*-global spapr-nvram.reg=xxxx*/
/* -global spapr-nvram.reg=xxxx */
+ QEMU_CAPS_LAST, /* this must always be the last item */ };
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 009d42d..41b8d78 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -53,6 +53,10 @@
#define VIR_FROM_THIS VIR_FROM_QEMU
+#define VIO_ADDR_NET 0x1000ul +#define VIO_ADDR_SCSI 0x2000ul +#define VIO_ADDR_SERIAL 0x30000000ul +#define VIO_ADDR_NVRAM 0x3000ul
This is much better.
VIR_ENUM_DECL(virDomainDiskQEMUBus) VIR_ENUM_IMPL(virDomainDiskQEMUBus, VIR_DOMAIN_DISK_BUS_LAST, @@ -1148,7 +1152,7 @@ int qemuDomainAssignSpaprVIOAddresses(virDomainDefPtr def, STREQ(def->nets[i]->model, "spapr-vlan")) def->nets[i]->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO; if (qemuAssignSpaprVIOAddress(def, &def->nets[i]->info, - 0x1000ul) < 0) + VIO_ADDR_NET) < 0) goto cleanup; }
@@ -1163,7 +1167,7 @@ int qemuDomainAssignSpaprVIOAddresses(virDomainDefPtr def, def->controllers[i]->type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI) def->controllers[i]->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO; if (qemuAssignSpaprVIOAddress(def, &def->controllers[i]->info, - 0x2000ul) < 0) + VIO_ADDR_SCSI) < 0) goto cleanup; }
@@ -1173,7 +1177,16 @@ int qemuDomainAssignSpaprVIOAddresses(virDomainDefPtr def, STREQ(def->os.machine, "pseries")) def->serials[i]->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO; if (qemuAssignSpaprVIOAddress(def, &def->serials[i]->info, - 0x30000000ul) < 0) + VIO_ADDR_SERIAL) < 0) + goto cleanup; + } + + if (def->nvram) { + if (def->os.arch == VIR_ARCH_PPC64 && + STREQ(def->os.machine, "pseries")) + def->nvram->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO; + if (qemuAssignSpaprVIOAddress(def, &def->nvram->info, + VIO_ADDR_NVRAM) < 0) goto cleanup; }
@@ -3969,6 +3982,32 @@ error: return NULL; }
+static char * +qemuBuildNVRAMDevStr(virDomainNVRAMDefPtr dev) +{ + virBuffer buf = VIR_BUFFER_INITIALIZER; + + if (dev->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO && + dev->info.addr.spaprvio.has_reg) { + virBufferAsprintf(&buf, "spapr-nvram.reg=0x%llx", + dev->info.addr.spaprvio.reg); + } else { + virReportError(VIR_ERR_XML_ERROR, + "%s", _("NVRAM address only can be spaprvio currently.\n"));
"nvram address type must be 'spaprvio'" No need for the ".\n".
+ goto error; + } + + if (virBufferError(&buf)) { + virReportOOMError(); + goto error; + } + + return virBufferContentAndReset(&buf); + +error: + virBufferFreeAndReset(&buf); + return NULL; +}
char * qemuBuildUSBInputDevStr(virDomainInputDefPtr dev, @@ -7776,6 +7815,30 @@ qemuBuildCommandLine(virConnectPtr conn, goto error; }
+ if (def->nvram) { + if (def->os.arch == VIR_ARCH_PPC64 && + STREQ(def->os.machine, "pseries")) { + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_NVRAM)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + "%s", _("NVRAM device is not available "
s/NVRAM/nvram/, to be consistent with the tag name in XML.
+ " with this QEMU binary"));
s/ with/with/
+ goto error; + } + + char *optstr; + virCommandAddArg(cmd, "-global"); + optstr = qemuBuildNVRAMDevStr(def->nvram); + if (!optstr) + goto error; + if (optstr) + virCommandAddArg(cmd, optstr); + VIR_FREE(optstr); + } else { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("NVRAM device is only supported for PPC64"));
s/NVRAM/nvram/,
+ goto error; + } + } if (snapshot) virCommandAddArgList(cmd, "-loadvm", snapshot->def->name, NULL);
@@ -9884,6 +9947,25 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr qemuCaps, goto error; }
+ } else if (STREQ(arg, "-global") && + STRPREFIX(progargv[i + 1], "spapr-nvram.reg=")) { + useless blank line.
+ WANT_VALUE(); + + if (VIR_ALLOC(def->nvram) < 0) + goto no_memory; + + def->nvram->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO; + def->nvram->info.addr.spaprvio.has_reg = true; + + val += strlen("spapr-nvram.reg="); + if (virStrToLong_ull(val, NULL, 16, + &def->nvram->info.addr.spaprvio.reg) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot parse nvram's address:" + "'%s'"), val); + goto error; + } } else if (STREQ(arg, "-S")) { /* ignore, always added by libvirt */ } else { diff --git a/tests/qemuargv2xmltest.c b/tests/qemuargv2xmltest.c index ee6c7a9..9f1bb24 100644 --- a/tests/qemuargv2xmltest.c +++ b/tests/qemuargv2xmltest.c @@ -244,6 +244,8 @@ mymain(void)
DO_TEST("hyperv");
+ DO_TEST("pseries-nvram"); + DO_TEST_FULL("restore-v1", 0, "stdio"); DO_TEST_FULL("restore-v2", 0, "stdio"); DO_TEST_FULL("restore-v2", 0, "exec:cat"); diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 4bf13f0..1c21a63 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -907,6 +907,7 @@ mymain(void) QEMU_CAPS_CHARDEV, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG); DO_TEST_ERROR("pseries-vio-address-clash", QEMU_CAPS_DRIVE, QEMU_CAPS_CHARDEV, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG); + DO_TEST("pseries-nvram", QEMU_CAPS_DEVICE_NVRAM); DO_TEST("disk-ide-drive-split", QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_IDE_CD); diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 7434190..1d10bf2 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -259,6 +259,8 @@ mymain(void) DO_TEST("virtio-rng-random"); DO_TEST("virtio-rng-egd");
+ DO_TEST("pseries-nvram"); + /* These tests generate different XML */ DO_TEST_DIFFERENT("balloon-device-auto"); DO_TEST_DIFFERENT("channel-virtio-auto");
ACK with the nits fixed.
On 24/04/13 20:17, Osier Yang wrote:
On 19/04/13 16:37, Li Zhang wrote:
From: Li Zhang <zhlcindy@linux.vnet.ibm.com>
This patch is to add command line builder and parser for NVRAM device, and add test cases.
Signed-off-by: Li Zhang <zhlcindy@linux.vnet.ibm.com> --- v5 -> v4: * Fix one memory leakage suggested by Eric Blake * Fix several code style suggested by Eric Blake
src/qemu/qemu_capabilities.c | 3 ++ src/qemu/qemu_capabilities.h | 2 + src/qemu/qemu_command.c | 88 ++++++++++++++++++++++++++++++++++++++++++-- tests/qemuargv2xmltest.c | 2 + tests/qemuxml2argvtest.c | 1 + tests/qemuxml2xmltest.c | 2 + 6 files changed, 95 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index ef291c0..1d54477 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -220,6 +220,8 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, "machine-usb-opt", "tpm-passthrough", "tpm-tis", + + "nvram", /* 140 */ ); struct _virQEMUCaps { @@ -1347,6 +1349,7 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = { { "virtio-rng-ccw", QEMU_CAPS_DEVICE_VIRTIO_RNG }, { "rng-random", QEMU_CAPS_OBJECT_RNG_RANDOM }, { "rng-egd", QEMU_CAPS_OBJECT_RNG_EGD }, + { "spapr-nvram", QEMU_CAPS_DEVICE_NVRAM }, }; static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsVirtioBlk[] = { diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 4e76799..85f47c4 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -179,6 +179,8 @@ enum virQEMUCapsFlags { QEMU_CAPS_DEVICE_TPM_PASSTHROUGH = 138, /* -tpmdev passthrough */ QEMU_CAPS_DEVICE_TPM_TIS = 139, /* -device tpm_tis */ + QEMU_CAPS_DEVICE_NVRAM = 140, /*-global spapr-nvram.reg=xxxx*/
/* -global spapr-nvram.reg=xxxx */
+ QEMU_CAPS_LAST, /* this must always be the last item */ }; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 009d42d..41b8d78 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -53,6 +53,10 @@ #define VIR_FROM_THIS VIR_FROM_QEMU +#define VIO_ADDR_NET 0x1000ul +#define VIO_ADDR_SCSI 0x2000ul +#define VIO_ADDR_SERIAL 0x30000000ul +#define VIO_ADDR_NVRAM 0x3000ul
This is much better.
VIR_ENUM_DECL(virDomainDiskQEMUBus) VIR_ENUM_IMPL(virDomainDiskQEMUBus, VIR_DOMAIN_DISK_BUS_LAST, @@ -1148,7 +1152,7 @@ int qemuDomainAssignSpaprVIOAddresses(virDomainDefPtr def, STREQ(def->nets[i]->model, "spapr-vlan")) def->nets[i]->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO; if (qemuAssignSpaprVIOAddress(def, &def->nets[i]->info, - 0x1000ul) < 0) + VIO_ADDR_NET) < 0) goto cleanup; } @@ -1163,7 +1167,7 @@ int qemuDomainAssignSpaprVIOAddresses(virDomainDefPtr def, def->controllers[i]->type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI) def->controllers[i]->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO; if (qemuAssignSpaprVIOAddress(def, &def->controllers[i]->info, - 0x2000ul) < 0) + VIO_ADDR_SCSI) < 0) goto cleanup; } @@ -1173,7 +1177,16 @@ int qemuDomainAssignSpaprVIOAddresses(virDomainDefPtr def, STREQ(def->os.machine, "pseries")) def->serials[i]->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO; if (qemuAssignSpaprVIOAddress(def, &def->serials[i]->info, - 0x30000000ul) < 0) + VIO_ADDR_SERIAL) < 0) + goto cleanup; + } + + if (def->nvram) { + if (def->os.arch == VIR_ARCH_PPC64 && + STREQ(def->os.machine, "pseries")) + def->nvram->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO; + if (qemuAssignSpaprVIOAddress(def, &def->nvram->info, + VIO_ADDR_NVRAM) < 0) goto cleanup; } @@ -3969,6 +3982,32 @@ error: return NULL; } +static char * +qemuBuildNVRAMDevStr(virDomainNVRAMDefPtr dev) +{ + virBuffer buf = VIR_BUFFER_INITIALIZER; + + if (dev->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO && + dev->info.addr.spaprvio.has_reg) { + virBufferAsprintf(&buf, "spapr-nvram.reg=0x%llx", + dev->info.addr.spaprvio.reg); + } else { + virReportError(VIR_ERR_XML_ERROR, + "%s", _("NVRAM address only can be spaprvio currently.\n"));
"nvram address type must be 'spaprvio'"
No need for the ".\n".
+ goto error; + } + + if (virBufferError(&buf)) { + virReportOOMError(); + goto error; + } + + return virBufferContentAndReset(&buf); + +error: + virBufferFreeAndReset(&buf); + return NULL; +} char * qemuBuildUSBInputDevStr(virDomainInputDefPtr dev, @@ -7776,6 +7815,30 @@ qemuBuildCommandLine(virConnectPtr conn, goto error; } + if (def->nvram) { + if (def->os.arch == VIR_ARCH_PPC64 && + STREQ(def->os.machine, "pseries")) { + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_NVRAM)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + "%s", _("NVRAM device is not available "
s/NVRAM/nvram/, to be consistent with the tag name in XML.
+ " with this QEMU binary"));
s/ with/with/
+ goto error; + } + + char *optstr; + virCommandAddArg(cmd, "-global"); + optstr = qemuBuildNVRAMDevStr(def->nvram); + if (!optstr) + goto error; + if (optstr) + virCommandAddArg(cmd, optstr); + VIR_FREE(optstr); + } else { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("NVRAM device is only supported for PPC64"));
s/NVRAM/nvram/,
+ goto error; + } + } if (snapshot) virCommandAddArgList(cmd, "-loadvm", snapshot->def->name, NULL); @@ -9884,6 +9947,25 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr qemuCaps, goto error; } + } else if (STREQ(arg, "-global") && + STRPREFIX(progargv[i + 1], "spapr-nvram.reg=")) { + useless blank line.
+ WANT_VALUE(); + + if (VIR_ALLOC(def->nvram) < 0) + goto no_memory; + + def->nvram->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO; + def->nvram->info.addr.spaprvio.has_reg = true; + + val += strlen("spapr-nvram.reg="); + if (virStrToLong_ull(val, NULL, 16, + &def->nvram->info.addr.spaprvio.reg) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot parse nvram's address:" + "'%s'"), val); + goto error; + } } else if (STREQ(arg, "-S")) { /* ignore, always added by libvirt */ } else { diff --git a/tests/qemuargv2xmltest.c b/tests/qemuargv2xmltest.c index ee6c7a9..9f1bb24 100644 --- a/tests/qemuargv2xmltest.c +++ b/tests/qemuargv2xmltest.c @@ -244,6 +244,8 @@ mymain(void) DO_TEST("hyperv"); + DO_TEST("pseries-nvram"); + DO_TEST_FULL("restore-v1", 0, "stdio"); DO_TEST_FULL("restore-v2", 0, "stdio"); DO_TEST_FULL("restore-v2", 0, "exec:cat"); diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 4bf13f0..1c21a63 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -907,6 +907,7 @@ mymain(void) QEMU_CAPS_CHARDEV, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG); DO_TEST_ERROR("pseries-vio-address-clash", QEMU_CAPS_DRIVE, QEMU_CAPS_CHARDEV, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG); + DO_TEST("pseries-nvram", QEMU_CAPS_DEVICE_NVRAM); DO_TEST("disk-ide-drive-split", QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_IDE_CD); diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 7434190..1d10bf2 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -259,6 +259,8 @@ mymain(void) DO_TEST("virtio-rng-random"); DO_TEST("virtio-rng-egd"); + DO_TEST("pseries-nvram"); + /* These tests generate different XML */ DO_TEST_DIFFERENT("balloon-device-auto"); DO_TEST_DIFFERENT("channel-virtio-auto");
ACK with the nits fixed.
I intended to push this with the attached diff squashed in. But it fails with test files missed: .............................I/O warning : failed to load external entity "/home/osier/work/git/libvirt/tests/qemuxml2argvdata/qemuxml2argv-pseries-nvram.xml" !.......... 280 . 281 FAIL FAIL: qemuxml2argvtest TEST: qemuxml2xmltest ........................................ 40 ........................................ 80 ..................................../home/osier/work/git/libvirt/tests/qemuxml2argvdata/qemuxml2argv-pseries-nvram.xml: failed to open: No such file or directory /home/osier/work/git/libvirt/tests/qemuxml2argvdata/qemuxml2argv-pseries-nvram.xml: failed to open: No such file or directory !... 120 ........... 131 FAIL FAIL: qemuxml2xmltest TEST: qemuxmlnstest ....... 7 OK PASS: qemuxmlnstest TEST: qemuargv2xmltest ........................................ 40 ...................................../home/osier/work/git/libvirt/tests/qemuxml2argvdata/qemuxml2argv-pseries-nvram.args: failed to open: No such file or directory
On 2013年04月24日 20:41, Osier Yang wrote:
On 24/04/13 20:17, Osier Yang wrote:
On 19/04/13 16:37, Li Zhang wrote:
From: Li Zhang <zhlcindy@linux.vnet.ibm.com>
This patch is to add command line builder and parser for NVRAM device, and add test cases.
Signed-off-by: Li Zhang <zhlcindy@linux.vnet.ibm.com> --- v5 -> v4: * Fix one memory leakage suggested by Eric Blake * Fix several code style suggested by Eric Blake
src/qemu/qemu_capabilities.c | 3 ++ src/qemu/qemu_capabilities.h | 2 + src/qemu/qemu_command.c | 88 ++++++++++++++++++++++++++++++++++++++++++-- tests/qemuargv2xmltest.c | 2 + tests/qemuxml2argvtest.c | 1 + tests/qemuxml2xmltest.c | 2 + 6 files changed, 95 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index ef291c0..1d54477 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -220,6 +220,8 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, "machine-usb-opt", "tpm-passthrough", "tpm-tis", + + "nvram", /* 140 */ ); struct _virQEMUCaps { @@ -1347,6 +1349,7 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = { { "virtio-rng-ccw", QEMU_CAPS_DEVICE_VIRTIO_RNG }, { "rng-random", QEMU_CAPS_OBJECT_RNG_RANDOM }, { "rng-egd", QEMU_CAPS_OBJECT_RNG_EGD }, + { "spapr-nvram", QEMU_CAPS_DEVICE_NVRAM }, }; static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsVirtioBlk[] = { diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 4e76799..85f47c4 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -179,6 +179,8 @@ enum virQEMUCapsFlags { QEMU_CAPS_DEVICE_TPM_PASSTHROUGH = 138, /* -tpmdev passthrough */ QEMU_CAPS_DEVICE_TPM_TIS = 139, /* -device tpm_tis */ + QEMU_CAPS_DEVICE_NVRAM = 140, /*-global spapr-nvram.reg=xxxx*/
/* -global spapr-nvram.reg=xxxx */
+ QEMU_CAPS_LAST, /* this must always be the last item */ }; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 009d42d..41b8d78 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -53,6 +53,10 @@ #define VIR_FROM_THIS VIR_FROM_QEMU +#define VIO_ADDR_NET 0x1000ul +#define VIO_ADDR_SCSI 0x2000ul +#define VIO_ADDR_SERIAL 0x30000000ul +#define VIO_ADDR_NVRAM 0x3000ul
This is much better.
VIR_ENUM_DECL(virDomainDiskQEMUBus) VIR_ENUM_IMPL(virDomainDiskQEMUBus, VIR_DOMAIN_DISK_BUS_LAST, @@ -1148,7 +1152,7 @@ int qemuDomainAssignSpaprVIOAddresses(virDomainDefPtr def, STREQ(def->nets[i]->model, "spapr-vlan")) def->nets[i]->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO; if (qemuAssignSpaprVIOAddress(def, &def->nets[i]->info, - 0x1000ul) < 0) + VIO_ADDR_NET) < 0) goto cleanup; } @@ -1163,7 +1167,7 @@ int qemuDomainAssignSpaprVIOAddresses(virDomainDefPtr def, def->controllers[i]->type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI) def->controllers[i]->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO; if (qemuAssignSpaprVIOAddress(def, &def->controllers[i]->info, - 0x2000ul) < 0) + VIO_ADDR_SCSI) < 0) goto cleanup; } @@ -1173,7 +1177,16 @@ int qemuDomainAssignSpaprVIOAddresses(virDomainDefPtr def, STREQ(def->os.machine, "pseries")) def->serials[i]->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO; if (qemuAssignSpaprVIOAddress(def, &def->serials[i]->info, - 0x30000000ul) < 0) + VIO_ADDR_SERIAL) < 0) + goto cleanup; + } + + if (def->nvram) { + if (def->os.arch == VIR_ARCH_PPC64 && + STREQ(def->os.machine, "pseries")) + def->nvram->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO; + if (qemuAssignSpaprVIOAddress(def, &def->nvram->info, + VIO_ADDR_NVRAM) < 0) goto cleanup; } @@ -3969,6 +3982,32 @@ error: return NULL; } +static char * +qemuBuildNVRAMDevStr(virDomainNVRAMDefPtr dev) +{ + virBuffer buf = VIR_BUFFER_INITIALIZER; + + if (dev->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO && + dev->info.addr.spaprvio.has_reg) { + virBufferAsprintf(&buf, "spapr-nvram.reg=0x%llx", + dev->info.addr.spaprvio.reg); + } else { + virReportError(VIR_ERR_XML_ERROR, + "%s", _("NVRAM address only can be spaprvio currently.\n"));
"nvram address type must be 'spaprvio'"
No need for the ".\n".
+ goto error; + } + + if (virBufferError(&buf)) { + virReportOOMError(); + goto error; + } + + return virBufferContentAndReset(&buf); + +error: + virBufferFreeAndReset(&buf); + return NULL; +} char * qemuBuildUSBInputDevStr(virDomainInputDefPtr dev, @@ -7776,6 +7815,30 @@ qemuBuildCommandLine(virConnectPtr conn, goto error; } + if (def->nvram) { + if (def->os.arch == VIR_ARCH_PPC64 && + STREQ(def->os.machine, "pseries")) { + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_NVRAM)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + "%s", _("NVRAM device is not available "
s/NVRAM/nvram/, to be consistent with the tag name in XML.
+ " with this QEMU binary"));
s/ with/with/
+ goto error; + } + + char *optstr; + virCommandAddArg(cmd, "-global"); + optstr = qemuBuildNVRAMDevStr(def->nvram); + if (!optstr) + goto error; + if (optstr) + virCommandAddArg(cmd, optstr); + VIR_FREE(optstr); + } else { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("NVRAM device is only supported for PPC64"));
s/NVRAM/nvram/,
+ goto error; + } + } if (snapshot) virCommandAddArgList(cmd, "-loadvm", snapshot->def->name, NULL); @@ -9884,6 +9947,25 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr qemuCaps, goto error; } + } else if (STREQ(arg, "-global") && + STRPREFIX(progargv[i + 1], "spapr-nvram.reg=")) { + useless blank line.
+ WANT_VALUE(); + + if (VIR_ALLOC(def->nvram) < 0) + goto no_memory; + + def->nvram->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO; + def->nvram->info.addr.spaprvio.has_reg = true; + + val += strlen("spapr-nvram.reg="); + if (virStrToLong_ull(val, NULL, 16, + &def->nvram->info.addr.spaprvio.reg) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot parse nvram's address:" + "'%s'"), val); + goto error; + } } else if (STREQ(arg, "-S")) { /* ignore, always added by libvirt */ } else { diff --git a/tests/qemuargv2xmltest.c b/tests/qemuargv2xmltest.c index ee6c7a9..9f1bb24 100644 --- a/tests/qemuargv2xmltest.c +++ b/tests/qemuargv2xmltest.c @@ -244,6 +244,8 @@ mymain(void) DO_TEST("hyperv"); + DO_TEST("pseries-nvram"); + DO_TEST_FULL("restore-v1", 0, "stdio"); DO_TEST_FULL("restore-v2", 0, "stdio"); DO_TEST_FULL("restore-v2", 0, "exec:cat"); diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 4bf13f0..1c21a63 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -907,6 +907,7 @@ mymain(void) QEMU_CAPS_CHARDEV, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG); DO_TEST_ERROR("pseries-vio-address-clash", QEMU_CAPS_DRIVE, QEMU_CAPS_CHARDEV, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG); + DO_TEST("pseries-nvram", QEMU_CAPS_DEVICE_NVRAM); DO_TEST("disk-ide-drive-split", QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_IDE_CD); diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 7434190..1d10bf2 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -259,6 +259,8 @@ mymain(void) DO_TEST("virtio-rng-random"); DO_TEST("virtio-rng-egd"); + DO_TEST("pseries-nvram"); + /* These tests generate different XML */ DO_TEST_DIFFERENT("balloon-device-auto"); DO_TEST_DIFFERENT("channel-virtio-auto");
ACK with the nits fixed.
I intended to push this with the attached diff squashed in. But it fails with test files missed:
Ah, sorry. I forget to add these files to my patch. I will resend it later. :)
.............................I/O warning : failed to load external entity "/home/osier/work/git/libvirt/tests/qemuxml2argvdata/qemuxml2argv-pseries-nvram.xml" !.......... 280 . 281 FAIL FAIL: qemuxml2argvtest TEST: qemuxml2xmltest ........................................ 40 ........................................ 80 ..................................../home/osier/work/git/libvirt/tests/qemuxml2argvdata/qemuxml2argv-pseries-nvram.xml: failed to open: No such file or directory /home/osier/work/git/libvirt/tests/qemuxml2argvdata/qemuxml2argv-pseries-nvram.xml: failed to open: No such file or directory !... 120 ........... 131 FAIL FAIL: qemuxml2xmltest TEST: qemuxmlnstest ....... 7 OK PASS: qemuxmlnstest TEST: qemuargv2xmltest ........................................ 40 ...................................../home/osier/work/git/libvirt/tests/qemuxml2argvdata/qemuxml2argv-pseries-nvram.args: failed to open: No such file or directory
ping ... On 2013年04月19日 16:37, Li Zhang wrote:
From: Li Zhang <zhlcindy@linux.vnet.ibm.com>
For pSeries guest in QEMU, NVRAM is one kind of spapr-vio device. Users are allowed to specify spapr-vio devices'address. But NVRAM is not supported in libvirt. So this patch is to add NVRAM device to allow users to specify its address.
In QEMU, NVRAM device's address is specified by "-global spapr-nvram.reg=xxxxx".
In libvirt, XML file is defined as the following:
<nvram> <address type='spapr-vio' reg='0x3000'/> </nvram>
Signed-off-by: Li Zhang <zhlcindy@linux.vnet.ibm.com> --- v5 -> v4: * Add NVRAM capability suggested by Osier Yang. * Define macros for VIO device address. * Define qemuBuildNVRAMDevStr as static func. * Correct several error messages. * Add xml2xml test case
v4 -> v3: * Seperate NVRAM definition from qemu command line parser.
v3 -> v2: * Add NVRAM in domaincommon.rng and formatdomain.xml.in suggested by Daniel P.Berrange * Add NVRAM test cases suggested by Daniel P.Berrange * Remove nvram allocation when it is not specified suggested by Daniel P.Berrange * Add several error reports suggested by Daniel P.Berrange
v2 -> v1: * Add NVRAM parameters parsing in qemuParseCommandLine
docs/formatdomain.html.in | 35 +++++++++++++++++++ docs/schemas/domaincommon.rng | 10 ++++++ src/conf/domain_conf.c | 81 +++++++++++++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 10 ++++++ 4 files changed, 136 insertions(+)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 0cc56d9..4a7a66f 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -4502,6 +4502,41 @@ qemu-kvm -net nic,model=? /dev/null </dd> </dl>
+ <h4><a name="elementsNVRAM">NVRAM device</a></h4> + <p> + One NVRAM device is always added to pSeries guests on PPC64. + And on PPC64, NVRAM devices' address type are VIO which + allows users to change.<code>nvram</code> element in XML file + is provided to specify its address. + Currently, libvirt only considers configuration for pSeries guests. + </p> + <p> + Example: usage of NVRAM configuration + </p> +<pre> + ... + <devices> + <nvram> + <address type='spapr-vio' reg='0x3000'/> + </nvram> + </devices> + ... +</pre> + <dl> + <dt><code>spapr-vio</code></dt> + <dd> + <p> + VIO device address type, this is only for PPC64. + </p> + </dd> + <dt><code>reg</code></dt> + <dd> + <p> + Devices' address + </p> + </dd> + </dl> + <h3><a name="seclabel">Security label</a></h3>
<p> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 3976b82..86ef540 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -2793,6 +2793,13 @@ </optional> </element> </define> + <define name="nvram"> + <element name="nvram"> + <optional> + <ref name="address"/> + </optional> + </element> + </define> <define name="memballoon"> <element name="memballoon"> <attribute name="model"> @@ -3280,6 +3287,9 @@ <optional> <ref name="memballoon"/> </optional> + <optional> + <ref name="nvram"/> + </optional> </interleave> </element> </define> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 1643f30..acaec20 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -194,6 +194,7 @@ VIR_ENUM_IMPL(virDomainDevice, VIR_DOMAIN_DEVICE_LAST, "smartcard", "chr", "memballoon", + "nvram", "rng")
VIR_ENUM_IMPL(virDomainDeviceAddress, VIR_DOMAIN_DEVICE_ADDRESS_TYPE_LAST, @@ -1560,6 +1561,16 @@ void virDomainMemballoonDefFree(virDomainMemballoonDefPtr def) VIR_FREE(def); }
+void virDomainNVRAMDefFree(virDomainNVRAMDefPtr def) +{ + if (!def) + return; + + virDomainDeviceInfoClear(&def->info); + + VIR_FREE(def); +} + void virDomainWatchdogDefFree(virDomainWatchdogDefPtr def) { if (!def) @@ -1743,6 +1754,7 @@ void virDomainDeviceDefFree(virDomainDeviceDefPtr def) case VIR_DOMAIN_DEVICE_SMARTCARD: case VIR_DOMAIN_DEVICE_CHR: case VIR_DOMAIN_DEVICE_MEMBALLOON: + case VIR_DOMAIN_DEVICE_NVRAM: case VIR_DOMAIN_DEVICE_LAST: break; } @@ -1951,6 +1963,7 @@ void virDomainDefFree(virDomainDefPtr def) virDomainWatchdogDefFree(def->watchdog);
virDomainMemballoonDefFree(def->memballoon); + virDomainNVRAMDefFree(def->nvram);
for (i = 0; i < def->nseclabels; i++) virSecurityLabelDefFree(def->seclabels[i]); @@ -2519,6 +2532,12 @@ virDomainDeviceInfoIterateInternal(virDomainDefPtr def, if (cb(def, &device, &def->rng->info, opaque) < 0) return -1; } + if (def->nvram) { + device.type = VIR_DOMAIN_DEVICE_NVRAM; + device.data.nvram = def->nvram; + if (cb(def, &device, &def->nvram->info, opaque) < 0) + return -1; + } device.type = VIR_DOMAIN_DEVICE_HUB; for (i = 0; i < def->nhubs ; i++) { device.data.hub = def->hubs[i]; @@ -2550,6 +2569,7 @@ virDomainDeviceInfoIterateInternal(virDomainDefPtr def, case VIR_DOMAIN_DEVICE_SMARTCARD: case VIR_DOMAIN_DEVICE_CHR: case VIR_DOMAIN_DEVICE_MEMBALLOON: + case VIR_DOMAIN_DEVICE_NVRAM: case VIR_DOMAIN_DEVICE_LAST: case VIR_DOMAIN_DEVICE_RNG: break; @@ -8130,6 +8150,28 @@ error: goto cleanup; }
+static virDomainNVRAMDefPtr +virDomainNVRAMDefParseXML(const xmlNodePtr node, + unsigned int flags) +{ + virDomainNVRAMDefPtr def; + + if (VIR_ALLOC(def) < 0) { + virReportOOMError(); + return NULL; + } + + if (virDomainDeviceInfoParseXML(node, NULL, &def->info, flags) < 0) + goto error; + + return def; + +error: + virDomainNVRAMDefFree(def); + def = NULL; + return def; +} + static virSysinfoDefPtr virSysinfoParseXML(const xmlNodePtr node, xmlXPathContextPtr ctxt) @@ -11304,6 +11346,26 @@ virDomainDefParseXML(xmlDocPtr xml, } VIR_FREE(nodes);
+ def->nvram = NULL; + if ((n = virXPathNodeSet("./devices/nvram", ctxt, &nodes)) < 0) { + goto error; + } + + if (n > 1) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("only a single nvram device is supported")); + goto error; + } + + if (n > 0) { + virDomainNVRAMDefPtr nvram = + virDomainNVRAMDefParseXML(nodes[0], flags); + if (!nvram) + goto error; + def->nvram = nvram; + VIR_FREE(nodes); + } + /* analysis of the hub devices */ if ((n = virXPathNodeSet("./devices/hub", ctxt, &nodes)) < 0) { goto error; @@ -14391,6 +14453,21 @@ virDomainMemballoonDefFormat(virBufferPtr buf, }
static int +virDomainNVRAMDefFormat(virBufferPtr buf, + virDomainNVRAMDefPtr def, + unsigned int flags) +{ + virBufferAddLit(buf, " <nvram>\n"); + if (virDomainDeviceInfoIsSet(&def->info, flags) && + virDomainDeviceInfoFormat(buf, &def->info, flags) < 0) + return -1; + + virBufferAddLit(buf, " </nvram>\n"); + + return 0; +} + +static int virDomainSysinfoDefFormat(virBufferPtr buf, virSysinfoDefPtr def) { @@ -15720,6 +15797,9 @@ virDomainDefFormatInternal(virDomainDefPtr def, if (def->rng) virDomainRNGDefFormat(buf, def->rng, flags);
+ if (def->nvram) + virDomainNVRAMDefFormat(buf, def->nvram, flags); + virBufferAddLit(buf, " </devices>\n");
virBufferAdjustIndent(buf, 2); @@ -17002,6 +17082,7 @@ virDomainDeviceDefCopy(virDomainDeviceDefPtr src, case VIR_DOMAIN_DEVICE_SMARTCARD: case VIR_DOMAIN_DEVICE_CHR: case VIR_DOMAIN_DEVICE_MEMBALLOON: + case VIR_DOMAIN_DEVICE_NVRAM: case VIR_DOMAIN_DEVICE_LAST: virReportError(VIR_ERR_INTERNAL_ERROR, _("Copying definition of '%d' type " diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index f1f01fa..502629a 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -109,6 +109,9 @@ typedef virDomainChrDef *virDomainChrDefPtr; typedef struct _virDomainMemballoonDef virDomainMemballoonDef; typedef virDomainMemballoonDef *virDomainMemballoonDefPtr;
+typedef struct _virDomainNVRAMDef virDomainNVRAMDef; +typedef virDomainNVRAMDef *virDomainNVRAMDefPtr; + typedef struct _virDomainSnapshotObj virDomainSnapshotObj; typedef virDomainSnapshotObj *virDomainSnapshotObjPtr;
@@ -137,6 +140,7 @@ typedef enum { VIR_DOMAIN_DEVICE_SMARTCARD, VIR_DOMAIN_DEVICE_CHR, VIR_DOMAIN_DEVICE_MEMBALLOON, + VIR_DOMAIN_DEVICE_NVRAM, VIR_DOMAIN_DEVICE_RNG,
VIR_DOMAIN_DEVICE_LAST @@ -163,6 +167,7 @@ struct _virDomainDeviceDef { virDomainSmartcardDefPtr smartcard; virDomainChrDefPtr chr; virDomainMemballoonDefPtr memballoon; + virDomainNVRAMDefPtr nvram; virDomainRNGDefPtr rng; } data; }; @@ -1469,6 +1474,9 @@ struct _virDomainMemballoonDef { virDomainDeviceInfo info; };
+struct _virDomainNVRAMDef { + virDomainDeviceInfo info; +};
enum virDomainSmbiosMode { VIR_DOMAIN_SMBIOS_NONE = 0, @@ -1916,6 +1924,7 @@ struct _virDomainDef { /* Only 1 */ virDomainWatchdogDefPtr watchdog; virDomainMemballoonDefPtr memballoon; + virDomainNVRAMDefPtr nvram; virDomainTPMDefPtr tpm; virCPUDefPtr cpu; virSysinfoDefPtr sysinfo; @@ -2076,6 +2085,7 @@ int virDomainChrSourceDefCopy(virDomainChrSourceDefPtr src, void virDomainSoundCodecDefFree(virDomainSoundCodecDefPtr def); void virDomainSoundDefFree(virDomainSoundDefPtr def); void virDomainMemballoonDefFree(virDomainMemballoonDefPtr def); +void virDomainNVRAMDefFree(virDomainNVRAMDefPtr def); void virDomainWatchdogDefFree(virDomainWatchdogDefPtr def); void virDomainVideoDefFree(virDomainVideoDefPtr def); virDomainHostdevDefPtr virDomainHostdevDefAlloc(void);
On 19/04/13 16:37, Li Zhang wrote:
From: Li Zhang <zhlcindy@linux.vnet.ibm.com>
For pSeries guest in QEMU, NVRAM is one kind of spapr-vio device. Users are allowed to specify spapr-vio devices'address. But NVRAM is not supported in libvirt. So this patch is to add NVRAM device to allow users to specify its address.
In QEMU, NVRAM device's address is specified by "-global spapr-nvram.reg=xxxxx".
In libvirt, XML file is defined as the following:
<nvram> <address type='spapr-vio' reg='0x3000'/> </nvram>
Signed-off-by: Li Zhang <zhlcindy@linux.vnet.ibm.com> --- v5 -> v4: * Add NVRAM capability suggested by Osier Yang. * Define macros for VIO device address. * Define qemuBuildNVRAMDevStr as static func. * Correct several error messages. * Add xml2xml test case
v4 -> v3: * Seperate NVRAM definition from qemu command line parser.
v3 -> v2: * Add NVRAM in domaincommon.rng and formatdomain.xml.in suggested by Daniel P.Berrange * Add NVRAM test cases suggested by Daniel P.Berrange * Remove nvram allocation when it is not specified suggested by Daniel P.Berrange * Add several error reports suggested by Daniel P.Berrange
v2 -> v1: * Add NVRAM parameters parsing in qemuParseCommandLine
docs/formatdomain.html.in | 35 +++++++++++++++++++ docs/schemas/domaincommon.rng | 10 ++++++ src/conf/domain_conf.c | 81 +++++++++++++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 10 ++++++ 4 files changed, 136 insertions(+)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 0cc56d9..4a7a66f 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -4502,6 +4502,41 @@ qemu-kvm -net nic,model=? /dev/null </dd> </dl>
+ <h4><a name="elementsNVRAM">NVRAM device</a></h4> + <p> + One NVRAM device is always added to pSeries guests on PPC64. + And on PPC64, NVRAM devices' address type are VIO which
s/are/is/, s/VIO which/VIO, which/,
+ allows users to change.<code>nvram</code> element in XML file
s/change\./change\. /,
+ is provided to specify its address. + Currently, libvirt only considers configuration for pSeries guests. + </p>
How about rewords the documents like: nvram device is always added to pSeries guest on PPC64, and its address is allowed to be changed. Element <code>nvram</code> is provided to enable the address setting.
+ <p> + Example: usage of NVRAM configuration + </p> +<pre> + ... + <devices> + <nvram> + <address type='spapr-vio' reg='0x3000'/> + </nvram> + </devices> + ... +</pre> + <dl> + <dt><code>spapr-vio</code></dt> + <dd> + <p> + VIO device address type, this is only for PPC64.
s/this is only for/only valid for/,
+ </p> + </dd> + <dt><code>reg</code></dt> + <dd> + <p> + Devices' address
s/Devices'/Device's/
+ </p> + </dd> + </dl> + <h3><a name="seclabel">Security label</a></h3>
<p> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 3976b82..86ef540 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -2793,6 +2793,13 @@ </optional> </element> </define> + <define name="nvram"> + <element name="nvram"> + <optional> + <ref name="address"/> + </optional> + </element> + </define> <define name="memballoon"> <element name="memballoon"> <attribute name="model"> @@ -3280,6 +3287,9 @@ <optional> <ref name="memballoon"/> </optional> + <optional> + <ref name="nvram"/> + </optional> </interleave> </element> </define> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 1643f30..acaec20 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -194,6 +194,7 @@ VIR_ENUM_IMPL(virDomainDevice, VIR_DOMAIN_DEVICE_LAST, "smartcard", "chr", "memballoon", + "nvram", "rng")
VIR_ENUM_IMPL(virDomainDeviceAddress, VIR_DOMAIN_DEVICE_ADDRESS_TYPE_LAST, @@ -1560,6 +1561,16 @@ void virDomainMemballoonDefFree(virDomainMemballoonDefPtr def) VIR_FREE(def); }
+void virDomainNVRAMDefFree(virDomainNVRAMDefPtr def) +{ + if (!def) + return; + + virDomainDeviceInfoClear(&def->info); + + VIR_FREE(def); +} + void virDomainWatchdogDefFree(virDomainWatchdogDefPtr def) { if (!def) @@ -1743,6 +1754,7 @@ void virDomainDeviceDefFree(virDomainDeviceDefPtr def) case VIR_DOMAIN_DEVICE_SMARTCARD: case VIR_DOMAIN_DEVICE_CHR: case VIR_DOMAIN_DEVICE_MEMBALLOON: + case VIR_DOMAIN_DEVICE_NVRAM:
Any special reason to not use virDomainNVRAMDefFree here? and same question for other device types.
case VIR_DOMAIN_DEVICE_LAST: break; } @@ -1951,6 +1963,7 @@ void virDomainDefFree(virDomainDefPtr def) virDomainWatchdogDefFree(def->watchdog);
virDomainMemballoonDefFree(def->memballoon); + virDomainNVRAMDefFree(def->nvram);
for (i = 0; i < def->nseclabels; i++) virSecurityLabelDefFree(def->seclabels[i]); @@ -2519,6 +2532,12 @@ virDomainDeviceInfoIterateInternal(virDomainDefPtr def, if (cb(def, &device, &def->rng->info, opaque) < 0) return -1; } + if (def->nvram) { + device.type = VIR_DOMAIN_DEVICE_NVRAM; + device.data.nvram = def->nvram; + if (cb(def, &device, &def->nvram->info, opaque) < 0) + return -1; + } device.type = VIR_DOMAIN_DEVICE_HUB; for (i = 0; i < def->nhubs ; i++) { device.data.hub = def->hubs[i]; @@ -2550,6 +2569,7 @@ virDomainDeviceInfoIterateInternal(virDomainDefPtr def, case VIR_DOMAIN_DEVICE_SMARTCARD: case VIR_DOMAIN_DEVICE_CHR: case VIR_DOMAIN_DEVICE_MEMBALLOON: + case VIR_DOMAIN_DEVICE_NVRAM: case VIR_DOMAIN_DEVICE_LAST: case VIR_DOMAIN_DEVICE_RNG: break; @@ -8130,6 +8150,28 @@ error: goto cleanup; }
+static virDomainNVRAMDefPtr +virDomainNVRAMDefParseXML(const xmlNodePtr node, + unsigned int flags) +{ + virDomainNVRAMDefPtr def; + + if (VIR_ALLOC(def) < 0) { + virReportOOMError(); + return NULL; + } + + if (virDomainDeviceInfoParseXML(node, NULL, &def->info, flags) < 0) + goto error; + + return def; + +error: + virDomainNVRAMDefFree(def); + def = NULL; + return def;
"return NULL;" is enough.
+} + static virSysinfoDefPtr virSysinfoParseXML(const xmlNodePtr node, xmlXPathContextPtr ctxt) @@ -11304,6 +11346,26 @@ virDomainDefParseXML(xmlDocPtr xml, } VIR_FREE(nodes);
+ def->nvram = NULL; + if ((n = virXPathNodeSet("./devices/nvram", ctxt, &nodes)) < 0) { + goto error; + } + + if (n > 1) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("only a single nvram device is supported")); + goto error; + } + + if (n > 0) { } else if (n == 1) { + virDomainNVRAMDefPtr nvram = + virDomainNVRAMDefParseXML(nodes[0], flags); + if (!nvram) + goto error; + def->nvram = nvram; + VIR_FREE(nodes); + } + /* analysis of the hub devices */ if ((n = virXPathNodeSet("./devices/hub", ctxt, &nodes)) < 0) { goto error; @@ -14391,6 +14453,21 @@ virDomainMemballoonDefFormat(virBufferPtr buf, }
static int +virDomainNVRAMDefFormat(virBufferPtr buf, + virDomainNVRAMDefPtr def, + unsigned int flags) +{ + virBufferAddLit(buf, " <nvram>\n"); + if (virDomainDeviceInfoIsSet(&def->info, flags) && + virDomainDeviceInfoFormat(buf, &def->info, flags) < 0) + return -1; + + virBufferAddLit(buf, " </nvram>\n"); + + return 0; +} + +static int virDomainSysinfoDefFormat(virBufferPtr buf, virSysinfoDefPtr def) { @@ -15720,6 +15797,9 @@ virDomainDefFormatInternal(virDomainDefPtr def, if (def->rng) virDomainRNGDefFormat(buf, def->rng, flags);
+ if (def->nvram) + virDomainNVRAMDefFormat(buf, def->nvram, flags); + virBufferAddLit(buf, " </devices>\n");
virBufferAdjustIndent(buf, 2); @@ -17002,6 +17082,7 @@ virDomainDeviceDefCopy(virDomainDeviceDefPtr src, case VIR_DOMAIN_DEVICE_SMARTCARD: case VIR_DOMAIN_DEVICE_CHR: case VIR_DOMAIN_DEVICE_MEMBALLOON: + case VIR_DOMAIN_DEVICE_NVRAM: case VIR_DOMAIN_DEVICE_LAST: virReportError(VIR_ERR_INTERNAL_ERROR, _("Copying definition of '%d' type " diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index f1f01fa..502629a 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -109,6 +109,9 @@ typedef virDomainChrDef *virDomainChrDefPtr; typedef struct _virDomainMemballoonDef virDomainMemballoonDef; typedef virDomainMemballoonDef *virDomainMemballoonDefPtr;
+typedef struct _virDomainNVRAMDef virDomainNVRAMDef; +typedef virDomainNVRAMDef *virDomainNVRAMDefPtr; + typedef struct _virDomainSnapshotObj virDomainSnapshotObj; typedef virDomainSnapshotObj *virDomainSnapshotObjPtr;
@@ -137,6 +140,7 @@ typedef enum { VIR_DOMAIN_DEVICE_SMARTCARD, VIR_DOMAIN_DEVICE_CHR, VIR_DOMAIN_DEVICE_MEMBALLOON, + VIR_DOMAIN_DEVICE_NVRAM, VIR_DOMAIN_DEVICE_RNG,
VIR_DOMAIN_DEVICE_LAST @@ -163,6 +167,7 @@ struct _virDomainDeviceDef { virDomainSmartcardDefPtr smartcard; virDomainChrDefPtr chr; virDomainMemballoonDefPtr memballoon; + virDomainNVRAMDefPtr nvram; virDomainRNGDefPtr rng; } data; }; @@ -1469,6 +1474,9 @@ struct _virDomainMemballoonDef { virDomainDeviceInfo info; };
+struct _virDomainNVRAMDef { + virDomainDeviceInfo info; +};
enum virDomainSmbiosMode { VIR_DOMAIN_SMBIOS_NONE = 0, @@ -1916,6 +1924,7 @@ struct _virDomainDef { /* Only 1 */ virDomainWatchdogDefPtr watchdog; virDomainMemballoonDefPtr memballoon; + virDomainNVRAMDefPtr nvram; virDomainTPMDefPtr tpm; virCPUDefPtr cpu; virSysinfoDefPtr sysinfo; @@ -2076,6 +2085,7 @@ int virDomainChrSourceDefCopy(virDomainChrSourceDefPtr src, void virDomainSoundCodecDefFree(virDomainSoundCodecDefPtr def); void virDomainSoundDefFree(virDomainSoundDefPtr def); void virDomainMemballoonDefFree(virDomainMemballoonDefPtr def); +void virDomainNVRAMDefFree(virDomainNVRAMDefPtr def); void virDomainWatchdogDefFree(virDomainWatchdogDefPtr def); void virDomainVideoDefFree(virDomainVideoDefPtr def); virDomainHostdevDefPtr virDomainHostdevDefAlloc(void);
ACK with the nits fixed, and if a good answer for the question.
On 24/04/13 19:58, Osier Yang wrote:
On 19/04/13 16:37, Li Zhang wrote:
From: Li Zhang <zhlcindy@linux.vnet.ibm.com>
For pSeries guest in QEMU, NVRAM is one kind of spapr-vio device. Users are allowed to specify spapr-vio devices'address. But NVRAM is not supported in libvirt. So this patch is to add NVRAM device to allow users to specify its address.
In QEMU, NVRAM device's address is specified by "-global spapr-nvram.reg=xxxxx".
In libvirt, XML file is defined as the following:
<nvram> <address type='spapr-vio' reg='0x3000'/> </nvram>
Signed-off-by: Li Zhang <zhlcindy@linux.vnet.ibm.com> --- v5 -> v4: * Add NVRAM capability suggested by Osier Yang. * Define macros for VIO device address. * Define qemuBuildNVRAMDevStr as static func. * Correct several error messages. * Add xml2xml test case
v4 -> v3: * Seperate NVRAM definition from qemu command line parser.
v3 -> v2: * Add NVRAM in domaincommon.rng and formatdomain.xml.in suggested by Daniel P.Berrange * Add NVRAM test cases suggested by Daniel P.Berrange * Remove nvram allocation when it is not specified suggested by Daniel P.Berrange * Add several error reports suggested by Daniel P.Berrange
v2 -> v1: * Add NVRAM parameters parsing in qemuParseCommandLine
docs/formatdomain.html.in | 35 +++++++++++++++++++ docs/schemas/domaincommon.rng | 10 ++++++ src/conf/domain_conf.c | 81 +++++++++++++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 10 ++++++ 4 files changed, 136 insertions(+)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 0cc56d9..4a7a66f 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -4502,6 +4502,41 @@ qemu-kvm -net nic,model=? /dev/null </dd> </dl> + <h4><a name="elementsNVRAM">NVRAM device</a></h4> + <p> + One NVRAM device is always added to pSeries guests on PPC64. + And on PPC64, NVRAM devices' address type are VIO which
s/are/is/, s/VIO which/VIO, which/,
+ allows users to change.<code>nvram</code> element in XML file
s/change\./change\. /,
+ is provided to specify its address. + Currently, libvirt only considers configuration for pSeries guests. + </p>
How about rewords the documents like:
nvram device is always added to pSeries guest on PPC64, and its address is allowed to be changed. Element <code>nvram</code> is provided to enable the address setting.
+ <p> + Example: usage of NVRAM configuration + </p> +<pre> + ... + <devices> + <nvram> + <address type='spapr-vio' reg='0x3000'/> + </nvram> + </devices> + ... +</pre> + <dl> + <dt><code>spapr-vio</code></dt> + <dd> + <p> + VIO device address type, this is only for PPC64.
s/this is only for/only valid for/,
+ </p> + </dd> + <dt><code>reg</code></dt> + <dd> + <p> + Devices' address
s/Devices'/Device's/
+ </p> + </dd> + </dl> + <h3><a name="seclabel">Security label</a></h3> <p> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 3976b82..86ef540 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -2793,6 +2793,13 @@ </optional> </element> </define> + <define name="nvram"> + <element name="nvram"> + <optional> + <ref name="address"/> + </optional> + </element> + </define> <define name="memballoon"> <element name="memballoon"> <attribute name="model"> @@ -3280,6 +3287,9 @@ <optional> <ref name="memballoon"/> </optional> + <optional> + <ref name="nvram"/> + </optional> </interleave> </element> </define> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 1643f30..acaec20 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -194,6 +194,7 @@ VIR_ENUM_IMPL(virDomainDevice, VIR_DOMAIN_DEVICE_LAST, "smartcard", "chr", "memballoon", + "nvram", "rng") VIR_ENUM_IMPL(virDomainDeviceAddress, VIR_DOMAIN_DEVICE_ADDRESS_TYPE_LAST, @@ -1560,6 +1561,16 @@ void virDomainMemballoonDefFree(virDomainMemballoonDefPtr def) VIR_FREE(def); } +void virDomainNVRAMDefFree(virDomainNVRAMDefPtr def) +{ + if (!def) + return; + + virDomainDeviceInfoClear(&def->info); + + VIR_FREE(def); +} + void virDomainWatchdogDefFree(virDomainWatchdogDefPtr def) { if (!def) @@ -1743,6 +1754,7 @@ void virDomainDeviceDefFree(virDomainDeviceDefPtr def) case VIR_DOMAIN_DEVICE_SMARTCARD: case VIR_DOMAIN_DEVICE_CHR: case VIR_DOMAIN_DEVICE_MEMBALLOON: + case VIR_DOMAIN_DEVICE_NVRAM:
Any special reason to not use virDomainNVRAMDefFree here? and same question for other device types.
case VIR_DOMAIN_DEVICE_LAST: break; } @@ -1951,6 +1963,7 @@ void virDomainDefFree(virDomainDefPtr def) virDomainWatchdogDefFree(def->watchdog); virDomainMemballoonDefFree(def->memballoon); + virDomainNVRAMDefFree(def->nvram); for (i = 0; i < def->nseclabels; i++) virSecurityLabelDefFree(def->seclabels[i]); @@ -2519,6 +2532,12 @@ virDomainDeviceInfoIterateInternal(virDomainDefPtr def, if (cb(def, &device, &def->rng->info, opaque) < 0) return -1; } + if (def->nvram) { + device.type = VIR_DOMAIN_DEVICE_NVRAM; + device.data.nvram = def->nvram; + if (cb(def, &device, &def->nvram->info, opaque) < 0) + return -1; + } device.type = VIR_DOMAIN_DEVICE_HUB; for (i = 0; i < def->nhubs ; i++) { device.data.hub = def->hubs[i]; @@ -2550,6 +2569,7 @@ virDomainDeviceInfoIterateInternal(virDomainDefPtr def, case VIR_DOMAIN_DEVICE_SMARTCARD: case VIR_DOMAIN_DEVICE_CHR: case VIR_DOMAIN_DEVICE_MEMBALLOON: + case VIR_DOMAIN_DEVICE_NVRAM: case VIR_DOMAIN_DEVICE_LAST: case VIR_DOMAIN_DEVICE_RNG: break; @@ -8130,6 +8150,28 @@ error: goto cleanup; } +static virDomainNVRAMDefPtr +virDomainNVRAMDefParseXML(const xmlNodePtr node, + unsigned int flags) +{ + virDomainNVRAMDefPtr def; + + if (VIR_ALLOC(def) < 0) { + virReportOOMError(); + return NULL; + } + + if (virDomainDeviceInfoParseXML(node, NULL, &def->info, flags) < 0) + goto error; + + return def; + +error: + virDomainNVRAMDefFree(def); + def = NULL; + return def;
"return NULL;" is enough.
+} + static virSysinfoDefPtr virSysinfoParseXML(const xmlNodePtr node, xmlXPathContextPtr ctxt) @@ -11304,6 +11346,26 @@ virDomainDefParseXML(xmlDocPtr xml, } VIR_FREE(nodes); + def->nvram = NULL; + if ((n = virXPathNodeSet("./devices/nvram", ctxt, &nodes)) < 0) { + goto error; + } + + if (n > 1) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("only a single nvram device is supported")); + goto error; + } + + if (n > 0) { } else if (n == 1) { + virDomainNVRAMDefPtr nvram = + virDomainNVRAMDefParseXML(nodes[0], flags); + if (!nvram) + goto error; + def->nvram = nvram; + VIR_FREE(nodes); + } + /* analysis of the hub devices */ if ((n = virXPathNodeSet("./devices/hub", ctxt, &nodes)) < 0) { goto error; @@ -14391,6 +14453,21 @@ virDomainMemballoonDefFormat(virBufferPtr buf, } static int +virDomainNVRAMDefFormat(virBufferPtr buf, + virDomainNVRAMDefPtr def, + unsigned int flags) +{ + virBufferAddLit(buf, " <nvram>\n"); + if (virDomainDeviceInfoIsSet(&def->info, flags) && + virDomainDeviceInfoFormat(buf, &def->info, flags) < 0) + return -1; + + virBufferAddLit(buf, " </nvram>\n"); + + return 0; +} + +static int virDomainSysinfoDefFormat(virBufferPtr buf, virSysinfoDefPtr def) { @@ -15720,6 +15797,9 @@ virDomainDefFormatInternal(virDomainDefPtr def, if (def->rng) virDomainRNGDefFormat(buf, def->rng, flags); + if (def->nvram) + virDomainNVRAMDefFormat(buf, def->nvram, flags); + virBufferAddLit(buf, " </devices>\n"); virBufferAdjustIndent(buf, 2); @@ -17002,6 +17082,7 @@ virDomainDeviceDefCopy(virDomainDeviceDefPtr src, case VIR_DOMAIN_DEVICE_SMARTCARD: case VIR_DOMAIN_DEVICE_CHR: case VIR_DOMAIN_DEVICE_MEMBALLOON: + case VIR_DOMAIN_DEVICE_NVRAM: case VIR_DOMAIN_DEVICE_LAST: virReportError(VIR_ERR_INTERNAL_ERROR, _("Copying definition of '%d' type " diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index f1f01fa..502629a 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -109,6 +109,9 @@ typedef virDomainChrDef *virDomainChrDefPtr; typedef struct _virDomainMemballoonDef virDomainMemballoonDef; typedef virDomainMemballoonDef *virDomainMemballoonDefPtr; +typedef struct _virDomainNVRAMDef virDomainNVRAMDef; +typedef virDomainNVRAMDef *virDomainNVRAMDefPtr; + typedef struct _virDomainSnapshotObj virDomainSnapshotObj; typedef virDomainSnapshotObj *virDomainSnapshotObjPtr; @@ -137,6 +140,7 @@ typedef enum { VIR_DOMAIN_DEVICE_SMARTCARD, VIR_DOMAIN_DEVICE_CHR, VIR_DOMAIN_DEVICE_MEMBALLOON, + VIR_DOMAIN_DEVICE_NVRAM, VIR_DOMAIN_DEVICE_RNG, VIR_DOMAIN_DEVICE_LAST @@ -163,6 +167,7 @@ struct _virDomainDeviceDef { virDomainSmartcardDefPtr smartcard; virDomainChrDefPtr chr; virDomainMemballoonDefPtr memballoon; + virDomainNVRAMDefPtr nvram; virDomainRNGDefPtr rng; } data; }; @@ -1469,6 +1474,9 @@ struct _virDomainMemballoonDef { virDomainDeviceInfo info; }; +struct _virDomainNVRAMDef { + virDomainDeviceInfo info; +}; enum virDomainSmbiosMode { VIR_DOMAIN_SMBIOS_NONE = 0, @@ -1916,6 +1924,7 @@ struct _virDomainDef { /* Only 1 */ virDomainWatchdogDefPtr watchdog; virDomainMemballoonDefPtr memballoon; + virDomainNVRAMDefPtr nvram; virDomainTPMDefPtr tpm; virCPUDefPtr cpu; virSysinfoDefPtr sysinfo; @@ -2076,6 +2085,7 @@ int virDomainChrSourceDefCopy(virDomainChrSourceDefPtr src, void virDomainSoundCodecDefFree(virDomainSoundCodecDefPtr def); void virDomainSoundDefFree(virDomainSoundDefPtr def); void virDomainMemballoonDefFree(virDomainMemballoonDefPtr def); +void virDomainNVRAMDefFree(virDomainNVRAMDefPtr def); void virDomainWatchdogDefFree(virDomainWatchdogDefPtr def); void virDomainVideoDefFree(virDomainVideoDefPtr def); virDomainHostdevDefPtr virDomainHostdevDefAlloc(void);
ACK with the nits fixed, and if a good answer for the question.
I'm going to push the patch with the attached diff squashed in. For the "question", we can have a later patch.
On 2013年04月24日 19:58, Osier Yang wrote:
On 19/04/13 16:37, Li Zhang wrote:
From: Li Zhang <zhlcindy@linux.vnet.ibm.com>
For pSeries guest in QEMU, NVRAM is one kind of spapr-vio device. Users are allowed to specify spapr-vio devices'address. But NVRAM is not supported in libvirt. So this patch is to add NVRAM device to allow users to specify its address.
In QEMU, NVRAM device's address is specified by "-global spapr-nvram.reg=xxxxx".
In libvirt, XML file is defined as the following:
<nvram> <address type='spapr-vio' reg='0x3000'/> </nvram>
Signed-off-by: Li Zhang <zhlcindy@linux.vnet.ibm.com> --- v5 -> v4: * Add NVRAM capability suggested by Osier Yang. * Define macros for VIO device address. * Define qemuBuildNVRAMDevStr as static func. * Correct several error messages. * Add xml2xml test case
v4 -> v3: * Seperate NVRAM definition from qemu command line parser.
v3 -> v2: * Add NVRAM in domaincommon.rng and formatdomain.xml.in suggested by Daniel P.Berrange * Add NVRAM test cases suggested by Daniel P.Berrange * Remove nvram allocation when it is not specified suggested by Daniel P.Berrange * Add several error reports suggested by Daniel P.Berrange
v2 -> v1: * Add NVRAM parameters parsing in qemuParseCommandLine
docs/formatdomain.html.in | 35 +++++++++++++++++++ docs/schemas/domaincommon.rng | 10 ++++++ src/conf/domain_conf.c | 81 +++++++++++++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 10 ++++++ 4 files changed, 136 insertions(+)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 0cc56d9..4a7a66f 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -4502,6 +4502,41 @@ qemu-kvm -net nic,model=? /dev/null </dd> </dl> + <h4><a name="elementsNVRAM">NVRAM device</a></h4> + <p> + One NVRAM device is always added to pSeries guests on PPC64. + And on PPC64, NVRAM devices' address type are VIO which
s/are/is/, s/VIO which/VIO, which/,
+ allows users to change.<code>nvram</code> element in XML file
s/change\./change\. /,
+ is provided to specify its address. + Currently, libvirt only considers configuration for pSeries guests. + </p>
How about rewords the documents like:
nvram device is always added to pSeries guest on PPC64, and its address is allowed to be changed. Element <code>nvram</code> is provided to enable the address setting.
It's better. Thanks.
+ <p> + Example: usage of NVRAM configuration + </p> +<pre> + ... + <devices> + <nvram> + <address type='spapr-vio' reg='0x3000'/> + </nvram> + </devices> + ... +</pre> + <dl> + <dt><code>spapr-vio</code></dt> + <dd> + <p> + VIO device address type, this is only for PPC64.
s/this is only for/only valid for/,
+ </p> + </dd> + <dt><code>reg</code></dt> + <dd> + <p> + Devices' address
s/Devices'/Device's/
+ </p> + </dd> + </dl> + <h3><a name="seclabel">Security label</a></h3> <p> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 3976b82..86ef540 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -2793,6 +2793,13 @@ </optional> </element> </define> + <define name="nvram"> + <element name="nvram"> + <optional> + <ref name="address"/> + </optional> + </element> + </define> <define name="memballoon"> <element name="memballoon"> <attribute name="model"> @@ -3280,6 +3287,9 @@ <optional> <ref name="memballoon"/> </optional> + <optional> + <ref name="nvram"/> + </optional> </interleave> </element> </define> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 1643f30..acaec20 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -194,6 +194,7 @@ VIR_ENUM_IMPL(virDomainDevice, VIR_DOMAIN_DEVICE_LAST, "smartcard", "chr", "memballoon", + "nvram", "rng") VIR_ENUM_IMPL(virDomainDeviceAddress, VIR_DOMAIN_DEVICE_ADDRESS_TYPE_LAST, @@ -1560,6 +1561,16 @@ void virDomainMemballoonDefFree(virDomainMemballoonDefPtr def) VIR_FREE(def); } +void virDomainNVRAMDefFree(virDomainNVRAMDefPtr def) +{ + if (!def) + return; + + virDomainDeviceInfoClear(&def->info); + + VIR_FREE(def); +} + void virDomainWatchdogDefFree(virDomainWatchdogDefPtr def) { if (!def) @@ -1743,6 +1754,7 @@ void virDomainDeviceDefFree(virDomainDeviceDefPtr def) case VIR_DOMAIN_DEVICE_SMARTCARD: case VIR_DOMAIN_DEVICE_CHR: case VIR_DOMAIN_DEVICE_MEMBALLOON: + case VIR_DOMAIN_DEVICE_NVRAM:
Any special reason to not use virDomainNVRAMDefFree here? and same question for other device types.
It's freed when domain def is freed. I think it is created when the domain is defined, so it is freed when the domain is freed. But I am not sure whether it's same with other device types. :)
case VIR_DOMAIN_DEVICE_LAST: break; } @@ -1951,6 +1963,7 @@ void virDomainDefFree(virDomainDefPtr def) virDomainWatchdogDefFree(def->watchdog); virDomainMemballoonDefFree(def->memballoon); + virDomainNVRAMDefFree(def->nvram); for (i = 0; i < def->nseclabels; i++) virSecurityLabelDefFree(def->seclabels[i]); @@ -2519,6 +2532,12 @@ virDomainDeviceInfoIterateInternal(virDomainDefPtr def, if (cb(def, &device, &def->rng->info, opaque) < 0) return -1; } + if (def->nvram) { + device.type = VIR_DOMAIN_DEVICE_NVRAM; + device.data.nvram = def->nvram; + if (cb(def, &device, &def->nvram->info, opaque) < 0) + return -1; + } device.type = VIR_DOMAIN_DEVICE_HUB; for (i = 0; i < def->nhubs ; i++) { device.data.hub = def->hubs[i]; @@ -2550,6 +2569,7 @@ virDomainDeviceInfoIterateInternal(virDomainDefPtr def, case VIR_DOMAIN_DEVICE_SMARTCARD: case VIR_DOMAIN_DEVICE_CHR: case VIR_DOMAIN_DEVICE_MEMBALLOON: + case VIR_DOMAIN_DEVICE_NVRAM: case VIR_DOMAIN_DEVICE_LAST: case VIR_DOMAIN_DEVICE_RNG: break; @@ -8130,6 +8150,28 @@ error: goto cleanup; } +static virDomainNVRAMDefPtr +virDomainNVRAMDefParseXML(const xmlNodePtr node, + unsigned int flags) +{ + virDomainNVRAMDefPtr def; + + if (VIR_ALLOC(def) < 0) { + virReportOOMError(); + return NULL; + } + + if (virDomainDeviceInfoParseXML(node, NULL, &def->info, flags) < 0) + goto error; + + return def; + +error: + virDomainNVRAMDefFree(def); + def = NULL; + return def; "return NULL;" is enough. +} + static virSysinfoDefPtr virSysinfoParseXML(const xmlNodePtr node, xmlXPathContextPtr ctxt) @@ -11304,6 +11346,26 @@ virDomainDefParseXML(xmlDocPtr xml, } VIR_FREE(nodes); + def->nvram = NULL; + if ((n = virXPathNodeSet("./devices/nvram", ctxt, &nodes)) < 0) { + goto error; + } + + if (n > 1) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("only a single nvram device is supported")); + goto error; + } + + if (n > 0) { } else if (n == 1) { + virDomainNVRAMDefPtr nvram = + virDomainNVRAMDefParseXML(nodes[0], flags); + if (!nvram) + goto error; + def->nvram = nvram; + VIR_FREE(nodes); + } + /* analysis of the hub devices */ if ((n = virXPathNodeSet("./devices/hub", ctxt, &nodes)) < 0) { goto error; @@ -14391,6 +14453,21 @@ virDomainMemballoonDefFormat(virBufferPtr buf, } static int +virDomainNVRAMDefFormat(virBufferPtr buf, + virDomainNVRAMDefPtr def, + unsigned int flags) +{ + virBufferAddLit(buf, " <nvram>\n"); + if (virDomainDeviceInfoIsSet(&def->info, flags) && + virDomainDeviceInfoFormat(buf, &def->info, flags) < 0) + return -1; + + virBufferAddLit(buf, " </nvram>\n"); + + return 0; +} + +static int virDomainSysinfoDefFormat(virBufferPtr buf, virSysinfoDefPtr def) { @@ -15720,6 +15797,9 @@ virDomainDefFormatInternal(virDomainDefPtr def, if (def->rng) virDomainRNGDefFormat(buf, def->rng, flags); + if (def->nvram) + virDomainNVRAMDefFormat(buf, def->nvram, flags); + virBufferAddLit(buf, " </devices>\n"); virBufferAdjustIndent(buf, 2); @@ -17002,6 +17082,7 @@ virDomainDeviceDefCopy(virDomainDeviceDefPtr src, case VIR_DOMAIN_DEVICE_SMARTCARD: case VIR_DOMAIN_DEVICE_CHR: case VIR_DOMAIN_DEVICE_MEMBALLOON: + case VIR_DOMAIN_DEVICE_NVRAM: case VIR_DOMAIN_DEVICE_LAST: virReportError(VIR_ERR_INTERNAL_ERROR, _("Copying definition of '%d' type " diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index f1f01fa..502629a 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -109,6 +109,9 @@ typedef virDomainChrDef *virDomainChrDefPtr; typedef struct _virDomainMemballoonDef virDomainMemballoonDef; typedef virDomainMemballoonDef *virDomainMemballoonDefPtr; +typedef struct _virDomainNVRAMDef virDomainNVRAMDef; +typedef virDomainNVRAMDef *virDomainNVRAMDefPtr; + typedef struct _virDomainSnapshotObj virDomainSnapshotObj; typedef virDomainSnapshotObj *virDomainSnapshotObjPtr; @@ -137,6 +140,7 @@ typedef enum { VIR_DOMAIN_DEVICE_SMARTCARD, VIR_DOMAIN_DEVICE_CHR, VIR_DOMAIN_DEVICE_MEMBALLOON, + VIR_DOMAIN_DEVICE_NVRAM, VIR_DOMAIN_DEVICE_RNG, VIR_DOMAIN_DEVICE_LAST @@ -163,6 +167,7 @@ struct _virDomainDeviceDef { virDomainSmartcardDefPtr smartcard; virDomainChrDefPtr chr; virDomainMemballoonDefPtr memballoon; + virDomainNVRAMDefPtr nvram; virDomainRNGDefPtr rng; } data; }; @@ -1469,6 +1474,9 @@ struct _virDomainMemballoonDef { virDomainDeviceInfo info; }; +struct _virDomainNVRAMDef { + virDomainDeviceInfo info; +}; enum virDomainSmbiosMode { VIR_DOMAIN_SMBIOS_NONE = 0, @@ -1916,6 +1924,7 @@ struct _virDomainDef { /* Only 1 */ virDomainWatchdogDefPtr watchdog; virDomainMemballoonDefPtr memballoon; + virDomainNVRAMDefPtr nvram; virDomainTPMDefPtr tpm; virCPUDefPtr cpu; virSysinfoDefPtr sysinfo; @@ -2076,6 +2085,7 @@ int virDomainChrSourceDefCopy(virDomainChrSourceDefPtr src, void virDomainSoundCodecDefFree(virDomainSoundCodecDefPtr def); void virDomainSoundDefFree(virDomainSoundDefPtr def); void virDomainMemballoonDefFree(virDomainMemballoonDefPtr def); +void virDomainNVRAMDefFree(virDomainNVRAMDefPtr def); void virDomainWatchdogDefFree(virDomainWatchdogDefPtr def); void virDomainVideoDefFree(virDomainVideoDefPtr def); virDomainHostdevDefPtr virDomainHostdevDefAlloc(void);
ACK with the nits fixed, and if a good answer for the question.
On 2013年04月24日 19:58, Osier Yang wrote:
On 19/04/13 16:37, Li Zhang wrote:
From: Li Zhang <zhlcindy@linux.vnet.ibm.com>
For pSeries guest in QEMU, NVRAM is one kind of spapr-vio device. Users are allowed to specify spapr-vio devices'address. But NVRAM is not supported in libvirt. So this patch is to add NVRAM device to allow users to specify its address.
In QEMU, NVRAM device's address is specified by "-global spapr-nvram.reg=xxxxx".
In libvirt, XML file is defined as the following:
<nvram> <address type='spapr-vio' reg='0x3000'/> </nvram>
Signed-off-by: Li Zhang <zhlcindy@linux.vnet.ibm.com> --- v5 -> v4: * Add NVRAM capability suggested by Osier Yang. * Define macros for VIO device address. * Define qemuBuildNVRAMDevStr as static func. * Correct several error messages. * Add xml2xml test case
v4 -> v3: * Seperate NVRAM definition from qemu command line parser.
v3 -> v2: * Add NVRAM in domaincommon.rng and formatdomain.xml.in suggested by Daniel P.Berrange * Add NVRAM test cases suggested by Daniel P.Berrange * Remove nvram allocation when it is not specified suggested by Daniel P.Berrange * Add several error reports suggested by Daniel P.Berrange
v2 -> v1: * Add NVRAM parameters parsing in qemuParseCommandLine
docs/formatdomain.html.in | 35 +++++++++++++++++++ docs/schemas/domaincommon.rng | 10 ++++++ src/conf/domain_conf.c | 81 +++++++++++++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 10 ++++++ 4 files changed, 136 insertions(+)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 0cc56d9..4a7a66f 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -4502,6 +4502,41 @@ qemu-kvm -net nic,model=? /dev/null </dd> </dl> + <h4><a name="elementsNVRAM">NVRAM device</a></h4> + <p> + One NVRAM device is always added to pSeries guests on PPC64. + And on PPC64, NVRAM devices' address type are VIO which
s/are/is/, s/VIO which/VIO, which/,
+ allows users to change.<code>nvram</code> element in XML file
s/change\./change\. /,
+ is provided to specify its address. + Currently, libvirt only considers configuration for pSeries guests. + </p>
How about rewords the documents like:
nvram device is always added to pSeries guest on PPC64, and its address is allowed to be changed. Element <code>nvram</code> is provided to enable the address setting.
It's better. Thanks.
+ <p> + Example: usage of NVRAM configuration + </p> +<pre> + ... + <devices> + <nvram> + <address type='spapr-vio' reg='0x3000'/> + </nvram> + </devices> + ... +</pre> + <dl> + <dt><code>spapr-vio</code></dt> + <dd> + <p> + VIO device address type, this is only for PPC64.
s/this is only for/only valid for/,
+ </p> + </dd> + <dt><code>reg</code></dt> + <dd> + <p> + Devices' address
s/Devices'/Device's/
+ </p> + </dd> + </dl> + <h3><a name="seclabel">Security label</a></h3> <p> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 3976b82..86ef540 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -2793,6 +2793,13 @@ </optional> </element> </define> + <define name="nvram"> + <element name="nvram"> + <optional> + <ref name="address"/> + </optional> + </element> + </define> <define name="memballoon"> <element name="memballoon"> <attribute name="model"> @@ -3280,6 +3287,9 @@ <optional> <ref name="memballoon"/> </optional> + <optional> + <ref name="nvram"/> + </optional> </interleave> </element> </define> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 1643f30..acaec20 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -194,6 +194,7 @@ VIR_ENUM_IMPL(virDomainDevice, VIR_DOMAIN_DEVICE_LAST, "smartcard", "chr", "memballoon", + "nvram", "rng") VIR_ENUM_IMPL(virDomainDeviceAddress, VIR_DOMAIN_DEVICE_ADDRESS_TYPE_LAST, @@ -1560,6 +1561,16 @@ void virDomainMemballoonDefFree(virDomainMemballoonDefPtr def) VIR_FREE(def); } +void virDomainNVRAMDefFree(virDomainNVRAMDefPtr def) +{ + if (!def) + return; + + virDomainDeviceInfoClear(&def->info); + + VIR_FREE(def); +} + void virDomainWatchdogDefFree(virDomainWatchdogDefPtr def) { if (!def) @@ -1743,6 +1754,7 @@ void virDomainDeviceDefFree(virDomainDeviceDefPtr def) case VIR_DOMAIN_DEVICE_SMARTCARD: case VIR_DOMAIN_DEVICE_CHR: case VIR_DOMAIN_DEVICE_MEMBALLOON: + case VIR_DOMAIN_DEVICE_NVRAM:
Any special reason to not use virDomainNVRAMDefFree here? and same question for other device types.
It's freed when domain def is freed. I think it is created when the domain is defined, so it is freed when the domain is freed.
But I am not sure whether it's same with other device types. :) No, you misunderstood, virDomainDeviceDefFree is a wrapper of the all
On 25/04/13 10:13, Li Zhang wrote: the free helpers of each device type. Itself can be used as helper in the source too. I see you add virDomainNVRAMDeviceFree in virDomainDefFree, but it's different things, sometimes you will want to use virDomainDeviceDefFree to free the device def, without careing about what the device type is.. Osier
On 2013年04月25日 10:42, Osier Yang wrote:
On 2013年04月24日 19:58, Osier Yang wrote:
On 19/04/13 16:37, Li Zhang wrote:
From: Li Zhang <zhlcindy@linux.vnet.ibm.com>
For pSeries guest in QEMU, NVRAM is one kind of spapr-vio device. Users are allowed to specify spapr-vio devices'address. But NVRAM is not supported in libvirt. So this patch is to add NVRAM device to allow users to specify its address.
In QEMU, NVRAM device's address is specified by "-global spapr-nvram.reg=xxxxx".
In libvirt, XML file is defined as the following:
<nvram> <address type='spapr-vio' reg='0x3000'/> </nvram>
Signed-off-by: Li Zhang <zhlcindy@linux.vnet.ibm.com> --- v5 -> v4: * Add NVRAM capability suggested by Osier Yang. * Define macros for VIO device address. * Define qemuBuildNVRAMDevStr as static func. * Correct several error messages. * Add xml2xml test case
v4 -> v3: * Seperate NVRAM definition from qemu command line parser.
v3 -> v2: * Add NVRAM in domaincommon.rng and formatdomain.xml.in suggested by Daniel P.Berrange * Add NVRAM test cases suggested by Daniel P.Berrange * Remove nvram allocation when it is not specified suggested by Daniel P.Berrange * Add several error reports suggested by Daniel P.Berrange
v2 -> v1: * Add NVRAM parameters parsing in qemuParseCommandLine
docs/formatdomain.html.in | 35 +++++++++++++++++++ docs/schemas/domaincommon.rng | 10 ++++++ src/conf/domain_conf.c | 81 +++++++++++++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 10 ++++++ 4 files changed, 136 insertions(+)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 0cc56d9..4a7a66f 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -4502,6 +4502,41 @@ qemu-kvm -net nic,model=? /dev/null </dd> </dl> + <h4><a name="elementsNVRAM">NVRAM device</a></h4> + <p> + One NVRAM device is always added to pSeries guests on PPC64. + And on PPC64, NVRAM devices' address type are VIO which
s/are/is/, s/VIO which/VIO, which/,
+ allows users to change.<code>nvram</code> element in XML file
s/change\./change\. /,
+ is provided to specify its address. + Currently, libvirt only considers configuration for pSeries guests. + </p>
How about rewords the documents like:
nvram device is always added to pSeries guest on PPC64, and its address is allowed to be changed. Element <code>nvram</code> is provided to enable the address setting.
It's better. Thanks.
+ <p> + Example: usage of NVRAM configuration + </p> +<pre> + ... + <devices> + <nvram> + <address type='spapr-vio' reg='0x3000'/> + </nvram> + </devices> + ... +</pre> + <dl> + <dt><code>spapr-vio</code></dt> + <dd> + <p> + VIO device address type, this is only for PPC64.
s/this is only for/only valid for/,
+ </p> + </dd> + <dt><code>reg</code></dt> + <dd> + <p> + Devices' address
s/Devices'/Device's/
+ </p> + </dd> + </dl> + <h3><a name="seclabel">Security label</a></h3> <p> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 3976b82..86ef540 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -2793,6 +2793,13 @@ </optional> </element> </define> + <define name="nvram"> + <element name="nvram"> + <optional> + <ref name="address"/> + </optional> + </element> + </define> <define name="memballoon"> <element name="memballoon"> <attribute name="model"> @@ -3280,6 +3287,9 @@ <optional> <ref name="memballoon"/> </optional> + <optional> + <ref name="nvram"/> + </optional> </interleave> </element> </define> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 1643f30..acaec20 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -194,6 +194,7 @@ VIR_ENUM_IMPL(virDomainDevice, VIR_DOMAIN_DEVICE_LAST, "smartcard", "chr", "memballoon", + "nvram", "rng") VIR_ENUM_IMPL(virDomainDeviceAddress, VIR_DOMAIN_DEVICE_ADDRESS_TYPE_LAST, @@ -1560,6 +1561,16 @@ void virDomainMemballoonDefFree(virDomainMemballoonDefPtr def) VIR_FREE(def); } +void virDomainNVRAMDefFree(virDomainNVRAMDefPtr def) +{ + if (!def) + return; + + virDomainDeviceInfoClear(&def->info); + + VIR_FREE(def); +} + void virDomainWatchdogDefFree(virDomainWatchdogDefPtr def) { if (!def) @@ -1743,6 +1754,7 @@ void virDomainDeviceDefFree(virDomainDeviceDefPtr def) case VIR_DOMAIN_DEVICE_SMARTCARD: case VIR_DOMAIN_DEVICE_CHR: case VIR_DOMAIN_DEVICE_MEMBALLOON: + case VIR_DOMAIN_DEVICE_NVRAM:
Any special reason to not use virDomainNVRAMDefFree here? and same question for other device types.
It's freed when domain def is freed. I think it is created when the domain is defined, so it is freed when the domain is freed.
But I am not sure whether it's same with other device types. :) No, you misunderstood, virDomainDeviceDefFree is a wrapper of the all
On 25/04/13 10:13, Li Zhang wrote: the free helpers of each device type. Itself can be used as helper in the source too.
I see you add virDomainNVRAMDeviceFree in virDomainDefFree, but it's different things, sometimes you will want to use virDomainDeviceDefFree to free the device def, without careing about what the device type is..
Ah, I am considering when this device is freed. For example, when deleting one device from this domain, it may call this this to free this device. But actually, nvram device can't be deleted from this domain. Is it okay if freeing the device def? Logically, it seems that this should be fine. But I am not sure whether this is reasonable.
Osier
On 25/04/13 11:07, Li Zhang wrote:
On 2013年04月25日 10:42, Osier Yang wrote:
On 2013年04月24日 19:58, Osier Yang wrote:
On 19/04/13 16:37, Li Zhang wrote:
From: Li Zhang <zhlcindy@linux.vnet.ibm.com>
For pSeries guest in QEMU, NVRAM is one kind of spapr-vio device. Users are allowed to specify spapr-vio devices'address. But NVRAM is not supported in libvirt. So this patch is to add NVRAM device to allow users to specify its address.
In QEMU, NVRAM device's address is specified by "-global spapr-nvram.reg=xxxxx".
In libvirt, XML file is defined as the following:
<nvram> <address type='spapr-vio' reg='0x3000'/> </nvram>
Signed-off-by: Li Zhang <zhlcindy@linux.vnet.ibm.com> --- v5 -> v4: * Add NVRAM capability suggested by Osier Yang. * Define macros for VIO device address. * Define qemuBuildNVRAMDevStr as static func. * Correct several error messages. * Add xml2xml test case
v4 -> v3: * Seperate NVRAM definition from qemu command line parser.
v3 -> v2: * Add NVRAM in domaincommon.rng and formatdomain.xml.in suggested by Daniel P.Berrange * Add NVRAM test cases suggested by Daniel P.Berrange * Remove nvram allocation when it is not specified suggested by Daniel P.Berrange * Add several error reports suggested by Daniel P.Berrange
v2 -> v1: * Add NVRAM parameters parsing in qemuParseCommandLine
docs/formatdomain.html.in | 35 +++++++++++++++++++ docs/schemas/domaincommon.rng | 10 ++++++ src/conf/domain_conf.c | 81 +++++++++++++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 10 ++++++ 4 files changed, 136 insertions(+)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 0cc56d9..4a7a66f 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -4502,6 +4502,41 @@ qemu-kvm -net nic,model=? /dev/null </dd> </dl> + <h4><a name="elementsNVRAM">NVRAM device</a></h4> + <p> + One NVRAM device is always added to pSeries guests on PPC64. + And on PPC64, NVRAM devices' address type are VIO which
s/are/is/, s/VIO which/VIO, which/,
+ allows users to change.<code>nvram</code> element in XML file
s/change\./change\. /,
+ is provided to specify its address. + Currently, libvirt only considers configuration for pSeries guests. + </p>
How about rewords the documents like:
nvram device is always added to pSeries guest on PPC64, and its address is allowed to be changed. Element <code>nvram</code> is provided to enable the address setting.
It's better. Thanks.
+ <p> + Example: usage of NVRAM configuration + </p> +<pre> + ... + <devices> + <nvram> + <address type='spapr-vio' reg='0x3000'/> + </nvram> + </devices> + ... +</pre> + <dl> + <dt><code>spapr-vio</code></dt> + <dd> + <p> + VIO device address type, this is only for PPC64.
s/this is only for/only valid for/,
+ </p> + </dd> + <dt><code>reg</code></dt> + <dd> + <p> + Devices' address
s/Devices'/Device's/
+ </p> + </dd> + </dl> + <h3><a name="seclabel">Security label</a></h3> <p> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 3976b82..86ef540 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -2793,6 +2793,13 @@ </optional> </element> </define> + <define name="nvram"> + <element name="nvram"> + <optional> + <ref name="address"/> + </optional> + </element> + </define> <define name="memballoon"> <element name="memballoon"> <attribute name="model"> @@ -3280,6 +3287,9 @@ <optional> <ref name="memballoon"/> </optional> + <optional> + <ref name="nvram"/> + </optional> </interleave> </element> </define> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 1643f30..acaec20 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -194,6 +194,7 @@ VIR_ENUM_IMPL(virDomainDevice, VIR_DOMAIN_DEVICE_LAST, "smartcard", "chr", "memballoon", + "nvram", "rng") VIR_ENUM_IMPL(virDomainDeviceAddress, VIR_DOMAIN_DEVICE_ADDRESS_TYPE_LAST, @@ -1560,6 +1561,16 @@ void virDomainMemballoonDefFree(virDomainMemballoonDefPtr def) VIR_FREE(def); } +void virDomainNVRAMDefFree(virDomainNVRAMDefPtr def) +{ + if (!def) + return; + + virDomainDeviceInfoClear(&def->info); + + VIR_FREE(def); +} + void virDomainWatchdogDefFree(virDomainWatchdogDefPtr def) { if (!def) @@ -1743,6 +1754,7 @@ void virDomainDeviceDefFree(virDomainDeviceDefPtr def) case VIR_DOMAIN_DEVICE_SMARTCARD: case VIR_DOMAIN_DEVICE_CHR: case VIR_DOMAIN_DEVICE_MEMBALLOON: + case VIR_DOMAIN_DEVICE_NVRAM:
Any special reason to not use virDomainNVRAMDefFree here? and same question for other device types.
It's freed when domain def is freed. I think it is created when the domain is defined, so it is freed when the domain is freed.
But I am not sure whether it's same with other device types. :) No, you misunderstood, virDomainDeviceDefFree is a wrapper of the all
On 25/04/13 10:13, Li Zhang wrote: the free helpers of each device type. Itself can be used as helper in the source too.
I see you add virDomainNVRAMDeviceFree in virDomainDefFree, but it's different things, sometimes you will want to use virDomainDeviceDefFree to free the device def, without careing about what the device type is..
Ah, I am considering when this device is freed. For example, when deleting one device from this domain, it may call this this to free this device. But actually, nvram device can't be deleted from this domain.
There are other places one will need to free the device def, not only when detaching...
Is it okay if freeing the device def? Logically, it seems that this should be fine. But I am not sure whether this is reasonable.
It's fine to keep it not freed in virDomainDeviceFree, just like what you do in this patch, because same other device type, such as SMARTCARD also has the problem, we can fix it as a later patch together.
On 2013年04月25日 11:19, Osier Yang wrote:
On 25/04/13 11:07, Li Zhang wrote:
On 2013年04月25日 10:42, Osier Yang wrote:
On 25/04/13 10:13, Li Zhang wrote:
On 2013年04月24日 19:58, Osier Yang wrote:
On 19/04/13 16:37, Li Zhang wrote:
From: Li Zhang <zhlcindy@linux.vnet.ibm.com>
For pSeries guest in QEMU, NVRAM is one kind of spapr-vio device. Users are allowed to specify spapr-vio devices'address. But NVRAM is not supported in libvirt. So this patch is to add NVRAM device to allow users to specify its address.
In QEMU, NVRAM device's address is specified by "-global spapr-nvram.reg=xxxxx".
In libvirt, XML file is defined as the following:
<nvram> <address type='spapr-vio' reg='0x3000'/> </nvram>
Signed-off-by: Li Zhang <zhlcindy@linux.vnet.ibm.com> --- v5 -> v4: * Add NVRAM capability suggested by Osier Yang. * Define macros for VIO device address. * Define qemuBuildNVRAMDevStr as static func. * Correct several error messages. * Add xml2xml test case
v4 -> v3: * Seperate NVRAM definition from qemu command line parser.
v3 -> v2: * Add NVRAM in domaincommon.rng and formatdomain.xml.in suggested by Daniel P.Berrange * Add NVRAM test cases suggested by Daniel P.Berrange * Remove nvram allocation when it is not specified suggested by Daniel P.Berrange * Add several error reports suggested by Daniel P.Berrange
v2 -> v1: * Add NVRAM parameters parsing in qemuParseCommandLine
docs/formatdomain.html.in | 35 +++++++++++++++++++ docs/schemas/domaincommon.rng | 10 ++++++ src/conf/domain_conf.c | 81 +++++++++++++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 10 ++++++ 4 files changed, 136 insertions(+)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 0cc56d9..4a7a66f 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -4502,6 +4502,41 @@ qemu-kvm -net nic,model=? /dev/null </dd> </dl> + <h4><a name="elementsNVRAM">NVRAM device</a></h4> + <p> + One NVRAM device is always added to pSeries guests on PPC64. + And on PPC64, NVRAM devices' address type are VIO which
s/are/is/, s/VIO which/VIO, which/,
+ allows users to change.<code>nvram</code> element in XML file
s/change\./change\. /,
+ is provided to specify its address. + Currently, libvirt only considers configuration for pSeries guests. + </p>
How about rewords the documents like:
nvram device is always added to pSeries guest on PPC64, and its address is allowed to be changed. Element <code>nvram</code> is provided to enable the address setting.
It's better. Thanks.
+ <p> + Example: usage of NVRAM configuration + </p> +<pre> + ... + <devices> + <nvram> + <address type='spapr-vio' reg='0x3000'/> + </nvram> + </devices> + ... +</pre> + <dl> + <dt><code>spapr-vio</code></dt> + <dd> + <p> + VIO device address type, this is only for PPC64.
s/this is only for/only valid for/,
+ </p> + </dd> + <dt><code>reg</code></dt> + <dd> + <p> + Devices' address
s/Devices'/Device's/
+ </p> + </dd> + </dl> + <h3><a name="seclabel">Security label</a></h3> <p> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 3976b82..86ef540 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -2793,6 +2793,13 @@ </optional> </element> </define> + <define name="nvram"> + <element name="nvram"> + <optional> + <ref name="address"/> + </optional> + </element> + </define> <define name="memballoon"> <element name="memballoon"> <attribute name="model"> @@ -3280,6 +3287,9 @@ <optional> <ref name="memballoon"/> </optional> + <optional> + <ref name="nvram"/> + </optional> </interleave> </element> </define> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 1643f30..acaec20 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -194,6 +194,7 @@ VIR_ENUM_IMPL(virDomainDevice, VIR_DOMAIN_DEVICE_LAST, "smartcard", "chr", "memballoon", + "nvram", "rng") VIR_ENUM_IMPL(virDomainDeviceAddress, VIR_DOMAIN_DEVICE_ADDRESS_TYPE_LAST, @@ -1560,6 +1561,16 @@ void virDomainMemballoonDefFree(virDomainMemballoonDefPtr def) VIR_FREE(def); } +void virDomainNVRAMDefFree(virDomainNVRAMDefPtr def) +{ + if (!def) + return; + + virDomainDeviceInfoClear(&def->info); + + VIR_FREE(def); +} + void virDomainWatchdogDefFree(virDomainWatchdogDefPtr def) { if (!def) @@ -1743,6 +1754,7 @@ void virDomainDeviceDefFree(virDomainDeviceDefPtr def) case VIR_DOMAIN_DEVICE_SMARTCARD: case VIR_DOMAIN_DEVICE_CHR: case VIR_DOMAIN_DEVICE_MEMBALLOON: + case VIR_DOMAIN_DEVICE_NVRAM:
Any special reason to not use virDomainNVRAMDefFree here? and same question for other device types.
It's freed when domain def is freed. I think it is created when the domain is defined, so it is freed when the domain is freed.
But I am not sure whether it's same with other device types. :) No, you misunderstood, virDomainDeviceDefFree is a wrapper of the all the free helpers of each device type. Itself can be used as helper in the source too.
I see you add virDomainNVRAMDeviceFree in virDomainDefFree, but it's different things, sometimes you will want to use virDomainDeviceDefFree to free the device def, without careing about what the device type is..
Ah, I am considering when this device is freed. For example, when deleting one device from this domain, it may call this this to free this device. But actually, nvram device can't be deleted from this domain.
There are other places one will need to free the device def, not only when detaching...
Is it okay if freeing the device def? Logically, it seems that this should be fine. But I am not sure whether this is reasonable.
It's fine to keep it not freed in virDomainDeviceFree, just like what you do in this patch, because same other device type, such as SMARTCARD also has the problem, we can fix it as a later patch together.
Okay. So I will resend you [2/2] patch with files added later. Thanks for your review. :)
participants (2)
-
Li Zhang -
Osier Yang