[libvirt] [PATCH v3 08/17] conf: Introduce @access to <memory/>

Michal Privoznik posted 17 patches 8 years, 11 months ago
[libvirt] [PATCH v3 08/17] conf: Introduce @access to <memory/>
Posted by Michal Privoznik 8 years, 11 months ago
Now that NVDIMM has found its way into libvirt, users might want
to fine tune some settings for each module separately. One such
setting is 'share=on|off' for the memory-backend-file object.
This setting - just like its name suggest already - enables
sharing the nvdimm module with other applications. Under the hood
it controls whether qemu mmaps() the file as MAP_PRIVATE or
MAP_SHARED.

Yet again, we have such config knob in domain XML, but it's just
an attribute to numa <cell/>. This does not give fine enough
tuning on per-memdevice basis so we need to have the attribute
for each device too.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 docs/formatdomain.html.in                          | 16 ++++++-
 docs/schemas/domaincommon.rng                      |  8 ++++
 src/conf/domain_conf.c                             | 15 +++++-
 src/conf/domain_conf.h                             |  2 +
 .../qemuxml2argv-memory-hotplug-nvdimm-access.xml  | 56 ++++++++++++++++++++++
 ...qemuxml2xmlout-memory-hotplug-nvdimm-access.xml |  1 +
 tests/qemuxml2xmltest.c                            |  1 +
 7 files changed, 96 insertions(+), 3 deletions(-)
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug-nvdimm-access.xml
 create mode 120000 tests/qemuxml2xmloutdata/qemuxml2xmlout-memory-hotplug-nvdimm-access.xml

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 6e89bfe3a..4bc3d92f9 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -1417,7 +1417,7 @@
       <span class='since'>Since 1.2.9</span> the optional attribute
       <code>memAccess</code> can control whether the memory is to be
       mapped as "shared" or "private".  This is valid only for
-      hugepages-backed memory.
+      hugepages-backed memory and nvdimm modules.
     </p>
 
     <p>
@@ -7099,7 +7099,7 @@ qemu-kvm -net nic,model=? /dev/null
 <pre>
 ...
 &lt;devices&gt;
-  &lt;memory model='dimm'&gt;
+  &lt;memory model='dimm' access='private'&gt;
     &lt;target&gt;
       &lt;size unit='KiB'&gt;524287&lt;/size&gt;
       &lt;node&gt;0&lt;/node&gt;
@@ -7138,6 +7138,18 @@ qemu-kvm -net nic,model=? /dev/null
         </p>
       </dd>
 
+      <dt><code>access</code></dt>
+      <dd>
+        <p>
+          An optional attribute <code>access</code>
+          (<span class="since">since 3.2.0</span>) that provides
+          capability to fine tune mapping of the memory on per
+          module basis. Values are the same as
+          <a href="#elementsMemoryBacking">Memory Backing</a>:
+          <code>shared</code> and <code>private</code>.
+        </p>
+      </dd>
+
       <dt><code>source</code></dt>
       <dd>
         <p>
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 07b3c52ad..5e7e75950 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -4749,6 +4749,14 @@
           <value>nvdimm</value>
         </choice>
       </attribute>
+      <optional>
+        <attribute name="access">
+          <choice>
+            <value>shared</value>
+            <value>private</value>
+          </choice>
+        </attribute>
+      </optional>
       <interleave>
         <optional>
           <ref name="memorydev-source"/>
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index e23ce0015..bf3f2fb86 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -13874,6 +13874,15 @@ virDomainMemoryDefParseXML(xmlNodePtr memdevNode,
     }
     VIR_FREE(tmp);
 
+    tmp = virXMLPropString(memdevNode, "access");
+    if (tmp &&
+        (def->access = virDomainMemoryAccessTypeFromString(tmp)) <= 0) {
+        virReportError(VIR_ERR_XML_ERROR,
+                       _("invalid access mode '%s'"), tmp);
+        goto error;
+    }
+    VIR_FREE(tmp);
+
     /* source */
     if ((node = virXPathNode("./source", ctxt)) &&
         virDomainMemorySourceDefParseXML(node, ctxt, def) < 0)
