comments below
On 01/13/15 14:41, Michal Privoznik wrote:
> It would be nice if we expose the capability of choosing the NVRAM
> store file format among with all the possibilities.
>
> Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx>
> ---
> docs/formatdomaincaps.html.in | 16 ++++++++++++++++
> docs/schemas/domaincaps.rng | 13 +++++++++++++
> src/conf/domain_capabilities.c | 17 +++++++++++++++++
> src/conf/domain_capabilities.h | 1 +
> src/qemu/qemu_capabilities.c | 7 +++++--
> tests/domaincapsschemadata/domaincaps-full.xml | 6 ++++++
> tests/domaincapsschemadata/domaincaps-qemu_1.6.50-1.xml | 6 ++++++
> tests/domaincapstest.c | 1 +
> 8 files changed, 65 insertions(+), 2 deletions(-)
>
> diff --git a/docs/formatdomaincaps.html.in b/docs/formatdomaincaps.html.in
> index 850109f..05a67b8 100644
> --- a/docs/formatdomaincaps.html.in
> +++ b/docs/formatdomaincaps.html.in
> @@ -118,6 +118,12 @@
> <value>no</value>
> </enum>
> </loader>
> + <nvram supported='yes'>
> + <enum name='format'>
> + <value>raw</value>
> + <value>qcow2</value>
> + </enum>
> + </nvram>
> </os>
> ...
> <domainCapabilities>
> @@ -142,6 +148,16 @@
> <loader/> element.</dd>
> </dl>
>
> + <p>For the <code>nvram</code> element, the following can occur:</p>
s/occur/be listed/; I think. (May not be right for the capability docs.)
Also, mention <loader>.
> +
> + <dl>
> + <dt>format</dt>
> + <dd>What formats of NVRAM file are supported. The default is
> + <code>raw</code> which is delivered in the packages. However, in some
> + distributions, the <code>qcow2</code> may be delivered too. This refers
> + to <code>format</code> attribute of the <nvram/> element.</dd>
> + </dl>
> +
I think the language about packages and distributions should be omitted.
> <h3><a name="elementsDevices">Devices</a></h3>
>
> <p>
> diff --git a/docs/schemas/domaincaps.rng b/docs/schemas/domaincaps.rng
> index 35d3745..e8a1e59 100644
> --- a/docs/schemas/domaincaps.rng
> +++ b/docs/schemas/domaincaps.rng
> @@ -54,6 +54,16 @@
> </element>
> </define>
>
> + <define name='nvram'>
> + <element name='nvram'>
> + <ref name='supported'/>
> + <optional>
> + <ref name='value'/>
> + </optional>
> + <ref name='enum'/>
> + </element>
> + </define>
> +
> <define name='os'>
> <element name='os'>
> <interleave>
> @@ -61,6 +71,9 @@
> <optional>
> <ref name='loader'/>
> </optional>
> + <optional>
> + <ref name='nvram'/>
> + </optional>
> </interleave>
> </element>
> </define>
> diff --git a/src/conf/domain_capabilities.c b/src/conf/domain_capabilities.c
> index 7c59912..d545fcc 100644
> --- a/src/conf/domain_capabilities.c
> +++ b/src/conf/domain_capabilities.c
> @@ -221,6 +221,22 @@ virDomainCapsLoaderFormat(virBufferPtr buf,
> }
>
> static void
> +virDomainCapsNVRAMFormat(virBufferPtr buf,
> + virDomainCapsLoaderPtr nvram)
> +{
> + /* Even though the type of @nvram would suggest naming it
> + * differently than 'nvram' ('loader' for instance), keep in
> + * mind that the FORMAT_PROLOGUE macro emits the variable
> + * name into the XML where we want to see 'nvram'. Really. */
> + FORMAT_PROLOGUE(nvram);
> +
> + virDomainCapsEnumFormat(buf, &nvram->nvramFormat, "format",
> + virDomainLoaderNVRAMFormatTypeToString);
> +
> + FORMAT_EPILOGUE(nvram);
> +}
> +
> +static void
> virDomainCapsOSFormat(virBufferPtr buf,
> virDomainCapsOSPtr os)
> {
> @@ -229,6 +245,7 @@ virDomainCapsOSFormat(virBufferPtr buf,
> FORMAT_PROLOGUE(os);
>
> virDomainCapsLoaderFormat(buf, loader);
> + virDomainCapsNVRAMFormat(buf, loader);
>
> FORMAT_EPILOGUE(os);
> }
> diff --git a/src/conf/domain_capabilities.h b/src/conf/domain_capabilities.h
> index 597ac75..f878a67 100644
> --- a/src/conf/domain_capabilities.h
> +++ b/src/conf/domain_capabilities.h
> @@ -57,6 +57,7 @@ struct _virDomainCapsLoader {
> virDomainCapsStringValues values; /* Info about values for the element */
> virDomainCapsEnum type; /* Info about virDomainLoader */
> virDomainCapsEnum readonly; /* Info about readonly:virTristateBool */
> + virDomainCapsEnum nvramFormat; /* Info about virDomainLoaderNVRAMFormat */
> };
>
> typedef struct _virDomainCapsOS virDomainCapsOS;
> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> index 13f3cd3..858ef8d 100644
> --- a/src/qemu/qemu_capabilities.c
> +++ b/src/qemu/qemu_capabilities.c
> @@ -3706,10 +3706,13 @@ virQEMUCapsFillDomainLoaderCaps(virQEMUCapsPtr qemuCaps,
> VIR_DOMAIN_LOADER_TYPE_ROM);
>
> if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DRIVE) &&
> - virQEMUCapsGet(qemuCaps, QEMU_CAPS_DRIVE_FORMAT))
> + virQEMUCapsGet(qemuCaps, QEMU_CAPS_DRIVE_FORMAT)) {
> VIR_DOMAIN_CAPS_ENUM_SET(capsLoader->type,
> VIR_DOMAIN_LOADER_TYPE_PFLASH);
> -
> + VIR_DOMAIN_CAPS_ENUM_SET(capsLoader->nvramFormat,
> + VIR_DOMAIN_LOADER_NVRAM_FORMAT_RAW,
> + VIR_DOMAIN_LOADER_NVRAM_FORMAT_QCOW2);
> + }
>
> if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DRIVE_READONLY))
> VIR_DOMAIN_CAPS_ENUM_SET(capsLoader->readonly,
> diff --git a/tests/domaincapsschemadata/domaincaps-full.xml b/tests/domaincapsschemadata/domaincaps-full.xml
> index 96202bc..8a3902e 100644
> --- a/tests/domaincapsschemadata/domaincaps-full.xml
> +++ b/tests/domaincapsschemadata/domaincaps-full.xml
> @@ -18,6 +18,12 @@
> <value>no</value>
> </enum>
> </loader>
> + <nvram supported='yes'>
> + <enum name='format'>
> + <value>raw</value>
> + <value>qcow2</value>
> + </enum>
> + </nvram>
> </os>
> <devices>
> <disk supported='yes'>
> diff --git a/tests/domaincapsschemadata/domaincaps-qemu_1.6.50-1.xml b/tests/domaincapsschemadata/domaincaps-qemu_1.6.50-1.xml
> index 346ef65..4ba0324 100644
> --- a/tests/domaincapsschemadata/domaincaps-qemu_1.6.50-1.xml
> +++ b/tests/domaincapsschemadata/domaincaps-qemu_1.6.50-1.xml
> @@ -15,6 +15,12 @@
> <value>no</value>
> </enum>
> </loader>
> + <nvram supported='yes'>
> + <enum name='format'>
> + <value>raw</value>
> + <value>qcow2</value>
> + </enum>
> + </nvram>
> </os>
> <devices>
> <disk supported='yes'>
> diff --git a/tests/domaincapstest.c b/tests/domaincapstest.c
> index 70d2ef3..328ce5e 100644
> --- a/tests/domaincapstest.c
> +++ b/tests/domaincapstest.c
> @@ -70,6 +70,7 @@ fillAll(virDomainCapsPtr domCaps,
> loader->device.supported = true;
> SET_ALL_BITS(loader->type);
> SET_ALL_BITS(loader->readonly);
> + SET_ALL_BITS(loader->nvramFormat);
> if (fillStringValues(&loader->values,
> "/foo/bar",
> "/tmp/my_path",
>
The rest seems okay to me (but you could sell me anything around these
parts :))
Thanks
Laszlo
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list