Currently, inside of virt-aa-helper code the domain definition is parsed and then all def->mems are iterated over and for NVDIMM models corresponding nvdimmPath is set label on. Conceptually, this code works (except the label should be set for VIRTIO_PMEM model too, but that is addressed in the next commit), but it can be written in more extensible way. Firstly, there's no need to check whether def->mems[i] is not NULL because we're inside a for() loop that's counting through def->nmems. Secondly, we can have a helper variable ('mem') to make the code more readable (just like we do in other loops). Then, we can use switch() to allow compiler warn us on new memory model. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- src/security/virt-aa-helper.c | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index da1e4fc3e4..a0c76d24a8 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -1147,10 +1147,20 @@ get_files(vahControl * ctl) } for (i = 0; i < ctl->def->nmems; i++) { - if (ctl->def->mems[i] && - ctl->def->mems[i]->model == VIR_DOMAIN_MEMORY_MODEL_NVDIMM) { - if (vah_add_file(&buf, ctl->def->mems[i]->nvdimmPath, "rw") != 0) + virDomainMemoryDef *mem = ctl->def->mems[i]; + + switch (mem->model) { + case VIR_DOMAIN_MEMORY_MODEL_NVDIMM: + if (vah_add_file(&buf, mem->nvdimmPath, "rw") != 0) goto cleanup; + break; + case VIR_DOMAIN_MEMORY_MODEL_DIMM: + case VIR_DOMAIN_MEMORY_MODEL_VIRTIO_PMEM: + case VIR_DOMAIN_MEMORY_MODEL_VIRTIO_MEM: + case VIR_DOMAIN_MEMORY_MODEL_SGX_EPC: + case VIR_DOMAIN_MEMORY_MODEL_NONE: + case VIR_DOMAIN_MEMORY_MODEL_LAST: + break; } } -- 2.41.0

As of v7.9.0-rc1~296 users have ability to adjust what portion of virtio-mem is exposed to the guest. Then, as of v9.4.0-rc2~5 they have ability to set address where the memory is mapped. But due to a missing check it was possible to feed virDomainUpdateDeviceFlags() API with memory device XML that changes the address. This is of course not possible and should be forbidden. Add the missing check. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- src/qemu/qemu_hotplug.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 2e3c99760d..03a50191ef 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -7102,6 +7102,13 @@ qemuDomainChangeMemoryLiveValidateChange(const virDomainMemoryDef *oldDef, return false; } + if (oldDef->address != newDef->address) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("cannot modify memory address from '0x%1$llx' to '0x%2$llx'"), + oldDef->address, newDef->address); + return false; + } + return true; } -- 2.41.0

