[libvirt] [PATCH v3 1/3] 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> --- 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 src/conf/domain_conf.c | 76 +++++++++++++++++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 10 +++++++ src/qemu/qemu_command.c | 67 +++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_command.h | 2 ++ 4 files changed, 155 insertions(+) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 3278e9c..c37487c 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -190,6 +190,7 @@ VIR_ENUM_IMPL(virDomainDevice, VIR_DOMAIN_DEVICE_LAST, "smartcard", "chr", "memballoon", + "nvram", "rng") VIR_ENUM_IMPL(virDomainDeviceAddress, VIR_DOMAIN_DEVICE_ADDRESS_TYPE_LAST, @@ -1515,6 +1516,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) @@ -1678,6 +1689,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; } @@ -1869,6 +1881,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]); @@ -2429,6 +2442,12 @@ int virDomainDeviceInfoIterate(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]; @@ -2460,6 +2479,7 @@ int virDomainDeviceInfoIterate(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; @@ -7703,6 +7723,23 @@ 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 ) + return NULL; + + return def; +} + static virSysinfoDefPtr virSysinfoParseXML(const xmlNodePtr node, xmlXPathContextPtr ctxt) @@ -10844,6 +10881,26 @@ virDomainDefParseXML(virCapsPtr caps, 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; @@ -13854,6 +13911,21 @@ virDomainMemballoonDefFormat(virBufferPtr buf, } static int +virDomainNVRAMDefFormat(virBufferPtr buf, + virDomainNVRAMDefPtr def, + unsigned int flags) +{ + virBufferAsprintf(buf, " <nvram>\n"); + if (virDomainDeviceInfoIsSet(&def->info, flags)) + if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0) + return -1; + + virBufferAddLit(buf, " </nvram>\n"); + + return 0; +} + +static int virDomainSysinfoDefFormat(virBufferPtr buf, virSysinfoDefPtr def) { @@ -15161,6 +15233,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); @@ -16443,6 +16518,7 @@ virDomainDeviceDefCopy(virCapsPtr caps, 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 96f11ba..03b7c9a 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -108,6 +108,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; @@ -136,6 +139,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 @@ -162,6 +166,7 @@ struct _virDomainDeviceDef { virDomainSmartcardDefPtr smartcard; virDomainChrDefPtr chr; virDomainMemballoonDefPtr memballoon; + virDomainNVRAMDefPtr nvram; virDomainRNGDefPtr rng; } data; }; @@ -1423,6 +1428,9 @@ struct _virDomainMemballoonDef { virDomainDeviceInfo info; }; +struct _virDomainNVRAMDef { + virDomainDeviceInfo info; +}; enum virDomainSmbiosMode { VIR_DOMAIN_SMBIOS_NONE = 0, @@ -1883,6 +1891,7 @@ struct _virDomainDef { /* Only 1 */ virDomainWatchdogDefPtr watchdog; virDomainMemballoonDefPtr memballoon; + virDomainNVRAMDefPtr nvram; virCPUDefPtr cpu; virSysinfoDefPtr sysinfo; virDomainRedirFilterDefPtr redirfilter; @@ -2015,6 +2024,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); diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 4891b65..26c3001 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1169,6 +1169,15 @@ int qemuDomainAssignSpaprVIOAddresses(virDomainDefPtr def, 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, + 0x3000ul) < 0) + goto cleanup; + } + /* No other devices are currently supported on spapr-vio */ ret = 0; @@ -3809,6 +3818,32 @@ error: return NULL; } +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_CONFIG_UNSUPPORTED, + "%s", _("NVRAM device address is not supported.\n")); + goto error; + } + + if (virBufferError(&buf)) { + virReportOOMError(); + goto error; + } + + return virBufferContentAndReset(&buf); + +error: + virBufferFreeAndReset(&buf); + return NULL; +} char * qemuBuildUSBInputDevStr(virDomainInputDefPtr dev, @@ -7492,6 +7527,21 @@ qemuBuildCommandLine(virConnectPtr conn, goto error; } + if (def->nvram) { + if (def->os.arch == VIR_ARCH_PPC64 && + STREQ(def->os.machine, "pseries")) { + 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 not supported")); + } if (snapshot) virCommandAddArgList(cmd, "-loadvm", snapshot->def->name, NULL); @@ -9584,6 +9634,23 @@ 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; + + val += strlen("spapr-nvram.reg="); + if (virStrToLong_ull(val, NULL, 16, + &def->nvram->info.addr.spaprvio.reg) < 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("invalid value for spapr-nvram.reg :" + "'%s'"), val); + goto error; + } + } else if (STREQ(arg, "-S")) { /* ignore, always added by libvirt */ } else { diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index 17687f4..2b60997 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -118,6 +118,8 @@ char * qemuBuildWatchdogDevStr(virDomainWatchdogDefPtr dev, char * qemuBuildMemballoonDevStr(virDomainMemballoonDefPtr dev, virQEMUCapsPtr qemuCaps); +char * qemuBuildNVRAMDevStr(virDomainNVRAMDefPtr dev); + char * qemuBuildUSBInputDevStr(virDomainInputDefPtr dev, virQEMUCapsPtr qemuCaps); -- 1.7.10.1
From: Li Zhang <zhlcindy@linux.vnet.ibm.com> This patch is to add NVRAM docs in formatdomain.html.in and domaincommon.rng Signed-off-by: Li Zhang <zhlcindy@linux.vnet.ibm.com> --- docs/formatdomain.html.in | 35 +++++++++++++++++++++++++++++++++++ docs/schemas/domaincommon.rng | 10 ++++++++++ 2 files changed, 45 insertions(+) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 8a3c3b7..35f9c9b 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -4365,6 +4365,41 @@ qemu-kvm -net nic,model=? /dev/null </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 9792065..3769d39 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -2705,6 +2705,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"> @@ -3135,6 +3142,9 @@ <optional> <ref name="memballoon"/> </optional> + <optional> + <ref name="nvram"/> + </optional> </interleave> </element> </define> -- 1.7.10.1
Any more comment? Thanks. On 2013年03月27日 13:07, Li Zhang wrote:
From: Li Zhang <zhlcindy@linux.vnet.ibm.com>
This patch is to add NVRAM docs in formatdomain.html.in and domaincommon.rng
Signed-off-by: Li Zhang <zhlcindy@linux.vnet.ibm.com> --- docs/formatdomain.html.in | 35 +++++++++++++++++++++++++++++++++++ docs/schemas/domaincommon.rng | 10 ++++++++++ 2 files changed, 45 insertions(+)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 8a3c3b7..35f9c9b 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -4365,6 +4365,41 @@ qemu-kvm -net nic,model=? /dev/null
</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 9792065..3769d39 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -2705,6 +2705,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"> @@ -3135,6 +3142,9 @@ <optional> <ref name="memballoon"/> </optional> + <optional> + <ref name="nvram"/> + </optional> </interleave> </element> </define>
From: Li Zhang <zhlcindy@linux.vnet.ibm.com> This patch is to add NVRAM test cases. Signed-off-by: Li Zhang <zhlcindy@linux.vnet.ibm.com> --- .../qemuxml2argv-pseries-nvram.args | 1 + .../qemuxml2argv-pseries-nvram.xml | 20 ++++++++++++++++++++ tests/qemuxml2argvtest.c | 2 ++ 3 files changed, 23 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-nvram.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-nvram.xml diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pseries-nvram.args b/tests/qemuxml2argvdata/qemuxml2argv-pseries-nvram.args new file mode 100644 index 0000000..a3b4bcb --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-pseries-nvram.args @@ -0,0 +1 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu-system-ppc64 -S -M pseries -m 512 -smp 1 -nographic -nodefconfig -nodefaults -chardev socket,id=charmonitor,path=/tmp/test-monitor,server,nowait -mon chardev=charmonitor,id=monitor,mode=readline -no-acpi -boot c -usb -chardev pty,id=charserial0 -device spapr-vty,chardev=charserial0,reg=0x30000000 -global spapr-nvram.reg=0x4000 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pseries-nvram.xml b/tests/qemuxml2argvdata/qemuxml2argv-pseries-nvram.xml new file mode 100644 index 0000000..e6850e5 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-pseries-nvram.xml @@ -0,0 +1,20 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>1ccfd97d-5eb4-478a-bbe6-88d254c16db7</uuid> + <memory unit='KiB'>524288</memory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='ppc64' machine='pseries'>hvm</type> + </os> + <clock offset='utc'/> + <devices> + <emulator>/usr/bin/qemu-system-ppc64</emulator> + <console type='pty'> + <address type="spapr-vio"/> + </console> + <memballoon model="none"/> + <nvram> + <address type='spapr-vio' reg='0x4000'/> + </nvram> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index e76d844..a6cbf0c 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -887,6 +887,8 @@ 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_CHARDEV, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG); DO_TEST("disk-ide-drive-split", QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_IDE_CD); -- 1.7.10.1
Any more comment? Thanks a lot. :) On 2013年03月27日 13:07, Li Zhang wrote:
From: Li Zhang <zhlcindy@linux.vnet.ibm.com>
This patch is to add NVRAM test cases.
Signed-off-by: Li Zhang <zhlcindy@linux.vnet.ibm.com> --- .../qemuxml2argv-pseries-nvram.args | 1 + .../qemuxml2argv-pseries-nvram.xml | 20 ++++++++++++++++++++ tests/qemuxml2argvtest.c | 2 ++ 3 files changed, 23 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-nvram.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-nvram.xml
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pseries-nvram.args b/tests/qemuxml2argvdata/qemuxml2argv-pseries-nvram.args new file mode 100644 index 0000000..a3b4bcb --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-pseries-nvram.args @@ -0,0 +1 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu-system-ppc64 -S -M pseries -m 512 -smp 1 -nographic -nodefconfig -nodefaults -chardev socket,id=charmonitor,path=/tmp/test-monitor,server,nowait -mon chardev=charmonitor,id=monitor,mode=readline -no-acpi -boot c -usb -chardev pty,id=charserial0 -device spapr-vty,chardev=charserial0,reg=0x30000000 -global spapr-nvram.reg=0x4000 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pseries-nvram.xml b/tests/qemuxml2argvdata/qemuxml2argv-pseries-nvram.xml new file mode 100644 index 0000000..e6850e5 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-pseries-nvram.xml @@ -0,0 +1,20 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>1ccfd97d-5eb4-478a-bbe6-88d254c16db7</uuid> + <memory unit='KiB'>524288</memory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='ppc64' machine='pseries'>hvm</type> + </os> + <clock offset='utc'/> + <devices> + <emulator>/usr/bin/qemu-system-ppc64</emulator> + <console type='pty'> + <address type="spapr-vio"/> + </console> + <memballoon model="none"/> + <nvram> + <address type='spapr-vio' reg='0x4000'/> + </nvram> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index e76d844..a6cbf0c 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -887,6 +887,8 @@ 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_CHARDEV, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG); DO_TEST("disk-ide-drive-split", QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_IDE_CD);
On Wed, Mar 27, 2013 at 01:07:56PM +0800, Li Zhang wrote:
From: Li Zhang <zhlcindy@linux.vnet.ibm.com>
This patch is to add NVRAM test cases.
Signed-off-by: Li Zhang <zhlcindy@linux.vnet.ibm.com> --- .../qemuxml2argv-pseries-nvram.args | 1 + .../qemuxml2argv-pseries-nvram.xml | 20 ++++++++++++++++++++ tests/qemuxml2argvtest.c | 2 ++
You should also add the same data files in tests/qemuargv2xmltest.c to validate your code for parsing ARGV. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On 2013年04月11日 17:37, Daniel P. Berrange wrote:
On Wed, Mar 27, 2013 at 01:07:56PM +0800, Li Zhang wrote:
From: Li Zhang <zhlcindy@linux.vnet.ibm.com>
This patch is to add NVRAM test cases.
Signed-off-by: Li Zhang <zhlcindy@linux.vnet.ibm.com> --- .../qemuxml2argv-pseries-nvram.args | 1 + .../qemuxml2argv-pseries-nvram.xml | 20 ++++++++++++++++++++ tests/qemuxml2argvtest.c | 2 ++ You should also add the same data files in tests/qemuargv2xmltest.c to validate your code for parsing ARGV. OK, I will add it.
Daniel
Any more comment? Thanks. :) On 2013年03月27日 13:07, 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> --- 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
src/conf/domain_conf.c | 76 +++++++++++++++++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 10 +++++++ src/qemu/qemu_command.c | 67 +++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_command.h | 2 ++ 4 files changed, 155 insertions(+)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 3278e9c..c37487c 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -190,6 +190,7 @@ VIR_ENUM_IMPL(virDomainDevice, VIR_DOMAIN_DEVICE_LAST, "smartcard", "chr", "memballoon", + "nvram", "rng")
VIR_ENUM_IMPL(virDomainDeviceAddress, VIR_DOMAIN_DEVICE_ADDRESS_TYPE_LAST, @@ -1515,6 +1516,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) @@ -1678,6 +1689,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; } @@ -1869,6 +1881,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]); @@ -2429,6 +2442,12 @@ int virDomainDeviceInfoIterate(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]; @@ -2460,6 +2479,7 @@ int virDomainDeviceInfoIterate(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; @@ -7703,6 +7723,23 @@ 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 ) + return NULL; + + return def; +} + static virSysinfoDefPtr virSysinfoParseXML(const xmlNodePtr node, xmlXPathContextPtr ctxt) @@ -10844,6 +10881,26 @@ virDomainDefParseXML(virCapsPtr caps, 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; @@ -13854,6 +13911,21 @@ virDomainMemballoonDefFormat(virBufferPtr buf, }
static int +virDomainNVRAMDefFormat(virBufferPtr buf, + virDomainNVRAMDefPtr def, + unsigned int flags) +{ + virBufferAsprintf(buf, " <nvram>\n"); + if (virDomainDeviceInfoIsSet(&def->info, flags)) + if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0) + return -1; + + virBufferAddLit(buf, " </nvram>\n"); + + return 0; +} + +static int virDomainSysinfoDefFormat(virBufferPtr buf, virSysinfoDefPtr def) { @@ -15161,6 +15233,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); @@ -16443,6 +16518,7 @@ virDomainDeviceDefCopy(virCapsPtr caps, 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 96f11ba..03b7c9a 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -108,6 +108,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;
@@ -136,6 +139,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 @@ -162,6 +166,7 @@ struct _virDomainDeviceDef { virDomainSmartcardDefPtr smartcard; virDomainChrDefPtr chr; virDomainMemballoonDefPtr memballoon; + virDomainNVRAMDefPtr nvram; virDomainRNGDefPtr rng; } data; }; @@ -1423,6 +1428,9 @@ struct _virDomainMemballoonDef { virDomainDeviceInfo info; };
+struct _virDomainNVRAMDef { + virDomainDeviceInfo info; +};
enum virDomainSmbiosMode { VIR_DOMAIN_SMBIOS_NONE = 0, @@ -1883,6 +1891,7 @@ struct _virDomainDef { /* Only 1 */ virDomainWatchdogDefPtr watchdog; virDomainMemballoonDefPtr memballoon; + virDomainNVRAMDefPtr nvram; virCPUDefPtr cpu; virSysinfoDefPtr sysinfo; virDomainRedirFilterDefPtr redirfilter; @@ -2015,6 +2024,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); diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 4891b65..26c3001 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1169,6 +1169,15 @@ int qemuDomainAssignSpaprVIOAddresses(virDomainDefPtr def, 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, + 0x3000ul) < 0) + goto cleanup; + } + /* No other devices are currently supported on spapr-vio */
ret = 0; @@ -3809,6 +3818,32 @@ error: return NULL; }
+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_CONFIG_UNSUPPORTED, + "%s", _("NVRAM device address is not supported.\n")); + goto error; + } + + if (virBufferError(&buf)) { + virReportOOMError(); + goto error; + } + + return virBufferContentAndReset(&buf); + +error: + virBufferFreeAndReset(&buf); + return NULL; +}
char * qemuBuildUSBInputDevStr(virDomainInputDefPtr dev, @@ -7492,6 +7527,21 @@ qemuBuildCommandLine(virConnectPtr conn, goto error; }
+ if (def->nvram) { + if (def->os.arch == VIR_ARCH_PPC64 && + STREQ(def->os.machine, "pseries")) { + 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 not supported")); + } if (snapshot) virCommandAddArgList(cmd, "-loadvm", snapshot->def->name, NULL);
@@ -9584,6 +9634,23 @@ 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; + + val += strlen("spapr-nvram.reg="); + if (virStrToLong_ull(val, NULL, 16, + &def->nvram->info.addr.spaprvio.reg) < 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("invalid value for spapr-nvram.reg :" + "'%s'"), val); + goto error; + } + } else if (STREQ(arg, "-S")) { /* ignore, always added by libvirt */ } else { diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index 17687f4..2b60997 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -118,6 +118,8 @@ char * qemuBuildWatchdogDevStr(virDomainWatchdogDefPtr dev, char * qemuBuildMemballoonDevStr(virDomainMemballoonDefPtr dev, virQEMUCapsPtr qemuCaps);
+char * qemuBuildNVRAMDevStr(virDomainNVRAMDefPtr dev); + char * qemuBuildUSBInputDevStr(virDomainInputDefPtr dev, virQEMUCapsPtr qemuCaps);
On Wed, Mar 27, 2013 at 01:07:54PM +0800, 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> --- 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
src/conf/domain_conf.c | 76 +++++++++++++++++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 10 +++++++
src/qemu/qemu_command.c | 67 +++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_command.h | 2 ++ 4 files changed, 155 insertions(+)
The QEMU pieces should be separate from the parser pieces. Basically instead of the 3 patches you have here you should have 2 patches with the following files in each one: 1. src/conf/domain_conf.*, docs/schemas/* and docs/formatdomain.html.in 2. src/qemu/* and tests/qemuxml2argv*
@@ -13854,6 +13911,21 @@ virDomainMemballoonDefFormat(virBufferPtr buf, }
static int +virDomainNVRAMDefFormat(virBufferPtr buf, + virDomainNVRAMDefPtr def, + unsigned int flags) +{ + virBufferAsprintf(buf, " <nvram>\n"); + if (virDomainDeviceInfoIsSet(&def->info, flags)) + if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0) + return -1; + + virBufferAddLit(buf, " </nvram>\n"); + + return 0;
There's inconsistent indentation here
qemuBuildUSBInputDevStr(virDomainInputDefPtr dev, @@ -7492,6 +7527,21 @@ qemuBuildCommandLine(virConnectPtr conn, goto error; }
+ if (def->nvram) { + if (def->os.arch == VIR_ARCH_PPC64 && + STREQ(def->os.machine, "pseries")) { + 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 not supported"));
Missing a 'goto error' here
@@ -9584,6 +9634,23 @@ 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; + + val += strlen("spapr-nvram.reg="); + if (virStrToLong_ull(val, NULL, 16, + &def->nvram->info.addr.spaprvio.reg) < 0) {
Don't you need to set the address type too.
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("invalid value for spapr-nvram.reg :" + "'%s'"), val); + goto error; + } + } else if (STREQ(arg, "-S")) { /* ignore, always added by libvirt */ } else {
Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On 2013年04月11日 17:36, Daniel P. Berrange wrote:
On Wed, Mar 27, 2013 at 01:07:54PM +0800, 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> --- 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
src/conf/domain_conf.c | 76 +++++++++++++++++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 10 +++++++
src/qemu/qemu_command.c | 67 +++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_command.h | 2 ++ 4 files changed, 155 insertions(+) The QEMU pieces should be separate from the parser pieces. Basically instead of the 3 patches you have here you should have 2 patches with the following files in each one:
1. src/conf/domain_conf.*, docs/schemas/* and docs/formatdomain.html.in
2. src/qemu/* and tests/qemuxml2argv* OK, got it.
@@ -13854,6 +13911,21 @@ virDomainMemballoonDefFormat(virBufferPtr buf, }
static int +virDomainNVRAMDefFormat(virBufferPtr buf, + virDomainNVRAMDefPtr def, + unsigned int flags) +{ + virBufferAsprintf(buf, " <nvram>\n"); + if (virDomainDeviceInfoIsSet(&def->info, flags)) + if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0) + return -1; + + virBufferAddLit(buf, " </nvram>\n"); + + return 0; There's inconsistent indentation here
I will correct it.
qemuBuildUSBInputDevStr(virDomainInputDefPtr dev, @@ -7492,6 +7527,21 @@ qemuBuildCommandLine(virConnectPtr conn, goto error; }
+ if (def->nvram) { + if (def->os.arch == VIR_ARCH_PPC64 && + STREQ(def->os.machine, "pseries")) { + 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 not supported"));
Missing a 'goto error' here
Ok, I will add it.
@@ -9584,6 +9634,23 @@ 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; + + val += strlen("spapr-nvram.reg="); + if (virStrToLong_ull(val, NULL, 16, + &def->nvram->info.addr.spaprvio.reg) < 0) { Don't you need to set the address type too.
Ah, yes. I will add it.
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("invalid value for spapr-nvram.reg :" + "'%s'"), val); + goto error; + } + } else if (STREQ(arg, "-S")) { /* ignore, always added by libvirt */ } else {
Daniel
participants (2)
-
Daniel P. Berrange -
Li Zhang