[libvirt] [PATCH] conf: Introduce align for hostmem-file

Jie Wang posted 1 patch 5 years, 11 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/1527597532-37999-1-git-send-email-wangjie88@huawei.com
Test syntax-check failed
docs/formatdomain.html.in                          | 18 +++++++
docs/schemas/domaincommon.rng                      |  7 +++
src/conf/domain_conf.c                             | 14 +++++
src/conf/domain_conf.h                             |  1 +
src/qemu/qemu_command.c                            |  4 ++
.../memory-hotplug-nvdimm-align.args               | 31 +++++++++++
.../memory-hotplug-nvdimm-align.xml                | 63 ++++++++++++++++++++++
tests/qemuxml2argvtest.c                           |  3 ++
.../memory-hotplug-nvdimm-align.xml                |  1 +
tests/qemuxml2xmltest.c                            |  1 +
10 files changed, 143 insertions(+)
create mode 100644 tests/qemuxml2argvdata/memory-hotplug-nvdimm-align.args
create mode 100644 tests/qemuxml2argvdata/memory-hotplug-nvdimm-align.xml
create mode 100644 tests/qemuxml2xmloutdata/memory-hotplug-nvdimm-align.xml
[libvirt] [PATCH] conf: Introduce align for hostmem-file
Posted by Jie Wang 5 years, 11 months ago
QEMU has add the 'align' option to 'memory-backend-file'. Expose
this option to users by new element align.

Signed-off-by: Jie Wang <wangjie88@huawei.com>
---
 docs/formatdomain.html.in                          | 18 +++++++
 docs/schemas/domaincommon.rng                      |  7 +++
 src/conf/domain_conf.c                             | 14 +++++
 src/conf/domain_conf.h                             |  1 +
 src/qemu/qemu_command.c                            |  4 ++
 .../memory-hotplug-nvdimm-align.args               | 31 +++++++++++
 .../memory-hotplug-nvdimm-align.xml                | 63 ++++++++++++++++++++++
 tests/qemuxml2argvtest.c                           |  3 ++
 .../memory-hotplug-nvdimm-align.xml                |  1 +
 tests/qemuxml2xmltest.c                            |  1 +
 10 files changed, 143 insertions(+)
 create mode 100644 tests/qemuxml2argvdata/memory-hotplug-nvdimm-align.args
 create mode 100644 tests/qemuxml2argvdata/memory-hotplug-nvdimm-align.xml
 create mode 100644 tests/qemuxml2xmloutdata/memory-hotplug-nvdimm-align.xml

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 0d0fd3b..29fe145 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -7932,6 +7932,9 @@ qemu-kvm -net nic,model=? /dev/null
     &lt;/target&gt;
   &lt;/memory&gt;
   &lt;memory model='nvdimm'&gt;
+    &lt;align&gt;
+      &lt;sieze unit='KiB'&gt;2048&lt;/size&gt;
+    &lt;/align&gt;
     &lt;source&gt;
       &lt;path&gt;/tmp/nvdimm&lt;/path&gt;
     &lt;/source&gt;
@@ -7983,6 +7986,21 @@ qemu-kvm -net nic,model=? /dev/null
         </p>
       </dd>
 
+      <dt><code>align</code></dt>
+      <dd>
+        <p>
+          For NVDIMM type devices one can optionally use
+          <code>align</code> and its subelement <code>size</code>
+          to configure the size of alignment 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 alignment must be multiples of page size 4KiB,</li>
+        </ol>
+      </dd>
+
       <dt><code>source</code></dt>
       <dd>
         <p>
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 71ac3d0..9e994b1 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -5145,6 +5145,13 @@
           <ref name="virYesNo"/>
         </attribute>
       </optional>
+      <optional>
+        <element name="align">
+          <element name="size">
+            <ref name="scaledInteger"/>
+          </element>
+        </element>
+      </optional>
       <interleave>
         <optional>
           <ref name="memorydev-source"/>
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 3689ac0..bf91167 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -15739,6 +15739,12 @@ virDomainMemoryDefParseXML(virDomainXMLOptionPtr xmlopt,
     }
     VIR_FREE(tmp);
 
+    if ((node = virXPathNode("./align", ctxt))) {
+        if (virDomainParseMemory("./align/size", "./align/size/@unit", ctxt,
+                                 &def->align, true, false) < 0)
+            goto error;
+    }
+
     /* source */
     if ((node = virXPathNode("./source", ctxt)) &&
         virDomainMemorySourceDefParseXML(node, ctxt, def) < 0)
