[libvirt] [PATCH v5 1/2] Add NVRAM device
Osier Yang
jyang at redhat.com
Wed Apr 24 11:58:07 UTC 2013
On 19/04/13 16:37, Li Zhang wrote:
> From: Li Zhang <zhlcindy at 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 at 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.
More information about the libvir-list
mailing list