On Fri, Apr 23, 2021 at 15:24:27 +0200, Michal Privoznik wrote:
[...]
diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
index 1b9b221611..44e478c88a 100644
--- a/docs/formatdomain.rst
+++ b/docs/formatdomain.rst
[...]
@@ -7793,7 +7807,8 @@ Example: usage of the memory devices
added memory from the perspective of the guest.
The mandatory ``size`` subelement configures the size of the added memory as
- a scaled integer.
+ a scaled integer. For ``virtio-mem`` this represents the maximum possible
+ size exposed to the guest.
The ``node`` subelement configures the guest NUMA node to attach the memory
to. The element shall be used only if the guest has NUMA nodes configured.
@@ -7820,6 +7835,17 @@ Example: usage of the memory devices
so other backend types should use the ``readonly`` element. :since:`Since
5.0.0`
+ ``block``
+ For ``virtio-mem`` only.
+ The size of an individual block, granularity of division of memory module.
I'd refrain from using 'memory module' as this is specifically
paravirtualized -> no modules.
+ Must be power of two and at least equal to size of a
transparent hugepage
+ (2MiB on x84_64). The default is hypervisor dependent.
+
+ ``requested``
+ For ``virtio-mem`` only.
+ The total size exposed to the guest. Must respect ``block`` granularity
+ and be smaller or equal to ``size``.
+
:anchor:`<a id="elementsIommu"/>`
IOMMU devices
[...]
diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c
index 686b9e8d16..16fa65bc6b 100644
--- a/src/conf/domain_validate.c
+++ b/src/conf/domain_validate.c
[...]
@@ -1896,6 +1899,42 @@ virDomainMemoryDefValidate(const
virDomainMemoryDef *mem,
_("virtio-pmem does not support NUMA nodes"));
return -1;
}
+ break;
+
+ case VIR_DOMAIN_MEMORY_MODEL_VIRTIO_MEM:
+ if (mem->requestedsize > mem->size) {
+ virReportError(VIR_ERR_XML_DETAIL, "%s",
+ _("requested size must be smaller than @size"));
+ return -1;
+ }
I'd expect that 'mem->requestedsize == mem->size' is also okay
otherwise
it'll rob you from using the last block.
+
+ if (!VIR_IS_POW2(mem->blocksize)) {
+ virReportError(VIR_ERR_XML_DETAIL, "%s",
+ _("block size must be a power of two"));
+ return -1;
+ }
+
+ if (virHostMemGetTHPSize(&thpSize) < 0) {
+ /* We failed to get THP size, fall back to a sane default. On
+ * almost every architecture the size will be 2MiB, except for some
+ * funky arches like sparc and m68k. Use 2MiB and refine later if
+ * somebody complains. */
+ thpSize = 2048;
+ }
[...]
diff --git a/src/qemu/qemu_alias.c b/src/qemu/qemu_alias.c
index 63638b1402..e1caeec006 100644
--- a/src/qemu/qemu_alias.c
+++ b/src/qemu/qemu_alias.c
@@ -523,6 +523,7 @@ qemuAssignDeviceMemoryAlias(virDomainDef *def,
case VIR_DOMAIN_MEMORY_MODEL_VIRTIO_PMEM:
prefix = "virtiopmem";
break;
+ case VIR_DOMAIN_MEMORY_MODEL_VIRTIO_MEM:
case VIR_DOMAIN_MEMORY_MODEL_NONE:
case VIR_DOMAIN_MEMORY_MODEL_LAST:
default:
In this hunk you refrain from adding qemuisms and they are added
later, but all the following hunks add qemu-specific details.
Specifically the last one adding qemuCaps check is questionable.
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 6e3e3555c7..c182b47f38 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -8968,6 +8968,16 @@ qemuDomainDefValidateMemoryHotplugDevice(const virDomainMemoryDef
*mem,
needsNuma = false;
break;
+ case VIR_DOMAIN_MEMORY_MODEL_VIRTIO_MEM:
+ if (mem->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI &&
+ mem->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("only 'pci' addresses are supported for the %s
device"),
+ virDomainMemoryModelTypeToString(mem->model));
+ return -1;
+ }
+ break;
+
case VIR_DOMAIN_MEMORY_MODEL_NONE:
case VIR_DOMAIN_MEMORY_MODEL_LAST:
return -1;
diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c
index 1ee75b8f2e..c632b055f1 100644
--- a/src/qemu/qemu_domain_address.c
+++ b/src/qemu/qemu_domain_address.c
@@ -380,9 +380,18 @@ qemuDomainPrimeVirtioDeviceAddresses(virDomainDef *def,
}
for (i = 0; i < def->nmems; i++) {
- if (def->mems[i]->model == VIR_DOMAIN_MEMORY_MODEL_VIRTIO_PMEM &&
- def->mems[i]->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE)
- def->mems[i]->info.type = type;
+ switch (def->mems[i]->model) {
+ case VIR_DOMAIN_MEMORY_MODEL_VIRTIO_PMEM:
+ case VIR_DOMAIN_MEMORY_MODEL_VIRTIO_MEM:
+ if (def->mems[i]->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE)
+ def->mems[i]->info.type = type;
+ break;
+ case VIR_DOMAIN_MEMORY_MODEL_NONE:
+ case VIR_DOMAIN_MEMORY_MODEL_DIMM:
+ case VIR_DOMAIN_MEMORY_MODEL_NVDIMM:
+ case VIR_DOMAIN_MEMORY_MODEL_LAST:
+ break;
+ }
}
if (type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) {
[...]
@@ -2439,12 +2449,19 @@ qemuDomainAssignDevicePCISlots(virDomainDef
*def,
for (i = 0; i < def->nmems; i++) {
virDomainMemoryDef *mem = def->mems[i];
- if (mem->model != VIR_DOMAIN_MEMORY_MODEL_VIRTIO_PMEM ||
- !virDeviceInfoPCIAddressIsWanted(&mem->info))
- continue;
-
- if (qemuDomainPCIAddressReserveNextAddr(addrs, &mem->info) < 0)
- return -1;
+ switch (mem->model) {
+ case VIR_DOMAIN_MEMORY_MODEL_VIRTIO_PMEM:
+ case VIR_DOMAIN_MEMORY_MODEL_VIRTIO_MEM:
+ if (virDeviceInfoPCIAddressIsWanted(&mem->info) &&
+ qemuDomainPCIAddressReserveNextAddr(addrs, &mem->info) < 0)
+ return -1;
+ break;
+ case VIR_DOMAIN_MEMORY_MODEL_NONE:
+ case VIR_DOMAIN_MEMORY_MODEL_DIMM:
+ case VIR_DOMAIN_MEMORY_MODEL_NVDIMM:
+ case VIR_DOMAIN_MEMORY_MODEL_LAST:
+ break;
+ }
}
return 0;
[...]
diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
index 255d653118..b02b33a0f1 100644
--- a/src/qemu/qemu_validate.c
+++ b/src/qemu/qemu_validate.c
@@ -4895,6 +4895,14 @@ qemuValidateDomainDeviceDefMemory(virDomainMemoryDef *mem,
}
break;
+ case VIR_DOMAIN_MEMORY_MODEL_VIRTIO_MEM:
+ if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIRTIO_MEM_PCI)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("virtio-mem isn't supported by this QEMU
binary"));
+ return -1;
+ }
+ break;
+
case VIR_DOMAIN_MEMORY_MODEL_NONE:
case VIR_DOMAIN_MEMORY_MODEL_LAST:
break;
With the nits addressed:
Reviewed-by: Peter Krempa <pkrempa(a)redhat.com>