Re: [PATCH v5 1/2] Add NVRAM device

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxxxxxxxxxx>

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@xxxxxxxxxxxxxxxxxx>
---
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>
+ ...
+ &lt;devices&gt;
+ &lt;nvram&gt;
+ &lt;address type='spapr-vio' reg='0x3000'/&gt;
+ &lt;/nvram&gt;
+ &lt;/devices&gt;
+ ...
+</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.

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

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list





[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]