[libvirt] [PATCH v5 03/11] nvdimm: introduce 'readonly' element into xml for NVDIMM memory

[John Ferlan]

On 12/20/18 4:14 AM, Luyao Zhong wrote:
> The 'readonly' option allows users to mark vNVDIMM read-only:
> 
> <devices>
>   ...
>   <memory model='nvdimm' access='shared'>
>       <source>
>           <path>/dev/dax0.0</path>
>       </source>
>       <target>
>           <size unit='MiB'>4094</size>
>           <node>0</node>
>           <label>
>               <size unit='MiB'>2</size>
>           </label>
>           <readonly/>
>       </target>
>   </memory>
>   ...
> </devices>
> 
> Signed-off-by: Luyao Zhong <luyao.zhong at intel.com>
> ---
>  docs/formatdomain.html.in                          | 45 ++++++++++++-----
>  docs/schemas/domaincommon.rng                      |  5 ++
>  src/conf/domain_conf.c                             | 12 +++++
>  src/conf/domain_conf.h                             |  1 +
>  .../memory-hotplug-nvdimm-readonly.xml             | 58 ++++++++++++++++++++++
>  .../memory-hotplug-nvdimm-readonly.xml             |  1 +
>  tests/qemuxml2xmltest.c                            |  1 +
>  7 files changed, 111 insertions(+), 12 deletions(-)
>  create mode 100644 tests/qemuxml2argvdata/memory-hotplug-nvdimm-readonly.xml
>  create mode 120000 tests/qemuxml2xmloutdata/memory-hotplug-nvdimm-readonly.xml
> 
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index 0781f24..2ca7a46 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -8331,6 +8331,7 @@ qemu-kvm -net nic,model=? /dev/null
>        <label>
>          <size unit='KiB'>128</size>
>        </label>
> +      <readonly/>

Building off the previous patch... Since the description below indicates
something about persistence and real backends, I'm "assuming" that means
that a source marked with <pmem> would not want to use <readonly/>;
however, something like the source path /tmp/nvdimm should be marked as
<readonly/>.

Does that description fit your view?  If so, I'll modify the /tmp/nvdimm
to have this <readonly/> option, while the /dev/dax0.0 with the <pmem/>
wouldn't have it.

>      </target>
>    </memory>
>  </devices>
> @@ -8454,19 +8455,39 @@ qemu-kvm -net nic,model=? /dev/null
>            NUMA nodes configured.
>          </p>
>          <p>
> -          For NVDIMM type devices one can optionally use
> -          <code>label</code> and its subelement <code>size</code>
> -          to configure the size of namespaces label storage
> -          within the NVDIMM module. The <code>size</code> element
> -          has usual meaning described
> -          <a href="#elementsMemoryAllocation">here</a>.
> -          For QEMU domains the following restrictions apply:
> +          Besides, the following optional elements can be provided as well for
> +          NVDIMM type devices:

Similar to patch1:

s/Besides, ... devices:/The following optional elements may be used:

>          </p>
> -        <ol>
> -          <li>the minimum label size is 128KiB,</li>
> -          <li>the remaining size (total-size - label-size) has to be aligned to
> -            4KiB</li>
> -        </ol>
> +
> +        <dl>
> +          <dt><code>label</code></dt>
> +          <dd>
> +            <p>
> +              For NVDIMM type devices one can optionally use
> +              <code>label</code> and its subelement <code>size</code>
> +              to configure the size of namespaces label storage
> +              within the NVDIMM module. The <code>size</code> element
> +              has usual meaning described
> +              <a href="#elementsMemoryAllocation">here</a>.
> +              For QEMU domains the following restrictions apply:
> +            </p>
> +            <ol>
> +              <li>the minimum label size is 128KiB,</li>
> +              <li>the remaining size (total-size - label-size) will be aligned to
> +                4KiB as default.</li>
> +            </ol>
> +          </dd>
> +
> +          <dt><code>readonly</code></dt>
> +          <dd>
> +            <p>
> +              The <code>readonly</code> element can be used to mark vNVDIMM read-only.

s/can be/is/

s/mark/mark the/

s/read-only/as read-only/


> +              Currently, only real NVDIMM device backend can guarantee the guest write

s/Currently, only/Only/

s/real/the real/

> +              persistence, so please set <code>readonly</code> when using other types
> +              of backends. <span class="since">Since 5.0.0</span>

s/, so please ... backends/, so other backend types should use the
<code>readonly</code> element.

Again, with your OK I can make these changes for you...

Reviewed-by: John Ferlan <jferlan at redhat.com>

John

[...]