@@ -22680,7 +22689,11 @@ virDomainMemoryDefFormat(virBufferPtr buf,
 {
     const char *model = virDomainMemoryModelTypeToString(def->model);
 
-    virBufferAsprintf(buf, "<memory model='%s'>\n", model);
+    virBufferAsprintf(buf, "<memory model='%s'", model);
+    if (def->access)
+        virBufferAsprintf(buf, " access='%s'",
+                          virDomainMemoryAccessTypeToString(def->access));
+    virBufferAddLit(buf, ">\n");
     virBufferAdjustIndent(buf, 2);
 
     if (virDomainMemorySourceDefFormat(buf, def) < 0)
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 823582253..fa27708ce 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2002,6 +2002,8 @@ typedef enum {
 } virDomainMemoryModel;
 
 struct _virDomainMemoryDef {
+    virDomainMemoryAccess access;
+
     /* source */
     virBitmapPtr sourceNodes;
     unsigned long long pagesize; /* kibibytes */
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug-nvdimm-access.xml b/tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug-nvdimm-access.xml
new file mode 100644
index 000000000..700e961a6
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug-nvdimm-access.xml
@@ -0,0 +1,56 @@
+<domain type='qemu'>
+  <name>QEMUGuest1</name>
+  <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+  <maxMemory slots='16' unit='KiB'>1099511627776</maxMemory>
+  <memory unit='KiB'>1267710</memory>
+  <currentMemory unit='KiB'>1267710</currentMemory>
+  <vcpu placement='static' cpuset='0-1'>2</vcpu>
+  <os>
+    <type arch='i686' machine='pc'>hvm</type>
+    <boot dev='hd'/>
+  </os>
+  <idmap>
+    <uid start='0' target='1000' count='10'/>
+    <gid start='0' target='1000' count='10'/>
+  </idmap>
+  <cpu>
+    <topology sockets='2' cores='1' threads='1'/>
+    <numa>
+      <cell id='0' cpus='0-1' memory='219136' unit='KiB'/>
+    </numa>
+  </cpu>
+  <clock offset='utc'/>
+  <on_poweroff>destroy</on_poweroff>
+  <on_reboot>restart</on_reboot>
+  <on_crash>destroy</on_crash>
+  <devices>
+    <emulator>/usr/bin/qemu</emulator>
+    <disk type='block' device='disk'>
+      <source dev='/dev/HostVG/QEMUGuest1'/>
+      <target dev='hda' bus='ide'/>
+      <address type='drive' controller='0' bus='0' target='0' unit='0'/>
+    </disk>
+    <controller type='ide' index='0'>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/>
+    </controller>
+    <controller type='usb' index='0'>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/>
+    </controller>
+    <controller type='pci' index='0' model='pci-root'/>
+    <input type='mouse' bus='ps2'/>
+    <input type='keyboard' bus='ps2'/>
+    <memballoon model='virtio'>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/>
+    </memballoon>
+    <memory model='nvdimm' access='private'>
+      <source>
+        <path>/tmp/nvdimm</path>
+      </source>
+      <target>
+        <size unit='KiB'>523264</size>
+        <node>0</node>
+      </target>
+      <address type='dimm' slot='0'/>
+    </memory>
+  </devices>
+</domain>
diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-memory-hotplug-nvdimm-access.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-memory-hotplug-nvdimm-access.xml
new file mode 120000
index 000000000..4b0496ad5
--- /dev/null
+++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-memory-hotplug-nvdimm-access.xml
@@ -0,0 +1 @@
+/home/zippy/work/libvirt/libvirt.git/tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug-nvdimm-access.xml
\ No newline at end of file
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index e1c341dd5..ef49a79ca 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -1079,6 +1079,7 @@ mymain(void)
     DO_TEST("memory-hotplug-nonuma", NONE);
     DO_TEST("memory-hotplug-dimm", NONE);
     DO_TEST("memory-hotplug-nvdimm", NONE);
+    DO_TEST("memory-hotplug-nvdimm-access", NONE);
     DO_TEST("net-udp", NONE);
 
     DO_TEST("video-virtio-gpu-device", NONE);
-- 
2.11.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 08/17] conf: Introduce @access to <memory/>
Posted by John Ferlan 8 years, 10 months ago

On 03/09/2017 11:06 AM, Michal Privoznik wrote:
> Now that NVDIMM has found its way into libvirt, users might want
> to fine tune some settings for each module separately. One such
> setting is 'share=on|off' for the memory-backend-file object.
> This setting - just like its name suggest already - enables
> sharing the nvdimm module with other applications. Under the hood
> it controls whether qemu mmaps() the file as MAP_PRIVATE or
> MAP_SHARED.
> 
> Yet again, we have such config knob in domain XML, but it's just
> an attribute to numa <cell/>. This does not give fine enough
> tuning on per-memdevice basis so we need to have the attribute
> for each device too.
> 
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
>  docs/formatdomain.html.in                          | 16 ++++++-
>  docs/schemas/domaincommon.rng                      |  8 ++++
>  src/conf/domain_conf.c                             | 15 +++++-
>  src/conf/domain_conf.h                             |  2 +
>  .../qemuxml2argv-memory-hotplug-nvdimm-access.xml  | 56 ++++++++++++++++++++++
>  ...qemuxml2xmlout-memory-hotplug-nvdimm-access.xml |  1 +
>  tests/qemuxml2xmltest.c                            |  1 +
>  7 files changed, 96 insertions(+), 3 deletions(-)
>  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug-nvdimm-access.xml
>  create mode 120000 tests/qemuxml2xmloutdata/qemuxml2xmlout-memory-hotplug-nvdimm-access.xml
> 

This patch fails make check for me because your qemuxml2xmloutdata has
the wrong softlink set up.  I don't have "/home/zippy/work/...", but I
would have

e.g. from the tests/qemuxml2xmloutdata directory use:

ln -sf ../qemuxml2argvdata/qemuxml2argv-memory-hotplug-nvdimm-access.xml
qemuxml2xmlout-memory-hotplug-nvdimm-access.xml

> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index 6e89bfe3a..4bc3d92f9 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -1417,7 +1417,7 @@
>        <span class='since'>Since 1.2.9</span> the optional attribute
>        <code>memAccess</code> can control whether the memory is to be
>        mapped as "shared" or "private".  This is valid only for
> -      hugepages-backed memory.
> +      hugepages-backed memory and nvdimm modules.
>      </p>
>  
>      <p>
> @@ -7099,7 +7099,7 @@ qemu-kvm -net nic,model=? /dev/null
>  <pre>
>  ...
>  &lt;devices&gt;
> -  &lt;memory model='dimm'&gt;
> +  &lt;memory model='dimm' access='private'&gt;
>      &lt;target&gt;
>        &lt;size unit='KiB'&gt;524287&lt;/size&gt;
>        &lt;node&gt;0&lt;/node&gt;
> @@ -7138,6 +7138,18 @@ qemu-kvm -net nic,model=? /dev/null
>          </p>
>        </dd>
>  
> +      <dt><code>access</code></dt>
> +      <dd>
> +        <p>
> +          An optional attribute <code>access</code>
> +          (<span class="since">since 3.2.0</span>) that provides
> +          capability to fine tune mapping of the memory on per
> +          module basis. Values are the same as
> +          <a href="#elementsMemoryBacking">Memory Backing</a>:
> +          <code>shared</code> and <code>private</code>.
> +        </p>
> +      </dd>
> +

>From the v2 code review comment/response - can/should we state right
here that the access setting in/for nvdimm would override any
memoryBacking setting?'

If so, then would it be furthermore interesting to modify the XML here
to set the memoryBacking as private and override that, but it's not
required...

ACK w/ the fixed link to the output file... Your call on the doc change


John

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