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(a)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);