[PATCH v3 4/5] qemu_domain.c: post parse pSeries NVDIMM align with PARSE_ABI_UPDATE

A previous patch removed the pSeries NVDIMM align that wasn't being done properly. This patch reintroduces it in the right fashion, making it reliant on VIR_DOMAIN_DEF_PARSE_ABI_UPDATE. This makes it complying with the intended design defined by commit c7d7ba85a624. Since the PARSE_ABI_UPDATE_FLAG is more restrictive than checking for !migrate && !snapshot, like is being currently done with qemuDomainAlignMemorySizes(), this means that we'll align the pSeries NVDIMMs in two places - in post parse time for new guests, and in qemuDomainAlignMemorySizes() for all guests that aren't migrating or in a snapshot. Another difference is that the logic is now in the QEMU driver instead of domain_conf.c. This was necessary because all considerations made about the PARSE_ABI_UPDATE flag were done under QEMU. Given that no other driver supports ppc64 there is no impact in this change. A new test was added to exercise what we're doing. It consists of a a copy of the existing 'memory-hotplug-nvdimm-ppc64' xml2xml test, called with the PARSE_ABI_UPDATE flag. As intended, we're not changing QEMU command line or any XML without the flag, while the pseries NVDIMM memory is being aligned when the flag is used. qemuDomainNVDimmAlignSizePseries() was moved up to be used by qemuDomainMemoryDefPostParse(). The alignment for pSeries is always 256 * 1024 KiB, making a call to qemuDomainGetMemorySizeAlignment() and the need for the 'def' arg obsolete as well. Reviewed-by: Michal Privoznik <mprivozn(a)redhat.com&gt; Signed-off-by: Daniel Henrique Barboza <danielhb413(a)gmail.com&gt; --- src/qemu/qemu_domain.c | 112 +++++++++++------- ...emory-hotplug-nvdimm-ppc64-abi-update.args | 30 +++++ ...memory-hotplug-nvdimm-ppc64-abi-update.xml | 1 + tests/qemuxml2argvtest.c | 7 ++ ...memory-hotplug-nvdimm-ppc64-abi-update.xml | 46 +++++++ tests/qemuxml2xmltest.c | 7 ++ 6 files changed, 162 insertions(+), 41 deletions(-) create mode 100644 tests/qemuxml2argvdata/memory-hotplug-nvdimm-ppc64-abi-update.args create mode 120000 tests/qemuxml2argvdata/memory-hotplug-nvdimm-ppc64-abi-update.xml create mode 100644 tests/qemuxml2xmloutdata/memory-hotplug-nvdimm-ppc64-abi-update.xml diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index af3c0e269a..5fff7d312d 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -5341,6 +5341,70 @@ qemuDomainTPMDefPostParse(virDomainTPMDefPtr tpm, } +static int +qemuDomainNVDimmAlignSizePseries(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 = 256 * 1024; + unsigned long long guestArea = mem->size - mem->labelsize; + + /* Align down guest_area. 256MiB is the minimum size. Error + * out if target_size is smaller than 256MiB + label_size, + * since aligning it up will cause QEMU errors. */ + if (mem->size < (ppc64AlignSize + mem->labelsize)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("minimum target size for the NVDIMM " + "must be 256MB plus the label size")); + return -1; + } + + guestArea = (guestArea/ppc64AlignSize) * ppc64AlignSize; + mem->size = guestArea + mem->labelsize; + + return 0; +} + + +static int +qemuDomainMemoryDefPostParse(virDomainMemoryDefPtr mem, virArch arch, + unsigned int parseFlags) +{ + /* Memory alignment can't be done for migration or snapshot + * scenarios. This logic was defined by commit c7d7ba85a624. + * + * There is no easy way to replicate at this point the same conditions + * used to call qemuDomainAlignMemorySizes(), which means checking if + * we're not migrating and not in a snapshot. + * + * We can use the PARSE_ABI_UPDATE flag, which is more strict - + * existing guests will not activate the flag to avoid breaking + * boot ABI. This means that any alignment done here will be replicated + * later on by qemuDomainAlignMemorySizes() to contemplate existing + * guests as well. */ + if (parseFlags & VIR_DOMAIN_DEF_PARSE_ABI_UPDATE) { + if (ARCH_IS_PPC64(arch) && + mem->model == VIR_DOMAIN_MEMORY_MODEL_NVDIMM && + qemuDomainNVDimmAlignSizePseries(mem) < 0) + return -1; + } + + return 0; +} + + static int qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, const virDomainDef *def, @@ -5398,6 +5462,11 @@ qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, ret = qemuDomainTPMDefPostParse(dev->data.tpm, def->os.arch); break; + case VIR_DOMAIN_DEVICE_MEMORY: + ret = qemuDomainMemoryDefPostParse(dev->data.memory, def->os.arch, + parseFlags); + break; + case VIR_DOMAIN_DEVICE_LEASE: case VIR_DOMAIN_DEVICE_FS: case VIR_DOMAIN_DEVICE_INPUT: @@ -5410,7 +5479,6 @@ qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, case VIR_DOMAIN_DEVICE_MEMBALLOON: case VIR_DOMAIN_DEVICE_NVRAM: case VIR_DOMAIN_DEVICE_RNG: - case VIR_DOMAIN_DEVICE_MEMORY: case VIR_DOMAIN_DEVICE_IOMMU: case VIR_DOMAIN_DEVICE_AUDIO: ret = 0; @@ -8044,44 +8112,6 @@ qemuDomainGetMemoryModuleSizeAlignment(const virDomainDef *def, } -static int -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. Error - * out if target_size is smaller than 256MiB + label_size, - * since aligning it up will cause QEMU errors. */ - if (mem->size < (ppc64AlignSize + mem->labelsize)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("minimum target size for the NVDIMM " - "must be 256MB plus the label size")); - return -1; - } - - guestArea = (guestArea/ppc64AlignSize) * ppc64AlignSize; - mem->size = guestArea + mem->labelsize; - - return 0; -} - - int qemuDomainAlignMemorySizes(virDomainDefPtr def) { @@ -8130,7 +8160,7 @@ qemuDomainAlignMemorySizes(virDomainDefPtr def) for (i = 0; i < def->nmems; i++) { if (def->mems[i]->model == VIR_DOMAIN_MEMORY_MODEL_NVDIMM && ARCH_IS_PPC64(def->os.arch)) { - if (qemuDomainNVDimmAlignSizePseries(def, def->mems[i]) < 0) + if (qemuDomainNVDimmAlignSizePseries(def->mems[i]) < 0) return -1; } else { align = qemuDomainGetMemoryModuleSizeAlignment(def, def->mems[i]); @@ -8167,7 +8197,7 @@ qemuDomainMemoryDeviceAlignSize(virDomainDefPtr def, { if (mem->model == VIR_DOMAIN_MEMORY_MODEL_NVDIMM && ARCH_IS_PPC64(def->os.arch)) { - return qemuDomainNVDimmAlignSizePseries(def, mem); + return qemuDomainNVDimmAlignSizePseries(mem); } else { mem->size = VIR_ROUND_UP(mem->size, qemuDomainGetMemorySizeAlignment(def)); diff --git a/tests/qemuxml2argvdata/memory-hotplug-nvdimm-ppc64-abi-update.args b/tests/qemuxml2argvdata/memory-hotplug-nvdimm-ppc64-abi-update.args new file mode 100644 index 0000000000..f50444e47e --- /dev/null +++ b/tests/qemuxml2argvdata/memory-hotplug-nvdimm-ppc64-abi-update.args @@ -0,0 +1,30 @@ +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,mem-path=/tmp/nvdimm,prealloc=yes,\ +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 diff --git a/tests/qemuxml2argvdata/memory-hotplug-nvdimm-ppc64-abi-update.xml b/tests/qemuxml2argvdata/memory-hotplug-nvdimm-ppc64-abi-update.xml new file mode 120000 index 0000000000..c7d71906f6 --- /dev/null +++ b/tests/qemuxml2argvdata/memory-hotplug-nvdimm-ppc64-abi-update.xml @@ -0,0 +1 @@ +memory-hotplug-nvdimm-ppc64.xml \ No newline at end of file diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 8c8426e699..a9bd210ca3 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -3014,6 +3014,13 @@ mymain(void) DO_TEST("memory-hotplug-nvdimm-ppc64", QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE, QEMU_CAPS_OBJECT_MEMORY_FILE, QEMU_CAPS_DEVICE_NVDIMM); + DO_TEST_FULL("memory-hotplug-nvdimm-ppc64-abi-update", + ARG_PARSEFLAGS, VIR_DOMAIN_DEF_PARSE_ABI_UPDATE, + ARG_QEMU_CAPS, + QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE, + QEMU_CAPS_OBJECT_MEMORY_FILE, + QEMU_CAPS_DEVICE_NVDIMM, + QEMU_CAPS_LAST); DO_TEST("machine-aeskeywrap-on-caps", QEMU_CAPS_AES_KEY_WRAP, diff --git a/tests/qemuxml2xmloutdata/memory-hotplug-nvdimm-ppc64-abi-update.xml b/tests/qemuxml2xmloutdata/memory-hotplug-nvdimm-ppc64-abi-update.xml new file mode 100644 index 0000000000..3999b1a99f --- /dev/null +++ b/tests/qemuxml2xmloutdata/memory-hotplug-nvdimm-ppc64-abi-update.xml @@ -0,0 +1,46 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <maxMemory slots='16' unit='KiB'>1099511627776</maxMemory> + <memory unit='KiB'>1572992</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' model='none'/> + <controller type='pci' index='0' model='pci-root'> + <model name='spapr-pci-host-bridge'/> + <target index='0'/> + </controller> + <memballoon model='none'/> + <panic model='pseries'/> + <memory model='nvdimm'> + <uuid>49545eb3-75e1-2d0a-acdd-f0294406c99e</uuid> + <source> + <path>/tmp/nvdimm</path> + </source> + <target> + <size unit='KiB'>524416</size> + <node>0</node> + <label> + <size unit='KiB'>128</size> + </label> + </target> + <address type='dimm' slot='0'/> + </memory> + </devices> +</domain> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 83e5dd0cbe..18faea2b33 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -1244,6 +1244,13 @@ mymain(void) DO_TEST("memory-hotplug-nvdimm-ppc64", QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE, QEMU_CAPS_OBJECT_MEMORY_FILE, QEMU_CAPS_DEVICE_NVDIMM); + DO_TEST_FULL("memory-hotplug-nvdimm-ppc64-abi-update", WHEN_BOTH, + ARG_PARSEFLAGS, VIR_DOMAIN_DEF_PARSE_ABI_UPDATE, + ARG_QEMU_CAPS, + QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE, + QEMU_CAPS_OBJECT_MEMORY_FILE, + QEMU_CAPS_DEVICE_NVDIMM, + QEMU_CAPS_LAST); DO_TEST("net-udp", NONE); -- 2.26.2