[libvirt] [PATCH v3 05/17] Introduce NVDIMM memory model

[John Ferlan]

On 03/09/2017 11:06 AM, Michal Privoznik wrote:
> NVDIMM is new type of memory introduced into QEMU 2.6. The idea
> is that we have a Non-Volatile memory module that keeps the data
> persistent across domain reboots.
> 
> At the domain XML level, we already have some representation of
> 'dimm' modules. Long story short, we have <memory/> element that

(From my v2 review):

Starting with "Long story short..."

how about instead:

NVDIMM will utilize the existing <memory/> element that lives under
<devices/> by adding a new attribute 'nvdimm' to the existing @model and
introduce a new <path/> element for <source/> while reusing other
fields. The resulting XML would appear as:

> lives under <devices/>. Now, the element even has @model
> attribute which we can use to introduce new memory type:
> 
>     <memory model='nvdimm'>
>       <source>
>         <path>/tmp/nvdimm</path>
>       </source>
>       <target>
>         <size unit='KiB'>523264</size>
>         <node>0</node>
>       </target>
>       <address type='dimm' slot='0'/>
>     </memory>
> 
> So far, this is just a XML parser/formatter extension. QEMU
> driver implementation is in the next commit.

Would it be important to indicate that the size is included with the
overall domain memory size?

> 
> For more info on NVDIMM visit the following web page:
> 
>     http://pmem.io/
> 

I know this is one of those almost hate to mention it, but should the
link be listed in formatdomain? It would be "lost" in a commit message,
but then again if the website domain changes, then we have a dead link.


> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
> ---
>  docs/formatdomain.html.in                          | 56 +++++++++----
>  docs/schemas/domaincommon.rng                      | 32 ++++---
>  src/conf/domain_conf.c                             | 97 ++++++++++++++++------
>  src/conf/domain_conf.h                             |  2 +
>  src/qemu/qemu_command.c                            |  6 ++
>  src/qemu/qemu_domain.c                             |  5 ++
>  .../qemuxml2argv-memory-hotplug-nvdimm.xml         | 56 +++++++++++++
>  .../qemuxml2xmlout-memory-hotplug-nvdimm.xml       |  1 +
>  tests/qemuxml2xmltest.c                            |  1 +
>  9 files changed, 205 insertions(+), 51 deletions(-)
>  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug-nvdimm.xml
>  create mode 120000 tests/qemuxml2xmloutdata/qemuxml2xmlout-memory-hotplug-nvdimm.xml
> 

ACK whether or not you alter the commit message and/or formatdomain

John