Re: [PATCH 4/4] domaincaps: Expose NVRAM store file format capabilities

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

 



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 @@
>          &lt;value&gt;no&lt;/value&gt;
>        &lt;/enum&gt;
>      &lt;/loader&gt;
> +    &lt;nvram supported='yes'&gt;
> +      &lt;enum name='format'&gt;
> +        &lt;value&gt;raw&lt;/value&gt;
> +        &lt;value&gt;qcow2&lt;/value&gt;
> +      &lt;/enum&gt;
> +    &lt;/nvram&gt;
>    &lt;/os&gt;
>    ...
>  &lt;domainCapabilities&gt;
> @@ -142,6 +148,16 @@
>        &lt;loader/&gt; 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 &lt;nvram/&gt; 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



[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]