9:02 a.m.
The structure has two sets of members: some for the target and
some for the source. The latter is model dependant (e.g. path to
a nvdimm device applies only to NVDIMM model). Move the members
into an union so that it is obvious which members apply to which
model. This way it's easier to maintain code cleanliness when
introducing a new model.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/conf/domain_conf.c | 57 ++++++++++++++++++------------
src/conf/domain_conf.h | 16 ++++++---
src/qemu/qemu_alias.c | 13 +++++--
src/qemu/qemu_cgroup.c | 10 +++---
src/qemu/qemu_command.c | 60 +++++++++++++++++++++-----------
src/qemu/qemu_namespace.c | 2 +-
src/qemu/qemu_process.c | 8 ++---
src/security/security_apparmor.c | 6 ++--
src/security/security_dac.c | 4 +--
src/security/security_selinux.c | 4 +--
src/security/virt-aa-helper.c | 2 +-
11 files changed, 113 insertions(+), 69 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index c4da972f1a..60a0fda27e 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -3136,8 +3136,18 @@ void virDomainMemoryDefFree(virDomainMemoryDefPtr def)
if (!def)
return;
- VIR_FREE(def->nvdimmPath);
- virBitmapFree(def->sourceNodes);
+ switch (def->model) {
+ case VIR_DOMAIN_MEMORY_MODEL_DIMM:
+ virBitmapFree(def->s.dimm.sourceNodes);
+ break;
+ case VIR_DOMAIN_MEMORY_MODEL_NVDIMM:
+ VIR_FREE(def->s.nvdimm.path);
+ break;
+ case VIR_DOMAIN_MEMORY_MODEL_NONE:
+ case VIR_DOMAIN_MEMORY_MODEL_LAST:
+ break;
+ }
+
virDomainDeviceInfoClear(&def->info);
VIR_FREE(def);
}
@@ -6699,7 +6709,7 @@ virDomainMemoryDefValidate(const virDomainMemoryDef *mem,
const virDomainDef *def)
{
if (mem->model == VIR_DOMAIN_MEMORY_MODEL_NVDIMM) {
- if (!mem->nvdimmPath) {
+ if (!mem->s.nvdimm.path) {
virReportError(VIR_ERR_XML_DETAIL, "%s",
_("path is required for model 'nvdimm'"));
return -1;
@@ -16702,15 +16712,15 @@ virDomainMemorySourceDefParseXML(xmlNodePtr node,
switch (def->model) {
case VIR_DOMAIN_MEMORY_MODEL_DIMM:
if (virDomainParseMemory("./pagesize", "./pagesize/@unit",
ctxt,
- &def->pagesize, false, false) < 0)
+ &def->s.dimm.pagesize, false, false) < 0)
return -1;
if ((nodemask = virXPathString("string(./nodemask)", ctxt))) {
- if (virBitmapParse(nodemask, &def->sourceNodes,
+ if (virBitmapParse(nodemask, &def->s.dimm.sourceNodes,
VIR_DOMAIN_CPUMASK_LEN) < 0)
return -1;
- if (virBitmapIsAllClear(def->sourceNodes)) {
+ if (virBitmapIsAllClear(def->s.dimm.sourceNodes)) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("Invalid value of 'nodemask': %s"),
nodemask);
return -1;
@@ -16719,14 +16729,14 @@ virDomainMemorySourceDefParseXML(xmlNodePtr node,
break;
case VIR_DOMAIN_MEMORY_MODEL_NVDIMM:
- def->nvdimmPath = virXPathString("string(./path)", ctxt);
+ def->s.nvdimm.path = virXPathString("string(./path)", ctxt);
if (virDomainParseMemory("./alignsize", "./alignsize/@unit",
ctxt,
- &def->alignsize, false, false) < 0)
+ &def->s.nvdimm.alignsize, false, false) < 0)
return -1;
if (virXPathBoolean("boolean(./pmem)", ctxt))
- def->nvdimmPmem = true;
+ def->s.nvdimm.pmem = true;
break;
@@ -18602,15 +18612,15 @@ virDomainMemoryFindByDefInternal(virDomainDefPtr def,
switch (mem->model) {
case VIR_DOMAIN_MEMORY_MODEL_DIMM:
/* source stuff -> match with device */
- if (tmp->pagesize != mem->pagesize)
+ if (tmp->s.dimm.pagesize != mem->s.dimm.pagesize)
continue;
- if (!virBitmapEqual(tmp->sourceNodes, mem->sourceNodes))
+ if (!virBitmapEqual(tmp->s.dimm.sourceNodes, mem->s.dimm.sourceNodes))
continue;
break;
case VIR_DOMAIN_MEMORY_MODEL_NVDIMM:
- if (STRNEQ(tmp->nvdimmPath, mem->nvdimmPath))
+ if (STRNEQ(tmp->s.nvdimm.path, mem->s.nvdimm.path))
continue;
break;
@@ -24220,15 +24230,15 @@ virDomainMemoryDefCheckABIStability(virDomainMemoryDefPtr src,
return false;
}
- if (src->alignsize != dst->alignsize) {
+ if (src->s.nvdimm.alignsize != dst->s.nvdimm.alignsize) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("Target NVDIMM alignment '%llu' doesn't
match "
"source NVDIMM alignment '%llu'"),
- src->alignsize, dst->alignsize);
+ src->s.nvdimm.alignsize, dst->s.nvdimm.alignsize);
return false;
}
- if (src->nvdimmPmem != dst->nvdimmPmem) {
+ if (src->s.nvdimm.pmem != dst->s.nvdimm.pmem) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("Target NVDIMM pmem flag doesn't match "
"source NVDIMM pmem flag"));
@@ -27882,26 +27892,27 @@ virDomainMemorySourceDefFormat(virBufferPtr buf,
switch (def->model) {
case VIR_DOMAIN_MEMORY_MODEL_DIMM:
- if (def->sourceNodes) {
- if (!(bitmap = virBitmapFormat(def->sourceNodes)))
+ if (def->s.dimm.sourceNodes) {
+ if (!(bitmap = virBitmapFormat(def->s.dimm.sourceNodes)))
return -1;
virBufferAsprintf(&childBuf,
"<nodemask>%s</nodemask>\n", bitmap);
}
- if (def->pagesize)
+ if (def->s.dimm.pagesize) {
virBufferAsprintf(&childBuf, "<pagesize
unit='KiB'>%llu</pagesize>\n",
- def->pagesize);
+ def->s.dimm.pagesize);
+ }
break;
case VIR_DOMAIN_MEMORY_MODEL_NVDIMM:
- virBufferEscapeString(&childBuf, "<path>%s</path>\n",
def->nvdimmPath);
+ virBufferEscapeString(&childBuf, "<path>%s</path>\n",
def->s.nvdimm.path);
- if (def->alignsize)
+ if (def->s.nvdimm.alignsize)
virBufferAsprintf(&childBuf, "<alignsize
unit='KiB'>%llu</alignsize>\n",
- def->alignsize);
+ def->s.nvdimm.alignsize);
- if (def->nvdimmPmem)
+ if (def->s.nvdimm.pmem)
virBufferAddLit(&childBuf, "<pmem/>\n");
break;
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 8ac23fd7d9..611dac8bc4 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2315,11 +2315,17 @@ struct _virDomainMemoryDef {
virTristateBool discard;
/* source */
- virBitmapPtr sourceNodes;
- unsigned long long pagesize; /* kibibytes */
- char *nvdimmPath;
- unsigned long long alignsize; /* kibibytes; valid only for NVDIMM */
- bool nvdimmPmem; /* valid only for NVDIMM */
+ union {
+ struct {
+ virBitmapPtr sourceNodes;
+ unsigned long long pagesize; /* kibibytes */
+ } dimm; /* VIR_DOMAIN_MEMORY_MODEL_DIMM */
+ struct {
+ char *path;
+ unsigned long long alignsize; /* kibibytes */
+ bool pmem;
+ } nvdimm; /* VIR_DOMAIN_MEMORY_MODEL_NVDIMM */
+ } s;
/* target */
virDomainMemoryModel model;
diff --git a/src/qemu/qemu_alias.c b/src/qemu/qemu_alias.c
index dcb6c7156d..5ebcd766a9 100644
--- a/src/qemu/qemu_alias.c
+++ b/src/qemu/qemu_alias.c
@@ -486,15 +486,22 @@ qemuAssignDeviceMemoryAlias(virDomainDefPtr def,
size_t i;
int maxidx = 0;
int idx;
- const char *prefix;
+ const char *prefix = NULL;
if (mem->info.alias)
return 0;
- if (mem->model == VIR_DOMAIN_MEMORY_MODEL_DIMM)
+ switch (mem->model) {
+ case VIR_DOMAIN_MEMORY_MODEL_DIMM:
prefix = "dimm";
- else
+ break;
+ case VIR_DOMAIN_MEMORY_MODEL_NVDIMM:
prefix = "nvdimm";
+ break;
+ case VIR_DOMAIN_MEMORY_MODEL_NONE:
+ case VIR_DOMAIN_MEMORY_MODEL_LAST:
+ break;
+ }
if (oldAlias) {
for (i = 0; i < def->nmems; i++) {
diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index 10fdc7444d..519158eccd 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -508,11 +508,11 @@ qemuSetupMemoryDevicesCgroup(virDomainObjPtr vm,
if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_DEVICES))
return 0;
- VIR_DEBUG("Setting devices Cgroup for NVDIMM device: %s",
mem->nvdimmPath);
- rv = virCgroupAllowDevicePath(priv->cgroup, mem->nvdimmPath,
+ VIR_DEBUG("Setting devices Cgroup for NVDIMM device: %s",
mem->s.nvdimm.path);
+ rv = virCgroupAllowDevicePath(priv->cgroup, mem->s.nvdimm.path,
VIR_CGROUP_DEVICE_RW, false);
virDomainAuditCgroupPath(vm, priv->cgroup, "allow",
- mem->nvdimmPath, "rw", rv);
+ mem->s.nvdimm.path, "rw", rv);
return rv;
}
@@ -531,10 +531,10 @@ qemuTeardownMemoryDevicesCgroup(virDomainObjPtr vm,
if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_DEVICES))
return 0;
- rv = virCgroupDenyDevicePath(priv->cgroup, mem->nvdimmPath,
+ rv = virCgroupDenyDevicePath(priv->cgroup, mem->s.nvdimm.path,
VIR_CGROUP_DEVICE_RWM, false);
virDomainAuditCgroupPath(vm, priv->cgroup,
- "deny", mem->nvdimmPath, "rwm", rv);
+ "deny", mem->s.nvdimm.path, "rwm",
rv);
return rv;
}
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 568c0f6054..d3321b30e9 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -2941,10 +2941,13 @@ qemuBuildMemoryBackendProps(virJSONValuePtr *backendProps,
int rc;
g_autoptr(virJSONValue) props = NULL;
bool nodeSpecified = virDomainNumatuneNodeSpecified(def->numa,
mem->targetNode);
- unsigned long long pagesize = mem->pagesize;
- bool needHugepage = !!pagesize;
- bool useHugepage = !!pagesize;
+ unsigned long long pagesize = 0;
+ bool needHugepage = false;
+ bool useHugepage = false;
int discard = mem->discard;
+ const char *nvdimmPath = NULL;
+ unsigned long long alignsize = 0;
+ bool nvdimmPmem = false;
/* The difference between @needHugepage and @useHugepage is that the latter
* is true whenever huge page is defined for the current memory cell.
@@ -2954,6 +2957,23 @@ qemuBuildMemoryBackendProps(virJSONValuePtr *backendProps,
*backendProps = NULL;
+ switch (mem->model) {
+ case VIR_DOMAIN_MEMORY_MODEL_DIMM:
+ pagesize = mem->s.dimm.pagesize;
+ needHugepage = !!pagesize;
+ useHugepage = !!pagesize;
+ nodemask = mem->s.dimm.sourceNodes;
+ break;
+ case VIR_DOMAIN_MEMORY_MODEL_NVDIMM:
+ nvdimmPath = mem->s.nvdimm.path;
+ alignsize = mem->s.nvdimm.alignsize;
+ nvdimmPmem = mem->s.nvdimm.pmem;
+ break;
+ case VIR_DOMAIN_MEMORY_MODEL_NONE:
+ case VIR_DOMAIN_MEMORY_MODEL_LAST:
+ break;
+ }
+
if (mem->targetNode >= 0) {
/* memory devices could provide a invalid guest node */
if (mem->targetNode >= virDomainNumaGetNodeCount(def->numa)) {
@@ -3059,11 +3079,11 @@ qemuBuildMemoryBackendProps(virJSONValuePtr *backendProps,
if (qemuBuildMemoryBackendPropsShare(props, memAccess) < 0)
return -1;
- } 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);
prealloc = true;
} else if (useHugepage) {
if (qemuGetDomainHupageMemPath(priv->driver, def, pagesize, &memPath)
< 0)
@@ -3081,7 +3101,7 @@ qemuBuildMemoryBackendProps(virJSONValuePtr *backendProps,
NULL) < 0)
return -1;
- if (!mem->nvdimmPath &&
+ if (!nvdimmPath &&
discard == VIR_TRISTATE_BOOL_YES) {
if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_OBJECT_MEMORY_FILE_DISCARD))
{
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
@@ -3108,18 +3128,18 @@ qemuBuildMemoryBackendProps(virJSONValuePtr *backendProps,
if (virJSONValueObjectAdd(props, "U:size", mem->size * 1024, NULL) <
0)
return -1;
- if (mem->alignsize) {
+ if (alignsize) {
if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_OBJECT_MEMORY_FILE_ALIGN)) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("nvdimm align property is not available "
"with this QEMU binary"));
return -1;
}
- if (virJSONValueObjectAdd(props, "U:align", mem->alignsize * 1024,
NULL) < 0)
+ if (virJSONValueObjectAdd(props, "U:align", alignsize * 1024, NULL)
< 0)
return -1;
}
- if (mem->nvdimmPmem) {
+ if (nvdimmPmem) {
if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_OBJECT_MEMORY_FILE_PMEM)) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("nvdimm pmem property is not available "
@@ -3130,13 +3150,10 @@ qemuBuildMemoryBackendProps(virJSONValuePtr *backendProps,
return -1;
}
- if (mem->sourceNodes) {
- nodemask = mem->sourceNodes;
- } else {
- if (virDomainNumatuneMaybeGetNodeset(def->numa, priv->autoNodeset,
- &nodemask, mem->targetNode) < 0)
- return -1;
- }
+ if (!nodemask &&
+ virDomainNumatuneMaybeGetNodeset(def->numa, priv->autoNodeset,
+ &nodemask, mem->targetNode) < 0)
+ return -1;
if (nodemask) {
if (!virNumaNodesetIsAvailable(nodemask))
@@ -3149,8 +3166,11 @@ qemuBuildMemoryBackendProps(virJSONValuePtr *backendProps,
}
/* If none of the following is requested... */
- if (!needHugepage && !mem->sourceNodes && !nodeSpecified
&&
- !mem->nvdimmPath &&
+ if (!needHugepage &&
+ !(mem->model == VIR_DOMAIN_MEMORY_MODEL_DIMM &&
+ mem->s.dimm.sourceNodes) &&
+ !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_namespace.c b/src/qemu/qemu_namespace.c
index 1030309b5a..ba1e6365f1 100644
--- a/src/qemu/qemu_namespace.c
+++ b/src/qemu/qemu_namespace.c
@@ -354,7 +354,7 @@ qemuDomainSetupMemory(virDomainMemoryDefPtr mem,
if (mem->model != VIR_DOMAIN_MEMORY_MODEL_NVDIMM)
return 0;
- return virStringListAdd(paths, mem->nvdimmPath);
+ return virStringListAdd(paths, mem->s.nvdimm.path);
}
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 579b3c3713..e68df5abe7 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3885,15 +3885,15 @@ qemuProcessNeedHugepagesPath(virDomainDefPtr def,
for (i = 0; i < def->nmems; i++) {
if (def->mems[i]->model == VIR_DOMAIN_MEMORY_MODEL_DIMM &&
- def->mems[i]->pagesize &&
- def->mems[i]->pagesize != system_pagesize)
+ def->mems[i]->s.dimm.pagesize &&
+ def->mems[i]->s.dimm.pagesize != system_pagesize)
return true;
}
if (mem &&
mem->model == VIR_DOMAIN_MEMORY_MODEL_DIMM &&
- mem->pagesize &&
- mem->pagesize != system_pagesize)
+ mem->s.dimm.pagesize &&
+ mem->s.dimm.pagesize != system_pagesize)
return true;
return false;
diff --git a/src/security/security_apparmor.c b/src/security/security_apparmor.c
index eed66e460f..78136e751e 100644
--- a/src/security/security_apparmor.c
+++ b/src/security/security_apparmor.c
@@ -686,13 +686,13 @@ AppArmorSetMemoryLabel(virSecurityManagerPtr mgr,
switch (mem->model) {
case VIR_DOMAIN_MEMORY_MODEL_NVDIMM:
- if (!virFileExists(mem->nvdimmPath)) {
+ if (!virFileExists(mem->s.nvdimm.path)) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("%s: \'%s\' does not exist"),
- __func__, mem->nvdimmPath);
+ __func__, mem->s.nvdimm.path);
return -1;
}
- return reload_profile(mgr, def, mem->nvdimmPath, true);
+ return reload_profile(mgr, def, mem->s.nvdimm.path, true);
case VIR_DOMAIN_MEMORY_MODEL_NONE:
case VIR_DOMAIN_MEMORY_MODEL_DIMM:
case VIR_DOMAIN_MEMORY_MODEL_LAST:
diff --git a/src/security/security_dac.c b/src/security/security_dac.c
index 4f4a0a069e..44ee42e1bd 100644
--- a/src/security/security_dac.c
+++ b/src/security/security_dac.c
@@ -1889,7 +1889,7 @@ virSecurityDACRestoreMemoryLabel(virSecurityManagerPtr mgr,
switch (mem->model) {
case VIR_DOMAIN_MEMORY_MODEL_NVDIMM:
- ret = virSecurityDACRestoreFileLabel(mgr, mem->nvdimmPath);
+ ret = virSecurityDACRestoreFileLabel(mgr, mem->s.nvdimm.path);
break;
case VIR_DOMAIN_MEMORY_MODEL_DIMM:
@@ -2070,7 +2070,7 @@ virSecurityDACSetMemoryLabel(virSecurityManagerPtr mgr,
return -1;
ret = virSecurityDACSetOwnership(mgr, NULL,
- mem->nvdimmPath,
+ mem->s.nvdimm.path,
user, group, true);
break;
diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
index e9cd95916e..294c9f1db5 100644
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -1577,7 +1577,7 @@ virSecuritySELinuxSetMemoryLabel(virSecurityManagerPtr mgr,
if (!seclabel || !seclabel->relabel)
return 0;
- if (virSecuritySELinuxSetFilecon(mgr, mem->nvdimmPath,
+ if (virSecuritySELinuxSetFilecon(mgr, mem->s.nvdimm.path,
seclabel->imagelabel, true) < 0)
return -1;
break;
@@ -1606,7 +1606,7 @@ virSecuritySELinuxRestoreMemoryLabel(virSecurityManagerPtr mgr,
if (!seclabel || !seclabel->relabel)
return 0;
- ret = virSecuritySELinuxRestoreFileLabel(mgr, mem->nvdimmPath, true);
+ ret = virSecuritySELinuxRestoreFileLabel(mgr, mem->s.nvdimm.path, true);
break;
case VIR_DOMAIN_MEMORY_MODEL_DIMM:
diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
index 5a6f4a5f7d..a8a05a0a90 100644
--- a/src/security/virt-aa-helper.c
+++ b/src/security/virt-aa-helper.c
@@ -1170,7 +1170,7 @@ 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)
+ if (vah_add_file(&buf, ctl->def->mems[i]->s.nvdimm.path,
"rw") != 0)
goto cleanup;
}
}
--
2.26.2