For NVDIMM devices it is optionally possible to specify the size
of internal storage for namespaces. Namespaces are a feature that
allows users to partition the NVDIMM for different uses.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
docs/formatdomain.html.in | 9 ++++
docs/schemas/domaincommon.rng | 7 +++
src/conf/domain_conf.c | 19 +++++++
src/conf/domain_conf.h | 1 +
src/qemu/qemu_command.c | 3 ++
.../qemuxml2argv-memory-hotplug-nvdimm-label.args | 26 ++++++++++
.../qemuxml2argv-memory-hotplug-nvdimm-label.xml | 59 ++++++++++++++++++++++
tests/qemuxml2argvtest.c | 2 +
.../qemuxml2xmlout-memory-hotplug-nvdimm-label.xml | 1 +
tests/qemuxml2xmltest.c | 1 +
10 files changed, 128 insertions(+)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug-nvdimm-label.args
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug-nvdimm-label.xml
create mode 120000 tests/qemuxml2xmloutdata/qemuxml2xmlout-memory-hotplug-nvdimm-label.xml
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 00c0df2ce..4a078b577 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -7038,6 +7038,9 @@ qemu-kvm -net nic,model=? /dev/null
<target>
<size unit='KiB'>524288</size>
<node>1</node>
+ <label>
+ <size unit='KiB'>128</size>
+ </label>
</target>
</memory>
</devices>
@@ -7117,6 +7120,12 @@ qemu-kvm -net nic,model=? /dev/null
attach the memory to. The element shall be used only if the guest has
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.
+ </p>
</dd>
</dl>
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 78195aae9..aafc731f5 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -4800,6 +4800,13 @@
<ref name="unsignedInt"/>
</element>
</optional>
+ <optional>
+ <element name="label">
+ <element name="size">
+ <ref name="scaledInteger"/>
+ </element>
+ </element>
+ </optional>
</interleave>
</element>
</define>
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index a31114cc7..7840f8e02 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -13824,6 +13824,18 @@ virDomainMemoryTargetDefParseXML(xmlNodePtr node,
&def->size, true, false) < 0)
goto cleanup;
+ if (def->model == VIR_DOMAIN_MEMORY_MODEL_NVDIMM) {
+ if (virDomainParseMemory("./label/size", "./label/size/@unit", ctxt,
+ &def->labelsize, false, false) < 0)
+ goto cleanup;
+
+ if (def->labelsize && def->labelsize < 128) {
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("nvdimm label must be at least 128KiB"));
+ goto cleanup;
+ }
+ }
+
ret = 0;
cleanup:
@@ -22663,6 +22675,13 @@ virDomainMemoryTargetDefFormat(virBufferPtr buf,
virBufferAsprintf(buf, "<size unit='KiB'>%llu</size>\n", def->size);
if (def->targetNode >= 0)
virBufferAsprintf(buf, "<node>%d</node>\n", def->targetNode);
+ if (def->labelsize) {
+ virBufferAddLit(buf, "<label>\n");
+ virBufferAdjustIndent(buf, 2);
+ virBufferAsprintf(buf, "<size unit='KiB'>%llu</size>\n", def->labelsize);
+ virBufferAdjustIndent(buf, -2);
+ virBufferAddLit(buf, "</label>\n");
+ }
virBufferAdjustIndent(buf, -2);
virBufferAddLit(buf, "</target>\n");
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 87516ca69..0e68f5770 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2013,6 +2013,7 @@ struct _virDomainMemoryDef {
int model; /* virDomainMemoryModel */
int targetNode;
unsigned long long size; /* kibibytes */
+ unsigned long long labelsize; /* kibibytes; valid only for NVDIMM */
virDomainDeviceInfo info;
};
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 287387055..91ace8e38 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3440,6 +3440,9 @@ qemuBuildMemoryDeviceStr(virDomainMemoryDefPtr mem)
if (mem->targetNode >= 0)
virBufferAsprintf(&buf, "node=%d,", mem->targetNode);
+ if (mem->labelsize)
+ virBufferAsprintf(&buf, "label-size=%llu,", mem->labelsize * 1024);
+
virBufferAsprintf(&buf, "memdev=mem%s,id=%s",
mem->info.alias, mem->info.alias);
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug-nvdimm-label.args b/tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug-nvdimm-label.args
new file mode 100644
index 000000000..8669f2d84
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug-nvdimm-label.args
@@ -0,0 +1,26 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/home/test \
+USER=test \
+LOGNAME=test \
+QEMU_AUDIO_DRV=none \
+/usr/bin/qemu \
+-name QEMUGuest1 \
+-S \
+-machine pc,accel=tcg,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 \
+-device nvdimm,node=0,label-size=131072,memdev=memnvdimm0,id=nvdimm0,slot=0 \
+-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
+-nographic \
+-nodefaults \
+-monitor unix:/tmp/lib/domain--1-QEMUGuest1/monitor.sock,server,nowait \
+-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/qemuxml2argv-memory-hotplug-nvdimm-label.xml b/tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug-nvdimm-label.xml
new file mode 100644
index 000000000..425385251
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug-nvdimm-label.xml
@@ -0,0 +1,59 @@
+<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>
+ <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 97a2c55cd..5c05556a3 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -2337,6 +2337,8 @@ mymain(void)
QEMU_CAPS_NUMA, QEMU_CAPS_OBJECT_MEMORY_RAM, QEMU_CAPS_OBJECT_MEMORY_FILE);
DO_TEST("memory-hotplug-nvdimm-access", QEMU_CAPS_MACHINE_OPT, QEMU_CAPS_DEVICE_NVDIMM,
QEMU_CAPS_NUMA, QEMU_CAPS_OBJECT_MEMORY_RAM, QEMU_CAPS_OBJECT_MEMORY_FILE);
+ DO_TEST("memory-hotplug-nvdimm-label", QEMU_CAPS_MACHINE_OPT, 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_MACHINE_OPT, QEMU_CAPS_AES_KEY_WRAP,
diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-memory-hotplug-nvdimm-label.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-memory-hotplug-nvdimm-label.xml
new file mode 120000
index 000000000..e357ec582
--- /dev/null
+++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-memory-hotplug-nvdimm-label.xml
@@ -0,0 +1 @@
+/home/zippy/work/libvirt/libvirt.git/tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug-nvdimm-label.xml
\ No newline at end of file
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index ef49a79ca..bf62dbbb2 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -1080,6 +1080,7 @@ mymain(void)
DO_TEST("memory-hotplug-dimm", NONE);
DO_TEST("memory-hotplug-nvdimm", NONE);
DO_TEST("memory-hotplug-nvdimm-access", NONE);
+ DO_TEST("memory-hotplug-nvdimm-label", 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
On 02/27/2017 08:19 AM, Michal Privoznik wrote:
> For NVDIMM devices it is optionally possible to specify the size
> of internal storage for namespaces. Namespaces are a feature that
> allows users to partition the NVDIMM for different uses.
>
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
> docs/formatdomain.html.in | 9 ++++
> docs/schemas/domaincommon.rng | 7 +++
> src/conf/domain_conf.c | 19 +++++++
> src/conf/domain_conf.h | 1 +
> src/qemu/qemu_command.c | 3 ++
> .../qemuxml2argv-memory-hotplug-nvdimm-label.args | 26 ++++++++++
> .../qemuxml2argv-memory-hotplug-nvdimm-label.xml | 59 ++++++++++++++++++++++
> tests/qemuxml2argvtest.c | 2 +
> .../qemuxml2xmlout-memory-hotplug-nvdimm-label.xml | 1 +
> tests/qemuxml2xmltest.c | 1 +
> 10 files changed, 128 insertions(+)
> create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug-nvdimm-label.args
> create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug-nvdimm-label.xml
> create mode 120000 tests/qemuxml2xmloutdata/qemuxml2xmlout-memory-hotplug-nvdimm-label.xml
>
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index 00c0df2ce..4a078b577 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -7038,6 +7038,9 @@ qemu-kvm -net nic,model=? /dev/null
> <target>
> <size unit='KiB'>524288</size>
> <node>1</node>
> + <label>
> + <size unit='KiB'>128</size>
> + </label>
> </target>
> </memory>
> </devices>
> @@ -7117,6 +7120,12 @@ qemu-kvm -net nic,model=? /dev/null
> attach the memory to. The element shall be used only if the guest has
> 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.
and the "unit=" is also required, right?
> + </p>
> </dd>
> </dl>
>
> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
> index 78195aae9..aafc731f5 100644
> --- a/docs/schemas/domaincommon.rng
> +++ b/docs/schemas/domaincommon.rng
> @@ -4800,6 +4800,13 @@
> <ref name="unsignedInt"/>
> </element>
> </optional>
> + <optional>
> + <element name="label">
> + <element name="size">
> + <ref name="scaledInteger"/>
> + </element>
> + </element>
> + </optional>
> </interleave>
> </element>
> </define>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index a31114cc7..7840f8e02 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -13824,6 +13824,18 @@ virDomainMemoryTargetDefParseXML(xmlNodePtr node,
> &def->size, true, false) < 0)
> goto cleanup;
>
> + if (def->model == VIR_DOMAIN_MEMORY_MODEL_NVDIMM) {
> + if (virDomainParseMemory("./label/size", "./label/size/@unit", ctxt,
> + &def->labelsize, false, false) < 0)
So one could provide a fairly large size for this?
Why is '@required' false?
> + goto cleanup;
> +
> + if (def->labelsize && def->labelsize < 128) {
This makes it seem labelsize is optional, but it's not clear (to me at
least) from the description above whether must provide the size as well
as the label.
Of course as I read on - labelsize is expected.... Let's face it, if
label is provided and labelsize is 0, then well not much is going to be
allowed is it?
> + virReportError(VIR_ERR_XML_ERROR, "%s",
> + _("nvdimm label must be at least 128KiB"));
> + goto cleanup;
> + }
> + }
> +
> ret = 0;
>
> cleanup:
> @@ -22663,6 +22675,13 @@ virDomainMemoryTargetDefFormat(virBufferPtr buf,
> virBufferAsprintf(buf, "<size unit='KiB'>%llu</size>\n", def->size);
> if (def->targetNode >= 0)
> virBufferAsprintf(buf, "<node>%d</node>\n", def->targetNode);
> + if (def->labelsize) {
> + virBufferAddLit(buf, "<label>\n");
> + virBufferAdjustIndent(buf, 2);
> + virBufferAsprintf(buf, "<size unit='KiB'>%llu</size>\n", def->labelsize);
There's no check here if labelsize wasn't provided.
> + virBufferAdjustIndent(buf, -2);
> + virBufferAddLit(buf, "</label>\n");
> + }
>
> virBufferAdjustIndent(buf, -2);
> virBufferAddLit(buf, "</target>\n");
Similar comments from before about ABI...
Additionally if NVDIMM's are included in the total memory allocation's
(from my comments in patch2), wouldn't the size of this label need to
also be included?
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index 87516ca69..0e68f5770 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -2013,6 +2013,7 @@ struct _virDomainMemoryDef {
> int model; /* virDomainMemoryModel */
> int targetNode;
> unsigned long long size; /* kibibytes */
> + unsigned long long labelsize; /* kibibytes; valid only for NVDIMM */
>
> virDomainDeviceInfo info;
> };
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 287387055..91ace8e38 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -3440,6 +3440,9 @@ qemuBuildMemoryDeviceStr(virDomainMemoryDefPtr mem)
> if (mem->targetNode >= 0)
> virBufferAsprintf(&buf, "node=%d,", mem->targetNode);
>
> + if (mem->labelsize)
> + virBufferAsprintf(&buf, "label-size=%llu,", mem->labelsize * 1024);
> +
And this code checks for labelsize using the "assumption" (I suppose
that if label, then we have a labelsize too.
John
> virBufferAsprintf(&buf, "memdev=mem%s,id=%s",
> mem->info.alias, mem->info.alias);
>
> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug-nvdimm-label.args b/tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug-nvdimm-label.args
> new file mode 100644
> index 000000000..8669f2d84
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug-nvdimm-label.args
> @@ -0,0 +1,26 @@
> +LC_ALL=C \
> +PATH=/bin \
> +HOME=/home/test \
> +USER=test \
> +LOGNAME=test \
> +QEMU_AUDIO_DRV=none \
> +/usr/bin/qemu \
> +-name QEMUGuest1 \
> +-S \
> +-machine pc,accel=tcg,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 \
> +-device nvdimm,node=0,label-size=131072,memdev=memnvdimm0,id=nvdimm0,slot=0 \
> +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
> +-nographic \
> +-nodefaults \
> +-monitor unix:/tmp/lib/domain--1-QEMUGuest1/monitor.sock,server,nowait \
> +-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/qemuxml2argv-memory-hotplug-nvdimm-label.xml b/tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug-nvdimm-label.xml
> new file mode 100644
> index 000000000..425385251
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug-nvdimm-label.xml
> @@ -0,0 +1,59 @@
> +<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>
> + <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 97a2c55cd..5c05556a3 100644
> --- a/tests/qemuxml2argvtest.c
> +++ b/tests/qemuxml2argvtest.c
> @@ -2337,6 +2337,8 @@ mymain(void)
> QEMU_CAPS_NUMA, QEMU_CAPS_OBJECT_MEMORY_RAM, QEMU_CAPS_OBJECT_MEMORY_FILE);
> DO_TEST("memory-hotplug-nvdimm-access", QEMU_CAPS_MACHINE_OPT, QEMU_CAPS_DEVICE_NVDIMM,
> QEMU_CAPS_NUMA, QEMU_CAPS_OBJECT_MEMORY_RAM, QEMU_CAPS_OBJECT_MEMORY_FILE);
> + DO_TEST("memory-hotplug-nvdimm-label", QEMU_CAPS_MACHINE_OPT, 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_MACHINE_OPT, QEMU_CAPS_AES_KEY_WRAP,
> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-memory-hotplug-nvdimm-label.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-memory-hotplug-nvdimm-label.xml
> new file mode 120000
> index 000000000..e357ec582
> --- /dev/null
> +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-memory-hotplug-nvdimm-label.xml
> @@ -0,0 +1 @@
> +/home/zippy/work/libvirt/libvirt.git/tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug-nvdimm-label.xml
> \ No newline at end of file
> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
> index ef49a79ca..bf62dbbb2 100644
> --- a/tests/qemuxml2xmltest.c
> +++ b/tests/qemuxml2xmltest.c
> @@ -1080,6 +1080,7 @@ mymain(void)
> DO_TEST("memory-hotplug-dimm", NONE);
> DO_TEST("memory-hotplug-nvdimm", NONE);
> DO_TEST("memory-hotplug-nvdimm-access", NONE);
> + DO_TEST("memory-hotplug-nvdimm-label", 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
On 03/07/2017 05:55 PM, John Ferlan wrote:
>
>
> On 02/27/2017 08:19 AM, Michal Privoznik wrote:
>> For NVDIMM devices it is optionally possible to specify the size
>> of internal storage for namespaces. Namespaces are a feature that
>> allows users to partition the NVDIMM for different uses.
>>
>> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
>> ---
>> docs/formatdomain.html.in | 9 ++++
>> docs/schemas/domaincommon.rng | 7 +++
>> src/conf/domain_conf.c | 19 +++++++
>> src/conf/domain_conf.h | 1 +
>> src/qemu/qemu_command.c | 3 ++
>> .../qemuxml2argv-memory-hotplug-nvdimm-label.args | 26 ++++++++++
>> .../qemuxml2argv-memory-hotplug-nvdimm-label.xml | 59 ++++++++++++++++++++++
>> tests/qemuxml2argvtest.c | 2 +
>> .../qemuxml2xmlout-memory-hotplug-nvdimm-label.xml | 1 +
>> tests/qemuxml2xmltest.c | 1 +
>> 10 files changed, 128 insertions(+)
>> create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug-nvdimm-label.args
>> create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug-nvdimm-label.xml
>> create mode 120000 tests/qemuxml2xmloutdata/qemuxml2xmlout-memory-hotplug-nvdimm-label.xml
>>
>> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
>> index 00c0df2ce..4a078b577 100644
>> --- a/docs/formatdomain.html.in
>> +++ b/docs/formatdomain.html.in
>> @@ -7038,6 +7038,9 @@ qemu-kvm -net nic,model=? /dev/null
>> <target>
>> <size unit='KiB'>524288</size>
>> <node>1</node>
>> + <label>
>> + <size unit='KiB'>128</size>
>> + </label>
>> </target>
>> </memory>
>> </devices>
>> @@ -7117,6 +7120,12 @@ qemu-kvm -net nic,model=? /dev/null
>> attach the memory to. The element shall be used only if the guest has
>> 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.
>
> and the "unit=" is also required, right?
No. By default the unit is KiB. If users want to use different one, they
have to specify it in @unit attribute. This is just like domain memory.
That's why we can use the same parsing function.
>
>> + </p>
>> </dd>
>> </dl>
>>
>> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
>> index 78195aae9..aafc731f5 100644
>> --- a/docs/schemas/domaincommon.rng
>> +++ b/docs/schemas/domaincommon.rng
>> @@ -4800,6 +4800,13 @@
>> <ref name="unsignedInt"/>
>> </element>
>> </optional>
>> + <optional>
>> + <element name="label">
>> + <element name="size">
>> + <ref name="scaledInteger"/>
>> + </element>
>> + </element>
>> + </optional>
>> </interleave>
>> </element>
>> </define>
>> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>> index a31114cc7..7840f8e02 100644
>> --- a/src/conf/domain_conf.c
>> +++ b/src/conf/domain_conf.c
>> @@ -13824,6 +13824,18 @@ virDomainMemoryTargetDefParseXML(xmlNodePtr node,
>> &def->size, true, false) < 0)
>> goto cleanup;
>>
>> + if (def->model == VIR_DOMAIN_MEMORY_MODEL_NVDIMM) {
>> + if (virDomainParseMemory("./label/size", "./label/size/@unit", ctxt,
>> + &def->labelsize, false, false) < 0)
>
> So one could provide a fairly large size for this?
Yeah, if they have enough (disk/actual nvdimm) space. Now that I'm
thinking about this, maybe we can at least check if the label-size is
not greater than the size of nvdimm itself.
>
> Why is '@required' false?
Because it is not required to specify label size.
>
>> + goto cleanup;
>> +
>> + if (def->labelsize && def->labelsize < 128) {
>
> This makes it seem labelsize is optional, but it's not clear (to me at
> least) from the description above whether must provide the size as well
> as the label.
>
> Of course as I read on - labelsize is expected.... Let's face it, if
> label is provided and labelsize is 0, then well not much is going to be
> allowed is it?
I think you can still use it as a data storage inside the guest. Labels
were not introduced in qemu at the same time as NVDIMM, so clearly you
can use NVDIMMs even without labels.
>
>
>> + virReportError(VIR_ERR_XML_ERROR, "%s",
>> + _("nvdimm label must be at least 128KiB"));
>> + goto cleanup;
>> + }
>> + }
>> +
>> ret = 0;
>>
>> cleanup:
>> @@ -22663,6 +22675,13 @@ virDomainMemoryTargetDefFormat(virBufferPtr buf,
>> virBufferAsprintf(buf, "<size unit='KiB'>%llu</size>\n", def->size);
>> if (def->targetNode >= 0)
>> virBufferAsprintf(buf, "<node>%d</node>\n", def->targetNode);
>> + if (def->labelsize) {
>> + virBufferAddLit(buf, "<label>\n");
>> + virBufferAdjustIndent(buf, 2);
>> + virBufferAsprintf(buf, "<size unit='KiB'>%llu</size>\n", def->labelsize);
>
> There's no check here if labelsize wasn't provided.
Yes, there is.
>
>> + virBufferAdjustIndent(buf, -2);
>> + virBufferAddLit(buf, "</label>\n");
>> + }
>>
>> virBufferAdjustIndent(buf, -2);
>> virBufferAddLit(buf, "</target>\n");
>
> Similar comments from before about ABI...
This might actually require ABI check as the label-size is visible
inside the guest. Will update it.
>
> Additionally if NVDIMM's are included in the total memory allocation's
> (from my comments in patch2), wouldn't the size of this label need to
> also be included?
>
No. This is not some additional memory. Label size specifies the size of
a portion of NVDIMM that is used to store labels. For instance, if I
have 512MiB NVDIMM and set label-size to 2MiB, there's only 510MiB
available for data. If the label-size is 128MiB then there's only 128MiB
left for data.
>> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
>> index 87516ca69..0e68f5770 100644
>> --- a/src/conf/domain_conf.h
>> +++ b/src/conf/domain_conf.h
>> @@ -2013,6 +2013,7 @@ struct _virDomainMemoryDef {
>> int model; /* virDomainMemoryModel */
>> int targetNode;
>> unsigned long long size; /* kibibytes */
>> + unsigned long long labelsize; /* kibibytes; valid only for NVDIMM */
>>
>> virDomainDeviceInfo info;
>> };
>> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
>> index 287387055..91ace8e38 100644
>> --- a/src/qemu/qemu_command.c
>> +++ b/src/qemu/qemu_command.c
>> @@ -3440,6 +3440,9 @@ qemuBuildMemoryDeviceStr(virDomainMemoryDefPtr mem)
>> if (mem->targetNode >= 0)
>> virBufferAsprintf(&buf, "node=%d,", mem->targetNode);
>>
>> + if (mem->labelsize)
>> + virBufferAsprintf(&buf, "label-size=%llu,", mem->labelsize * 1024);
>> +
>
> And this code checks for labelsize using the "assumption" (I suppose
> that if label, then we have a labelsize too.
No. If mem->labelsize then mem->labelsize. Or maybe I'm misunderstanding
something?
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2026 Red Hat, Inc.