[libvirt] [PATCH v2 07/14] qemu: Introduce label-size for NVDIMMs
John Ferlan
jferlan at redhat.com
Tue Mar 7 16:55:43 UTC 2017
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 at 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);
>
More information about the libvir-list
mailing list