[PATCH v2 0/2] domain_validate: Account for NVDIMM label size properly when checking for memory conflicts
9:53 a.m.
v2 of:
https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/thread/WP...
diff to v1:
- Amended commit message to 1/2 (appended Fixes: with respective commits),
- Added new test case
Michal Prívozník (2):
domain_validate: Account for NVDIMM label size properly when checking
for memory conflicts
qemuxmlconftest: Introduce memory-hotplug-nvdimm-overlap test case
src/conf/domain_validate.c | 50 +++++++++++-
...-hotplug-nvdimm-overlap.x86_64-latest.args | 40 ++++++++++
...y-hotplug-nvdimm-overlap.x86_64-latest.xml | 1 +
.../memory-hotplug-nvdimm-overlap.xml | 77 +++++++++++++++++++
tests/qemuxmlconftest.c | 4 +
5 files changed, 170 insertions(+), 2 deletions(-)
create mode 100644
tests/qemuxmlconfdata/memory-hotplug-nvdimm-overlap.x86_64-latest.args
create mode 120000 tests/qemuxmlconfdata/memory-hotplug-nvdimm-overlap.x86_64-latest.xml
create mode 100644 tests/qemuxmlconfdata/memory-hotplug-nvdimm-overlap.xml
--
2.43.0
9:53 a.m.
New subject: [PATCH v2 1/2] domain_validate: Account for NVDIMM label size properly when checking for memory conflicts
As of v9.8.0-rc1~7 we check whether two <memory/> devices don't
overlap (since we allow setting where a <memory/> device should
be mapped to). We do this pretty straightforward, by comparing
start and end address of each <memory/> device combination.
But since only the start address is given (an exposed in the
XML), the end address is computed trivially as:
start + mem->size * 1024
And for majority of memory device types this works. Except for
NVDIMMs. For them the <memory/> device consists of two separate
regions: 1) actual memory device, and 2) label.
Label is where NVDIMM stores some additional information like
namespaces partition and so on. But it's not mapped into the
guest the same way as actual memory device. In fact, mem->size is
a sum of both actual memory device and label sizes. And to make
things a bit worse, both sizes are subject to alignment (either
the alignsize value specified in XML, or system page size if not
specified in XML).
Therefore, to get the size of actual memory device we need to
take mem->size and substract label size rounded up to alignment.
If we don't do this we report there's an overlap between two
NVDIMMs even when in reality there's none.
Fixes: 3fd64fb0e236fc80ffa2cc977c0d471f11fc39bf
Fixes: 91f9a9fb4fc0d34ed8d7a869de3d9f87687c3618
Resolves: https://issues.redhat.com/browse/RHEL-4452?focusedId=23805174#comment-238...
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/conf/domain_validate.c | 50 ++++++++++++++++++++++++++++++++++++--
1 file changed, 48 insertions(+), 2 deletions(-)
diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c
index 46479f10f2..5a9398b545 100644
--- a/src/conf/domain_validate.c
+++ b/src/conf/domain_validate.c
@@ -2225,6 +2225,52 @@ virDomainHostdevDefValidate(const virDomainHostdevDef *hostdev)
}
+/**
+ * virDomainMemoryGetMappedSize:
+ * @mem: memory device definition
+ *
+ * For given memory device definition (@mem) calculate size mapped into
+ * the guest. This is usually mem->size, except for NVDIMM where its
+ * label is mapped elsewhere.
+ *
+ * Returns: Number of bytes a memory device takes when mapped into a
+ * guest.
+ */
+static unsigned long long
+virDomainMemoryGetMappedSize(const virDomainMemoryDef *mem)
+{
+ unsigned long long ret = mem->size;
+
+ if (mem->model == VIR_DOMAIN_MEMORY_MODEL_NVDIMM) {
+ unsigned long long alignsize = mem->source.nvdimm.alignsize;
+ unsigned long long labelsize = 0;
+
+ /* For NVDIMM the situation is a bit more complicated. Firstly,
+ * its <label/> is not mapped as a part of memory device, so we
+ * must subtract label size from NVDIMM size. Secondly, label is
+ * also subject to alignment so we need to round it up before
+ * subtraction. */
+
+ if (alignsize == 0) {
+ long pagesize = virGetSystemPageSizeKB();
+
+ /* If no alignment is specified in the XML, fallback to
+ * system page size alignment. */
+ if (pagesize > 0)
+ alignsize = pagesize;
+ }
+
+ if (alignsize > 0) {
+ labelsize = VIR_ROUND_UP(mem->target.nvdimm.labelsize, alignsize);
+
+ ret -= labelsize;
+ }
+ }
+
+ return ret * 1024;
+}
+
+
static int
virDomainMemoryDefCheckConflict(const virDomainMemoryDef *mem,
const virDomainDef *def)
@@ -2259,7 +2305,7 @@ virDomainMemoryDefCheckConflict(const virDomainMemoryDef *mem,
}
/* thisStart and thisEnd are in bytes, mem->size in kibibytes */
- thisEnd = thisStart + mem->size * 1024;
+ thisEnd = thisStart + virDomainMemoryGetMappedSize(mem);
for (i = 0; i < def->nmems; i++) {
const virDomainMemoryDef *other = def->mems[i];
@@ -2316,7 +2362,7 @@ virDomainMemoryDefCheckConflict(const virDomainMemoryDef *mem,
if (thisStart == 0 || otherStart == 0)
continue;
- otherEnd = otherStart + other->size * 1024;
+ otherEnd = otherStart + virDomainMemoryGetMappedSize(other);
if ((thisStart <= otherStart && thisEnd > otherStart) ||
(otherStart <= thisStart && otherEnd > thisStart)) {
--
2.43.0
10:29 a.m.
New subject: [PATCH v2 1/2] domain_validate: Account for NVDIMM label size
properly when checking for memory conflicts
On Tue, Feb 20, 2024 at 04:53:29PM +0100, Michal Privoznik wrote:
As of v9.8.0-rc1~7 we check whether two <memory/> devices
don't
overlap (since we allow setting where a <memory/> device should
be mapped to). We do this pretty straightforward, by comparing
start and end address of each <memory/> device combination.
But since only the start address is given (an exposed in the
XML), the end address is computed trivially as:
start + mem->size * 1024
And for majority of memory device types this works. Except for
NVDIMMs. For them the <memory/> device consists of two separate
regions: 1) actual memory device, and 2) label.
Label is where NVDIMM stores some additional information like
namespaces partition and so on. But it's not mapped into the
guest the same way as actual memory device. In fact, mem->size is
a sum of both actual memory device and label sizes. And to make
things a bit worse, both sizes are subject to alignment (either
the alignsize value specified in XML, or system page size if not
specified in XML).
Therefore, to get the size of actual memory device we need to
take mem->size and substract label size rounded up to alignment.
If we don't do this we report there's an overlap between two
NVDIMMs even when in reality there's none.
Fixes: 3fd64fb0e236fc80ffa2cc977c0d471f11fc39bf
Fixes: 91f9a9fb4fc0d34ed8d7a869de3d9f87687c3618
Resolves: https://issues.redhat.com/browse/RHEL-4452?focusedId=23805174#comment-238...
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/conf/domain_validate.c | 50 ++++++++++++++++++++++++++++++++++++--
1 file changed, 48 insertions(+), 2 deletions(-)
diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c
index 46479f10f2..5a9398b545 100644
--- a/src/conf/domain_validate.c
+++ b/src/conf/domain_validate.c
@@ -2225,6 +2225,52 @@ virDomainHostdevDefValidate(const virDomainHostdevDef *hostdev)
}
+/**
+ * virDomainMemoryGetMappedSize:
+ * @mem: memory device definition
+ *
+ * For given memory device definition (@mem) calculate size mapped into
+ * the guest. This is usually mem->size, except for NVDIMM where its
+ * label is mapped elsewhere.
+ *
+ * Returns: Number of bytes a memory device takes when mapped into a
+ * guest.
+ */
+static unsigned long long
+virDomainMemoryGetMappedSize(const virDomainMemoryDef *mem)
+{
+ unsigned long long ret = mem->size;
+
+ if (mem->model == VIR_DOMAIN_MEMORY_MODEL_NVDIMM) {
+ unsigned long long alignsize = mem->source.nvdimm.alignsize;
+ unsigned long long labelsize = 0;
+
+ /* For NVDIMM the situation is a bit more complicated. Firstly,
+ * its <label/> is not mapped as a part of memory device, so we
+ * must subtract label size from NVDIMM size. Secondly, label is
+ * also subject to alignment so we need to round it up before
+ * subtraction. */
+
+ if (alignsize == 0) {
+ long pagesize = virGetSystemPageSizeKB();
+
+ /* If no alignment is specified in the XML, fallback to
+ * system page size alignment. */
+ if (pagesize > 0)
+ alignsize = pagesize;
I'm not very well versed in memory modules and architectures, but isn't
this restricted a bit more, or rather isn't the QEMU default slightly
different? The following two functions suggest it might be:
qemuDomainGetMemorySizeAlignment
qemuDomainGetMemoryModuleSizeAlignment
10:40 a.m.
New subject: [PATCH v2 1/2] domain_validate: Account for NVDIMM label size
properly when checking for memory conflicts
On 2/20/24 17:29, Martin Kletzander wrote:
On Tue, Feb 20, 2024 at 04:53:29PM +0100, Michal Privoznik wrote:
> As of v9.8.0-rc1~7 we check whether two <memory/> devices don't
> overlap (since we allow setting where a <memory/> device should
> be mapped to). We do this pretty straightforward, by comparing
> start and end address of each <memory/> device combination.
> But since only the start address is given (an exposed in the
> XML), the end address is computed trivially as:
>
> start + mem->size * 1024
>
> And for majority of memory device types this works. Except for
> NVDIMMs. For them the <memory/> device consists of two separate
> regions: 1) actual memory device, and 2) label.
>
> Label is where NVDIMM stores some additional information like
> namespaces partition and so on. But it's not mapped into the
> guest the same way as actual memory device. In fact, mem->size is
> a sum of both actual memory device and label sizes. And to make
> things a bit worse, both sizes are subject to alignment (either
> the alignsize value specified in XML, or system page size if not
> specified in XML).
>
> Therefore, to get the size of actual memory device we need to
> take mem->size and substract label size rounded up to alignment.
>
> If we don't do this we report there's an overlap between two
> NVDIMMs even when in reality there's none.
>
> Fixes: 3fd64fb0e236fc80ffa2cc977c0d471f11fc39bf
> Fixes: 91f9a9fb4fc0d34ed8d7a869de3d9f87687c3618
> Resolves:
> https://issues.redhat.com/browse/RHEL-4452?focusedId=23805174#comment-238...
> Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
> ---
> src/conf/domain_validate.c | 50 ++++++++++++++++++++++++++++++++++++--
> 1 file changed, 48 insertions(+), 2 deletions(-)
>
> diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c
> index 46479f10f2..5a9398b545 100644
> --- a/src/conf/domain_validate.c
> +++ b/src/conf/domain_validate.c
> @@ -2225,6 +2225,52 @@ virDomainHostdevDefValidate(const
> virDomainHostdevDef *hostdev)
> }
>
>
> +/**
> + * virDomainMemoryGetMappedSize:
> + * @mem: memory device definition
> + *
> + * For given memory device definition (@mem) calculate size mapped into
> + * the guest. This is usually mem->size, except for NVDIMM where its
> + * label is mapped elsewhere.
> + *
> + * Returns: Number of bytes a memory device takes when mapped into a
> + * guest.
> + */
> +static unsigned long long
> +virDomainMemoryGetMappedSize(const virDomainMemoryDef *mem)
> +{
> + unsigned long long ret = mem->size;
> +
> + if (mem->model == VIR_DOMAIN_MEMORY_MODEL_NVDIMM) {
> + unsigned long long alignsize = mem->source.nvdimm.alignsize;
> + unsigned long long labelsize = 0;
> +
> + /* For NVDIMM the situation is a bit more complicated. Firstly,
> + * its <label/> is not mapped as a part of memory device, so we
> + * must subtract label size from NVDIMM size. Secondly, label is
> + * also subject to alignment so we need to round it up before
> + * subtraction. */
> +
> + if (alignsize == 0) {
> + long pagesize = virGetSystemPageSizeKB();
> +
> + /* If no alignment is specified in the XML, fallback to
> + * system page size alignment. */
> + if (pagesize > 0)
> + alignsize = pagesize;
I'm not very well versed in memory modules and architectures, but isn't
this restricted a bit more, or rather isn't the QEMU default slightly
different? The following two functions suggest it might be:
qemuDomainGetMemorySizeAlignment
qemuDomainGetMemoryModuleSizeAlignment
Looking into QEMU sources, this is independent. I'm looking at
nvdimm_prepare_memory_region() declared at hw/mem/nvdimm.c in which I
can find:
if (!dimm->hostmem) {
error_setg(errp, "'" PC_DIMM_MEMDEV_PROP "' property must
be set");
return;
}
mr = host_memory_backend_get_memory(dimm->hostmem);
align = memory_region_get_alignment(mr);
size = memory_region_size(mr);
pmem_size = size - nvdimm->label_size;
nvdimm->label_data = memory_region_get_ram_ptr(mr) + pmem_size;
pmem_size = QEMU_ALIGN_DOWN(pmem_size, align);
In here, dimm->hostmem points to corresponding memory-backend-file
object with NVDIMM path (/memory/source/path in our XML). IOW, any huge
pages setting made to domain is independent of this. Then,
memory_region_get_alignment() gets alignment of that memory region,
which (looking at file_backend_memory_alloc()) corresponds to .align
attribute of the aforementioned memory-backend-file object => it
corresponds to /memory/source/alignsize in our XML or
mem->source.nvdimm.alignsize in our code. Finally, size reflects memory
size (.size attribute => /memory/target/size => mem->size).
Long story short, my comment "label is also subject to alignment" is
misleading, as it is not. The remaining memory (after label_size was
substracted) is then aligned down.
Consider the following squashed into my commit:
diff --git i/src/conf/domain_validate.c w/src/conf/domain_validate.c
index 5a9398b545..faa7659f07 100644
--- i/src/conf/domain_validate.c
+++ w/src/conf/domain_validate.c
@@ -2247,9 +2247,10 @@ virDomainMemoryGetMappedSize(const virDomainMemoryDef *mem)
/* For NVDIMM the situation is a bit more complicated. Firstly,
* its <label/> is not mapped as a part of memory device, so we
- * must subtract label size from NVDIMM size. Secondly, label is
- * also subject to alignment so we need to round it up before
- * subtraction. */
+ * must subtract label size from NVDIMM size. Secondly,
+ * remaining memory is then aligned again (rounded down). But
+ * for our purposes we might just round label size up and
+ * achieve the same (numeric) result. */
if (alignsize == 0) {
long pagesize = virGetSystemPageSizeKB();
Nice catch!
Michal
3:51 a.m.
New subject: [PATCH v2 1/2] domain_validate: Account for NVDIMM label size
properly when checking for memory conflicts
On Wed, Feb 21, 2024 at 05:40:05PM +0100, Michal Prívozník wrote:
On 2/20/24 17:29, Martin Kletzander wrote:
> On Tue, Feb 20, 2024 at 04:53:29PM +0100, Michal Privoznik wrote:
>> As of v9.8.0-rc1~7 we check whether two <memory/> devices don't
>> overlap (since we allow setting where a <memory/> device should
>> be mapped to). We do this pretty straightforward, by comparing
>> start and end address of each <memory/> device combination.
>> But since only the start address is given (an exposed in the
>> XML), the end address is computed trivially as:
>>
>> start + mem->size * 1024
>>
>> And for majority of memory device types this works. Except for
>> NVDIMMs. For them the <memory/> device consists of two separate
>> regions: 1) actual memory device, and 2) label.
>>
>> Label is where NVDIMM stores some additional information like
>> namespaces partition and so on. But it's not mapped into the
>> guest the same way as actual memory device. In fact, mem->size is
>> a sum of both actual memory device and label sizes. And to make
>> things a bit worse, both sizes are subject to alignment (either
>> the alignsize value specified in XML, or system page size if not
>> specified in XML).
>>
>> Therefore, to get the size of actual memory device we need to
>> take mem->size and substract label size rounded up to alignment.
>>
>> If we don't do this we report there's an overlap between two
>> NVDIMMs even when in reality there's none.
>>
>> Fixes: 3fd64fb0e236fc80ffa2cc977c0d471f11fc39bf
>> Fixes: 91f9a9fb4fc0d34ed8d7a869de3d9f87687c3618
>> Resolves:
>> https://issues.redhat.com/browse/RHEL-4452?focusedId=23805174#comment-238...
>> Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
>> ---
>> src/conf/domain_validate.c | 50 ++++++++++++++++++++++++++++++++++++--
>> 1 file changed, 48 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c
>> index 46479f10f2..5a9398b545 100644
>> --- a/src/conf/domain_validate.c
>> +++ b/src/conf/domain_validate.c
>> @@ -2225,6 +2225,52 @@ virDomainHostdevDefValidate(const
>> virDomainHostdevDef *hostdev)
>> }
>>
>>
>> +/**
>> + * virDomainMemoryGetMappedSize:
>> + * @mem: memory device definition
>> + *
>> + * For given memory device definition (@mem) calculate size mapped into
>> + * the guest. This is usually mem->size, except for NVDIMM where its
>> + * label is mapped elsewhere.
>> + *
>> + * Returns: Number of bytes a memory device takes when mapped into a
>> + * guest.
>> + */
>> +static unsigned long long
>> +virDomainMemoryGetMappedSize(const virDomainMemoryDef *mem)
>> +{
>> + unsigned long long ret = mem->size;
>> +
>> + if (mem->model == VIR_DOMAIN_MEMORY_MODEL_NVDIMM) {
>> + unsigned long long alignsize = mem->source.nvdimm.alignsize;
>> + unsigned long long labelsize = 0;
>> +
>> + /* For NVDIMM the situation is a bit more complicated. Firstly,
>> + * its <label/> is not mapped as a part of memory device, so we
>> + * must subtract label size from NVDIMM size. Secondly, label is
>> + * also subject to alignment so we need to round it up before
>> + * subtraction. */
>> +
>> + if (alignsize == 0) {
>> + long pagesize = virGetSystemPageSizeKB();
>> +
>> + /* If no alignment is specified in the XML, fallback to
>> + * system page size alignment. */
>> + if (pagesize > 0)
>> + alignsize = pagesize;
>
> I'm not very well versed in memory modules and architectures, but isn't
> this restricted a bit more, or rather isn't the QEMU default slightly
> different? The following two functions suggest it might be:
>
> qemuDomainGetMemorySizeAlignment
>
> qemuDomainGetMemoryModuleSizeAlignment
Looking into QEMU sources, this is independent. I'm looking at
nvdimm_prepare_memory_region() declared at hw/mem/nvdimm.c in which I
can find:
if (!dimm->hostmem) {
error_setg(errp, "'" PC_DIMM_MEMDEV_PROP "' property must
be set");
return;
}
mr = host_memory_backend_get_memory(dimm->hostmem);
align = memory_region_get_alignment(mr);
size = memory_region_size(mr);
pmem_size = size - nvdimm->label_size;
nvdimm->label_data = memory_region_get_ram_ptr(mr) + pmem_size;
pmem_size = QEMU_ALIGN_DOWN(pmem_size, align);
In here, dimm->hostmem points to corresponding memory-backend-file
object with NVDIMM path (/memory/source/path in our XML). IOW, any huge
pages setting made to domain is independent of this. Then,
memory_region_get_alignment() gets alignment of that memory region,
which (looking at file_backend_memory_alloc()) corresponds to .align
attribute of the aforementioned memory-backend-file object => it
corresponds to /memory/source/alignsize in our XML or
mem->source.nvdimm.alignsize in our code. Finally, size reflects memory
size (.size attribute => /memory/target/size => mem->size).
I was more afraid about the QEMU vs. libvirt default of the alignment if
left unspecified. But since that would only apply to niche scenarios
and this patch makes the code strictly better, so with the below diff
squashed in
Reviewed-by: Martin Kletzander <mkletzan(a)redhat.com>
Long story short, my comment "label is also subject to
alignment" is
misleading, as it is not. The remaining memory (after label_size was
substracted) is then aligned down.
Consider the following squashed into my commit:
diff --git i/src/conf/domain_validate.c w/src/conf/domain_validate.c
index 5a9398b545..faa7659f07 100644
--- i/src/conf/domain_validate.c
+++ w/src/conf/domain_validate.c
@@ -2247,9 +2247,10 @@ virDomainMemoryGetMappedSize(const virDomainMemoryDef *mem)
/* For NVDIMM the situation is a bit more complicated. Firstly,
* its <label/> is not mapped as a part of memory device, so we
- * must subtract label size from NVDIMM size. Secondly, label is
- * also subject to alignment so we need to round it up before
- * subtraction. */
+ * must subtract label size from NVDIMM size. Secondly,
+ * remaining memory is then aligned again (rounded down). But
+ * for our purposes we might just round label size up and
+ * achieve the same (numeric) result. */
if (alignsize == 0) {
long pagesize = virGetSystemPageSizeKB();
Nice catch!
Michal
9:53 a.m.
New subject: [PATCH v2 2/2] qemuxmlconftest: Introduce memory-hotplug-nvdimm-overlap test case
This new test case checks whether we are handling NVDIMMs
correctly when checking for overlapping memory devices (see
previous commit). Without previous commit, this test case would
fail, yet it was produced in real life (at least the NVDIMM
part) and thus it is valid.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
...-hotplug-nvdimm-overlap.x86_64-latest.args | 40 ++++++++++
...y-hotplug-nvdimm-overlap.x86_64-latest.xml | 1 +
.../memory-hotplug-nvdimm-overlap.xml | 77 +++++++++++++++++++
tests/qemuxmlconftest.c | 4 +
4 files changed, 122 insertions(+)
create mode 100644
tests/qemuxmlconfdata/memory-hotplug-nvdimm-overlap.x86_64-latest.args
create mode 120000 tests/qemuxmlconfdata/memory-hotplug-nvdimm-overlap.x86_64-latest.xml
create mode 100644 tests/qemuxmlconfdata/memory-hotplug-nvdimm-overlap.xml
diff --git a/tests/qemuxmlconfdata/memory-hotplug-nvdimm-overlap.x86_64-latest.args
b/tests/qemuxmlconfdata/memory-hotplug-nvdimm-overlap.x86_64-latest.args
new file mode 100644
index 0000000000..e74d2d7013
--- /dev/null
+++ b/tests/qemuxmlconfdata/memory-hotplug-nvdimm-overlap.x86_64-latest.args
@@ -0,0 +1,40 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1 \
+USER=test \
+LOGNAME=test \
+XDG_DATA_HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1/.local/share \
+XDG_CACHE_HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1/.cache \
+XDG_CONFIG_HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1/.config \
+/usr/bin/qemu-system-x86_64 \
+-name guest=QEMUGuest1,debug-threads=on \
+-S \
+-object
'{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/var/lib/libvirt/qemu/domain--1-QEMUGuest1/master-key.aes"}'
\
+-machine pc,usb=off,dump-guest-core=off,nvdimm=on,acpi=off \
+-accel tcg \
+-cpu qemu64 \
+-m size=1048576k,slots=16,maxmem=1099511627776k \
+-overcommit mem-lock=off \
+-smp 2,sockets=2,dies=1,clusters=1,cores=1,threads=1 \
+-object
'{"qom-type":"memory-backend-ram","id":"ram-node0","size":1073741824}'
\
+-numa node,nodeid=0,cpus=0-1,memdev=ram-node0 \
+-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
+-display none \
+-no-user-config \
+-nodefaults \
+-chardev socket,id=charmonitor,fd=1729,server=on,wait=off \
+-mon chardev=charmonitor,id=monitor,mode=control \
+-rtc base=utc \
+-no-shutdown \
+-boot strict=on \
+-device
'{"driver":"piix3-usb-uhci","id":"usb","bus":"pci.0","addr":"0x1.0x2"}'
\
+-object
'{"qom-type":"memory-backend-file","id":"memnvdimm0","mem-path":"/tmp/nvdimm1","share":true,"prealloc":true,"size":536870912,"align":2097152}'
\
+-device
'{"driver":"nvdimm","node":0,"label-size":131072,"memdev":"memnvdimm0","id":"nvdimm0","slot":0,"addr":4294967296}'
\
+-object
'{"qom-type":"memory-backend-file","id":"memnvdimm1","mem-path":"/tmp/nvdimm2","share":true,"prealloc":true,"size":536870912,"align":2097152}'
\
+-device
'{"driver":"nvdimm","node":0,"label-size":131072,"memdev":"memnvdimm1","id":"nvdimm1","slot":1,"addr":4829741056}'
\
+-blockdev
'{"driver":"host_device","filename":"/dev/HostVG/QEMUGuest1","node-name":"libvirt-1-storage","read-only":false}'
\
+-device
'{"driver":"ide-hd","bus":"ide.0","unit":0,"drive":"libvirt-1-storage","id":"ide0-0-0","bootindex":1}'
\
+-audiodev
'{"id":"audio1","driver":"none"}' \
+-device
'{"driver":"virtio-balloon-pci","id":"balloon0","bus":"pci.0","addr":"0x3"}'
\
+-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \
+-msg timestamp=on
diff --git a/tests/qemuxmlconfdata/memory-hotplug-nvdimm-overlap.x86_64-latest.xml
b/tests/qemuxmlconfdata/memory-hotplug-nvdimm-overlap.x86_64-latest.xml
new file mode 120000
index 0000000000..c5c37c4de9
--- /dev/null
+++ b/tests/qemuxmlconfdata/memory-hotplug-nvdimm-overlap.x86_64-latest.xml
@@ -0,0 +1 @@
+memory-hotplug-nvdimm-overlap.xml
\ No newline at end of file
diff --git a/tests/qemuxmlconfdata/memory-hotplug-nvdimm-overlap.xml
b/tests/qemuxmlconfdata/memory-hotplug-nvdimm-overlap.xml
new file mode 100644
index 0000000000..707b00c75e
--- /dev/null
+++ b/tests/qemuxmlconfdata/memory-hotplug-nvdimm-overlap.xml
@@ -0,0 +1,77 @@
+<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='x86_64' 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 mode='custom' match='exact' check='none'>
+ <model fallback='forbid'>qemu64</model>
+ <topology sockets='2' dies='1' clusters='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-x86_64</emulator>
+ <disk type='block' device='disk'>
+ <driver name='qemu' type='raw'/>
+ <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' model='piix3-uhci'>
+ <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'/>
+ <audio id='1' type='none'/>
+ <memballoon model='virtio'>
+ <address type='pci' domain='0x0000' bus='0x00'
slot='0x03' function='0x0'/>
+ </memballoon>
+ <memory model='nvdimm' access='shared'>
+ <source>
+ <path>/tmp/nvdimm1</path>
+ <alignsize unit='KiB'>2048</alignsize>
+ </source>
+ <target>
+ <size unit='KiB'>524288</size>
+ <node>0</node>
+ <label>
+ <size unit='KiB'>128</size>
+ </label>
+ </target>
+ <address type='dimm' slot='0' base='0x100000000'/>
+ </memory>
+ <memory model='nvdimm' access='shared'>
+ <source>
+ <path>/tmp/nvdimm2</path>
+ <alignsize unit='KiB'>2048</alignsize>
+ </source>
+ <target>
+ <size unit='KiB'>524288</size>
+ <node>0</node>
+ <label>
+ <size unit='KiB'>128</size>
+ </label>
+ </target>
+ <address type='dimm' slot='1' base='0x11fe00000'/>
+ </memory>
+ </devices>
+</domain>
diff --git a/tests/qemuxmlconftest.c b/tests/qemuxmlconftest.c
index e1ee1fbce3..03453e8ec8 100644
--- a/tests/qemuxmlconftest.c
+++ b/tests/qemuxmlconftest.c
@@ -2554,6 +2554,10 @@ mymain(void)
DO_TEST_CAPS_LATEST("memory-hotplug-virtio-mem");
DO_TEST_CAPS_LATEST("memory-hotplug-multiple");
DO_TEST_CAPS_LATEST_PARSE_ERROR("memory-hotplug-virtio-mem-overlap-address");
+ /* Test whether overlap calculation done in
+ * virDomainMemoryDefCheckConflict() works for NVDIMMs which are special
+ * than other memory devices because of how they handle <labelsize/> */
+ DO_TEST_CAPS_LATEST("memory-hotplug-nvdimm-overlap");
DO_TEST_CAPS_ARCH_LATEST("machine-aeskeywrap-on-caps", "s390x");
DO_TEST_CAPS_ARCH_LATEST("machine-aeskeywrap-on-cap", "s390x");
--
2.43.0
3:51 a.m.
New subject: [PATCH v2 2/2] qemuxmlconftest: Introduce
memory-hotplug-nvdimm-overlap test case
On Tue, Feb 20, 2024 at 04:53:30PM +0100, Michal Privoznik wrote:
This new test case checks whether we are handling NVDIMMs
correctly when checking for overlapping memory devices (see
previous commit). Without previous commit, this test case would
fail, yet it was produced in real life (at least the NVDIMM
part) and thus it is valid.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
Reviewed-by: Martin Kletzander <mkletzan(a)redhat.com>
305
days inactive
307
days old
6 comments
3 participants
participants (3)
-
Martin Kletzander -
Michal Privoznik -
Michal Prívozník