@@ -25334,6 +25340,14 @@ virDomainMemoryDefFormat(virBufferPtr buf,
     virBufferAddLit(buf, ">\n");
     virBufferAdjustIndent(buf, 2);
 
+    if (def->align) {
+        virBufferAddLit(buf, "<align>\n");
+        virBufferAdjustIndent(buf, 2);
+        virBufferAsprintf(buf, "<size unit='KiB'>%llu</size>\n", def->align);
+        virBufferAdjustIndent(buf, -2);
+        virBufferAddLit(buf, "</align>\n");
+    }
+
     if (virDomainMemorySourceDefFormat(buf, def) < 0)
         return -1;
 
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index a78fdee..1155c84 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2107,6 +2107,7 @@ typedef enum {
 struct _virDomainMemoryDef {
     virDomainMemoryAccess access;
     virTristateBool discard;
+    unsigned long long align;
 
     /* source */
     virBitmapPtr sourceNodes;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index c423733..5862457 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3186,6 +3186,10 @@ qemuBuildMemoryBackendStr(virJSONValuePtr *backendProps,
     if (virJSONValueObjectAdd(props, "U:size", mem->size * 1024, NULL) < 0)
         goto cleanup;
 
+    if (mem->align &&
+        virJSONValueObjectAdd(props, "u:align", mem->align * 1024, NULL) < 0)
+        goto cleanup;
+
     if (mem->sourceNodes) {
         nodemask = mem->sourceNodes;
     } else {
diff --git a/tests/qemuxml2argvdata/memory-hotplug-nvdimm-align.args b/tests/qemuxml2argvdata/memory-hotplug-nvdimm-align.args
new file mode 100644
index 0000000..e6fcf58
--- /dev/null
+++ b/tests/qemuxml2argvdata/memory-hotplug-nvdimm-align.args
@@ -0,0 +1,31 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/home/test \
+USER=test \
+LOGNAME=test \
+QEMU_AUDIO_DRV=none \
+/usr/bin/qemu-system-i686 \
+-name QEMUGuest1 \
+-S \
+-machine pc,accel=tcg,usb=off,dump-guest-core=off,nvdimm=on \
+-m size=219136k,slots=16,maxmem=1099511627776k \
+-smp 2,sockets=2,cores=1,threads=1 \
+-numa node,nodeid=0,cpus=0-1,mem=214 \
+-object memory-backend-file,id=memnvdimm0,prealloc=yes,mem-path=/tmp/nvdimm,\
+share=no,size=536870912,align=2097152 \
+-device nvdimm,node=0,label-size=131072,memdev=memnvdimm0,id=nvdimm0,slot=0 \
+-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
+-display none \
+-no-user-config \
+-nodefaults \
+-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,\
+server,nowait \
+-mon chardev=charmonitor,id=monitor,mode=control \
+-rtc base=utc \
+-no-shutdown \
+-no-acpi \
+-boot c \
+-usb \
+-drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-0 \
+-device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \
+-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3
diff --git a/tests/qemuxml2argvdata/memory-hotplug-nvdimm-align.xml b/tests/qemuxml2argvdata/memory-hotplug-nvdimm-align.xml
new file mode 100644
index 0000000..aa9e99b
--- /dev/null
+++ b/tests/qemuxml2argvdata/memory-hotplug-nvdimm-align.xml
@@ -0,0 +1,63 @@
+<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-system-i686</emulator>
+    <disk type='block' device='disk'>
+      <driver name='qemu' type='raw'/>
+      <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'>
+      <align>
+        <size unit='KiB'>2048</size>
+      </align>
+      <source>
+        <path>/tmp/nvdimm</path>
+      </source>
+      <target>
+        <size unit='KiB'>523264</size>
+        <node>0</node>
+        <label>
+          <size unit='KiB'>128</size>
+        </label>
+      </target>
+      <address type='dimm' slot='0'/>
+    </memory>
+  </devices>
+</domain>
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 07e5ba1..4674ded 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -2588,6 +2588,9 @@ mymain(void)
     DO_TEST("memory-hotplug-nvdimm-label",
             QEMU_CAPS_DEVICE_NVDIMM,
             QEMU_CAPS_NUMA, QEMU_CAPS_OBJECT_MEMORY_RAM, QEMU_CAPS_OBJECT_MEMORY_FILE);
+    DO_TEST("memory-hotplug-nvdimm-align",
+            QEMU_CAPS_DEVICE_NVDIMM,
+            QEMU_CAPS_NUMA, QEMU_CAPS_OBJECT_MEMORY_RAM, QEMU_CAPS_OBJECT_MEMORY_FILE);
 
     DO_TEST("machine-aeskeywrap-on-caps",
             QEMU_CAPS_AES_KEY_WRAP,
diff --git a/tests/qemuxml2xmloutdata/memory-hotplug-nvdimm-align.xml b/tests/qemuxml2xmloutdata/memory-hotplug-nvdimm-align.xml
new file mode 100644
index 0000000..9fc6001
--- /dev/null
+++ b/tests/qemuxml2xmloutdata/memory-hotplug-nvdimm-align.xml
@@ -0,0 +1 @@
+../qemuxml2argvdata/memory-hotplug-nvdimm-align.xml
\ No newline at end of file
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index 7cedc2b..822e98a 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -1098,6 +1098,7 @@ mymain(void)
     DO_TEST("memory-hotplug-nvdimm", NONE);
     DO_TEST("memory-hotplug-nvdimm-access", NONE);
     DO_TEST("memory-hotplug-nvdimm-label", NONE);
+    DO_TEST("memory-hotplug-nvdimm-align", NONE);
     DO_TEST("net-udp", NONE);
 
     DO_TEST("video-virtio-gpu-device", NONE);
-- 
1.8.3.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] conf: Introduce align for hostmem-file
Posted by Ján Tomko 5 years, 10 months ago
On Tue, May 29, 2018 at 08:38:52PM +0800, Jie Wang wrote:
>QEMU has add the 'align' option to 'memory-backend-file'. Expose
>this option to users by new element align.
>
>Signed-off-by: Jie Wang <wangjie88@huawei.com>
>---
> docs/formatdomain.html.in                          | 18 +++++++
> docs/schemas/domaincommon.rng                      |  7 +++
> src/conf/domain_conf.c                             | 14 +++++
> src/conf/domain_conf.h                             |  1 +
> src/qemu/qemu_command.c                            |  4 ++
> .../memory-hotplug-nvdimm-align.args               | 31 +++++++++++
> .../memory-hotplug-nvdimm-align.xml                | 63 ++++++++++++++++++++++
> tests/qemuxml2argvtest.c                           |  3 ++
> .../memory-hotplug-nvdimm-align.xml                |  1 +
> tests/qemuxml2xmltest.c                            |  1 +
> 10 files changed, 143 insertions(+)
> create mode 100644 tests/qemuxml2argvdata/memory-hotplug-nvdimm-align.args
> create mode 100644 tests/qemuxml2argvdata/memory-hotplug-nvdimm-align.xml
> create mode 100644 tests/qemuxml2xmloutdata/memory-hotplug-nvdimm-align.xml

There's no need to cc: random people with patches:
https://libvirt.org/hacking.html

    As a rule, patches should be sent to the mailing list only: all developers
    are subscribed to libvir-list and read it regularly, so please don't CC
    individual developers unless they've explicitly asked you to.

Jano
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Ping Re: [PATCH] conf: Introduce align for hostmem-file
Posted by WangJie (Pluto) 5 years, 10 months ago
Ping?

On 2018/5/29 20:38, Jie Wang wrote:
> QEMU has add the 'align' option to 'memory-backend-file'. Expose
> this option to users by new element align.
> 
> Signed-off-by: Jie Wang <wangjie88@huawei.com>
> ---
>  docs/formatdomain.html.in                          | 18 +++++++
>  docs/schemas/domaincommon.rng                      |  7 +++
>  src/conf/domain_conf.c                             | 14 +++++
>  src/conf/domain_conf.h                             |  1 +
>  src/qemu/qemu_command.c                            |  4 ++
>  .../memory-hotplug-nvdimm-align.args               | 31 +++++++++++
>  .../memory-hotplug-nvdimm-align.xml                | 63 ++++++++++++++++++++++
>  tests/qemuxml2argvtest.c                           |  3 ++
>  .../memory-hotplug-nvdimm-align.xml                |  1 +
>  tests/qemuxml2xmltest.c                            |  1 +
>  10 files changed, 143 insertions(+)
>  create mode 100644 tests/qemuxml2argvdata/memory-hotplug-nvdimm-align.args
>  create mode 100644 tests/qemuxml2argvdata/memory-hotplug-nvdimm-align.xml
>  create mode 100644 tests/qemuxml2xmloutdata/memory-hotplug-nvdimm-align.xml
> 
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index 0d0fd3b..29fe145 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -7932,6 +7932,9 @@ qemu-kvm -net nic,model=? /dev/null
>      &lt;/target&gt;
>    &lt;/memory&gt;
>    &lt;memory model='nvdimm'&gt;
> +    &lt;align&gt;
> +      &lt;sieze unit='KiB'&gt;2048&lt;/size&gt;
> +    &lt;/align&gt;
>      &lt;source&gt;
>        &lt;path&gt;/tmp/nvdimm&lt;/path&gt;
>      &lt;/source&gt;
> @@ -7983,6 +7986,21 @@ qemu-kvm -net nic,model=? /dev/null
>          </p>
>        </dd>
>  
> +      <dt><code>align</code></dt>
> +      <dd>
> +        <p>
> +          For NVDIMM type devices one can optionally use
> +          <code>align</code> and its subelement <code>size</code>
> +          to configure the size of alignment 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 alignment must be multiples of page size 4KiB,</li>
> +        </ol>
> +      </dd>
> +
>        <dt><code>source</code></dt>
>        <dd>
>          <p>
> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
> index 71ac3d0..9e994b1 100644
> --- a/docs/schemas/domaincommon.rng
> +++ b/docs/schemas/domaincommon.rng
> @@ -5145,6 +5145,13 @@
>            <ref name="virYesNo"/>
>          </attribute>
>        </optional>
> +      <optional>
> +        <element name="align">
> +          <element name="size">
> +            <ref name="scaledInteger"/>
> +          </element>
> +        </element>
> +      </optional>
>        <interleave>
>          <optional>
>            <ref name="memorydev-source"/>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 3689ac0..bf91167 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -15739,6 +15739,12 @@ virDomainMemoryDefParseXML(virDomainXMLOptionPtr xmlopt,
>      }
>      VIR_FREE(tmp);
>  
> +    if ((node = virXPathNode("./align", ctxt))) {
> +        if (virDomainParseMemory("./align/size", "./align/size/@unit", ctxt,
> +                                 &def->align, true, false) < 0)
> +            goto error;
> +    }
> +
>      /* source */
>      if ((node = virXPathNode("./source", ctxt)) &&
>          virDomainMemorySourceDefParseXML(node, ctxt, def) < 0)
> @@ -25334,6 +25340,14 @@ virDomainMemoryDefFormat(virBufferPtr buf,
>      virBufferAddLit(buf, ">\n");
>      virBufferAdjustIndent(buf, 2);
>  
> +    if (def->align) {
> +        virBufferAddLit(buf, "<align>\n");
> +        virBufferAdjustIndent(buf, 2);
> +        virBufferAsprintf(buf, "<size unit='KiB'>%llu</size>\n", def->align);
> +        virBufferAdjustIndent(buf, -2);
> +        virBufferAddLit(buf, "</align>\n");
> +    }
> +
>      if (virDomainMemorySourceDefFormat(buf, def) < 0)
>          return -1;
>  
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index a78fdee..1155c84 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -2107,6 +2107,7 @@ typedef enum {
>  struct _virDomainMemoryDef {
>      virDomainMemoryAccess access;
>      virTristateBool discard;
> +    unsigned long long align;
>  
>      /* source */
>      virBitmapPtr sourceNodes;
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index c423733..5862457 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -3186,6 +3186,10 @@ qemuBuildMemoryBackendStr(virJSONValuePtr *backendProps,
>      if (virJSONValueObjectAdd(props, "U:size", mem->size * 1024, NULL) < 0)
>          goto cleanup;
>  
> +    if (mem->align &&
> +        virJSONValueObjectAdd(props, "u:align", mem->align * 1024, NULL) < 0)
> +        goto cleanup;
> +
>      if (mem->sourceNodes) {
>          nodemask = mem->sourceNodes;
>      } else {
> diff --git a/tests/qemuxml2argvdata/memory-hotplug-nvdimm-align.args b/tests/qemuxml2argvdata/memory-hotplug-nvdimm-align.args
> new file mode 100644
> index 0000000..e6fcf58
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/memory-hotplug-nvdimm-align.args
> @@ -0,0 +1,31 @@
> +LC_ALL=C \
> +PATH=/bin \
> +HOME=/home/test \
> +USER=test \
> +LOGNAME=test \
> +QEMU_AUDIO_DRV=none \
> +/usr/bin/qemu-system-i686 \
> +-name QEMUGuest1 \
> +-S \
> +-machine pc,accel=tcg,usb=off,dump-guest-core=off,nvdimm=on \
> +-m size=219136k,slots=16,maxmem=1099511627776k \
> +-smp 2,sockets=2,cores=1,threads=1 \
> +-numa node,nodeid=0,cpus=0-1,mem=214 \
> +-object memory-backend-file,id=memnvdimm0,prealloc=yes,mem-path=/tmp/nvdimm,\
> +share=no,size=536870912,align=2097152 \
> +-device nvdimm,node=0,label-size=131072,memdev=memnvdimm0,id=nvdimm0,slot=0 \
> +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
> +-display none \
> +-no-user-config \
> +-nodefaults \
> +-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,\
> +server,nowait \
> +-mon chardev=charmonitor,id=monitor,mode=control \
> +-rtc base=utc \
> +-no-shutdown \
> +-no-acpi \
> +-boot c \
> +-usb \
> +-drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-0 \
> +-device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \
> +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3
> diff --git a/tests/qemuxml2argvdata/memory-hotplug-nvdimm-align.xml b/tests/qemuxml2argvdata/memory-hotplug-nvdimm-align.xml
> new file mode 100644
> index 0000000..aa9e99b
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/memory-hotplug-nvdimm-align.xml
> @@ -0,0 +1,63 @@
> +<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-system-i686</emulator>
> +    <disk type='block' device='disk'>
> +      <driver name='qemu' type='raw'/>
> +      <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'>
> +      <align>
> +        <size unit='KiB'>2048</size>
> +      </align>
> +      <source>
> +        <path>/tmp/nvdimm</path>
> +      </source>
> +      <target>
> +        <size unit='KiB'>523264</size>
> +        <node>0</node>
> +        <label>
> +          <size unit='KiB'>128</size>
> +        </label>
> +      </target>
> +      <address type='dimm' slot='0'/>
> +    </memory>
> +  </devices>
> +</domain>
> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
> index 07e5ba1..4674ded 100644
> --- a/tests/qemuxml2argvtest.c
> +++ b/tests/qemuxml2argvtest.c
> @@ -2588,6 +2588,9 @@ mymain(void)
>      DO_TEST("memory-hotplug-nvdimm-label",
>              QEMU_CAPS_DEVICE_NVDIMM,
>              QEMU_CAPS_NUMA, QEMU_CAPS_OBJECT_MEMORY_RAM, QEMU_CAPS_OBJECT_MEMORY_FILE);
> +    DO_TEST("memory-hotplug-nvdimm-align",
> +            QEMU_CAPS_DEVICE_NVDIMM,
> +            QEMU_CAPS_NUMA, QEMU_CAPS_OBJECT_MEMORY_RAM, QEMU_CAPS_OBJECT_MEMORY_FILE);
>  
>      DO_TEST("machine-aeskeywrap-on-caps",
>              QEMU_CAPS_AES_KEY_WRAP,
> diff --git a/tests/qemuxml2xmloutdata/memory-hotplug-nvdimm-align.xml b/tests/qemuxml2xmloutdata/memory-hotplug-nvdimm-align.xml
> new file mode 100644
> index 0000000..9fc6001
> --- /dev/null
> +++ b/tests/qemuxml2xmloutdata/memory-hotplug-nvdimm-align.xml
> @@ -0,0 +1 @@
> +../qemuxml2argvdata/memory-hotplug-nvdimm-align.xml
> \ No newline at end of file
> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
> index 7cedc2b..822e98a 100644
> --- a/tests/qemuxml2xmltest.c
> +++ b/tests/qemuxml2xmltest.c
> @@ -1098,6 +1098,7 @@ mymain(void)
>      DO_TEST("memory-hotplug-nvdimm", NONE);
>      DO_TEST("memory-hotplug-nvdimm-access", NONE);
>      DO_TEST("memory-hotplug-nvdimm-label", NONE);
> +    DO_TEST("memory-hotplug-nvdimm-align", NONE);
>      DO_TEST("net-udp", NONE);
>  
>      DO_TEST("video-virtio-gpu-device", NONE);
> 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] conf: Introduce align for hostmem-file
Posted by John Ferlan 5 years, 10 months ago
[removed developers from To since they read libvir-list anyway]

On 05/29/2018 08:38 AM, Jie Wang wrote:
> QEMU has add the 'align' option to 'memory-backend-file'. Expose
> this option to users by new element align.
> 

Would perhaps be nice to have a few more details about what this is as
part of the commit message... and the expected usage.

I see that "align" was added in 2.12 by commit id '98376843', so you'll
need to add a capability for the switch; otherwise, supplying align=XXX
to a pre-2.12 emulator would probably fail, correct?

See libvirt commit '72c1770aa0' done for
"memory-backend-file.discard-data" for the methodology, but I expect the
next round to contain:

    { "align", QEMU_CAPS_OBJECT_MEMORY_FILE_ALIGN },

in virQEMUCapsObjectPropsMemoryBackendFile along with changes to the
various tests/qemucapabilitiesdata/caps_2.12*.xml files which you can
generate via "VIR_TEST_REGENERATE_OUTPUT=1 tests/qemucapabilitiestest"
once you have compiled the qemu_capabilities.{c,h} changes.


> Signed-off-by: Jie Wang <wangjie88@huawei.com>
> ---
>  docs/formatdomain.html.in                          | 18 +++++++
>  docs/schemas/domaincommon.rng                      |  7 +++
>  src/conf/domain_conf.c                             | 14 +++++
>  src/conf/domain_conf.h                             |  1 +
>  src/qemu/qemu_command.c                            |  4 ++
>  .../memory-hotplug-nvdimm-align.args               | 31 +++++++++++
>  .../memory-hotplug-nvdimm-align.xml                | 63 ++++++++++++++++++++++
>  tests/qemuxml2argvtest.c                           |  3 ++
>  .../memory-hotplug-nvdimm-align.xml                |  1 +
>  tests/qemuxml2xmltest.c                            |  1 +
>  10 files changed, 143 insertions(+)
>  create mode 100644 tests/qemuxml2argvdata/memory-hotplug-nvdimm-align.args
>  create mode 100644 tests/qemuxml2argvdata/memory-hotplug-nvdimm-align.xml
>  create mode 100644 tests/qemuxml2xmloutdata/memory-hotplug-nvdimm-align.xml
> 

Split the patches into domain_conf, formatdomain, xml2xml testing as one
patch and qemu_command, qemuxml2argv tests in another patch. In between
the two patches, sandwich in the qemu_capabilities changes. You'll also
need a patch 4 to adjust docs/news.xml to briefly describe the enhancement.

Before reposting, please be sure to pass the "make [-j N] check
syntax-check" as check fails with:

FAIL: virschematest
FAIL: qemuxml2xmltest

and syntax-check fails with (at least):
tests/qemuxml2xmloutdata/memory-hotplug-nvdimm-align.xml
maint.mk: empty line(s) or no newline at EOF
make: *** [maint.mk:930: sc_prohibit_empty_lines_at_EOF] Error 1
make: *** Waiting for unfinished jobs....

	
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index 0d0fd3b..29fe145 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -7932,6 +7932,9 @@ qemu-kvm -net nic,model=? /dev/null
>      &lt;/target&gt;
>    &lt;/memory&gt;
>    &lt;memory model='nvdimm'&gt;
> +    &lt;align&gt;
> +      &lt;sieze unit='KiB'&gt;2048&lt;/size&gt;

s/sieze/size/

> +    &lt;/align&gt;

[1] see notes regarding formatting...

>      &lt;source&gt;
>        &lt;path&gt;/tmp/nvdimm&lt;/path&gt;
>      &lt;/source&gt;
> @@ -7983,6 +7986,21 @@ qemu-kvm -net nic,model=? /dev/null
>          </p>
>        </dd>
>  
> +      <dt><code>align</code></dt>
> +      <dd>
> +        <p>
> +          For NVDIMM type devices one can optionally use
> +          <code>align</code> and its subelement <code>size</code>
> +          to configure the size of alignment within the NVDIMM module.
> +          The <code>size</code> element has usual meaning described
> +          <a href="#elementsMemoryAllocation">here</a>.

I see you copied the NVDIMM target size description, but I think it's
better described using something like:

"The size of the XXXX. The value by default is in kilobytes, but the
unit attribute can be used to scale the value."

Search around the formatdomain.html.in for other uses of size for examples.

Beyond that, the description in/from the qemu commit regarding how this
is used, might be good to at least paraphrase in the libvirt description.

> +          For QEMU domains the following restrictions apply:
> +        </p>
> +        <ol>
> +          <li>the alignment must be multiples of page size 4KiB,</li>
> +        </ol>

Strange to just have 1 element in a list - in that case, just indicate
that for QEMU domains the alignment size must be...

BTW: I would have expected in some qemu_domain post parse function to
see a VIR_ROUND_UP for this or a comparison forcing failure because the
value isn't aligned properly. Hint, qemuDomainGetMemorySizeAlignment

> +      </dd>
> +
>        <dt><code>source</code></dt>
>        <dd>
>          <p>
> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
> index 71ac3d0..9e994b1 100644
> --- a/docs/schemas/domaincommon.rng
> +++ b/docs/schemas/domaincommon.rng
> @@ -5145,6 +5145,13 @@
>            <ref name="virYesNo"/>
>          </attribute>
>        </optional>
> +      <optional>
> +        <element name="align">
> +          <element name="size">
> +            <ref name="scaledInteger"/>
> +          </element>
> +        </element>
> +      </optional>

[1]
This should just follow the "pagesize" value. There's no need for:

  <align>
    <size unit='KiB'>2048</size>
  </align>

it should be :

  <align unit='KiB'>2048</align>

and it should be a child of <source>, just like <pagesize>

Thus changing:

  <group>
    <element name="path">
      <ref name="absFilePath"/>
    </element>
  </group>

to be:

  <group>
    <interleave>
      <element name="path">
        <ref name="absFilePath"/>
      </element>
      <optional>
        <element name="align">
          <ref name="scaledInteger"/>
        </element>
      </optional>
    </interleave>
  </group>

That way it's *very obvious* that this align is only for nvdimm

>        <interleave>
>          <optional>
>            <ref name="memorydev-source"/>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 3689ac0..bf91167 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -15739,6 +15739,12 @@ virDomainMemoryDefParseXML(virDomainXMLOptionPtr xmlopt,
>      }
>      VIR_FREE(tmp);
>  
> +    if ((node = virXPathNode("./align", ctxt))) {
> +        if (virDomainParseMemory("./align/size", "./align/size/@unit", ctxt,
> +                                 &def->align, true, false) < 0)
> +            goto error;
> +    }
> +

This then moves into the source parsing.

>      /* source */
>      if ((node = virXPathNode("./source", ctxt)) &&
>          virDomainMemorySourceDefParseXML(node, ctxt, def) < 0)
> @@ -25334,6 +25340,14 @@ virDomainMemoryDefFormat(virBufferPtr buf,
>      virBufferAddLit(buf, ">\n");
>      virBufferAdjustIndent(buf, 2);
>  
> +    if (def->align) {

I prefer, def->align > 0 - the value isn't a bool or a pointer.

> +        virBufferAddLit(buf, "<align>\n");
> +        virBufferAdjustIndent(buf, 2);
> +        virBufferAsprintf(buf, "<size unit='KiB'>%llu</size>\n", def->align);
> +        virBufferAdjustIndent(buf, -2);
> +        virBufferAddLit(buf, "</align>\n");
> +    }
> +

and all of this moves into source def format

>      if (virDomainMemorySourceDefFormat(buf, def) < 0)
>          return -1;
>  

Should virDomainMemoryDefCheckABIStability ensure that align sizes
between @src and @dst match?

Use cscope to search the sources for VIR_DOMAIN_MEMORY_MODEL_NVDIMM to
make sure there's no other places that may need adjustment... I think
you probably need a change to the virDomainMemoryDefValidate to check if
align is supplied for memory types other than nvdimm and then fail, alt

> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index a78fdee..1155c84 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -2107,6 +2107,7 @@ typedef enum {
>  struct _virDomainMemoryDef {
>      virDomainMemoryAccess access;
>      virTristateBool discard;
> +    unsigned long long align;

Since this alignment only for NVDIMM, it should be renamed to
nvdimmAlign and put under nvdimmPath.

>  
>      /* source */
>      virBitmapPtr sourceNodes;
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index c423733..5862457 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -3186,6 +3186,10 @@ qemuBuildMemoryBackendStr(virJSONValuePtr *backendProps,
>      if (virJSONValueObjectAdd(props, "U:size", mem->size * 1024, NULL) < 0)
>          goto cleanup;
>  
> +    if (mem->align &&
> +        virJSONValueObjectAdd(props, "u:align", mem->align * 1024, NULL) < 0)

If you use "U:align" you don't need the mem->align condition.

> +        goto cleanup;
> +
>      if (mem->sourceNodes) {
>          nodemask = mem->sourceNodes;
>      } else {
> diff --git a/tests/qemuxml2argvdata/memory-hotplug-nvdimm-align.args b/tests/qemuxml2argvdata/memory-hotplug-nvdimm-align.args
> new file mode 100644
> index 0000000..e6fcf58
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/memory-hotplug-nvdimm-align.args
> @@ -0,0 +1,31 @@
> +LC_ALL=C \
> +PATH=/bin \
> +HOME=/home/test \
> +USER=test \
> +LOGNAME=test \
> +QEMU_AUDIO_DRV=none \
> +/usr/bin/qemu-system-i686 \
> +-name QEMUGuest1 \
> +-S \
> +-machine pc,accel=tcg,usb=off,dump-guest-core=off,nvdimm=on \
> +-m size=219136k,slots=16,maxmem=1099511627776k \
> +-smp 2,sockets=2,cores=1,threads=1 \
> +-numa node,nodeid=0,cpus=0-1,mem=214 \
> +-object memory-backend-file,id=memnvdimm0,prealloc=yes,mem-path=/tmp/nvdimm,\
> +share=no,size=536870912,align=2097152 \
> +-device nvdimm,node=0,label-size=131072,memdev=memnvdimm0,id=nvdimm0,slot=0 \
> +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
> +-display none \
> +-no-user-config \
> +-nodefaults \
> +-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,\
> +server,nowait \
> +-mon chardev=charmonitor,id=monitor,mode=control \
> +-rtc base=utc \
> +-no-shutdown \
> +-no-acpi \
> +-boot c \
> +-usb \
> +-drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-0 \
> +-device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \
> +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3
> diff --git a/tests/qemuxml2argvdata/memory-hotplug-nvdimm-align.xml b/tests/qemuxml2argvdata/memory-hotplug-nvdimm-align.xml
> new file mode 100644
> index 0000000..aa9e99b
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/memory-hotplug-nvdimm-align.xml
> @@ -0,0 +1,63 @@
> +<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-system-i686</emulator>
> +    <disk type='block' device='disk'>
> +      <driver name='qemu' type='raw'/>
> +      <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'>
> +      <align>
> +        <size unit='KiB'>2048</size>
> +      </align>

[1] From above will change this...

> +      <source>
> +        <path>/tmp/nvdimm</path>
> +      </source>
> +      <target>
> +        <size unit='KiB'>523264</size>
> +        <node>0</node>
> +        <label>
> +          <size unit='KiB'>128</size>
> +        </label>
> +      </target>
> +      <address type='dimm' slot='0'/>
> +    </memory>
> +  </devices>
> +</domain>
> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
> index 07e5ba1..4674ded 100644
> --- a/tests/qemuxml2argvtest.c
> +++ b/tests/qemuxml2argvtest.c
> @@ -2588,6 +2588,9 @@ mymain(void)
>      DO_TEST("memory-hotplug-nvdimm-label",
>              QEMU_CAPS_DEVICE_NVDIMM,
>              QEMU_CAPS_NUMA, QEMU_CAPS_OBJECT_MEMORY_RAM, QEMU_CAPS_OBJECT_MEMORY_FILE);
> +    DO_TEST("memory-hotplug-nvdimm-align",
> +            QEMU_CAPS_DEVICE_NVDIMM,
> +            QEMU_CAPS_NUMA, QEMU_CAPS_OBJECT_MEMORY_RAM, QEMU_CAPS_OBJECT_MEMORY_FILE);

Use DO_TEST_CAPS_LATEST

>  
>      DO_TEST("machine-aeskeywrap-on-caps",
>              QEMU_CAPS_AES_KEY_WRAP,
> diff --git a/tests/qemuxml2xmloutdata/memory-hotplug-nvdimm-align.xml b/tests/qemuxml2xmloutdata/memory-hotplug-nvdimm-align.xml
> new file mode 100644
> index 0000000..9fc6001
> --- /dev/null
> +++ b/tests/qemuxml2xmloutdata/memory-hotplug-nvdimm-align.xml
> @@ -0,0 +1 @@
> +../qemuxml2argvdata/memory-hotplug-nvdimm-align.xml
> \ No newline at end of file

This isn't a link to the file - the contents of the file is a link...
That's probably why schematest and xml2xmltest fail

John

No need to CC me on the next series, I follow the list. As time permits
in my schedule, I do reviews unless someone else gets to them before me.

> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
> index 7cedc2b..822e98a 100644
> --- a/tests/qemuxml2xmltest.c
> +++ b/tests/qemuxml2xmltest.c
> @@ -1098,6 +1098,7 @@ mymain(void)
>      DO_TEST("memory-hotplug-nvdimm", NONE);
>      DO_TEST("memory-hotplug-nvdimm-access", NONE);
>      DO_TEST("memory-hotplug-nvdimm-label", NONE);
> +    DO_TEST("memory-hotplug-nvdimm-align", NONE);
>      DO_TEST("net-udp", NONE);
>  
>      DO_TEST("video-virtio-gpu-device", NONE);
> 

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