[PATCH v3 4/5] qemu_domain.c: post parse pSeries NVDIMM align with PARSE_ABI_UPDATE
Daniel Henrique Barboza
danielhb413 at gmail.com
Fri Dec 4 01:05:28 UTC 2020
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 at redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413 at gmail.com>
---
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
More information about the libvir-list
mailing list