This is similar to one of my previous commits. Simply speaking, users can specify address where a memory device is mapped to. And as such, we should include it when looking up corresponding device in domain definition (e.g. on device hot unplug). Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- src/conf/domain_conf.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 47693a49bf..9a4a26d875 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -15254,7 +15254,8 @@ virDomainMemoryFindByDefInternal(virDomainDef *def, tmp->targetNode != mem->targetNode || tmp->size != mem->size || tmp->blocksize != mem->blocksize || - tmp->requestedsize != mem->requestedsize) + tmp->requestedsize != mem->requestedsize || + tmp->address != mem->address) continue; switch (mem->model) { -- 2.41.0

The _virDomainMemoryDef struct is getting a bit messy. It has various members and only some of them are valid for given model. Worse, some are re-used for different models. We tried to make this more bearable by putting a comment next to each member describing what models the member is valid for, but that gets messy too. Therefore, do what we do elsewhere: introduce an union of structs and move individual members into their respective groups. This allows us to shorten some names (e.g. nvdimmPath or sourceNodes) as their purpose is obvious due to their placement. But to make this commit as small as possible, that'll be addressed later. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- src/conf/domain_conf.c | 125 +++++++++++++++++++++++-------- src/conf/domain_conf.h | 29 +++++-- src/conf/domain_validate.c | 4 +- src/qemu/qemu_cgroup.c | 12 ++- src/qemu/qemu_command.c | 97 ++++++++++++++++++------ src/qemu/qemu_hotplug.c | 10 +-- src/qemu/qemu_namespace.c | 4 +- src/qemu/qemu_process.c | 10 ++- src/qemu/qemu_validate.c | 4 +- src/security/security_apparmor.c | 24 ++++-- src/security/security_dac.c | 9 ++- src/security/security_selinux.c | 52 ++++++------- src/security/virt-aa-helper.c | 5 +- 13 files changed, 272 insertions(+), 113 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 9a4a26d875..a1dad679dd 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -3490,8 +3490,27 @@ void virDomainMemoryDefFree(virDomainMemoryDef *def) if (!def) return; - g_free(def->nvdimmPath); - virBitmapFree(def->sourceNodes); + switch (def->model) { + case VIR_DOMAIN_MEMORY_MODEL_DIMM: + virBitmapFree(def->source.dimm.sourceNodes); + break; + case VIR_DOMAIN_MEMORY_MODEL_NVDIMM: + g_free(def->source.nvdimm.nvdimmPath); + break; + case VIR_DOMAIN_MEMORY_MODEL_VIRTIO_PMEM: + g_free(def->source.virtio_pmem.nvdimmPath); + break; + case VIR_DOMAIN_MEMORY_MODEL_VIRTIO_MEM: + virBitmapFree(def->source.dimm.sourceNodes); + break; + case VIR_DOMAIN_MEMORY_MODEL_SGX_EPC: + virBitmapFree(def->source.sgx_epc.sourceNodes); + break; + case VIR_DOMAIN_MEMORY_MODEL_NONE: + case VIR_DOMAIN_MEMORY_MODEL_LAST: + break; + } + g_free(def->uuid); virDomainDeviceInfoClear(&def->info); g_free(def); @@ -13267,22 +13286,33 @@ virDomainMemorySourceDefParseXML(xmlNodePtr node, { VIR_XPATH_NODE_AUTORESTORE(ctxt) g_autofree char *nodemask = NULL; + unsigned long long *pagesize; + virBitmap **sourceNodes = NULL; ctxt->node = node; switch (def->model) { case VIR_DOMAIN_MEMORY_MODEL_DIMM: case VIR_DOMAIN_MEMORY_MODEL_VIRTIO_MEM: + + if (def->model == VIR_DOMAIN_MEMORY_MODEL_DIMM) { + pagesize = &def->source.dimm.pagesize; + sourceNodes = &def->source.dimm.sourceNodes; + } else { + pagesize = &def->source.virtio_mem.pagesize; + sourceNodes = &def->source.virtio_mem.sourceNodes; + } + if (virDomainParseMemory("./pagesize", "./pagesize/@unit", ctxt, - &def->pagesize, false, false) < 0) + pagesize, false, false) < 0) return -1; if ((nodemask = virXPathString("string(./nodemask)", ctxt))) { - if (virBitmapParse(nodemask, &def->sourceNodes, + if (virBitmapParse(nodemask, sourceNodes, VIR_DOMAIN_CPUMASK_LEN) < 0) return -1; - if (virBitmapIsAllClear(def->sourceNodes)) { + if (virBitmapIsAllClear(*sourceNodes)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Invalid value of 'nodemask': %1$s"), nodemask); return -1; @@ -13291,28 +13321,28 @@ virDomainMemorySourceDefParseXML(xmlNodePtr node, break; case VIR_DOMAIN_MEMORY_MODEL_NVDIMM: - def->nvdimmPath = virXPathString("string(./path)", ctxt); + def->source.nvdimm.nvdimmPath = virXPathString("string(./path)", ctxt); if (virDomainParseMemory("./alignsize", "./alignsize/@unit", ctxt, - &def->alignsize, false, false) < 0) + &def->source.nvdimm.alignsize, false, false) < 0) return -1; if (virXPathBoolean("boolean(./pmem)", ctxt)) - def->nvdimmPmem = true; + def->source.nvdimm.nvdimmPmem = true; break; case VIR_DOMAIN_MEMORY_MODEL_VIRTIO_PMEM: - def->nvdimmPath = virXPathString("string(./path)", ctxt); + def->source.virtio_pmem.nvdimmPath = virXPathString("string(./path)", ctxt); break; case VIR_DOMAIN_MEMORY_MODEL_SGX_EPC: if ((nodemask = virXPathString("string(./nodemask)", ctxt))) { - if (virBitmapParse(nodemask, &def->sourceNodes, + if (virBitmapParse(nodemask, &def->source.sgx_epc.sourceNodes, VIR_DOMAIN_CPUMASK_LEN) < 0) return -1; - if (virBitmapIsAllClear(def->sourceNodes)) { + if (virBitmapIsAllClear(def->source.sgx_epc.sourceNodes)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Invalid value of 'nodemask': %1$s"), nodemask); return -1; @@ -15260,27 +15290,36 @@ virDomainMemoryFindByDefInternal(virDomainDef *def, switch (mem->model) { case VIR_DOMAIN_MEMORY_MODEL_DIMM: + if (tmp->source.dimm.pagesize != mem->source.dimm.pagesize) + continue; + + if (!virBitmapEqual(tmp->source.dimm.sourceNodes, + mem->source.dimm.sourceNodes)) + continue; + break; case VIR_DOMAIN_MEMORY_MODEL_VIRTIO_MEM: - /* source stuff -> match with device */ - if (tmp->pagesize != mem->pagesize) + if (tmp->source.virtio_mem.pagesize != mem->source.virtio_mem.pagesize) continue; - if (!virBitmapEqual(tmp->sourceNodes, mem->sourceNodes)) + if (!virBitmapEqual(tmp->source.virtio_mem.sourceNodes, + mem->source.virtio_mem.sourceNodes)) continue; break; case VIR_DOMAIN_MEMORY_MODEL_NVDIMM: - if (STRNEQ(tmp->nvdimmPath, mem->nvdimmPath)) + if (STRNEQ(tmp->source.nvdimm.nvdimmPath, mem->source.nvdimm.nvdimmPath)) continue; break; case VIR_DOMAIN_MEMORY_MODEL_VIRTIO_PMEM: - if (STRNEQ(tmp->nvdimmPath, mem->nvdimmPath)) + if (STRNEQ(tmp->source.virtio_pmem.nvdimmPath, + mem->source.virtio_pmem.nvdimmPath)) continue; break; case VIR_DOMAIN_MEMORY_MODEL_SGX_EPC: - if (!virBitmapEqual(tmp->sourceNodes, mem->sourceNodes)) + if (!virBitmapEqual(tmp->source.sgx_epc.sourceNodes, + mem->source.sgx_epc.sourceNodes)) continue; break; @@ -21004,7 +21043,8 @@ virDomainMemoryDefCheckABIStability(virDomainMemoryDef *src, return false; } - if (src->model == VIR_DOMAIN_MEMORY_MODEL_NVDIMM) { + switch (src->model) { + case VIR_DOMAIN_MEMORY_MODEL_NVDIMM: if (src->labelsize != dst->labelsize) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Target NVDIMM label size '%1$llu' doesn't match source NVDIMM label size '%2$llu'"), @@ -21012,14 +21052,15 @@ virDomainMemoryDefCheckABIStability(virDomainMemoryDef *src, return false; } - if (src->alignsize != dst->alignsize) { + if (src->source.nvdimm.alignsize != dst->source.nvdimm.alignsize) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Target NVDIMM alignment '%1$llu' doesn't match source NVDIMM alignment '%2$llu'"), - src->alignsize, dst->alignsize); + src->source.nvdimm.alignsize, + dst->source.nvdimm.alignsize); return false; } - if (src->nvdimmPmem != dst->nvdimmPmem) { + if (src->source.nvdimm.nvdimmPmem != dst->source.nvdimm.nvdimmPmem) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Target NVDIMM pmem flag doesn't match " "source NVDIMM pmem flag")); @@ -21040,6 +21081,15 @@ virDomainMemoryDefCheckABIStability(virDomainMemoryDef *src, _("Target NVDIMM UUID doesn't match source NVDIMM")); return false; } + break; + + case VIR_DOMAIN_MEMORY_MODEL_DIMM: + case VIR_DOMAIN_MEMORY_MODEL_VIRTIO_PMEM: + case VIR_DOMAIN_MEMORY_MODEL_VIRTIO_MEM: + case VIR_DOMAIN_MEMORY_MODEL_SGX_EPC: + case VIR_DOMAIN_MEMORY_MODEL_NONE: + case VIR_DOMAIN_MEMORY_MODEL_LAST: + break; } return virDomainDeviceInfoCheckABIStability(&src->info, &dst->info); @@ -25097,37 +25147,48 @@ virDomainMemorySourceDefFormat(virBuffer *buf, switch (def->model) { case VIR_DOMAIN_MEMORY_MODEL_DIMM: + if (def->source.dimm.sourceNodes) { + if (!(bitmap = virBitmapFormat(def->source.dimm.sourceNodes))) + return -1; + + virBufferAsprintf(&childBuf, "<nodemask>%s</nodemask>\n", bitmap); + } + + if (def->source.dimm.pagesize) + virBufferAsprintf(&childBuf, "<pagesize unit='KiB'>%llu</pagesize>\n", + def->source.dimm.pagesize); + break; case VIR_DOMAIN_MEMORY_MODEL_VIRTIO_MEM: - if (def->sourceNodes) { - if (!(bitmap = virBitmapFormat(def->sourceNodes))) + if (def->source.virtio_mem.sourceNodes) { + if (!(bitmap = virBitmapFormat(def->source.virtio_mem.sourceNodes))) return -1; virBufferAsprintf(&childBuf, "<nodemask>%s</nodemask>\n", bitmap); } - if (def->pagesize) + if (def->source.virtio_mem.pagesize) virBufferAsprintf(&childBuf, "<pagesize unit='KiB'>%llu</pagesize>\n", - def->pagesize); + def->source.virtio_mem.pagesize); break; case VIR_DOMAIN_MEMORY_MODEL_NVDIMM: - virBufferEscapeString(&childBuf, "<path>%s</path>\n", def->nvdimmPath); + virBufferEscapeString(&childBuf, "<path>%s</path>\n", def->source.nvdimm.nvdimmPath); - if (def->alignsize) + if (def->source.nvdimm.alignsize) virBufferAsprintf(&childBuf, "<alignsize unit='KiB'>%llu</alignsize>\n", - def->alignsize); + def->source.nvdimm.alignsize); - if (def->nvdimmPmem) + if (def->source.nvdimm.nvdimmPmem) virBufferAddLit(&childBuf, "<pmem/>\n"); break; case VIR_DOMAIN_MEMORY_MODEL_VIRTIO_PMEM: - virBufferEscapeString(&childBuf, "<path>%s</path>\n", def->nvdimmPath); + virBufferEscapeString(&childBuf, "<path>%s</path>\n", def->source.virtio_pmem.nvdimmPath); break; case VIR_DOMAIN_MEMORY_MODEL_SGX_EPC: - if (def->sourceNodes) { - if (!(bitmap = virBitmapFormat(def->sourceNodes))) + if (def->source.sgx_epc.sourceNodes) { + if (!(bitmap = virBitmapFormat(def->source.sgx_epc.sourceNodes))) return -1; virBufferAsprintf(&childBuf, "<nodemask>%s</nodemask>\n", bitmap); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index c857ba556f..8770547590 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2634,18 +2634,33 @@ typedef enum { } virDomainMemoryModel; struct _virDomainMemoryDef { + virDomainMemoryModel model; virDomainMemoryAccess access; virTristateBool discard; - /* source */ - virBitmap *sourceNodes; - unsigned long long pagesize; /* kibibytes */ - char *nvdimmPath; /* valid for NVDIMM an VIRTIO_PMEM */ - unsigned long long alignsize; /* kibibytes; valid only for NVDIMM */ - bool nvdimmPmem; /* valid only for NVDIMM */ + union { + struct { + unsigned long long pagesize; /* kibibytes */ + virBitmap *sourceNodes; + } dimm; + struct { + char *nvdimmPath; + bool nvdimmPmem; + unsigned long long alignsize; /* kibibytes */ + } nvdimm; + struct { + char *nvdimmPath; + } virtio_pmem; + struct { + unsigned long long pagesize; /* kibibytes */ + virBitmap *sourceNodes; + } virtio_mem; + struct { + virBitmap *sourceNodes; + } sgx_epc; + } source; /* target */ - virDomainMemoryModel model; int targetNode; unsigned long long size; /* kibibytes */ unsigned long long labelsize; /* kibibytes; valid only for NVDIMM */ diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c index ad383b604e..2832fd010e 100644 --- a/src/conf/domain_validate.c +++ b/src/conf/domain_validate.c @@ -2260,7 +2260,7 @@ virDomainMemoryDefValidate(const virDomainMemoryDef *mem, switch (mem->model) { case VIR_DOMAIN_MEMORY_MODEL_NVDIMM: - if (!mem->nvdimmPath) { + if (!mem->source.nvdimm.nvdimmPath) { virReportError(VIR_ERR_XML_DETAIL, "%s", _("path is required for model 'nvdimm'")); return -1; @@ -2286,7 +2286,7 @@ virDomainMemoryDefValidate(const virDomainMemoryDef *mem, break; case VIR_DOMAIN_MEMORY_MODEL_VIRTIO_PMEM: - if (!mem->nvdimmPath) { + if (!mem->source.virtio_pmem.nvdimmPath) { virReportError(VIR_ERR_XML_DETAIL, _("path is required for model '%1$s'"), virDomainMemoryModelTypeToString(mem->model)); diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index fe78a0251a..b9d893fd86 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -553,8 +553,12 @@ qemuSetupMemoryDevicesCgroup(virDomainObj *vm, switch (mem->model) { case VIR_DOMAIN_MEMORY_MODEL_NVDIMM: + if (qemuCgroupAllowDevicePath(vm, mem->source.nvdimm.nvdimmPath, + VIR_CGROUP_DEVICE_RW, false) < 0) + return -1; + break; case VIR_DOMAIN_MEMORY_MODEL_VIRTIO_PMEM: - if (qemuCgroupAllowDevicePath(vm, mem->nvdimmPath, + if (qemuCgroupAllowDevicePath(vm, mem->source.virtio_pmem.nvdimmPath, VIR_CGROUP_DEVICE_RW, false) < 0) return -1; break; @@ -587,8 +591,12 @@ qemuTeardownMemoryDevicesCgroup(virDomainObj *vm, switch (mem->model) { case VIR_DOMAIN_MEMORY_MODEL_NVDIMM: + if (qemuCgroupDenyDevicePath(vm, mem->source.nvdimm.nvdimmPath, + VIR_CGROUP_DEVICE_RWM, false) < 0) + return -1; + break; case VIR_DOMAIN_MEMORY_MODEL_VIRTIO_PMEM: - if (qemuCgroupDenyDevicePath(vm, mem->nvdimmPath, + if (qemuCgroupDenyDevicePath(vm, mem->source.virtio_pmem.nvdimmPath, VIR_CGROUP_DEVICE_RWM, false) < 0) return -1; break; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 77c5335bde..2e06261ef2 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3146,11 +3146,34 @@ qemuBuildMemoryGetPagesize(virQEMUDriverConfig *cfg, bool *preallocRet) { const long system_page_size = virGetSystemPageSizeKB(); - unsigned long long pagesize = mem->pagesize; - bool needHugepage = !!pagesize; - bool useHugepage = !!pagesize; + unsigned long long pagesize = 0; + const char *nvdimmPath = NULL; + bool needHugepage = false; + bool useHugepage = false; bool prealloc = false; + switch (mem->model) { + case VIR_DOMAIN_MEMORY_MODEL_DIMM: + pagesize = mem->source.dimm.pagesize; + break; + case VIR_DOMAIN_MEMORY_MODEL_VIRTIO_MEM: + pagesize = mem->source.virtio_mem.pagesize; + break; + case VIR_DOMAIN_MEMORY_MODEL_NVDIMM: + nvdimmPath = mem->source.nvdimm.nvdimmPath; + break; + case VIR_DOMAIN_MEMORY_MODEL_VIRTIO_PMEM: + nvdimmPath = mem->source.virtio_pmem.nvdimmPath; + break; + case VIR_DOMAIN_MEMORY_MODEL_SGX_EPC: + case VIR_DOMAIN_MEMORY_MODEL_NONE: + case VIR_DOMAIN_MEMORY_MODEL_LAST: + break; + } + + needHugepage = !!pagesize; + useHugepage = !!pagesize; + if (pagesize == 0) { virDomainHugePage *master_hugepage = NULL; virDomainHugePage *hugepage = NULL; @@ -3216,10 +3239,11 @@ qemuBuildMemoryGetPagesize(virQEMUDriverConfig *cfg, /* If the NVDIMM is a real device then there's nothing to prealloc. * If anything, we would be only wearing off the device. * Similarly, virtio-pmem-pci doesn't need prealloc either. */ - if (mem->nvdimmPath) { - if (!mem->nvdimmPmem && - mem->model != VIR_DOMAIN_MEMORY_MODEL_VIRTIO_PMEM) + if (nvdimmPath) { + if (mem->model == VIR_DOMAIN_MEMORY_MODEL_NVDIMM && + !mem->source.nvdimm.nvdimmPmem) { prealloc = true; + } } else if (useHugepage) { prealloc = true; } @@ -3286,6 +3310,8 @@ qemuBuildMemoryBackendProps(virJSONValue **backendProps, unsigned long long pagesize = 0; bool needHugepage = false; bool useHugepage = false; + bool hasSourceNodes = false; + const char *nvdimmPath = NULL; int discard = mem->discard; bool disableCanonicalPath = false; @@ -3324,12 +3350,36 @@ qemuBuildMemoryBackendProps(virJSONValue **backendProps, props = virJSONValueNewObject(); + switch (mem->model) { + case VIR_DOMAIN_MEMORY_MODEL_DIMM: + nodemask = mem->source.dimm.sourceNodes; + break; + case VIR_DOMAIN_MEMORY_MODEL_VIRTIO_MEM: + nodemask = mem->source.virtio_mem.sourceNodes; + break; + case VIR_DOMAIN_MEMORY_MODEL_SGX_EPC: + nodemask = mem->source.sgx_epc.sourceNodes; + break; + case VIR_DOMAIN_MEMORY_MODEL_NVDIMM: + nvdimmPath = mem->source.nvdimm.nvdimmPath; + break; + case VIR_DOMAIN_MEMORY_MODEL_VIRTIO_PMEM: + nvdimmPath = mem->source.virtio_pmem.nvdimmPath; + break; + case VIR_DOMAIN_MEMORY_MODEL_NONE: + case VIR_DOMAIN_MEMORY_MODEL_LAST: + break; + } + + hasSourceNodes = !!nodemask; + if (mem->model == VIR_DOMAIN_MEMORY_MODEL_SGX_EPC) { backendType = "memory-backend-epc"; if (!priv->memPrealloc) prealloc = true; - } else if (!mem->nvdimmPath && - def->mem.source == VIR_DOMAIN_MEMORY_SOURCE_MEMFD) { + + } else if (!nvdimmPath && + def->mem.source == VIR_DOMAIN_MEMORY_SOURCE_MEMFD) { backendType = "memory-backend-memfd"; if (useHugepage && @@ -3343,11 +3393,11 @@ qemuBuildMemoryBackendProps(virJSONValue **backendProps, if (systemMemory) disableCanonicalPath = true; - } else if (useHugepage || mem->nvdimmPath || memAccess || - def->mem.source == VIR_DOMAIN_MEMORY_SOURCE_FILE) { + } else if (useHugepage || nvdimmPath || memAccess || + def->mem.source == VIR_DOMAIN_MEMORY_SOURCE_FILE) { - if (mem->nvdimmPath) { - memPath = g_strdup(mem->nvdimmPath); + if (nvdimmPath) { + memPath = g_strdup(nvdimmPath); } else if (useHugepage) { if (qemuGetDomainHupageMemPath(priv->driver, def, pagesize, &memPath) < 0) return -1; @@ -3363,7 +3413,7 @@ qemuBuildMemoryBackendProps(virJSONValue **backendProps, NULL) < 0) return -1; - if (!mem->nvdimmPath && + if (!nvdimmPath && virJSONValueObjectAdd(&props, "T:discard-data", discard, NULL) < 0) { @@ -3423,10 +3473,13 @@ qemuBuildMemoryBackendProps(virJSONValue **backendProps, if (virJSONValueObjectAdd(&props, "U:size", mem->size * 1024, NULL) < 0) return -1; - if (virJSONValueObjectAdd(&props, "P:align", mem->alignsize * 1024, NULL) < 0) + if (mem->model == VIR_DOMAIN_MEMORY_MODEL_NVDIMM && + virJSONValueObjectAdd(&props, "P:align", + mem->source.nvdimm.alignsize * 1024, NULL) < 0) return -1; - if (mem->nvdimmPmem) { + if (mem->model == VIR_DOMAIN_MEMORY_MODEL_NVDIMM && + mem->source.nvdimm.nvdimmPmem) { if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_OBJECT_MEMORY_FILE_PMEM)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("nvdimm pmem property is not available " @@ -3437,12 +3490,10 @@ qemuBuildMemoryBackendProps(virJSONValue **backendProps, return -1; } - if (mem->sourceNodes) { - nodemask = mem->sourceNodes; - } else { - if (virDomainNumatuneMaybeGetNodeset(def->numa, priv->autoNodeset, - &nodemask, mem->targetNode) < 0) - return -1; + if (!hasSourceNodes && + virDomainNumatuneMaybeGetNodeset(def->numa, priv->autoNodeset, + &nodemask, mem->targetNode) < 0) { + return -1; } if (nodemask) { @@ -3466,8 +3517,8 @@ qemuBuildMemoryBackendProps(virJSONValue **backendProps, } /* If none of the following is requested... */ - if (!needHugepage && !mem->sourceNodes && !nodeSpecified && - !mem->nvdimmPath && + if (!needHugepage && !hasSourceNodes && !nodeSpecified && + !nvdimmPath && memAccess == VIR_DOMAIN_MEMORY_ACCESS_DEFAULT && def->mem.source != VIR_DOMAIN_MEMORY_SOURCE_FILE && def->mem.source != VIR_DOMAIN_MEMORY_SOURCE_MEMFD && diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index e5f587ba65..ece5b2598d 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -7022,18 +7022,18 @@ qemuDomainChangeMemoryLiveValidateChange(const virDomainMemoryDef *oldDef, return false; } - if (!virBitmapEqual(oldDef->sourceNodes, - newDef->sourceNodes)) { + if (!virBitmapEqual(oldDef->source.virtio_mem.sourceNodes, + newDef->source.virtio_mem.sourceNodes)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("cannot modify memory source nodes")); return false; } - if (oldDef->pagesize != newDef->pagesize) { + if (oldDef->source.virtio_mem.pagesize != newDef->source.virtio_mem.pagesize) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("cannot modify memory pagesize from '%1$llu' to '%2$llu'"), - oldDef->pagesize, - newDef->pagesize); + oldDef->source.virtio_mem.pagesize, + newDef->source.virtio_mem.pagesize); return false; } diff --git a/src/qemu/qemu_namespace.c b/src/qemu/qemu_namespace.c index 7fa43253b1..206bbd7375 100644 --- a/src/qemu/qemu_namespace.c +++ b/src/qemu/qemu_namespace.c @@ -372,8 +372,10 @@ qemuDomainSetupMemory(virDomainMemoryDef *mem, { switch (mem->model) { case VIR_DOMAIN_MEMORY_MODEL_NVDIMM: + *paths = g_slist_prepend(*paths, g_strdup(mem->source.nvdimm.nvdimmPath)); + break; case VIR_DOMAIN_MEMORY_MODEL_VIRTIO_PMEM: - *paths = g_slist_prepend(*paths, g_strdup(mem->nvdimmPath)); + *paths = g_slist_prepend(*paths, g_strdup(mem->source.virtio_pmem.nvdimmPath)); break; case VIR_DOMAIN_MEMORY_MODEL_SGX_EPC: diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 0644f80161..aa2593fa9a 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3927,11 +3927,15 @@ static bool qemuProcessDomainMemoryDefNeedHugepagesPath(const virDomainMemoryDef *mem, const long system_pagesize) { + unsigned long long pagesize = 0; + switch (mem->model) { case VIR_DOMAIN_MEMORY_MODEL_DIMM: + pagesize = mem->source.dimm.pagesize; + break; case VIR_DOMAIN_MEMORY_MODEL_VIRTIO_MEM: - return mem->pagesize && mem->pagesize != system_pagesize; - + pagesize = mem->source.virtio_mem.pagesize; + break; case VIR_DOMAIN_MEMORY_MODEL_NONE: case VIR_DOMAIN_MEMORY_MODEL_NVDIMM: case VIR_DOMAIN_MEMORY_MODEL_VIRTIO_PMEM: @@ -3941,7 +3945,7 @@ qemuProcessDomainMemoryDefNeedHugepagesPath(const virDomainMemoryDef *mem, return false; } - return false; + return pagesize != 0 && pagesize != system_pagesize; } diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index d5c2b2cd44..9b548ee515 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -5074,8 +5074,8 @@ qemuValidateDomainDeviceDefMemory(virDomainMemoryDef *mem, return -1; } - if (mem->sourceNodes) { - while ((node = virBitmapNextSetBit(mem->sourceNodes, node)) >= 0) { + if (mem->source.sgx_epc.sourceNodes) { + while ((node = virBitmapNextSetBit(mem->source.sgx_epc.sourceNodes, node)) >= 0) { if (mem->size > sgxCaps->sgxSections[node].size) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("sgx epc size %1$lld on host node %2$zd is less than requested size %3$lld"), diff --git a/src/security/security_apparmor.c b/src/security/security_apparmor.c index 5752cd422d..4b904500ff 100644 --- a/src/security/security_apparmor.c +++ b/src/security/security_apparmor.c @@ -638,16 +638,15 @@ AppArmorSetMemoryLabel(virSecurityManager *mgr, virDomainDef *def, virDomainMemoryDef *mem) { + const char *path = NULL; + switch (mem->model) { case VIR_DOMAIN_MEMORY_MODEL_NVDIMM: + path = mem->source.nvdimm.nvdimmPath; + break; case VIR_DOMAIN_MEMORY_MODEL_VIRTIO_PMEM: - if (!virFileExists(mem->nvdimmPath)) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("%1$s: \'%2$s\' does not exist"), - __func__, mem->nvdimmPath); - return -1; - } - return reload_profile(mgr, def, mem->nvdimmPath, true); + path = mem->source.virtio_pmem.nvdimmPath; + break; case VIR_DOMAIN_MEMORY_MODEL_NONE: case VIR_DOMAIN_MEMORY_MODEL_DIMM: case VIR_DOMAIN_MEMORY_MODEL_VIRTIO_MEM: @@ -656,7 +655,16 @@ AppArmorSetMemoryLabel(virSecurityManager *mgr, break; } - return 0; + if (!path) + return 0; + + if (!virFileExists(path)) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("%1$s: \'%2$s\' does not exist"), + __func__, path); + return -1; + } + return reload_profile(mgr, def, path, true); } diff --git a/src/security/security_dac.c b/src/security/security_dac.c index c7dc145621..a615c6b09e 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -1857,8 +1857,9 @@ virSecurityDACRestoreMemoryLabel(virSecurityManager *mgr, { switch (mem->model) { case VIR_DOMAIN_MEMORY_MODEL_NVDIMM: + return virSecurityDACRestoreFileLabel(mgr, mem->source.nvdimm.nvdimmPath); case VIR_DOMAIN_MEMORY_MODEL_VIRTIO_PMEM: - return virSecurityDACRestoreFileLabel(mgr, mem->nvdimmPath); + return virSecurityDACRestoreFileLabel(mgr, mem->source.virtio_pmem.nvdimmPath); case VIR_DOMAIN_MEMORY_MODEL_SGX_EPC: /* We set label on SGX /dev nodes iff running with namespaces, so we @@ -2044,9 +2045,13 @@ virSecurityDACSetMemoryLabel(virSecurityManager *mgr, switch (mem->model) { case VIR_DOMAIN_MEMORY_MODEL_NVDIMM: + return virSecurityDACSetOwnership(mgr, NULL, + mem->source.nvdimm.nvdimmPath, + user, group, true); + case VIR_DOMAIN_MEMORY_MODEL_VIRTIO_PMEM: return virSecurityDACSetOwnership(mgr, NULL, - mem->nvdimmPath, + mem->source.virtio_pmem.nvdimmPath, user, group, true); case VIR_DOMAIN_MEMORY_MODEL_SGX_EPC: diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index e3e6a6115f..fdbfd5495f 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -1610,24 +1610,20 @@ virSecuritySELinuxSetMemoryLabel(virSecurityManager *mgr, virDomainMemoryDef *mem) { virSecurityLabelDef *seclabel; + const char *path = NULL; + + seclabel = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME); + if (!seclabel || !seclabel->relabel) + return 0; switch (mem->model) { case VIR_DOMAIN_MEMORY_MODEL_NVDIMM: + path = mem->source.nvdimm.nvdimmPath; + break; case VIR_DOMAIN_MEMORY_MODEL_VIRTIO_PMEM: - seclabel = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME); - if (!seclabel || !seclabel->relabel) - return 0; - - if (virSecuritySELinuxSetFilecon(mgr, mem->nvdimmPath, - seclabel->imagelabel, true) < 0) - return -1; + path = mem->source.virtio_pmem.nvdimmPath; break; - case VIR_DOMAIN_MEMORY_MODEL_SGX_EPC: - seclabel = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME); - if (!seclabel || !seclabel->relabel) - return 0; - if (virSecuritySELinuxSetFilecon(mgr, DEV_SGX_VEPC, seclabel->imagelabel, true) < 0 || virSecuritySELinuxSetFilecon(mgr, DEV_SGX_PROVISION, @@ -1642,6 +1638,12 @@ virSecuritySELinuxSetMemoryLabel(virSecurityManager *mgr, break; } + if (!path) + return 0; + + if (virSecuritySELinuxSetFilecon(mgr, path, + seclabel->imagelabel, true) < 0) + return -1; return 0; } @@ -1653,36 +1655,36 @@ virSecuritySELinuxRestoreMemoryLabel(virSecurityManager *mgr, { int ret = -1; virSecurityLabelDef *seclabel; + const char *path = NULL; + + seclabel = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME); + if (!seclabel || !seclabel->relabel) + return 0; switch (mem->model) { case VIR_DOMAIN_MEMORY_MODEL_NVDIMM: + path = mem->source.nvdimm.nvdimmPath; + break; case VIR_DOMAIN_MEMORY_MODEL_VIRTIO_PMEM: - seclabel = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME); - if (!seclabel || !seclabel->relabel) - return 0; - - ret = virSecuritySELinuxRestoreFileLabel(mgr, mem->nvdimmPath, true); + path = mem->source.virtio_pmem.nvdimmPath; break; - case VIR_DOMAIN_MEMORY_MODEL_SGX_EPC: - seclabel = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME); - if (!seclabel || !seclabel->relabel) - return 0; - ret = virSecuritySELinuxRestoreFileLabel(mgr, DEV_SGX_VEPC, true); if (virSecuritySELinuxRestoreFileLabel(mgr, DEV_SGX_PROVISION, true) < 0) ret = -1; - break; + return ret; case VIR_DOMAIN_MEMORY_MODEL_DIMM: case VIR_DOMAIN_MEMORY_MODEL_VIRTIO_MEM: case VIR_DOMAIN_MEMORY_MODEL_NONE: case VIR_DOMAIN_MEMORY_MODEL_LAST: - ret = 0; break; } - return ret; + if (!path) + return 0; + + return virSecuritySELinuxRestoreFileLabel(mgr, path, true); } diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index 23e3dba7f5..bf368cac47 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -1151,8 +1151,11 @@ get_files(vahControl * ctl) switch (mem->model) { case VIR_DOMAIN_MEMORY_MODEL_NVDIMM: + if (vah_add_file(&buf, mem->source.nvdimm.nvdimmPath, "rw") != 0) + goto cleanup; + break; case VIR_DOMAIN_MEMORY_MODEL_VIRTIO_PMEM: - if (vah_add_file(&buf, mem->nvdimmPath, "rw") != 0) + if (vah_add_file(&buf, mem->source.virtio_pmem.nvdimmPath, "rw") != 0) goto cleanup; break; case VIR_DOMAIN_MEMORY_MODEL_DIMM: -- 2.41.0

The _virDomainMemoryDef struct is getting a bit messy. It has various members and only some of them are valid for given model. Worse, some are re-used for different models. We tried to make this more bearable by putting a comment next to each member describing what models the member is valid for, but that gets messy too. Therefore, do what we do elsewhere: introduce an union of structs and move individual members into their respective groups. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- src/conf/domain_conf.c | 167 ++++++++++++++++++++++-------------- src/conf/domain_conf.h | 39 ++++++--- src/conf/domain_postparse.c | 6 +- src/conf/domain_validate.c | 14 +-- src/qemu/qemu_command.c | 34 +++++--- src/qemu/qemu_domain.c | 13 +-- src/qemu/qemu_driver.c | 8 +- src/qemu/qemu_hotplug.c | 14 +-- src/qemu/qemu_process.c | 4 +- src/qemu/qemu_validate.c | 2 +- 10 files changed, 182 insertions(+), 119 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index a1dad679dd..38d7250c6d 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -3496,6 +3496,7 @@ void virDomainMemoryDefFree(virDomainMemoryDef *def) break; case VIR_DOMAIN_MEMORY_MODEL_NVDIMM: g_free(def->source.nvdimm.nvdimmPath); + g_free(def->target.nvdimm.uuid); break; case VIR_DOMAIN_MEMORY_MODEL_VIRTIO_PMEM: g_free(def->source.virtio_pmem.nvdimmPath); @@ -3511,7 +3512,6 @@ void virDomainMemoryDefFree(virDomainMemoryDef *def) break; } - g_free(def->uuid); virDomainDeviceInfoClear(&def->info); g_free(def); } @@ -13366,6 +13366,7 @@ virDomainMemoryTargetDefParseXML(xmlNodePtr node, { VIR_XPATH_NODE_AUTORESTORE(ctxt) xmlNodePtr addrNode = NULL; + unsigned long long *addr; int rv; ctxt->node = node; @@ -13384,39 +13385,41 @@ virDomainMemoryTargetDefParseXML(xmlNodePtr node, switch (def->model) { case VIR_DOMAIN_MEMORY_MODEL_NVDIMM: if (virDomainParseMemory("./label/size", "./label/size/@unit", ctxt, - &def->labelsize, false, false) < 0) + &def->target.nvdimm.labelsize, false, false) < 0) return -1; - if (def->labelsize && def->labelsize < 128) { + if (def->target.nvdimm.labelsize && def->target.nvdimm.labelsize < 128) { virReportError(VIR_ERR_XML_ERROR, "%s", _("nvdimm label must be at least 128KiB")); return -1; } - if (def->labelsize >= def->size) { + if (def->target.nvdimm.labelsize >= def->size) { virReportError(VIR_ERR_XML_ERROR, "%s", _("label size must be smaller than NVDIMM size")); return -1; } if (virXPathBoolean("boolean(./readonly)", ctxt)) - def->readonly = true; + def->target.nvdimm.readonly = true; break; case VIR_DOMAIN_MEMORY_MODEL_VIRTIO_MEM: if (virDomainParseMemory("./block", "./block/@unit", ctxt, - &def->blocksize, false, false) < 0) + &def->target.virtio_mem.blocksize, false, false) < 0) return -1; if (virDomainParseMemory("./requested", "./requested/@unit", ctxt, - &def->requestedsize, false, false) < 0) + &def->target.virtio_mem.requestedsize, false, false) < 0) return -1; addrNode = virXPathNode("./address", ctxt); + addr = &def->target.virtio_mem.address; break; case VIR_DOMAIN_MEMORY_MODEL_VIRTIO_PMEM: addrNode = virXPathNode("./address", ctxt); + addr = &def->target.virtio_pmem.address; break; case VIR_DOMAIN_MEMORY_MODEL_NONE: @@ -13428,7 +13431,7 @@ virDomainMemoryTargetDefParseXML(xmlNodePtr node, if (addrNode && virXMLPropULongLong(addrNode, "base", 16, - VIR_XML_PROP_NONE, &def->address) < 0) { + VIR_XML_PROP_NONE, addr) < 0) { return -1; } @@ -13562,9 +13565,9 @@ virDomainMemoryDefParseXML(virDomainXMLOption *xmlopt, /* Extract NVDIMM UUID. */ if (def->model == VIR_DOMAIN_MEMORY_MODEL_NVDIMM && (tmp = virXPathString("string(./uuid[1])", ctxt))) { - def->uuid = g_new0(unsigned char, VIR_UUID_BUFLEN); + def->target.nvdimm.uuid = g_new0(unsigned char, VIR_UUID_BUFLEN); - if (virUUIDParse(tmp, def->uuid) < 0) { + if (virUUIDParse(tmp, def->target.nvdimm.uuid) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("malformed uuid element")); return NULL; @@ -15282,10 +15285,7 @@ virDomainMemoryFindByDefInternal(virDomainDef *def, /* target info -> always present */ if (tmp->model != mem->model || tmp->targetNode != mem->targetNode || - tmp->size != mem->size || - tmp->blocksize != mem->blocksize || - tmp->requestedsize != mem->requestedsize || - tmp->address != mem->address) + tmp->size != mem->size) continue; switch (mem->model) { @@ -15298,7 +15298,10 @@ virDomainMemoryFindByDefInternal(virDomainDef *def, continue; break; case VIR_DOMAIN_MEMORY_MODEL_VIRTIO_MEM: - if (tmp->source.virtio_mem.pagesize != mem->source.virtio_mem.pagesize) + if (tmp->source.virtio_mem.pagesize != mem->source.virtio_mem.pagesize || + tmp->target.virtio_mem.blocksize != mem->target.virtio_mem.blocksize || + tmp->target.virtio_mem.requestedsize != mem->target.virtio_mem.requestedsize || + tmp->target.virtio_mem.address != mem->target.virtio_mem.address) continue; if (!virBitmapEqual(tmp->source.virtio_mem.sourceNodes, @@ -15313,7 +15316,8 @@ virDomainMemoryFindByDefInternal(virDomainDef *def, case VIR_DOMAIN_MEMORY_MODEL_VIRTIO_PMEM: if (STRNEQ(tmp->source.virtio_pmem.nvdimmPath, - mem->source.virtio_pmem.nvdimmPath)) + mem->source.virtio_pmem.nvdimmPath) || + tmp->target.virtio_pmem.address != mem->target.virtio_pmem.address) continue; break; @@ -21022,33 +21026,13 @@ virDomainMemoryDefCheckABIStability(virDomainMemoryDef *src, return false; } - if (src->blocksize != dst->blocksize) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("Target memory device block size '%1$llu' doesn't match source memory device block size '%2$llu'"), - dst->blocksize, src->blocksize); - return false; - } - - if (src->requestedsize != dst->requestedsize) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("Target memory device requested size '%1$llu' doesn't match source memory device requested size '%2$llu'"), - dst->requestedsize, src->requestedsize); - return false; - } - - if (src->address != dst->address) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("Target memory device address '0x%1$llx' doesn't match source memory device address '0x%2$llx'"), - dst->address, src->address); - return false; - } - switch (src->model) { case VIR_DOMAIN_MEMORY_MODEL_NVDIMM: - if (src->labelsize != dst->labelsize) { + if (src->target.nvdimm.labelsize != dst->target.nvdimm.labelsize) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Target NVDIMM label size '%1$llu' doesn't match source NVDIMM label size '%2$llu'"), - src->labelsize, dst->labelsize); + src->target.nvdimm.labelsize, + dst->target.nvdimm.labelsize); return false; } @@ -21067,25 +21051,59 @@ virDomainMemoryDefCheckABIStability(virDomainMemoryDef *src, return false; } - if (src->readonly != dst->readonly) { + if (src->target.nvdimm.readonly != dst->target.nvdimm.readonly) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Target NVDIMM readonly flag doesn't match " "source NVDIMM readonly flag")); return false; } - if ((src->uuid || dst->uuid) && - !(src->uuid && dst->uuid && - memcmp(src->uuid, dst->uuid, VIR_UUID_BUFLEN) == 0)) { + if ((src->target.nvdimm.uuid || dst->target.nvdimm.uuid) && + !(src->target.nvdimm.uuid && dst->target.nvdimm.uuid && + memcmp(src->target.nvdimm.uuid, dst->target.nvdimm.uuid, VIR_UUID_BUFLEN) == 0)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Target NVDIMM UUID doesn't match source NVDIMM")); return false; } break; - case VIR_DOMAIN_MEMORY_MODEL_DIMM: case VIR_DOMAIN_MEMORY_MODEL_VIRTIO_PMEM: + if (src->target.virtio_pmem.address != dst->target.virtio_pmem.address) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Target memory device address '0x%1$llx' doesn't match source memory device address '0x%2$llx'"), + dst->target.virtio_pmem.address, + src->target.virtio_pmem.address); + return false; + } + break; + case VIR_DOMAIN_MEMORY_MODEL_VIRTIO_MEM: + if (src->target.virtio_mem.blocksize != dst->target.virtio_mem.blocksize) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Target memory device block size '%1$llu' doesn't match source memory device block size '%2$llu'"), + dst->target.virtio_mem.blocksize, + src->target.virtio_mem.blocksize); + return false; + } + + if (src->target.virtio_mem.requestedsize != dst->target.virtio_mem.requestedsize) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Target memory device requested size '%1$llu' doesn't match source memory device requested size '%2$llu'"), + dst->target.virtio_mem.requestedsize, + src->target.virtio_mem.requestedsize); + return false; + } + + if (src->target.virtio_mem.address != dst->target.virtio_mem.address) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Target memory device address '0x%1$llx' doesn't match source memory device address '0x%2$llx'"), + dst->target.virtio_mem.address, + src->target.virtio_mem.address); + return false; + } + break; + + case VIR_DOMAIN_MEMORY_MODEL_DIMM: case VIR_DOMAIN_MEMORY_MODEL_SGX_EPC: case VIR_DOMAIN_MEMORY_MODEL_NONE: case VIR_DOMAIN_MEMORY_MODEL_LAST: @@ -25216,29 +25234,49 @@ virDomainMemoryTargetDefFormat(virBuffer *buf, virBufferAsprintf(&childBuf, "<size unit='KiB'>%llu</size>\n", def->size); if (def->targetNode >= 0) virBufferAsprintf(&childBuf, "<node>%d</node>\n", def->targetNode); - if (def->labelsize) { - g_auto(virBuffer) labelChildBuf = VIR_BUFFER_INIT_CHILD(&childBuf); - virBufferAsprintf(&labelChildBuf, "<size unit='KiB'>%llu</size>\n", def->labelsize); - virXMLFormatElement(&childBuf, "label", NULL, &labelChildBuf); - } - if (def->readonly) - virBufferAddLit(&childBuf, "<readonly/>\n"); + switch (def->model) { + case VIR_DOMAIN_MEMORY_MODEL_NVDIMM: + if (def->target.nvdimm.labelsize) { + g_auto(virBuffer) labelChildBuf = VIR_BUFFER_INIT_CHILD(&childBuf); + + virBufferAsprintf(&labelChildBuf, "<size unit='KiB'>%llu</size>\n", + def->target.nvdimm.labelsize); + virXMLFormatElement(&childBuf, "label", NULL, &labelChildBuf); + } + if (def->target.nvdimm.readonly) + virBufferAddLit(&childBuf, "<readonly/>\n"); + break; - if (def->blocksize) { - virBufferAsprintf(&childBuf, "<block unit='KiB'>%llu</block>\n", - def->blocksize); + case VIR_DOMAIN_MEMORY_MODEL_VIRTIO_PMEM: + if (def->target.virtio_pmem.address) + virBufferAsprintf(&childBuf, "<address base='0x%llx'/>\n", + def->target.virtio_pmem.address); + break; - virBufferAsprintf(&childBuf, "<requested unit='KiB'>%llu</requested>\n", - def->requestedsize); - if (!(flags & VIR_DOMAIN_DEF_FORMAT_INACTIVE)) { - virBufferAsprintf(&childBuf, "<current unit='KiB'>%llu</current>\n", - def->currentsize); + case VIR_DOMAIN_MEMORY_MODEL_VIRTIO_MEM: + if (def->target.virtio_mem.blocksize) { + virBufferAsprintf(&childBuf, "<block unit='KiB'>%llu</block>\n", + def->target.virtio_mem.blocksize); + + virBufferAsprintf(&childBuf, "<requested unit='KiB'>%llu</requested>\n", + def->target.virtio_mem.requestedsize); + if (!(flags & VIR_DOMAIN_DEF_FORMAT_INACTIVE)) { + virBufferAsprintf(&childBuf, "<current unit='KiB'>%llu</current>\n", + def->target.virtio_mem.currentsize); + } } - } + if (def->target.virtio_mem.address) + virBufferAsprintf(&childBuf, "<address base='0x%llx'/>\n", + def->target.virtio_mem.address); + break; - if (def->address) - virBufferAsprintf(&childBuf, "<address base='0x%llx'/>\n", def->address); + case VIR_DOMAIN_MEMORY_MODEL_SGX_EPC: + case VIR_DOMAIN_MEMORY_MODEL_DIMM: + case VIR_DOMAIN_MEMORY_MODEL_NONE: + case VIR_DOMAIN_MEMORY_MODEL_LAST: + break; + } virXMLFormatElement(buf, "target", NULL, &childBuf); } @@ -25260,10 +25298,11 @@ virDomainMemoryDefFormat(virBuffer *buf, virBufferAddLit(buf, ">\n"); virBufferAdjustIndent(buf, 2); - if (def->uuid) { + if (def->model == VIR_DOMAIN_MEMORY_MODEL_NVDIMM && + def->target.nvdimm.uuid) { char uuidstr[VIR_UUID_STRING_BUFLEN]; - virUUIDFormat(def->uuid, uuidstr); + virUUIDFormat(def->target.nvdimm.uuid, uuidstr); virBufferAsprintf(buf, "<uuid>%s</uuid>\n", uuidstr); } diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 8770547590..cf4cb04a6f 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2638,6 +2638,9 @@ struct _virDomainMemoryDef { virDomainMemoryAccess access; virTristateBool discard; + unsigned long long size; /* kibibytes */ + int targetNode; + union { struct { unsigned long long pagesize; /* kibibytes */ @@ -2660,21 +2663,29 @@ struct _virDomainMemoryDef { } sgx_epc; } source; - /* target */ - int targetNode; - unsigned long long size; /* kibibytes */ - unsigned long long labelsize; /* kibibytes; valid only for NVDIMM */ - unsigned long long blocksize; /* kibibytes; valid only for VIRTIO_MEM */ - unsigned long long requestedsize; /* kibibytes; valid only for VIRTIO_MEM */ - unsigned long long currentsize; /* kibibytes, valid for VIRTIO_MEM and - active domain only, only to report never - parse */ - unsigned long long address; /* address where memory is mapped, valid for - VIRTIO_PMEM and VIRTIO_MEM only. */ - bool readonly; /* valid only for NVDIMM */ + union { + struct { + } dimm; + struct { + unsigned long long labelsize; /* kibibytes */ + bool readonly; - /* required for QEMU NVDIMM ppc64 support */ - unsigned char *uuid; /* VIR_UUID_BUFLEN bytes long */ + /* required for QEMU NVDIMM ppc64 support */ + unsigned char *uuid; /* VIR_UUID_BUFLEN bytes long */ + } nvdimm; + struct { + unsigned long long address; /* address where memory is mapped */ + } virtio_pmem; + struct { + unsigned long long blocksize; /* kibibytes */ + unsigned long long requestedsize; /* kibibytes */ + unsigned long long currentsize; /* kibibytes, valid for an active + domain only and parsed */ + unsigned long long address; /* address where memory is mapped */ + } virtio_mem; + struct { + } sgx_epc; + } target; virDomainDeviceInfo info; }; diff --git a/src/conf/domain_postparse.c b/src/conf/domain_postparse.c index f1dfdb7665..b76e8dcc5c 100644 --- a/src/conf/domain_postparse.c +++ b/src/conf/domain_postparse.c @@ -616,10 +616,10 @@ virDomainMemoryDefPostParse(virDomainMemoryDef *mem, case VIR_DOMAIN_MEMORY_MODEL_NVDIMM: /* If no NVDIMM UUID was provided in XML, generate one. */ if (ARCH_IS_PPC64(def->os.arch) && - !mem->uuid) { + !mem->target.nvdimm.uuid) { - mem->uuid = g_new0(unsigned char, VIR_UUID_BUFLEN); - if (virUUIDGenerate(mem->uuid) < 0) { + mem->target.nvdimm.uuid = g_new0(unsigned char, VIR_UUID_BUFLEN); + if (virUUIDGenerate(mem->target.nvdimm.uuid) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Failed to generate UUID")); return -1; diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c index 2832fd010e..ddf2e8effa 100644 --- a/src/conf/domain_validate.c +++ b/src/conf/domain_validate.c @@ -2273,12 +2273,12 @@ virDomainMemoryDefValidate(const virDomainMemoryDef *mem, } if (ARCH_IS_PPC64(def->os.arch)) { - if (mem->labelsize == 0) { + if (mem->target.nvdimm.labelsize == 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("label size is required for NVDIMM device")); return -1; } - } else if (mem->uuid) { + } else if (mem->target.nvdimm.uuid) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("UUID is not supported for NVDIMM device")); return -1; @@ -2314,14 +2314,14 @@ virDomainMemoryDefValidate(const virDomainMemoryDef *mem, break; case VIR_DOMAIN_MEMORY_MODEL_VIRTIO_MEM: - if (mem->requestedsize > mem->size) { + if (mem->target.virtio_mem.requestedsize > mem->size) { virReportError(VIR_ERR_XML_DETAIL, _("requested size must be smaller than or equal to @size (%1$lluKiB)"), mem->size); return -1; } - if (!VIR_IS_POW2(mem->blocksize)) { + if (!VIR_IS_POW2(mem->target.virtio_mem.blocksize)) { virReportError(VIR_ERR_XML_DETAIL, "%s", _("block size must be a power of two")); return -1; @@ -2335,20 +2335,20 @@ virDomainMemoryDefValidate(const virDomainMemoryDef *mem, thpSize = 2048; } - if (mem->blocksize < thpSize) { + if (mem->target.virtio_mem.blocksize < thpSize) { virReportError(VIR_ERR_XML_DETAIL, _("block size too small, must be at least %1$lluKiB"), thpSize); return -1; } - if (mem->requestedsize % mem->blocksize != 0) { + if (mem->target.virtio_mem.requestedsize % mem->target.virtio_mem.blocksize != 0) { virReportError(VIR_ERR_XML_DETAIL, "%s", _("requested size must be an integer multiple of block size")); return -1; } - if (mem->address % mem->blocksize != 0) { + if (mem->target.virtio_mem.address % mem->target.virtio_mem.blocksize != 0) { virReportError(VIR_ERR_XML_DETAIL, "%s", _("memory device address must be aligned to blocksize")); return -1; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 2e06261ef2..77601d27c5 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3622,6 +3622,10 @@ qemuBuildMemoryDeviceProps(virQEMUDriverConfig *cfg, g_autofree char *uuidstr = NULL; virTristateBool unarmed = VIR_TRISTATE_BOOL_ABSENT; g_autofree char *memdev = NULL; + unsigned long long labelsize = 0; + unsigned long long blocksize = 0; + unsigned long long requestedsize = 0; + unsigned long long address = 0; bool prealloc = false; if (!mem->info.alias) { @@ -3638,10 +3642,20 @@ qemuBuildMemoryDeviceProps(virQEMUDriverConfig *cfg, break; case VIR_DOMAIN_MEMORY_MODEL_NVDIMM: device = "nvdimm"; + if (mem->target.nvdimm.readonly) + unarmed = VIR_TRISTATE_BOOL_YES; + + if (mem->target.nvdimm.uuid) { + uuidstr = g_new0(char, VIR_UUID_STRING_BUFLEN); + virUUIDFormat(mem->target.nvdimm.uuid, uuidstr); + } + + labelsize = mem->target.nvdimm.labelsize; break; case VIR_DOMAIN_MEMORY_MODEL_VIRTIO_PMEM: device = "virtio-pmem-pci"; + address = mem->target.virtio_pmem.address; break; case VIR_DOMAIN_MEMORY_MODEL_VIRTIO_MEM: @@ -3650,6 +3664,10 @@ qemuBuildMemoryDeviceProps(virQEMUDriverConfig *cfg, if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE_VIRTIO_MEM_PCI_PREALLOC) && qemuBuildMemoryGetPagesize(cfg, def, mem, NULL, NULL, NULL, &prealloc) < 0) return NULL; + + blocksize = mem->target.virtio_mem.blocksize; + requestedsize = mem->target.virtio_mem.requestedsize; + address = mem->target.virtio_mem.address; break; case VIR_DOMAIN_MEMORY_MODEL_SGX_EPC: @@ -3661,25 +3679,17 @@ qemuBuildMemoryDeviceProps(virQEMUDriverConfig *cfg, break; } - if (mem->readonly) - unarmed = VIR_TRISTATE_BOOL_YES; - - if (mem->uuid) { - uuidstr = g_new0(char, VIR_UUID_STRING_BUFLEN); - virUUIDFormat(mem->uuid, uuidstr); - } - if (virJSONValueObjectAdd(&props, "s:driver", device, "k:node", mem->targetNode, - "P:label-size", mem->labelsize * 1024, - "P:block-size", mem->blocksize * 1024, - "P:requested-size", mem->requestedsize * 1024, + "P:label-size", labelsize * 1024, + "P:block-size", blocksize * 1024, + "P:requested-size", requestedsize * 1024, "S:uuid", uuidstr, "T:unarmed", unarmed, "s:memdev", memdev, "B:prealloc", prealloc, - "P:memaddr", mem->address, + "P:memaddr", address, "s:id", mem->info.alias, NULL) < 0) return NULL; diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 029238a9d7..2eda3ef59c 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -6080,7 +6080,7 @@ qemuDomainNVDimmAlignSizePseries(virDomainMemoryDef *mem) * target_size = AlignDown(target_size - label_size) + label_size */ unsigned long long ppc64AlignSize = 256 * 1024; - unsigned long long guestArea = mem->size - mem->labelsize; + unsigned long long guestArea = mem->size - mem->target.nvdimm.labelsize; /* Align down guestArea. We can't align down if guestArea is * smaller than the 256MiB alignment. */ @@ -6092,7 +6092,7 @@ qemuDomainNVDimmAlignSizePseries(virDomainMemoryDef *mem) } guestArea = (guestArea/ppc64AlignSize) * ppc64AlignSize; - mem->size = guestArea + mem->labelsize; + mem->size = guestArea + mem->target.nvdimm.labelsize; return 0; } @@ -8604,11 +8604,12 @@ qemuDomainUpdateMemoryDeviceInfo(virDomainObj *vm, switch (mem->model) { case VIR_DOMAIN_MEMORY_MODEL_VIRTIO_MEM: + mem->target.virtio_mem.currentsize = VIR_DIV_UP(dimm->size, 1024); + mem->target.virtio_mem.address = dimm->address; + break; + case VIR_DOMAIN_MEMORY_MODEL_VIRTIO_PMEM: - if (mem->model == VIR_DOMAIN_MEMORY_MODEL_VIRTIO_MEM) { - mem->currentsize = VIR_DIV_UP(dimm->size, 1024); - } - mem->address = dimm->address; + mem->target.virtio_pmem.address = dimm->address; break; case VIR_DOMAIN_MEMORY_MODEL_DIMM: diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index f0eda71c4f..73fa499e40 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4010,14 +4010,14 @@ processMemoryDeviceSizeChange(virQEMUDriver *driver, * It really contains just the balloon size. But in domain * definition we want to report also sum of @currentsize. Do * a bit of math to fix the domain definition. */ - balloon = vm->def->mem.cur_balloon - mem->currentsize; - mem->currentsize = VIR_DIV_UP(info->size, 1024); - balloon += mem->currentsize; + balloon = vm->def->mem.cur_balloon - mem->target.virtio_mem.currentsize; + mem->target.virtio_mem.currentsize = VIR_DIV_UP(info->size, 1024); + balloon += mem->target.virtio_mem.currentsize; vm->def->mem.cur_balloon = balloon; event = virDomainEventMemoryDeviceSizeChangeNewFromObj(vm, info->devAlias, - mem->currentsize); + mem->target.virtio_mem.currentsize); endjob: virDomainObjEndJob(vm); diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index ece5b2598d..c461c7454f 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -7051,20 +7051,22 @@ qemuDomainChangeMemoryLiveValidateChange(const virDomainMemoryDef *oldDef, return false; } - if (oldDef->blocksize != newDef->blocksize) { + if (oldDef->target.virtio_mem.blocksize != newDef->target.virtio_mem.blocksize) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("cannot modify memory block size from '%1$llu' to '%2$llu'"), - oldDef->blocksize, newDef->blocksize); + oldDef->target.virtio_mem.blocksize, + newDef->target.virtio_mem.blocksize); return false; } /* requestedsize can change */ - if (oldDef->address != newDef->address) { + if (oldDef->target.virtio_mem.address != newDef->target.virtio_mem.address) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("cannot modify memory address from '0x%1$llx' to '0x%2$llx'"), - oldDef->address, newDef->address); + oldDef->target.virtio_mem.address, + newDef->target.virtio_mem.address); return false; } @@ -7099,10 +7101,10 @@ qemuDomainChangeMemoryLive(virQEMUDriver *driver G_GNUC_UNUSED, return -1; if (qemuDomainChangeMemoryRequestedSize(vm, newDef->info.alias, - newDef->requestedsize) < 0) + newDef->target.virtio_mem.requestedsize) < 0) return -1; - oldDef->requestedsize = newDef->requestedsize; + oldDef->target.virtio_mem.requestedsize = newDef->target.virtio_mem.requestedsize; return 0; } diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index aa2593fa9a..5331996c93 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -1096,7 +1096,7 @@ qemuProcessHandleBalloonChange(qemuMonitor *mon G_GNUC_UNUSED, VIR_DEBUG("balloon size before fix is %lld", actual); for (i = 0; i < vm->def->nmems; i++) { if (vm->def->mems[i]->model == VIR_DOMAIN_MEMORY_MODEL_VIRTIO_MEM) - actual += vm->def->mems[i]->currentsize; + actual += vm->def->mems[i]->target.virtio_mem.currentsize; } VIR_DEBUG("Updating balloon from %lld to %lld kb", @@ -2286,7 +2286,7 @@ qemuProcessRefreshBalloonState(virDomainObj *vm, VIR_DEBUG("balloon size before fix is %lld", balloon); for (i = 0; i < vm->def->nmems; i++) { if (vm->def->mems[i]->model == VIR_DOMAIN_MEMORY_MODEL_VIRTIO_MEM) - balloon += vm->def->mems[i]->currentsize; + balloon += vm->def->mems[i]->target.virtio_mem.currentsize; } VIR_DEBUG("Updating balloon from %lld to %lld kb", vm->def->mem.cur_balloon, balloon); diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index 9b548ee515..a9a21843b5 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -5034,7 +5034,7 @@ qemuValidateDomainDeviceDefMemory(virDomainMemoryDef *mem, return -1; } - if (mem->readonly && + if (mem->target.nvdimm.readonly && !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_NVDIMM_UNARMED)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("nvdimm readonly property is not available " -- 2.41.0