[PATCH v2 0/5] NVDIMM suport for pSeries guests
changes in v2, all of them affecting just pSeries guests: - added 'label' requirement - added code to align down the NVDIMM device previous version: https://www.redhat.com/archives/libvir-list/2020-March/msg00165.html This patch series adds NVDIMM suport for ppc64 guests, which consists on adding an extra 'uuid' element in the nvdimm command line and the target label size must always be provided in the memory definition. No changes were made in the existing NVDIMM support for x86 and other archs. Daniel Henrique Barboza (5): conf: Introduce optional 'uuid' element for NVDIMM memory formatdomain.html.in: document the new 'uuid' NVDIMM element conf, qemu: enable NVDIMM support for ppc64 formatdomain.html.in: document NVDIMM 'label' requirement for pSeries news.xml: document the new NVDIMM support for Pseries guests docs/formatdomain.html.in | 24 +++++++-- docs/news.xml | 11 ++++ docs/schemas/domaincommon.rng | 5 ++ src/conf/domain_conf.c | 44 ++++++++++++++-- src/conf/domain_conf.h | 3 ++ src/qemu/qemu_command.c | 7 +++ src/qemu/qemu_domain.c | 47 +++++++++++++++-- .../memory-hotplug-nvdimm-ppc64.args | 32 ++++++++++++ .../memory-hotplug-nvdimm-ppc64.xml | 50 +++++++++++++++++++ tests/qemuxml2argvtest.c | 4 ++ .../memory-hotplug-nvdimm-ppc64.xml | 50 +++++++++++++++++++ tests/qemuxml2xmltest.c | 2 + 12 files changed, 268 insertions(+), 11 deletions(-) create mode 100644 tests/qemuxml2argvdata/memory-hotplug-nvdimm-ppc64.args create mode 100644 tests/qemuxml2argvdata/memory-hotplug-nvdimm-ppc64.xml create mode 100644 tests/qemuxml2xmloutdata/memory-hotplug-nvdimm-ppc64.xml -- 2.24.1
ppc64 NVDIMM support was implemented in QEMU by commit [1]. The support is similar to what x86 already does, aside from an extra 'uuid' element. This patch introduces a new optional 'uuid' element for the NVDIMM memory model. This element behaves like the 'uuid' element of the domain definition - if absent, we'll create a new one, otherwise use the one provided by the XML. The 'uuid' element is exclusive to pseries guests and are unavailable for other architectures. Next patch will use this new element to add NVDIMM support for ppc64. [1] https://github.com/qemu/qemu/commit/ee3a71e36654317b14ede0290e87628f8b79f850 Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- docs/schemas/domaincommon.rng | 5 ++ src/conf/domain_conf.c | 37 +++++++++++++-- src/conf/domain_conf.h | 3 ++ .../memory-hotplug-nvdimm-ppc64.xml | 47 +++++++++++++++++++ .../memory-hotplug-nvdimm-ppc64.xml | 47 +++++++++++++++++++ tests/qemuxml2xmltest.c | 2 + 6 files changed, 138 insertions(+), 3 deletions(-) create mode 100644 tests/qemuxml2argvdata/memory-hotplug-nvdimm-ppc64.xml create mode 100644 tests/qemuxml2xmloutdata/memory-hotplug-nvdimm-ppc64.xml diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 529a98fc05..119ebc9401 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -5660,6 +5660,11 @@ </attribute> </optional> <interleave> + <optional> + <element name="uuid"> + <ref name="UUID"/> + </element> + </optional> <optional> <ref name="memorydev-source"/> </optional> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index d2d97daf80..3ae6c181c2 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -16500,6 +16500,7 @@ static virDomainMemoryDefPtr virDomainMemoryDefParseXML(virDomainXMLOptionPtr xmlopt, xmlNodePtr memdevNode, xmlXPathContextPtr ctxt, + const virDomainDef *dom, unsigned int flags) { VIR_XPATH_NODE_AUTORESTORE(ctxt); @@ -16546,6 +16547,25 @@ virDomainMemoryDefParseXML(virDomainXMLOptionPtr xmlopt, def->discard = val; } + VIR_FREE(tmp); + + if (def->model == VIR_DOMAIN_MEMORY_MODEL_NVDIMM && + ARCH_IS_PPC64(dom->os.arch)) { + /* Extract nvdimm uuid or generate a new one */ + tmp = virXPathString("string(./uuid[1])", ctxt); + + if (!tmp) { + if (virUUIDGenerate(def->uuid) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("Failed to generate UUID")); + goto error; + } + } else if (virUUIDParse(tmp, def->uuid) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("malformed uuid element")); + goto error; + } + } /* source */ if ((node = virXPathNode("./source", ctxt)) && @@ -16843,7 +16863,8 @@ virDomainDeviceDefParse(const char *xmlStr, break; case VIR_DOMAIN_DEVICE_MEMORY: if (!(dev->data.memory = virDomainMemoryDefParseXML(xmlopt, node, - ctxt, flags))) + ctxt, def, + flags))) return NULL; break; case VIR_DOMAIN_DEVICE_IOMMU: @@ -21790,6 +21811,7 @@ virDomainDefParseXML(xmlDocPtr xml, virDomainMemoryDefPtr mem = virDomainMemoryDefParseXML(xmlopt, nodes[i], ctxt, + def, flags); if (!mem) goto error; @@ -26976,6 +26998,7 @@ virDomainMemoryTargetDefFormat(virBufferPtr buf, static int virDomainMemoryDefFormat(virBufferPtr buf, virDomainMemoryDefPtr def, + const virDomainDef *dom, unsigned int flags) { const char *model = virDomainMemoryModelTypeToString(def->model); @@ -26990,6 +27013,14 @@ virDomainMemoryDefFormat(virBufferPtr buf, virBufferAddLit(buf, ">\n"); virBufferAdjustIndent(buf, 2); + if (def->model == VIR_DOMAIN_MEMORY_MODEL_NVDIMM && + ARCH_IS_PPC64(dom->os.arch)) { + char uuidstr[VIR_UUID_STRING_BUFLEN]; + + virUUIDFormat(def->uuid, uuidstr); + virBufferAsprintf(buf, "<uuid>%s</uuid>\n", uuidstr); + } + if (virDomainMemorySourceDefFormat(buf, def) < 0) return -1; @@ -29319,7 +29350,7 @@ virDomainDefFormatInternalSetRootName(virDomainDefPtr def, } for (n = 0; n < def->nmems; n++) { - if (virDomainMemoryDefFormat(buf, def->mems[n], flags) < 0) + if (virDomainMemoryDefFormat(buf, def->mems[n], def, flags) < 0) goto error; } @@ -30432,7 +30463,7 @@ virDomainDeviceDefCopy(virDomainDeviceDefPtr src, rc = virDomainPanicDefFormat(&buf, src->data.panic); break; case VIR_DOMAIN_DEVICE_MEMORY: - rc = virDomainMemoryDefFormat(&buf, src->data.memory, flags); + rc = virDomainMemoryDefFormat(&buf, src->data.memory, def, flags); break; case VIR_DOMAIN_DEVICE_SHMEM: rc = virDomainShmemDefFormat(&buf, src->data.shmem, flags); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 91b776c28a..f2f2635957 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2189,6 +2189,9 @@ struct _virDomainMemoryDef { unsigned long long labelsize; /* kibibytes; valid only for NVDIMM */ bool readonly; /* valid only for NVDIMM */ + /* required for QEMU NVDIMM ppc64 support */ + unsigned char uuid[VIR_UUID_BUFLEN]; + virDomainDeviceInfo info; }; diff --git a/tests/qemuxml2argvdata/memory-hotplug-nvdimm-ppc64.xml b/tests/qemuxml2argvdata/memory-hotplug-nvdimm-ppc64.xml new file mode 100644 index 0000000000..59352d3c52 --- /dev/null +++ b/tests/qemuxml2argvdata/memory-hotplug-nvdimm-ppc64.xml @@ -0,0 +1,47 @@ +<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='ppc64' machine='pseries'>hvm</type> + <boot dev='hd'/> + </os> + <cpu> + <topology sockets='2' dies='1' cores='1' threads='1'/> + <numa> + <cell id='0' cpus='0-1' memory='1048576' 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-ppc64</emulator> + <controller type='usb' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/> + </controller> + <controller type='pci' index='0' model='pci-root'> + <model name='spapr-pci-host-bridge'/> + <target index='0'/> + </controller> + <memballoon model='virtio'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> + </memballoon> + <panic model='pseries'/> + <memory model='nvdimm'> + <uuid>49545eb3-75e1-2d0a-acdd-f0294406c99e</uuid> + <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/memory-hotplug-nvdimm-ppc64.xml b/tests/qemuxml2xmloutdata/memory-hotplug-nvdimm-ppc64.xml new file mode 100644 index 0000000000..59352d3c52 --- /dev/null +++ b/tests/qemuxml2xmloutdata/memory-hotplug-nvdimm-ppc64.xml @@ -0,0 +1,47 @@ +<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='ppc64' machine='pseries'>hvm</type> + <boot dev='hd'/> + </os> + <cpu> + <topology sockets='2' dies='1' cores='1' threads='1'/> + <numa> + <cell id='0' cpus='0-1' memory='1048576' 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-ppc64</emulator> + <controller type='usb' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/> + </controller> + <controller type='pci' index='0' model='pci-root'> + <model name='spapr-pci-host-bridge'/> + <target index='0'/> + </controller> + <memballoon model='virtio'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> + </memballoon> + <panic model='pseries'/> + <memory model='nvdimm'> + <uuid>49545eb3-75e1-2d0a-acdd-f0294406c99e</uuid> + <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/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 9b6a235777..a3c25b4a76 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -1246,6 +1246,8 @@ mymain(void) DO_TEST("memory-hotplug-nvdimm-align", QEMU_CAPS_DEVICE_NVDIMM); DO_TEST("memory-hotplug-nvdimm-pmem", QEMU_CAPS_DEVICE_NVDIMM); DO_TEST("memory-hotplug-nvdimm-readonly", QEMU_CAPS_DEVICE_NVDIMM); + DO_TEST("memory-hotplug-nvdimm-ppc64", QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE, + QEMU_CAPS_DEVICE_NVDIMM); DO_TEST("net-udp", NONE); DO_TEST("video-virtio-gpu-device", QEMU_CAPS_DEVICE_VIRTIO_GPU); -- 2.24.1
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- docs/formatdomain.html.in | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 7e7771725c..e3bf33f873 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -8857,6 +8857,7 @@ qemu-kvm -net nic,model=? /dev/null </target> </memory> <memory model='nvdimm'> + <uuid> <source> <path>/tmp/nvdimm</path> </source> @@ -8870,6 +8871,7 @@ qemu-kvm -net nic,model=? /dev/null </target> </memory> <memory model='nvdimm' access='shared'> + <uuid> <source> <path>/dev/dax0.0</path> <alignsize unit='KiB'>2048</alignsize> @@ -8925,6 +8927,17 @@ qemu-kvm -net nic,model=? /dev/null </p> </dd> + <dt><code>uuid</code></dt> + <dd> + <p> + For pSeries guests, an uuid can be set to identify the + nvdimm module. If absent, libvirt will generate an uuid. + automatically. This attribute is allowed only for + <code>model='nvdimm'</code> for pSeries guests. + <span class="since">Since 6.2.0</span> + </p> + </dd> + <dt><code>source</code></dt> <dd> <p> -- 2.24.1
Using the 'uuid' element for ppc64 NVDIMM memory added in the previous patch, use it in qemuBuildMemoryDeviceStr() to pass it over to QEMU. Another ppc64 restriction is the necessity of a mem->labelsize, given than ppc64 only support label-area backed NVDIMMs. Finally, we don't want ppc64 NVDIMMs to align up due to the high risk of going beyond the end of file with a 256MiB increment that the user didn't predict. Align it down instead. Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/conf/domain_conf.c | 7 +++ src/qemu/qemu_command.c | 7 +++ src/qemu/qemu_domain.c | 47 +++++++++++++++++-- .../memory-hotplug-nvdimm-ppc64.args | 32 +++++++++++++ .../memory-hotplug-nvdimm-ppc64.xml | 5 +- tests/qemuxml2argvtest.c | 4 ++ .../memory-hotplug-nvdimm-ppc64.xml | 5 +- 7 files changed, 102 insertions(+), 5 deletions(-) create mode 100644 tests/qemuxml2argvdata/memory-hotplug-nvdimm-ppc64.args diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 3ae6c181c2..7f8018fed2 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -16582,6 +16582,13 @@ virDomainMemoryDefParseXML(virDomainXMLOptionPtr xmlopt, if (virDomainMemoryTargetDefParseXML(node, ctxt, def) < 0) goto error; + if (def->model == VIR_DOMAIN_MEMORY_MODEL_NVDIMM && + ARCH_IS_PPC64(dom->os.arch) && def->labelsize == 0) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("label size is required for NVDIMM device")); + goto error; + } + if (virDomainDeviceInfoParseXML(xmlopt, memdevNode, &def->info, flags) < 0) goto error; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 9e0334a3e7..76f1247329 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3677,6 +3677,13 @@ qemuBuildMemoryDeviceStr(virDomainMemoryDefPtr mem, if (mem->labelsize) virBufferAsprintf(&buf, "label-size=%llu,", mem->labelsize * 1024); + if (virUUIDIsValid(mem->uuid)) { + char uuidstr[VIR_UUID_STRING_BUFLEN]; + + virUUIDFormat(mem->uuid, uuidstr); + virBufferAsprintf(&buf, "uuid=%s,", uuidstr); + } + if (mem->readonly) { if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE_NVDIMM_UNARMED)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 3d3f796d85..2f420a43cd 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -12418,6 +12418,35 @@ qemuDomainGetMemoryModuleSizeAlignment(const virDomainDef *def, } +static void +qemuDomainNVDimmAlignSizePseries(virDomainDefPtr def, + virDomainMemoryDefPtr mem) +{ + /* For NVDIMMs in ppc64 in we want to align down the guest + * visible space, instead of align up, to avoid writing + * beyond the end of file by adding a potential 256MiB + * to the user specified size. + * + * The label-size is mandatory for ppc64 as well, meaning that + * the guest visible space will be target_size-label_size. + * + * Finally, target_size must include label_size. + * + * The above can be summed up as follows: + * + * target_size = AlignDown(target_size - label_size) + label_size + */ + unsigned long long ppc64AlignSize = qemuDomainGetMemorySizeAlignment(def); + unsigned long long guestArea = mem->size - mem->labelsize; + + /* Align down guest_area. 256MiB is the minimum size. */ + guestArea = (guestArea/ppc64AlignSize) * ppc64AlignSize; + guestArea = MAX(guestArea, ppc64AlignSize); + + mem->size = guestArea + mem->labelsize; +} + + int qemuDomainAlignMemorySizes(virDomainDefPtr def) { @@ -12464,8 +12493,14 @@ qemuDomainAlignMemorySizes(virDomainDefPtr def) /* Align memory module sizes */ for (i = 0; i < def->nmems; i++) { - align = qemuDomainGetMemoryModuleSizeAlignment(def, def->mems[i]); - def->mems[i]->size = VIR_ROUND_UP(def->mems[i]->size, align); + if (def->mems[i]->model == VIR_DOMAIN_MEMORY_MODEL_NVDIMM && + ARCH_IS_PPC64(def->os.arch)) { + qemuDomainNVDimmAlignSizePseries(def, def->mems[i]); + } else { + align = qemuDomainGetMemoryModuleSizeAlignment(def, def->mems[i]); + def->mems[i]->size = VIR_ROUND_UP(def->mems[i]->size, align); + } + hotplugmem += def->mems[i]->size; if (def->mems[i]->size > maxmemkb) { @@ -12494,7 +12529,13 @@ void qemuDomainMemoryDeviceAlignSize(virDomainDefPtr def, virDomainMemoryDefPtr mem) { - mem->size = VIR_ROUND_UP(mem->size, qemuDomainGetMemorySizeAlignment(def)); + if (mem->model == VIR_DOMAIN_MEMORY_MODEL_NVDIMM && + ARCH_IS_PPC64(def->os.arch)) { + qemuDomainNVDimmAlignSizePseries(def, mem); + } else { + mem->size = VIR_ROUND_UP(mem->size, + qemuDomainGetMemorySizeAlignment(def)); + } } diff --git a/tests/qemuxml2argvdata/memory-hotplug-nvdimm-ppc64.args b/tests/qemuxml2argvdata/memory-hotplug-nvdimm-ppc64.args new file mode 100644 index 0000000000..92e6c538fb --- /dev/null +++ b/tests/qemuxml2argvdata/memory-hotplug-nvdimm-ppc64.args @@ -0,0 +1,32 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/tmp/lib/domain--1-QEMUGuest1 \ +USER=test \ +LOGNAME=test \ +XDG_DATA_HOME=/tmp/lib/domain--1-QEMUGuest1/.local/share \ +XDG_CACHE_HOME=/tmp/lib/domain--1-QEMUGuest1/.cache \ +XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest1/.config \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu-system-ppc64 \ +-name QEMUGuest1 \ +-S \ +-machine pseries,accel=tcg,usb=off,dump-guest-core=off,nvdimm=on \ +-m size=1048576k,slots=16,maxmem=1099511627776k \ +-realtime mlock=off \ +-smp 2,sockets=2,cores=1,threads=1 \ +-numa node,nodeid=0,cpus=0-1,mem=1024 \ +-object memory-backend-file,id=memnvdimm0,prealloc=yes,mem-path=/tmp/nvdimm,\ +size=537001984 \ +-device nvdimm,node=0,label-size=131072,\ +uuid=49545eb3-75e1-2d0a-acdd-f0294406c99e,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 \ +-usb \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x2 diff --git a/tests/qemuxml2argvdata/memory-hotplug-nvdimm-ppc64.xml b/tests/qemuxml2argvdata/memory-hotplug-nvdimm-ppc64.xml index 59352d3c52..ae5a17d3c8 100644 --- a/tests/qemuxml2argvdata/memory-hotplug-nvdimm-ppc64.xml +++ b/tests/qemuxml2argvdata/memory-hotplug-nvdimm-ppc64.xml @@ -38,8 +38,11 @@ <path>/tmp/nvdimm</path> </source> <target> - <size unit='KiB'>523264</size> + <size unit='KiB'>550000</size> <node>0</node> + <label> + <size unit='KiB'>128</size> + </label> </target> <address type='dimm' slot='0'/> </memory> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 35d413d40b..077f7e7650 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -2790,6 +2790,10 @@ mymain(void) DO_TEST_CAPS_LATEST("memory-hotplug-nvdimm-align"); DO_TEST_CAPS_LATEST("memory-hotplug-nvdimm-pmem"); DO_TEST_CAPS_LATEST("memory-hotplug-nvdimm-readonly"); + DO_TEST("memory-hotplug-nvdimm-ppc64", QEMU_CAPS_KVM, QEMU_CAPS_DEVICE_NVDIMM, + QEMU_CAPS_NUMA, QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE, + 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-ppc64.xml b/tests/qemuxml2xmloutdata/memory-hotplug-nvdimm-ppc64.xml index 59352d3c52..ae5a17d3c8 100644 --- a/tests/qemuxml2xmloutdata/memory-hotplug-nvdimm-ppc64.xml +++ b/tests/qemuxml2xmloutdata/memory-hotplug-nvdimm-ppc64.xml @@ -38,8 +38,11 @@ <path>/tmp/nvdimm</path> </source> <target> - <size unit='KiB'>523264</size> + <size unit='KiB'>550000</size> <node>0</node> + <label> + <size unit='KiB'>128</size> + </label> </target> <address type='dimm' slot='0'/> </memory> -- 2.24.1
On Thu, Mar 12, 2020 at 3:00 AM Daniel Henrique Barboza <danielhb413@gmail.com> wrote:
Using the 'uuid' element for ppc64 NVDIMM memory added in the previous patch, use it in qemuBuildMemoryDeviceStr() to pass it over to QEMU.
Another ppc64 restriction is the necessity of a mem->labelsize, given than ppc64 only support label-area backed NVDIMMs.
Finally, we don't want ppc64 NVDIMMs to align up due to the high risk of going beyond the end of file with a 256MiB increment that the user didn't predict. Align it down instead.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> ---
...
+static void +qemuDomainNVDimmAlignSizePseries(virDomainDefPtr def, + virDomainMemoryDefPtr mem) +{ + /* For NVDIMMs in ppc64 in we want to align down the guest + * visible space, instead of align up, to avoid writing + * beyond the end of file by adding a potential 256MiB + * to the user specified size. + * + * The label-size is mandatory for ppc64 as well, meaning that + * the guest visible space will be target_size-label_size. + * + * Finally, target_size must include label_size. + * + * The above can be summed up as follows: + * + * target_size = AlignDown(target_size - label_size) + label_size + */ + unsigned long long ppc64AlignSize = qemuDomainGetMemorySizeAlignment(def); + unsigned long long guestArea = mem->size - mem->labelsize; + + /* Align down guest_area. 256MiB is the minimum size. */ + guestArea = (guestArea/ppc64AlignSize) * ppc64AlignSize; + guestArea = MAX(guestArea, ppc64AlignSize);
The math is correct, but we need additional checks when a backing file of size less than 256MB is attempted to be hot-plugged. The qemu errors out like below if the backing file is 240MB and MAX of (240, 256) is chosen. -object memory-backend-file,id=memnvdimm1,prealloc=yes,mem-path =/tmp/nvdimm,share=yes,size=268566528: backing store size 0xf000000 does not match 'size' option 0x10020000
+ + mem->size = guestArea + mem->labelsize; +} + +.... + </label> </target> <address type='dimm' slot='0'/> </memory> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 35d413d40b..077f7e7650 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -2790,6 +2790,10 @@ mymain(void) DO_TEST_CAPS_LATEST("memory-hotplug-nvdimm-align"); DO_TEST_CAPS_LATEST("memory-hotplug-nvdimm-pmem"); DO_TEST_CAPS_LATEST("memory-hotplug-nvdimm-readonly"); + DO_TEST("memory-hotplug-nvdimm-ppc64", QEMU_CAPS_KVM, QEMU_CAPS_DEVICE_NVDIMM,
I see the qemucapabilitiesdata/caps-5*.ppc64* not updated with the nvdimm support. Can you update it as well? Then this can be changed to just DO_TEST_CAPS_LATEST_PPC64, without this elaborate list of caps.
+ QEMU_CAPS_NUMA, QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE, + QEMU_CAPS_OBJECT_MEMORY_RAM, ...
Thanks, Shiva
On 3/20/20 4:53 AM, Shivaprasad bhat wrote:
On Thu, Mar 12, 2020 at 3:00 AM Daniel Henrique Barboza <danielhb413@gmail.com> wrote:
Using the 'uuid' element for ppc64 NVDIMM memory added in the previous patch, use it in qemuBuildMemoryDeviceStr() to pass it over to QEMU.
Another ppc64 restriction is the necessity of a mem->labelsize, given than ppc64 only support label-area backed NVDIMMs.
Finally, we don't want ppc64 NVDIMMs to align up due to the high risk of going beyond the end of file with a 256MiB increment that the user didn't predict. Align it down instead.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> ---
...
+static void +qemuDomainNVDimmAlignSizePseries(virDomainDefPtr def, + virDomainMemoryDefPtr mem) +{ + /* For NVDIMMs in ppc64 in we want to align down the guest + * visible space, instead of align up, to avoid writing + * beyond the end of file by adding a potential 256MiB + * to the user specified size. + * + * The label-size is mandatory for ppc64 as well, meaning that + * the guest visible space will be target_size-label_size. + * + * Finally, target_size must include label_size. + * + * The above can be summed up as follows: + * + * target_size = AlignDown(target_size - label_size) + label_size + */ + unsigned long long ppc64AlignSize = qemuDomainGetMemorySizeAlignment(def); + unsigned long long guestArea = mem->size - mem->labelsize; + + /* Align down guest_area. 256MiB is the minimum size. */ + guestArea = (guestArea/ppc64AlignSize) * ppc64AlignSize; + guestArea = MAX(guestArea, ppc64AlignSize);
The math is correct, but we need additional checks when a backing file of size less than 256MB is attempted to be hot-plugged. The qemu errors out like below if the backing file is 240MB and MAX of (240, 256) is chosen.
-object memory-backend-file,id=memnvdimm1,prealloc=yes,mem-path =/tmp/nvdimm,share=yes,size=268566528: backing store size 0xf000000 does not match 'size' option 0x10020000
Got it. I'll error out in this case.
+ + mem->size = guestArea + mem->labelsize; +} + +.... + </label> </target> <address type='dimm' slot='0'/> </memory> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 35d413d40b..077f7e7650 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -2790,6 +2790,10 @@ mymain(void) DO_TEST_CAPS_LATEST("memory-hotplug-nvdimm-align"); DO_TEST_CAPS_LATEST("memory-hotplug-nvdimm-pmem"); DO_TEST_CAPS_LATEST("memory-hotplug-nvdimm-readonly"); + DO_TEST("memory-hotplug-nvdimm-ppc64", QEMU_CAPS_KVM, QEMU_CAPS_DEVICE_NVDIMM,
I see the qemucapabilitiesdata/caps-5*.ppc64* not updated with the nvdimm support. Can you update it as well? Then this can be changed to just DO_TEST_CAPS_LATEST_PPC64, without this elaborate list of caps.
+ QEMU_CAPS_NUMA, QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE, + QEMU_CAPS_OBJECT_MEMORY_RAM, ...
Roger that! DHB
Thanks, Shiva
On 11. 3. 2020 22:29, Daniel Henrique Barboza wrote:
Using the 'uuid' element for ppc64 NVDIMM memory added in the previous patch, use it in qemuBuildMemoryDeviceStr() to pass it over to QEMU.
Another ppc64 restriction is the necessity of a mem->labelsize, given than ppc64 only support label-area backed NVDIMMs.
Finally, we don't want ppc64 NVDIMMs to align up due to the high risk of going beyond the end of file with a 256MiB increment that the user didn't predict. Align it down instead.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/conf/domain_conf.c | 7 +++ src/qemu/qemu_command.c | 7 +++ src/qemu/qemu_domain.c | 47 +++++++++++++++++-- .../memory-hotplug-nvdimm-ppc64.args | 32 +++++++++++++ .../memory-hotplug-nvdimm-ppc64.xml | 5 +- tests/qemuxml2argvtest.c | 4 ++ .../memory-hotplug-nvdimm-ppc64.xml | 5 +- 7 files changed, 102 insertions(+), 5 deletions(-) create mode 100644 tests/qemuxml2argvdata/memory-hotplug-nvdimm-ppc64.args
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 3ae6c181c2..7f8018fed2 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -16582,6 +16582,13 @@ virDomainMemoryDefParseXML(virDomainXMLOptionPtr xmlopt, if (virDomainMemoryTargetDefParseXML(node, ctxt, def) < 0) goto error;
+ if (def->model == VIR_DOMAIN_MEMORY_MODEL_NVDIMM && + ARCH_IS_PPC64(dom->os.arch) && def->labelsize == 0) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("label size is required for NVDIMM device")); + goto error; + } +
I think this should go into virDomainMemoryDefValidate() instead, because this is not strictly a parse error (e.g. like invalid UUID or missing element). The rest looks good. Can you please post a v3 of just this patch? I will handle the review and merge. Sorry for delayed review. Michal
On 3/23/20 3:46 PM, Michal Prívozník wrote:
On 11. 3. 2020 22:29, Daniel Henrique Barboza wrote:
Using the 'uuid' element for ppc64 NVDIMM memory added in the previous patch, use it in qemuBuildMemoryDeviceStr() to pass it over to QEMU.
Another ppc64 restriction is the necessity of a mem->labelsize, given than ppc64 only support label-area backed NVDIMMs.
Finally, we don't want ppc64 NVDIMMs to align up due to the high risk of going beyond the end of file with a 256MiB increment that the user didn't predict. Align it down instead.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/conf/domain_conf.c | 7 +++ src/qemu/qemu_command.c | 7 +++ src/qemu/qemu_domain.c | 47 +++++++++++++++++-- .../memory-hotplug-nvdimm-ppc64.args | 32 +++++++++++++ .../memory-hotplug-nvdimm-ppc64.xml | 5 +- tests/qemuxml2argvtest.c | 4 ++ .../memory-hotplug-nvdimm-ppc64.xml | 5 +- 7 files changed, 102 insertions(+), 5 deletions(-) create mode 100644 tests/qemuxml2argvdata/memory-hotplug-nvdimm-ppc64.args
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 3ae6c181c2..7f8018fed2 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -16582,6 +16582,13 @@ virDomainMemoryDefParseXML(virDomainXMLOptionPtr xmlopt, if (virDomainMemoryTargetDefParseXML(node, ctxt, def) < 0) goto error;
+ if (def->model == VIR_DOMAIN_MEMORY_MODEL_NVDIMM && + ARCH_IS_PPC64(dom->os.arch) && def->labelsize == 0) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("label size is required for NVDIMM device")); + goto error; + } +
I think this should go into virDomainMemoryDefValidate() instead, because this is not strictly a parse error (e.g. like invalid UUID or missing element).
The rest looks good. Can you please post a v3 of just this patch? I will handle the review and merge. Sorry for delayed review.
No problem,. I'll send a v4 with this change (v3 is already in the ML with other changes). Thanks, DHB
Michal
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- docs/formatdomain.html.in | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index e3bf33f873..142cd17205 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -9026,12 +9026,13 @@ qemu-kvm -net nic,model=? /dev/null <dt><code>label</code></dt> <dd> <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. The <code>size</code> element - has usual meaning described + For NVDIMM type devices one can use <code>label</code> and its + subelement <code>size</code> to configure the size of + namespaces label storage within the NVDIMM module. The + <code>size</code> element has usual meaning described <a href="#elementsMemoryAllocation">here</a>. + <code>label</code> is mandatory for pSeries guests and optional + for all other architectures. For QEMU domains the following restrictions apply: </p> <ol> -- 2.24.1
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- docs/news.xml | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/docs/news.xml b/docs/news.xml index 7fd88f9998..6eb222cdb7 100644 --- a/docs/news.xml +++ b/docs/news.xml @@ -44,6 +44,17 @@ <libvirt> <release version="v6.2.0" date="unreleased"> <section title="New features"> + <change> + <summary> + qemu: NVDIMM support for pSeries guests + </summary> + <description> + QEMU 5.0 implements NVDIMM memory support for pSeries guests. This + is done by adding an 'uuid' element in the memory XML, which can + either be provided in the XML or, if ommited, generated + automatically. + </description> + </change> </section> <section title="Improvements"> </section> -- 2.24.1
Ping Also CC'ing Shiva since he implemented the QEMU side of this feature and might be interested in it. Shiva, mind tossing a review in patch 3/5? This is the one that contains the alignment logic and I'd like to be certain that I didn't mess it up. Thanks, DHB On 3/11/20 6:29 PM, Daniel Henrique Barboza wrote:
changes in v2, all of them affecting just pSeries guests: - added 'label' requirement - added code to align down the NVDIMM device previous version: https://www.redhat.com/archives/libvir-list/2020-March/msg00165.html
This patch series adds NVDIMM suport for ppc64 guests, which consists on adding an extra 'uuid' element in the nvdimm command line and the target label size must always be provided in the memory definition.
No changes were made in the existing NVDIMM support for x86 and other archs.
Daniel Henrique Barboza (5): conf: Introduce optional 'uuid' element for NVDIMM memory formatdomain.html.in: document the new 'uuid' NVDIMM element conf, qemu: enable NVDIMM support for ppc64 formatdomain.html.in: document NVDIMM 'label' requirement for pSeries news.xml: document the new NVDIMM support for Pseries guests
docs/formatdomain.html.in | 24 +++++++-- docs/news.xml | 11 ++++ docs/schemas/domaincommon.rng | 5 ++ src/conf/domain_conf.c | 44 ++++++++++++++-- src/conf/domain_conf.h | 3 ++ src/qemu/qemu_command.c | 7 +++ src/qemu/qemu_domain.c | 47 +++++++++++++++-- .../memory-hotplug-nvdimm-ppc64.args | 32 ++++++++++++ .../memory-hotplug-nvdimm-ppc64.xml | 50 +++++++++++++++++++ tests/qemuxml2argvtest.c | 4 ++ .../memory-hotplug-nvdimm-ppc64.xml | 50 +++++++++++++++++++ tests/qemuxml2xmltest.c | 2 + 12 files changed, 268 insertions(+), 11 deletions(-) create mode 100644 tests/qemuxml2argvdata/memory-hotplug-nvdimm-ppc64.args create mode 100644 tests/qemuxml2argvdata/memory-hotplug-nvdimm-ppc64.xml create mode 100644 tests/qemuxml2xmloutdata/memory-hotplug-nvdimm-ppc64.xml
participants (3)
-
Daniel Henrique Barboza -
Michal Prívozník -
Shivaprasad bhat