[libvirt] [PATCH v2 0/4] Predictable file names for memory-backend-file
v2 of: https://www.redhat.com/archives/libvir-list/2017-October/msg01063.html Patches are to be found here too: https://github.com/zippy2/libvirt/tree/qemu_mem_path_v3 diff to v1: -Dropped qemu.conf config knob -s/rmdir/virFileDeleteTree/ in qemuProcessBuildDestroyMemoryPathsImpl() because qemu leaves files behind and thus we need to unlink them too. Michal Privoznik (4): conf: s/virDomainObjGetShortName/virDomainDefGetShortName/ qemu: Move memPath generation from memoryBackingDir to a separate function qemu: Use predictable file names for memory-backend-file news: Document predictable file names for memory-backend-file docs/news.xml | 11 ++ src/conf/domain_conf.c | 4 +- src/conf/domain_conf.h | 2 +- src/libvirt_private.syms | 2 +- src/qemu/qemu_command.c | 9 +- src/qemu/qemu_conf.c | 71 ++++++++++- src/qemu/qemu_conf.h | 10 ++ src/qemu/qemu_domain.c | 4 +- src/qemu/qemu_driver.c | 19 ++- src/qemu/qemu_hotplug.c | 2 +- src/qemu/qemu_process.c | 137 +++++++++++++++------ src/qemu/qemu_process.h | 8 +- .../qemuxml2argv-cpu-numa-memshared.args | 6 +- .../qemuxml2argv-fd-memory-numa-topology.args | 3 +- .../qemuxml2argv-fd-memory-numa-topology2.args | 6 +- .../qemuxml2argv-fd-memory-numa-topology3.args | 9 +- .../qemuxml2argv-hugepages-memaccess2.args | 9 +- 17 files changed, 244 insertions(+), 68 deletions(-) -- 2.13.6
This function works over domain definition and not domain object. Its name is thus misleading. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/conf/domain_conf.c | 4 ++-- src/conf/domain_conf.h | 2 +- src/libvirt_private.syms | 2 +- src/qemu/qemu_conf.c | 2 +- src/qemu/qemu_domain.c | 4 ++-- src/qemu/qemu_driver.c | 2 +- 6 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 77c20c697..58b75c672 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -27871,14 +27871,14 @@ virDomainDefHasMemballoon(const virDomainDef *def) #define VIR_DOMAIN_SHORT_NAME_MAX 20 /** - * virDomainObjGetShortName: + * virDomainDefGetShortName: * @vm: Machine for which to get a name * @unique: Make sure the name is unique (use id as well) * * Shorten domain name to avoid possible path length limitations. */ char * -virDomainObjGetShortName(const virDomainDef *def) +virDomainDefGetShortName(const virDomainDef *def) { wchar_t wshortname[VIR_DOMAIN_SHORT_NAME_MAX + 1] = {0}; size_t len = 0; diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 38de70b15..bd541a9c5 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -3366,7 +3366,7 @@ int virDomainDefGetVcpuPinInfoHelper(virDomainDefPtr def, bool virDomainDefHasMemballoon(const virDomainDef *def) ATTRIBUTE_NONNULL(1); -char *virDomainObjGetShortName(const virDomainDef *def) ATTRIBUTE_NONNULL(1); +char *virDomainDefGetShortName(const virDomainDef *def) ATTRIBUTE_NONNULL(1); int virDomainGetBlkioParametersAssignFromDef(virDomainDefPtr def, diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 448d962b2..3ebff18aa 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -262,6 +262,7 @@ virDomainDefGetMemoryInitial; virDomainDefGetMemoryTotal; virDomainDefGetOnlineVcpumap; virDomainDefGetSecurityLabelDef; +virDomainDefGetShortName; virDomainDefGetVcpu; virDomainDefGetVcpuPinInfoHelper; virDomainDefGetVcpus; @@ -457,7 +458,6 @@ virDomainObjGetMetadata; virDomainObjGetOneDef; virDomainObjGetOneDefState; virDomainObjGetPersistentDef; -virDomainObjGetShortName; virDomainObjGetState; virDomainObjNew; virDomainObjParseNode; diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index ec61c9c52..bf6b334f4 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -1659,7 +1659,7 @@ qemuGetDomainHugepagePath(const virDomainDef *def, virHugeTLBFSPtr hugepage) { char *base = qemuGetBaseHugepagePath(hugepage); - char *domPath = virDomainObjGetShortName(def); + char *domPath = virDomainDefGetShortName(def); char *ret = NULL; if (base && domPath) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index c7c9e94da..e828fa2f6 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1705,7 +1705,7 @@ qemuDomainSetPrivatePaths(virQEMUDriverPtr driver, { virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); qemuDomainObjPrivatePtr priv = vm->privateData; - char *domname = virDomainObjGetShortName(vm->def); + char *domname = virDomainDefGetShortName(vm->def); int ret = -1; if (!domname) @@ -8203,7 +8203,7 @@ qemuDomainGetPreservedMountPath(virQEMUDriverConfigPtr cfg, char *path = NULL; char *tmp; const char *suffix = mountpoint + strlen(DEVPREFIX); - char *domname = virDomainObjGetShortName(vm->def); + char *domname = virDomainDefGetShortName(vm->def); size_t off; if (!domname) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 74fdfdb0f..135c20749 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4079,7 +4079,7 @@ getAutoDumpPath(virQEMUDriverPtr driver, virDomainObjPtr vm) { char *dumpfile = NULL; - char *domname = virDomainObjGetShortName(vm->def); + char *domname = virDomainDefGetShortName(vm->def); char timestr[100]; struct tm time_info; time_t curtime = time(NULL); -- 2.13.6
On 10/24/2017 07:41 AM, Michal Privoznik wrote:
This function works over domain definition and not domain object. Its name is thus misleading.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/conf/domain_conf.c | 4 ++-- src/conf/domain_conf.h | 2 +- src/libvirt_private.syms | 2 +- src/qemu/qemu_conf.c | 2 +- src/qemu/qemu_domain.c | 4 ++-- src/qemu/qemu_driver.c | 2 +- 6 files changed, 8 insertions(+), 8 deletions(-)
Reviewed-by: John Ferlan <jferlan@redhat.com> John BTW: I wonder if this function is somewhat broken - it's using def->id in order to generate the output, right? Which means for paths generated before the guest starts, they'll start with "-1"... IOW: Directory and file names that start with "-". This wasn't a problem originally (commit id a042275a) since the resulting path had "domain-" prepended to it - resulting in "domain--1-QEMUGuest1, for example). Personally, this looks REALLY odd when (re)using for example with a hugepage test where the path is "-mem-path /dev/hugepages2M/libvirt/qemu/-1-QEMUGuest1". OK - maybe it's just me, but seeing a filename start with "-" is really painful because it's not just a simple task of "rm $file", since "-" needs to escaped so as to not be considered an argument to rm <sigh>. I know, send a patch or just grumble and let someone else do it ;-)
On 11/02/2017 12:11 AM, John Ferlan wrote:
On 10/24/2017 07:41 AM, Michal Privoznik wrote:
This function works over domain definition and not domain object. Its name is thus misleading.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/conf/domain_conf.c | 4 ++-- src/conf/domain_conf.h | 2 +- src/libvirt_private.syms | 2 +- src/qemu/qemu_conf.c | 2 +- src/qemu/qemu_domain.c | 4 ++-- src/qemu/qemu_driver.c | 2 +- 6 files changed, 8 insertions(+), 8 deletions(-)
Reviewed-by: John Ferlan <jferlan@redhat.com>
John
BTW: I wonder if this function is somewhat broken - it's using def->id in order to generate the output, right? Which means for paths generated before the guest starts, they'll start with "-1"... IOW: Directory and file names that start with "-".
This wasn't a problem originally (commit id a042275a) since the resulting path had "domain-" prepended to it - resulting in "domain--1-QEMUGuest1, for example).
Personally, this looks REALLY odd when (re)using for example with a hugepage test where the path is "-mem-path /dev/hugepages2M/libvirt/qemu/-1-QEMUGuest1".
This is because we don't set an ID for domains when testing. However, in real scenarios this function is called over running domains. Therefore, the path will always start with a natural number. I don't think there's anything to fix really. We could go with "domain-" prefix but that'd be no better in fact. Yes, it would help with 'rm' case but other than that it has no added value. Michal
In near future we will need more than just a plain VIR_STRDUP(). Better implement that in a separate function and in qemuBuildMemoryBackendStr() which is complicated enough already. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_command.c | 2 +- src/qemu/qemu_conf.c | 18 ++++++++++++++++++ src/qemu/qemu_conf.h | 3 +++ 3 files changed, 22 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index b1cfafa79..82d2fb2a3 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3439,7 +3439,7 @@ qemuBuildMemoryBackendStr(virJSONValuePtr *backendProps, } else { /* We can have both pagesize and mem source. If that's the case, * prefer hugepages as those are more specific. */ - if (VIR_STRDUP(memPath, cfg->memoryBackingDir) < 0) + if (qemuGetMemoryBackingPath(cfg, &memPath) < 0) goto cleanup; } diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index bf6b334f4..23dcc25e1 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -1748,3 +1748,21 @@ qemuGetDomainHupageMemPath(const virDomainDef *def, return 0; } + + +/** + * qemuGetMemoryBackingPath: + * @cfg: the driver config + * @memPath: constructed path + * + * Constructs path to memory backing dir and stores it at @memPath. + * + * Returns: 0 on success, + * -1 otherwise (with error reported). + */ +int +qemuGetMemoryBackingPath(virQEMUDriverConfigPtr cfg, + char **memPath) +{ + return VIR_STRDUP(*memPath, cfg->memoryBackingDir); +} diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index 13b6f818a..9d6866816 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -363,4 +363,7 @@ int qemuGetDomainHupageMemPath(const virDomainDef *def, virQEMUDriverConfigPtr cfg, unsigned long long pagesize, char **memPath); + +int qemuGetMemoryBackingPath(virQEMUDriverConfigPtr cfg, + char **memPath); #endif /* __QEMUD_CONF_H */ -- 2.13.6
On 10/24/2017 07:41 AM, Michal Privoznik wrote:
In near future we will need more than just a plain VIR_STRDUP(). Better implement that in a separate function and in qemuBuildMemoryBackendStr() which is complicated enough already.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_command.c | 2 +- src/qemu/qemu_conf.c | 18 ++++++++++++++++++ src/qemu/qemu_conf.h | 3 +++ 3 files changed, 22 insertions(+), 1 deletion(-)
Reviewed-by: John Ferlan <jferlan@redhat.com> John
In some cases management application needs to allocate memory for qemu upfront and then just let qemu use that. Since we don't want to expose path for memory-backend-file anywhere in the domain XML, we can generate predictable paths. In this case: $memoryBackingDir/libvirt/qemu/$shortName/$alias where $shortName is result of virDomainObjGetShortName(). Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_command.c | 9 +- src/qemu/qemu_conf.c | 55 ++++++++- src/qemu/qemu_conf.h | 9 +- src/qemu/qemu_driver.c | 17 +++ src/qemu/qemu_hotplug.c | 2 +- src/qemu/qemu_process.c | 137 +++++++++++++++------ src/qemu/qemu_process.h | 8 +- .../qemuxml2argv-cpu-numa-memshared.args | 6 +- .../qemuxml2argv-fd-memory-numa-topology.args | 3 +- .../qemuxml2argv-fd-memory-numa-topology2.args | 6 +- .../qemuxml2argv-fd-memory-numa-topology3.args | 9 +- .../qemuxml2argv-hugepages-memaccess2.args | 9 +- 12 files changed, 207 insertions(+), 63 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 82d2fb2a3..d6d2654cd 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3439,7 +3439,7 @@ qemuBuildMemoryBackendStr(virJSONValuePtr *backendProps, } else { /* We can have both pagesize and mem source. If that's the case, * prefer hugepages as those are more specific. */ - if (qemuGetMemoryBackingPath(cfg, &memPath) < 0) + if (qemuGetMemoryBackingPath(def, cfg, mem->info.alias, &memPath) < 0) goto cleanup; } @@ -3542,12 +3542,13 @@ qemuBuildMemoryCellBackendStr(virDomainDefPtr def, unsigned long long memsize = virDomainNumaGetNodeMemorySize(def->numa, cell); + if (virAsprintf(&alias, "ram-node%zu", cell) < 0) + goto cleanup; + *backendStr = NULL; mem.size = memsize; mem.targetNode = cell; - - if (virAsprintf(&alias, "ram-node%zu", cell) < 0) - goto cleanup; + mem.info.alias = alias; if ((rc = qemuBuildMemoryBackendStr(&props, &backendType, cfg, priv->qemuCaps, def, &mem, priv->autoNodeset, false)) < 0) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 23dcc25e1..22c023255 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -1750,9 +1750,41 @@ qemuGetDomainHupageMemPath(const virDomainDef *def, } +int +qemuGetMemoryBackingBasePath(virQEMUDriverConfigPtr cfg, + char **path) +{ + return virAsprintf(path, "%s/libvirt/qemu", cfg->memoryBackingDir); +} + + +int +qemuGetMemoryBackingDomainPath(const virDomainDef *def, + virQEMUDriverConfigPtr cfg, + char **path) +{ + char *shortName = NULL; + char *base = NULL; + int ret = -1; + + if (!(shortName = virDomainDefGetShortName(def)) || + qemuGetMemoryBackingBasePath(cfg, &base) < 0 || + virAsprintf(path, "%s/%s", base, shortName) < 0) + goto cleanup; + + ret = 0; + cleanup: + VIR_FREE(base); + VIR_FREE(shortName); + return ret; +} + + /** * qemuGetMemoryBackingPath: + * @def: domain definition * @cfg: the driver config + * @alias: memory object alias * @memPath: constructed path * * Constructs path to memory backing dir and stores it at @memPath. @@ -1761,8 +1793,27 @@ qemuGetDomainHupageMemPath(const virDomainDef *def, * -1 otherwise (with error reported). */ int -qemuGetMemoryBackingPath(virQEMUDriverConfigPtr cfg, +qemuGetMemoryBackingPath(const virDomainDef *def, + virQEMUDriverConfigPtr cfg, + const char *alias, char **memPath) { - return VIR_STRDUP(*memPath, cfg->memoryBackingDir); + char *domainPath = NULL; + int ret = -1; + + if (!alias) { + /* This should never happen (TM) */ + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("memory device alias is not assigned")); + goto cleanup; + } + + if (qemuGetMemoryBackingDomainPath(def, cfg, &domainPath) < 0 || + virAsprintf(memPath, "%s/%s", domainPath, alias) < 0) + goto cleanup; + + ret = 0; + cleanup: + VIR_FREE(domainPath); + return ret; } diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index 9d6866816..a553e30e2 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -364,6 +364,13 @@ int qemuGetDomainHupageMemPath(const virDomainDef *def, unsigned long long pagesize, char **memPath); -int qemuGetMemoryBackingPath(virQEMUDriverConfigPtr cfg, +int qemuGetMemoryBackingBasePath(virQEMUDriverConfigPtr cfg, + char **path); +int qemuGetMemoryBackingDomainPath(const virDomainDef *def, + virQEMUDriverConfigPtr cfg, + char **path); +int qemuGetMemoryBackingPath(const virDomainDef *def, + virQEMUDriverConfigPtr cfg, + const char *alias, char **memPath); #endif /* __QEMUD_CONF_H */ diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 135c20749..b22fb7518 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -630,6 +630,7 @@ qemuStateInitialize(bool privileged, uid_t run_uid = -1; gid_t run_gid = -1; char *hugepagePath = NULL; + char *memoryBackingPath = NULL; size_t i; if (VIR_ALLOC(qemu_driver) < 0) @@ -888,6 +889,21 @@ qemuStateInitialize(bool privileged, VIR_FREE(hugepagePath); } + if (qemuGetMemoryBackingBasePath(cfg, &memoryBackingPath) < 0) + goto error; + + if (virFileMakePath(memoryBackingPath) < 0) { + virReportSystemError(errno, + _("unable to create memory backing path %s"), + memoryBackingPath); + goto error; + } + + if (privileged && + virFileUpdatePerm(memoryBackingPath, + 0, S_IXGRP | S_IXOTH) < 0) + goto error; + if (!(qemu_driver->closeCallbacks = virCloseCallbacksNew())) goto error; @@ -945,6 +961,7 @@ qemuStateInitialize(bool privileged, virObjectUnref(conn); VIR_FREE(driverConf); VIR_FREE(hugepagePath); + VIR_FREE(memoryBackingPath); qemuStateCleanup(); return -1; } diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 91f7f9ed6..5701c033b 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -2077,7 +2077,7 @@ qemuDomainAttachMemory(virQEMUDriverPtr driver, priv->qemuCaps, vm->def, mem, NULL, true) < 0) goto cleanup; - if (qemuProcessBuildDestroyHugepagesPath(driver, vm, mem, true) < 0) + if (qemuProcessBuildDestroyMemoryPaths(driver, vm, mem, true) < 0) goto cleanup; if (qemuDomainNamespaceSetupMemory(driver, vm, mem) < 0) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index fdc868912..4fe973c59 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3324,60 +3324,117 @@ qemuProcessNeedHugepagesPath(virDomainDefPtr def, } +static bool +qemuProcessNeedMemoryBackingPath(virDomainDefPtr def, + virDomainMemoryDefPtr mem) +{ + size_t i; + size_t numaNodes; + + if (def->mem.source == VIR_DOMAIN_MEMORY_SOURCE_FILE || + def->mem.access != VIR_DOMAIN_MEMORY_ACCESS_DEFAULT) + return true; + + numaNodes = virDomainNumaGetNodeCount(def->numa); + for (i = 0; i < numaNodes; i++) { + if (virDomainNumaGetNodeMemoryAccessMode(def->numa, i) + != VIR_DOMAIN_MEMORY_ACCESS_DEFAULT) + return true; + } + + if (mem && + mem->model == VIR_DOMAIN_MEMORY_MODEL_DIMM && + (mem->access != VIR_DOMAIN_MEMORY_ACCESS_DEFAULT || + (mem->targetNode >= 0 && + virDomainNumaGetNodeMemoryAccessMode(def->numa, mem->targetNode) + != VIR_DOMAIN_MEMORY_ACCESS_DEFAULT))) + return true; + + return false; +} + + +static int +qemuProcessBuildDestroyMemoryPathsImpl(virQEMUDriverPtr driver, + virDomainDefPtr def, + const char *path, + bool build) +{ + if (build) { + if (virFileExists(path)) + return 0; + + if (virFileMakePathWithMode(path, 0700) < 0) { + virReportSystemError(errno, + _("Unable to create %s"), + path); + return -1; + } + + if (qemuSecurityDomainSetPathLabel(driver->securityManager, + def, path) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unable to label %s"), path); + return -1; + } + } else { + if (virFileDeleteTree(path) < 0 && + errno != ENOENT) + VIR_WARN("Unable to remove hugepage path: %s (errno=%d)", + path, errno); + } + + return 0; +} + + int -qemuProcessBuildDestroyHugepagesPath(virQEMUDriverPtr driver, - virDomainObjPtr vm, - virDomainMemoryDefPtr mem, - bool build) +qemuProcessBuildDestroyMemoryPaths(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virDomainMemoryDefPtr mem, + bool build) { virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); - char *hugepagePath = NULL; + char *path = NULL; size_t i; - bool shouldBuild = false; + bool shouldBuildHP = false; + bool shouldBuildMB = false; int ret = -1; - if (build) - shouldBuild = qemuProcessNeedHugepagesPath(vm->def, mem); + if (build) { + shouldBuildHP = qemuProcessNeedHugepagesPath(vm->def, mem); + shouldBuildMB = qemuProcessNeedMemoryBackingPath(vm->def, mem); + } - if (!build || shouldBuild) { + if (!build || shouldBuildHP) { for (i = 0; i < cfg->nhugetlbfs; i++) { - VIR_FREE(hugepagePath); - hugepagePath = qemuGetDomainHugepagePath(vm->def, &cfg->hugetlbfs[i]); + path = qemuGetDomainHugepagePath(vm->def, &cfg->hugetlbfs[i]); - if (!hugepagePath) + if (!path) goto cleanup; - if (build) { - if (virFileExists(hugepagePath)) { - ret = 0; - goto cleanup; - } - - if (virFileMakePathWithMode(hugepagePath, 0700) < 0) { - virReportSystemError(errno, - _("Unable to create %s"), - hugepagePath); - goto cleanup; - } + if (qemuProcessBuildDestroyMemoryPathsImpl(driver, vm->def, + path, build) < 0) + goto cleanup; - if (qemuSecurityDomainSetPathLabel(driver->securityManager, - vm->def, hugepagePath) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("Unable to set huge path in security driver")); - goto cleanup; - } - } else { - if (rmdir(hugepagePath) < 0 && - errno != ENOENT) - VIR_WARN("Unable to remove hugepage path: %s (errno=%d)", - hugepagePath, errno); - } + VIR_FREE(path); } } + if (!build || shouldBuildMB) { + if (qemuGetMemoryBackingDomainPath(vm->def, cfg, &path) < 0) + goto cleanup; + + if (qemuProcessBuildDestroyMemoryPathsImpl(driver, vm->def, + path, build) < 0) + goto cleanup; + + VIR_FREE(path); + } + ret = 0; cleanup: - VIR_FREE(hugepagePath); + VIR_FREE(path); virObjectUnref(cfg); return ret; } @@ -5550,7 +5607,7 @@ qemuProcessPrepareHost(virQEMUDriverPtr driver, NULL) < 0) goto cleanup; - if (qemuProcessBuildDestroyHugepagesPath(driver, vm, NULL, true) < 0) + if (qemuProcessBuildDestroyMemoryPaths(driver, vm, NULL, true) < 0) goto cleanup; /* Ensure no historical cgroup for this VM is lying around bogus @@ -6254,7 +6311,7 @@ void qemuProcessStop(virQEMUDriverPtr driver, goto endjob; } - qemuProcessBuildDestroyHugepagesPath(driver, vm, NULL, false); + qemuProcessBuildDestroyMemoryPaths(driver, vm, NULL, false); vm->def->id = -1; @@ -7112,7 +7169,7 @@ qemuProcessReconnect(void *opaque) goto cleanup; } - if (qemuProcessBuildDestroyHugepagesPath(driver, obj, NULL, true) < 0) + if (qemuProcessBuildDestroyMemoryPaths(driver, obj, NULL, true) < 0) goto error; if ((qemuDomainAssignAddresses(obj->def, priv->qemuCaps, diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h index 814b86d8a..cd9a72031 100644 --- a/src/qemu/qemu_process.h +++ b/src/qemu/qemu_process.h @@ -38,10 +38,10 @@ int qemuProcessStopCPUs(virQEMUDriverPtr driver, virDomainPausedReason reason, qemuDomainAsyncJob asyncJob); -int qemuProcessBuildDestroyHugepagesPath(virQEMUDriverPtr driver, - virDomainObjPtr vm, - virDomainMemoryDefPtr mem, - bool build); +int qemuProcessBuildDestroyMemoryPaths(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virDomainMemoryDefPtr mem, + bool build); void qemuProcessAutostartAll(virQEMUDriverPtr driver); void qemuProcessReconnectAll(virConnectPtr conn, virQEMUDriverPtr driver); diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa-memshared.args b/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa-memshared.args index 5700c3413..352819429 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa-memshared.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa-memshared.args @@ -10,10 +10,12 @@ QEMU_AUDIO_DRV=none \ -M pc \ -m 214 \ -smp 16,sockets=2,cores=4,threads=2 \ --object memory-backend-file,id=ram-node0,mem-path=/var/lib/libvirt/qemu/ram,\ +-object memory-backend-file,id=ram-node0,\ +mem-path=/var/lib/libvirt/qemu/ram/libvirt/qemu/-1-QEMUGuest1/ram-node0,\ share=yes,size=112197632 \ -numa node,nodeid=0,cpus=0-7,memdev=ram-node0 \ --object memory-backend-file,id=ram-node1,mem-path=/var/lib/libvirt/qemu/ram,\ +-object memory-backend-file,id=ram-node1,\ +mem-path=/var/lib/libvirt/qemu/ram/libvirt/qemu/-1-QEMUGuest1/ram-node1,\ share=no,size=112197632 \ -numa node,nodeid=1,cpus=8-15,memdev=ram-node1 \ -uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ diff --git a/tests/qemuxml2argvdata/qemuxml2argv-fd-memory-numa-topology.args b/tests/qemuxml2argvdata/qemuxml2argv-fd-memory-numa-topology.args index 12f3d8ab8..fa1353259 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-fd-memory-numa-topology.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-fd-memory-numa-topology.args @@ -11,7 +11,8 @@ QEMU_AUDIO_DRV=none \ -m 14336 \ -mem-prealloc \ -smp 8,sockets=1,cores=8,threads=1 \ --object memory-backend-file,id=ram-node0,mem-path=/var/lib/libvirt/qemu/ram,\ +-object memory-backend-file,id=ram-node0,\ +mem-path=/var/lib/libvirt/qemu/ram/libvirt/qemu/-1-instance-00000092/ram-node0,\ share=yes,size=15032385536 \ -numa node,nodeid=0,cpus=0-7,memdev=ram-node0 \ -uuid 126f2720-6f8e-45ab-a886-ec9277079a67 \ diff --git a/tests/qemuxml2argvdata/qemuxml2argv-fd-memory-numa-topology2.args b/tests/qemuxml2argvdata/qemuxml2argv-fd-memory-numa-topology2.args index 585e4d506..6f73a1b99 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-fd-memory-numa-topology2.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-fd-memory-numa-topology2.args @@ -11,10 +11,12 @@ QEMU_AUDIO_DRV=none \ -m 28672 \ -mem-prealloc \ -smp 20,sockets=1,cores=8,threads=1 \ --object memory-backend-file,id=ram-node0,mem-path=/var/lib/libvirt/qemu/ram,\ +-object memory-backend-file,id=ram-node0,\ +mem-path=/var/lib/libvirt/qemu/ram/libvirt/qemu/-1-instance-00000092/ram-node0,\ share=no,size=15032385536 \ -numa node,nodeid=0,cpus=0-7,memdev=ram-node0 \ --object memory-backend-file,id=ram-node1,mem-path=/var/lib/libvirt/qemu/ram,\ +-object memory-backend-file,id=ram-node1,\ +mem-path=/var/lib/libvirt/qemu/ram/libvirt/qemu/-1-instance-00000092/ram-node1,\ share=yes,size=15032385536 \ -numa node,nodeid=1,cpus=8-15,memdev=ram-node1 \ -uuid 126f2720-6f8e-45ab-a886-ec9277079a67 \ diff --git a/tests/qemuxml2argvdata/qemuxml2argv-fd-memory-numa-topology3.args b/tests/qemuxml2argvdata/qemuxml2argv-fd-memory-numa-topology3.args index e9a57a69e..3c352fe03 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-fd-memory-numa-topology3.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-fd-memory-numa-topology3.args @@ -11,13 +11,16 @@ QEMU_AUDIO_DRV=none \ -m 43008 \ -mem-prealloc \ -smp 32,sockets=1,cores=24,threads=1 \ --object memory-backend-file,id=ram-node0,mem-path=/var/lib/libvirt/qemu/ram,\ +-object memory-backend-file,id=ram-node0,\ +mem-path=/var/lib/libvirt/qemu/ram/libvirt/qemu/-1-instance-00000092/ram-node0,\ share=yes,size=15032385536 \ -numa node,nodeid=0,cpus=0-1,memdev=ram-node0 \ --object memory-backend-file,id=ram-node1,mem-path=/var/lib/libvirt/qemu/ram,\ +-object memory-backend-file,id=ram-node1,\ +mem-path=/var/lib/libvirt/qemu/ram/libvirt/qemu/-1-instance-00000092/ram-node1,\ share=yes,size=15032385536 \ -numa node,nodeid=1,cpus=2-3,memdev=ram-node1 \ --object memory-backend-file,id=ram-node2,mem-path=/var/lib/libvirt/qemu/ram,\ +-object memory-backend-file,id=ram-node2,\ +mem-path=/var/lib/libvirt/qemu/ram/libvirt/qemu/-1-instance-00000092/ram-node2,\ share=no,size=15032385536 \ -numa node,nodeid=2,cpus=4-5,memdev=ram-node2 \ -uuid 126f2720-6f8e-45ab-a886-ec9277079a67 \ diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-memaccess2.args b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-memaccess2.args index 55db49171..d8e506c19 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-memaccess2.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-memaccess2.args @@ -10,17 +10,20 @@ QEMU_AUDIO_DRV=none \ -M pc \ -m size=4194304k,slots=16,maxmem=8388608k \ -smp 4,sockets=4,cores=1,threads=1 \ --object memory-backend-file,id=ram-node0,mem-path=/var/lib/libvirt/qemu/ram,\ +-object memory-backend-file,id=ram-node0,\ +mem-path=/var/lib/libvirt/qemu/ram/libvirt/qemu/-1-QEMUGuest1/ram-node0,\ share=no,size=1073741824,host-nodes=0-3,policy=bind \ -numa node,nodeid=0,cpus=0,memdev=ram-node0 \ -object memory-backend-file,id=ram-node1,prealloc=yes,\ mem-path=/dev/hugepages2M/libvirt/qemu/-1-QEMUGuest1,share=yes,size=1073741824,\ host-nodes=0-3,policy=bind \ -numa node,nodeid=1,cpus=1,memdev=ram-node1 \ --object memory-backend-file,id=ram-node2,mem-path=/var/lib/libvirt/qemu/ram,\ +-object memory-backend-file,id=ram-node2,\ +mem-path=/var/lib/libvirt/qemu/ram/libvirt/qemu/-1-QEMUGuest1/ram-node2,\ share=no,size=1073741824,host-nodes=0-3,policy=bind \ -numa node,nodeid=2,cpus=2,memdev=ram-node2 \ --object memory-backend-file,id=ram-node3,mem-path=/var/lib/libvirt/qemu/ram,\ +-object memory-backend-file,id=ram-node3,\ +mem-path=/var/lib/libvirt/qemu/ram/libvirt/qemu/-1-QEMUGuest1/ram-node3,\ share=no,size=1073741824,host-nodes=3,policy=bind \ -numa node,nodeid=3,cpus=3,memdev=ram-node3 \ -object memory-backend-file,id=memdimm0,prealloc=yes,\ -- 2.13.6
On 10/24/2017 07:41 AM, Michal Privoznik wrote:
In some cases management application needs to allocate memory for qemu upfront and then just let qemu use that. Since we don't want to expose path for memory-backend-file anywhere in the domain XML, we can generate predictable paths. In this case:
$memoryBackingDir/libvirt/qemu/$shortName/$alias
where $shortName is result of virDomainObjGetShortName().
s/Obj/Def
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_command.c | 9 +- src/qemu/qemu_conf.c | 55 ++++++++- src/qemu/qemu_conf.h | 9 +- src/qemu/qemu_driver.c | 17 +++ src/qemu/qemu_hotplug.c | 2 +- src/qemu/qemu_process.c | 137 +++++++++++++++------ src/qemu/qemu_process.h | 8 +- .../qemuxml2argv-cpu-numa-memshared.args | 6 +- .../qemuxml2argv-fd-memory-numa-topology.args | 3 +- .../qemuxml2argv-fd-memory-numa-topology2.args | 6 +- .../qemuxml2argv-fd-memory-numa-topology3.args | 9 +- .../qemuxml2argv-hugepages-memaccess2.args | 9 +- 12 files changed, 207 insertions(+), 63 deletions(-)
There's parts of this that would seemingly be able to be separated out so that it's easier to review...
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 82d2fb2a3..d6d2654cd 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3439,7 +3439,7 @@ qemuBuildMemoryBackendStr(virJSONValuePtr *backendProps, } else { /* We can have both pagesize and mem source. If that's the case, * prefer hugepages as those are more specific. */ - if (qemuGetMemoryBackingPath(cfg, &memPath) < 0) + if (qemuGetMemoryBackingPath(def, cfg, mem->info.alias, &memPath) < 0) goto cleanup; }
@@ -3542,12 +3542,13 @@ qemuBuildMemoryCellBackendStr(virDomainDefPtr def, unsigned long long memsize = virDomainNumaGetNodeMemorySize(def->numa, cell);
+ if (virAsprintf(&alias, "ram-node%zu", cell) < 0) + goto cleanup; +
This is just a simple trivial move and not related...
*backendStr = NULL; mem.size = memsize; mem.targetNode = cell; - - if (virAsprintf(&alias, "ram-node%zu", cell) < 0) - goto cleanup; + mem.info.alias = alias;
if ((rc = qemuBuildMemoryBackendStr(&props, &backendType, cfg, priv->qemuCaps, def, &mem, priv->autoNodeset, false)) < 0) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 23dcc25e1..22c023255 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -1750,9 +1750,41 @@ qemuGetDomainHupageMemPath(const virDomainDef *def, }
+int +qemuGetMemoryBackingBasePath(virQEMUDriverConfigPtr cfg, + char **path) +{ + return virAsprintf(path, "%s/libvirt/qemu", cfg->memoryBackingDir);
Not of a fan of redundancy, but I'm also not sure what better way there is... For privileged domains it's "/var/lib/libvirt/qemu/ram/libvirt/qemu/" and unpriv'd it's $XDG_CONFIG_HOME+"/qemu/ram/libvirt/qemu". The former repeats "libvirt/qemu" while the latter only repeats "qemu". I would think we could drop the "qemu" at the very least, but it doesn't matter that much to me. Random thought - do memory file paths even work for non priv'd domains?
+} + + +int +qemuGetMemoryBackingDomainPath(const virDomainDef *def, + virQEMUDriverConfigPtr cfg, + char **path) +{ + char *shortName = NULL; + char *base = NULL; + int ret = -1; + + if (!(shortName = virDomainDefGetShortName(def)) || + qemuGetMemoryBackingBasePath(cfg, &base) < 0 || + virAsprintf(path, "%s/%s", base, shortName) < 0) + goto cleanup; +
As I noted in review of patch 1... This continues the logic of creating paths that start with "-", which to me is, well, not right. Maybe others aren't offended by it. Is there anyway we can prepend something here to "shortName" - I'd say "ram", but that's redundant with the path, but that to me is at least better than a directory name starting with "-".
+ ret = 0; + cleanup: + VIR_FREE(base); + VIR_FREE(shortName); + return ret; +} + + /** * qemuGetMemoryBackingPath: + * @def: domain definition * @cfg: the driver config + * @alias: memory object alias * @memPath: constructed path * * Constructs path to memory backing dir and stores it at @memPath. @@ -1761,8 +1793,27 @@ qemuGetDomainHupageMemPath(const virDomainDef *def, * -1 otherwise (with error reported). */ int -qemuGetMemoryBackingPath(virQEMUDriverConfigPtr cfg, +qemuGetMemoryBackingPath(const virDomainDef *def, + virQEMUDriverConfigPtr cfg, + const char *alias, char **memPath) { - return VIR_STRDUP(*memPath, cfg->memoryBackingDir); + char *domainPath = NULL; + int ret = -1; + + if (!alias) { + /* This should never happen (TM) */ + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("memory device alias is not assigned")); + goto cleanup; + } + + if (qemuGetMemoryBackingDomainPath(def, cfg, &domainPath) < 0 || + virAsprintf(memPath, "%s/%s", domainPath, alias) < 0) + goto cleanup; + + ret = 0; + cleanup: + VIR_FREE(domainPath); + return ret; } diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index 9d6866816..a553e30e2 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -364,6 +364,13 @@ int qemuGetDomainHupageMemPath(const virDomainDef *def, unsigned long long pagesize, char **memPath);
-int qemuGetMemoryBackingPath(virQEMUDriverConfigPtr cfg, +int qemuGetMemoryBackingBasePath(virQEMUDriverConfigPtr cfg, + char **path); +int qemuGetMemoryBackingDomainPath(const virDomainDef *def, + virQEMUDriverConfigPtr cfg, + char **path); +int qemuGetMemoryBackingPath(const virDomainDef *def, + virQEMUDriverConfigPtr cfg, + const char *alias, char **memPath); #endif /* __QEMUD_CONF_H */ diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 135c20749..b22fb7518 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -630,6 +630,7 @@ qemuStateInitialize(bool privileged, uid_t run_uid = -1; gid_t run_gid = -1; char *hugepagePath = NULL; + char *memoryBackingPath = NULL; size_t i;
if (VIR_ALLOC(qemu_driver) < 0) @@ -888,6 +889,21 @@ qemuStateInitialize(bool privileged, VIR_FREE(hugepagePath); }
+ if (qemuGetMemoryBackingBasePath(cfg, &memoryBackingPath) < 0) + goto error; + + if (virFileMakePath(memoryBackingPath) < 0) { + virReportSystemError(errno, + _("unable to create memory backing path %s"), + memoryBackingPath); + goto error; + } + + if (privileged && + virFileUpdatePerm(memoryBackingPath, + 0, S_IXGRP | S_IXOTH) < 0) + goto error; + if (!(qemu_driver->closeCallbacks = virCloseCallbacksNew())) goto error;
@@ -945,6 +961,7 @@ qemuStateInitialize(bool privileged, virObjectUnref(conn); VIR_FREE(driverConf); VIR_FREE(hugepagePath); + VIR_FREE(memoryBackingPath); qemuStateCleanup(); return -1; } diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 91f7f9ed6..5701c033b 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -2077,7 +2077,7 @@ qemuDomainAttachMemory(virQEMUDriverPtr driver, priv->qemuCaps, vm->def, mem, NULL, true) < 0) goto cleanup;
- if (qemuProcessBuildDestroyHugepagesPath(driver, vm, mem, true) < 0) + if (qemuProcessBuildDestroyMemoryPaths(driver, vm, mem, true) < 0) goto cleanup;
if (qemuDomainNamespaceSetupMemory(driver, vm, mem) < 0) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index fdc868912..4fe973c59 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3324,60 +3324,117 @@ qemuProcessNeedHugepagesPath(virDomainDefPtr def, }
+static bool +qemuProcessNeedMemoryBackingPath(virDomainDefPtr def, + virDomainMemoryDefPtr mem) +{ + size_t i; + size_t numaNodes; + + if (def->mem.source == VIR_DOMAIN_MEMORY_SOURCE_FILE || + def->mem.access != VIR_DOMAIN_MEMORY_ACCESS_DEFAULT) + return true; + + numaNodes = virDomainNumaGetNodeCount(def->numa); + for (i = 0; i < numaNodes; i++) { + if (virDomainNumaGetNodeMemoryAccessMode(def->numa, i) + != VIR_DOMAIN_MEMORY_ACCESS_DEFAULT) + return true; + } + + if (mem && + mem->model == VIR_DOMAIN_MEMORY_MODEL_DIMM && + (mem->access != VIR_DOMAIN_MEMORY_ACCESS_DEFAULT || + (mem->targetNode >= 0 && + virDomainNumaGetNodeMemoryAccessMode(def->numa, mem->targetNode) + != VIR_DOMAIN_MEMORY_ACCESS_DEFAULT))) + return true; + + return false; +} + +
Starting here...
+static int +qemuProcessBuildDestroyMemoryPathsImpl(virQEMUDriverPtr driver, + virDomainDefPtr def, + const char *path, + bool build)
Could just be qemuProcessBuildDestroyPath as it's one of many possible calls within (at least) the hugepage loop.
+{ + if (build) { + if (virFileExists(path)) + return 0; + + if (virFileMakePathWithMode(path, 0700) < 0) { + virReportSystemError(errno, + _("Unable to create %s"), + path); + return -1; + } + + if (qemuSecurityDomainSetPathLabel(driver->securityManager, + def, path) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unable to label %s"), path); + return -1; + } + } else { + if (virFileDeleteTree(path) < 0 && + errno != ENOENT) + VIR_WARN("Unable to remove hugepage path: %s (errno=%d)",
This error message would need to change to remove "hugepage"
+ path, errno); + } + + return 0; +} + +
... this seems separable too, modulo the change to use virFileDeleteTree which then would be a follow-up patch to be accepted on its own merits... The other option being a check to add some sort of isDir type logic or flag is input considering that hugepages wouldn't need to delete the path, right? Or should they? As Dan points out in v1 - we do need to keep the $dir/libvirt/qemu though for other domains and we wouldn't want those subdirectories to get deleted from/for Hugepages before we do so for memory backing, right? I think the way you have this we'd delete those before getting to them in the logic, but my eyes are really tired right now.
int -qemuProcessBuildDestroyHugepagesPath(virQEMUDriverPtr driver, - virDomainObjPtr vm, - virDomainMemoryDefPtr mem, - bool build) +qemuProcessBuildDestroyMemoryPaths(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virDomainMemoryDefPtr mem, + bool build)
You could seemingly make the API name change separately too, IDC, but some do generally speaking of course...
{ virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); - char *hugepagePath = NULL; + char *path = NULL; size_t i; - bool shouldBuild = false; + bool shouldBuildHP = false; + bool shouldBuildMB = false; int ret = -1;
- if (build) - shouldBuild = qemuProcessNeedHugepagesPath(vm->def, mem); + if (build) { + shouldBuildHP = qemuProcessNeedHugepagesPath(vm->def, mem); + shouldBuildMB = qemuProcessNeedMemoryBackingPath(vm->def, mem); + }
- if (!build || shouldBuild) { + if (!build || shouldBuildHP) { for (i = 0; i < cfg->nhugetlbfs; i++) { - VIR_FREE(hugepagePath); - hugepagePath = qemuGetDomainHugepagePath(vm->def, &cfg->hugetlbfs[i]); + path = qemuGetDomainHugepagePath(vm->def, &cfg->hugetlbfs[i]);
- if (!hugepagePath) + if (!path) goto cleanup;
- if (build) { - if (virFileExists(hugepagePath)) { - ret = 0; - goto cleanup; - } - - if (virFileMakePathWithMode(hugepagePath, 0700) < 0) { - virReportSystemError(errno, - _("Unable to create %s"), - hugepagePath); - goto cleanup; - } + if (qemuProcessBuildDestroyMemoryPathsImpl(driver, vm->def, + path, build) < 0) + goto cleanup;
- if (qemuSecurityDomainSetPathLabel(driver->securityManager, - vm->def, hugepagePath) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("Unable to set huge path in security driver")); - goto cleanup; - } - } else { - if (rmdir(hugepagePath) < 0 && - errno != ENOENT) - VIR_WARN("Unable to remove hugepage path: %s (errno=%d)", - hugepagePath, errno); - } + VIR_FREE(path); } }
+ if (!build || shouldBuildMB) { + if (qemuGetMemoryBackingDomainPath(vm->def, cfg, &path) < 0) + goto cleanup; + + if (qemuProcessBuildDestroyMemoryPathsImpl(driver, vm->def, + path, build) < 0) + goto cleanup; + + VIR_FREE(path); + } + ret = 0; cleanup: - VIR_FREE(hugepagePath); + VIR_FREE(path); virObjectUnref(cfg); return ret; } @@ -5550,7 +5607,7 @@ qemuProcessPrepareHost(virQEMUDriverPtr driver, NULL) < 0) goto cleanup;
- if (qemuProcessBuildDestroyHugepagesPath(driver, vm, NULL, true) < 0) + if (qemuProcessBuildDestroyMemoryPaths(driver, vm, NULL, true) < 0) goto cleanup;
/* Ensure no historical cgroup for this VM is lying around bogus @@ -6254,7 +6311,7 @@ void qemuProcessStop(virQEMUDriverPtr driver, goto endjob; }
- qemuProcessBuildDestroyHugepagesPath(driver, vm, NULL, false); + qemuProcessBuildDestroyMemoryPaths(driver, vm, NULL, false);
vm->def->id = -1;
@@ -7112,7 +7169,7 @@ qemuProcessReconnect(void *opaque) goto cleanup; }
- if (qemuProcessBuildDestroyHugepagesPath(driver, obj, NULL, true) < 0) + if (qemuProcessBuildDestroyMemoryPaths(driver, obj, NULL, true) < 0) goto error;
if ((qemuDomainAssignAddresses(obj->def, priv->qemuCaps, diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h index 814b86d8a..cd9a72031 100644 --- a/src/qemu/qemu_process.h +++ b/src/qemu/qemu_process.h @@ -38,10 +38,10 @@ int qemuProcessStopCPUs(virQEMUDriverPtr driver, virDomainPausedReason reason, qemuDomainAsyncJob asyncJob);
-int qemuProcessBuildDestroyHugepagesPath(virQEMUDriverPtr driver, - virDomainObjPtr vm, - virDomainMemoryDefPtr mem, - bool build); +int qemuProcessBuildDestroyMemoryPaths(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virDomainMemoryDefPtr mem, + bool build);
void qemuProcessAutostartAll(virQEMUDriverPtr driver); void qemuProcessReconnectAll(virConnectPtr conn, virQEMUDriverPtr driver); diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa-memshared.args b/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa-memshared.args index 5700c3413..352819429 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa-memshared.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa-memshared.args @@ -10,10 +10,12 @@ QEMU_AUDIO_DRV=none \ -M pc \ -m 214 \ -smp 16,sockets=2,cores=4,threads=2 \ --object memory-backend-file,id=ram-node0,mem-path=/var/lib/libvirt/qemu/ram,\ +-object memory-backend-file,id=ram-node0,\ +mem-path=/var/lib/libvirt/qemu/ram/libvirt/qemu/-1-QEMUGuest1/ram-node0,\
And this is just where it really obvious what's happening - although it took me a bit to figure it all out, especially in my post KVM Forum travel haze. John
share=yes,size=112197632 \ -numa node,nodeid=0,cpus=0-7,memdev=ram-node0 \ --object memory-backend-file,id=ram-node1,mem-path=/var/lib/libvirt/qemu/ram,\ +-object memory-backend-file,id=ram-node1,\ +mem-path=/var/lib/libvirt/qemu/ram/libvirt/qemu/-1-QEMUGuest1/ram-node1,\ share=no,size=112197632 \ -numa node,nodeid=1,cpus=8-15,memdev=ram-node1 \ -uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ diff --git a/tests/qemuxml2argvdata/qemuxml2argv-fd-memory-numa-topology.args b/tests/qemuxml2argvdata/qemuxml2argv-fd-memory-numa-topology.args index 12f3d8ab8..fa1353259 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-fd-memory-numa-topology.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-fd-memory-numa-topology.args @@ -11,7 +11,8 @@ QEMU_AUDIO_DRV=none \ -m 14336 \ -mem-prealloc \ -smp 8,sockets=1,cores=8,threads=1 \ --object memory-backend-file,id=ram-node0,mem-path=/var/lib/libvirt/qemu/ram,\ +-object memory-backend-file,id=ram-node0,\ +mem-path=/var/lib/libvirt/qemu/ram/libvirt/qemu/-1-instance-00000092/ram-node0,\ share=yes,size=15032385536 \ -numa node,nodeid=0,cpus=0-7,memdev=ram-node0 \ -uuid 126f2720-6f8e-45ab-a886-ec9277079a67 \ diff --git a/tests/qemuxml2argvdata/qemuxml2argv-fd-memory-numa-topology2.args b/tests/qemuxml2argvdata/qemuxml2argv-fd-memory-numa-topology2.args index 585e4d506..6f73a1b99 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-fd-memory-numa-topology2.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-fd-memory-numa-topology2.args @@ -11,10 +11,12 @@ QEMU_AUDIO_DRV=none \ -m 28672 \ -mem-prealloc \ -smp 20,sockets=1,cores=8,threads=1 \ --object memory-backend-file,id=ram-node0,mem-path=/var/lib/libvirt/qemu/ram,\ +-object memory-backend-file,id=ram-node0,\ +mem-path=/var/lib/libvirt/qemu/ram/libvirt/qemu/-1-instance-00000092/ram-node0,\ share=no,size=15032385536 \ -numa node,nodeid=0,cpus=0-7,memdev=ram-node0 \ --object memory-backend-file,id=ram-node1,mem-path=/var/lib/libvirt/qemu/ram,\ +-object memory-backend-file,id=ram-node1,\ +mem-path=/var/lib/libvirt/qemu/ram/libvirt/qemu/-1-instance-00000092/ram-node1,\ share=yes,size=15032385536 \ -numa node,nodeid=1,cpus=8-15,memdev=ram-node1 \ -uuid 126f2720-6f8e-45ab-a886-ec9277079a67 \ diff --git a/tests/qemuxml2argvdata/qemuxml2argv-fd-memory-numa-topology3.args b/tests/qemuxml2argvdata/qemuxml2argv-fd-memory-numa-topology3.args index e9a57a69e..3c352fe03 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-fd-memory-numa-topology3.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-fd-memory-numa-topology3.args @@ -11,13 +11,16 @@ QEMU_AUDIO_DRV=none \ -m 43008 \ -mem-prealloc \ -smp 32,sockets=1,cores=24,threads=1 \ --object memory-backend-file,id=ram-node0,mem-path=/var/lib/libvirt/qemu/ram,\ +-object memory-backend-file,id=ram-node0,\ +mem-path=/var/lib/libvirt/qemu/ram/libvirt/qemu/-1-instance-00000092/ram-node0,\ share=yes,size=15032385536 \ -numa node,nodeid=0,cpus=0-1,memdev=ram-node0 \ --object memory-backend-file,id=ram-node1,mem-path=/var/lib/libvirt/qemu/ram,\ +-object memory-backend-file,id=ram-node1,\ +mem-path=/var/lib/libvirt/qemu/ram/libvirt/qemu/-1-instance-00000092/ram-node1,\ share=yes,size=15032385536 \ -numa node,nodeid=1,cpus=2-3,memdev=ram-node1 \ --object memory-backend-file,id=ram-node2,mem-path=/var/lib/libvirt/qemu/ram,\ +-object memory-backend-file,id=ram-node2,\ +mem-path=/var/lib/libvirt/qemu/ram/libvirt/qemu/-1-instance-00000092/ram-node2,\ share=no,size=15032385536 \ -numa node,nodeid=2,cpus=4-5,memdev=ram-node2 \ -uuid 126f2720-6f8e-45ab-a886-ec9277079a67 \ diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-memaccess2.args b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-memaccess2.args index 55db49171..d8e506c19 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-memaccess2.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-memaccess2.args @@ -10,17 +10,20 @@ QEMU_AUDIO_DRV=none \ -M pc \ -m size=4194304k,slots=16,maxmem=8388608k \ -smp 4,sockets=4,cores=1,threads=1 \ --object memory-backend-file,id=ram-node0,mem-path=/var/lib/libvirt/qemu/ram,\ +-object memory-backend-file,id=ram-node0,\ +mem-path=/var/lib/libvirt/qemu/ram/libvirt/qemu/-1-QEMUGuest1/ram-node0,\ share=no,size=1073741824,host-nodes=0-3,policy=bind \ -numa node,nodeid=0,cpus=0,memdev=ram-node0 \ -object memory-backend-file,id=ram-node1,prealloc=yes,\ mem-path=/dev/hugepages2M/libvirt/qemu/-1-QEMUGuest1,share=yes,size=1073741824,\ host-nodes=0-3,policy=bind \ -numa node,nodeid=1,cpus=1,memdev=ram-node1 \ --object memory-backend-file,id=ram-node2,mem-path=/var/lib/libvirt/qemu/ram,\ +-object memory-backend-file,id=ram-node2,\ +mem-path=/var/lib/libvirt/qemu/ram/libvirt/qemu/-1-QEMUGuest1/ram-node2,\ share=no,size=1073741824,host-nodes=0-3,policy=bind \ -numa node,nodeid=2,cpus=2,memdev=ram-node2 \ --object memory-backend-file,id=ram-node3,mem-path=/var/lib/libvirt/qemu/ram,\ +-object memory-backend-file,id=ram-node3,\ +mem-path=/var/lib/libvirt/qemu/ram/libvirt/qemu/-1-QEMUGuest1/ram-node3,\ share=no,size=1073741824,host-nodes=3,policy=bind \ -numa node,nodeid=3,cpus=3,memdev=ram-node3 \ -object memory-backend-file,id=memdimm0,prealloc=yes,\
On 11/02/2017 12:15 AM, John Ferlan wrote:
On 10/24/2017 07:41 AM, Michal Privoznik wrote:
In some cases management application needs to allocate memory for qemu upfront and then just let qemu use that. Since we don't want to expose path for memory-backend-file anywhere in the domain XML, we can generate predictable paths. In this case:
$memoryBackingDir/libvirt/qemu/$shortName/$alias
where $shortName is result of virDomainObjGetShortName().
s/Obj/Def
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_command.c | 9 +- src/qemu/qemu_conf.c | 55 ++++++++- src/qemu/qemu_conf.h | 9 +- src/qemu/qemu_driver.c | 17 +++ src/qemu/qemu_hotplug.c | 2 +- src/qemu/qemu_process.c | 137 +++++++++++++++------ src/qemu/qemu_process.h | 8 +- .../qemuxml2argv-cpu-numa-memshared.args | 6 +- .../qemuxml2argv-fd-memory-numa-topology.args | 3 +- .../qemuxml2argv-fd-memory-numa-topology2.args | 6 +- .../qemuxml2argv-fd-memory-numa-topology3.args | 9 +- .../qemuxml2argv-hugepages-memaccess2.args | 9 +- 12 files changed, 207 insertions(+), 63 deletions(-)
There's parts of this that would seemingly be able to be separated out so that it's easier to review...
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 82d2fb2a3..d6d2654cd 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3439,7 +3439,7 @@ qemuBuildMemoryBackendStr(virJSONValuePtr *backendProps, } else { /* We can have both pagesize and mem source. If that's the case, * prefer hugepages as those are more specific. */ - if (qemuGetMemoryBackingPath(cfg, &memPath) < 0) + if (qemuGetMemoryBackingPath(def, cfg, mem->info.alias, &memPath) < 0) goto cleanup; }
@@ -3542,12 +3542,13 @@ qemuBuildMemoryCellBackendStr(virDomainDefPtr def, unsigned long long memsize = virDomainNumaGetNodeMemorySize(def->numa, cell);
+ if (virAsprintf(&alias, "ram-node%zu", cell) < 0) + goto cleanup; +
This is just a simple trivial move and not related...
*backendStr = NULL; mem.size = memsize; mem.targetNode = cell; - - if (virAsprintf(&alias, "ram-node%zu", cell) < 0) - goto cleanup; + mem.info.alias = alias;
Nope. It is related. So the idea I'm implementing here is that the file name we put onto qemu cmd line ends with the alias of the device. And the alias is taken from mem.info.alias. Therefore I need to set it.
if ((rc = qemuBuildMemoryBackendStr(&props, &backendType, cfg, priv->qemuCaps, def, &mem, priv->autoNodeset, false)) < 0) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 23dcc25e1..22c023255 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -1750,9 +1750,41 @@ qemuGetDomainHupageMemPath(const virDomainDef *def, }
+int +qemuGetMemoryBackingBasePath(virQEMUDriverConfigPtr cfg, + char **path) +{ + return virAsprintf(path, "%s/libvirt/qemu", cfg->memoryBackingDir);
Not of a fan of redundancy, but I'm also not sure what better way there is...
For privileged domains it's "/var/lib/libvirt/qemu/ram/libvirt/qemu/" and unpriv'd it's $XDG_CONFIG_HOME+"/qemu/ram/libvirt/qemu".
The former repeats "libvirt/qemu" while the latter only repeats "qemu".
I would think we could drop the "qemu" at the very least, but it doesn't matter that much to me.
Yeah. I hear you. But trying to hack around this would cause more problems than it would solve IMO.
Random thought - do memory file paths even work for non priv'd domains?
They should. I don't see a reason why they shouldn't.
+} + + +int +qemuGetMemoryBackingDomainPath(const virDomainDef *def, + virQEMUDriverConfigPtr cfg, + char **path) +{ + char *shortName = NULL; + char *base = NULL; + int ret = -1; + + if (!(shortName = virDomainDefGetShortName(def)) || + qemuGetMemoryBackingBasePath(cfg, &base) < 0 || + virAsprintf(path, "%s/%s", base, shortName) < 0) + goto cleanup; +
As I noted in review of patch 1... This continues the logic of creating paths that start with "-", which to me is, well, not right. Maybe others aren't offended by it.
Is there anyway we can prepend something here to "shortName" - I'd say "ram", but that's redundant with the path, but that to me is at least better than a directory name starting with "-".
This won't ever happen. This code runs when ID has already been allocated for the domain.
+ ret = 0; + cleanup: + VIR_FREE(base); + VIR_FREE(shortName); + return ret; +} + + /** * qemuGetMemoryBackingPath: + * @def: domain definition * @cfg: the driver config + * @alias: memory object alias * @memPath: constructed path * * Constructs path to memory backing dir and stores it at @memPath. @@ -1761,8 +1793,27 @@ qemuGetDomainHupageMemPath(const virDomainDef *def, * -1 otherwise (with error reported). */ int -qemuGetMemoryBackingPath(virQEMUDriverConfigPtr cfg, +qemuGetMemoryBackingPath(const virDomainDef *def, + virQEMUDriverConfigPtr cfg, + const char *alias, char **memPath) { - return VIR_STRDUP(*memPath, cfg->memoryBackingDir); + char *domainPath = NULL; + int ret = -1; + + if (!alias) { + /* This should never happen (TM) */ + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("memory device alias is not assigned")); + goto cleanup; + } + + if (qemuGetMemoryBackingDomainPath(def, cfg, &domainPath) < 0 || + virAsprintf(memPath, "%s/%s", domainPath, alias) < 0) + goto cleanup; + + ret = 0; + cleanup: + VIR_FREE(domainPath); + return ret; } diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index 9d6866816..a553e30e2 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -364,6 +364,13 @@ int qemuGetDomainHupageMemPath(const virDomainDef *def, unsigned long long pagesize, char **memPath);
-int qemuGetMemoryBackingPath(virQEMUDriverConfigPtr cfg, +int qemuGetMemoryBackingBasePath(virQEMUDriverConfigPtr cfg, + char **path); +int qemuGetMemoryBackingDomainPath(const virDomainDef *def, + virQEMUDriverConfigPtr cfg, + char **path); +int qemuGetMemoryBackingPath(const virDomainDef *def, + virQEMUDriverConfigPtr cfg, + const char *alias, char **memPath); #endif /* __QEMUD_CONF_H */ diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 135c20749..b22fb7518 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -630,6 +630,7 @@ qemuStateInitialize(bool privileged, uid_t run_uid = -1; gid_t run_gid = -1; char *hugepagePath = NULL; + char *memoryBackingPath = NULL; size_t i;
if (VIR_ALLOC(qemu_driver) < 0) @@ -888,6 +889,21 @@ qemuStateInitialize(bool privileged, VIR_FREE(hugepagePath); }
+ if (qemuGetMemoryBackingBasePath(cfg, &memoryBackingPath) < 0) + goto error; + + if (virFileMakePath(memoryBackingPath) < 0) { + virReportSystemError(errno, + _("unable to create memory backing path %s"), + memoryBackingPath); + goto error; + } + + if (privileged && + virFileUpdatePerm(memoryBackingPath, + 0, S_IXGRP | S_IXOTH) < 0) + goto error; + if (!(qemu_driver->closeCallbacks = virCloseCallbacksNew())) goto error;
@@ -945,6 +961,7 @@ qemuStateInitialize(bool privileged, virObjectUnref(conn); VIR_FREE(driverConf); VIR_FREE(hugepagePath); + VIR_FREE(memoryBackingPath); qemuStateCleanup(); return -1; } diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 91f7f9ed6..5701c033b 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -2077,7 +2077,7 @@ qemuDomainAttachMemory(virQEMUDriverPtr driver, priv->qemuCaps, vm->def, mem, NULL, true) < 0) goto cleanup;
- if (qemuProcessBuildDestroyHugepagesPath(driver, vm, mem, true) < 0) + if (qemuProcessBuildDestroyMemoryPaths(driver, vm, mem, true) < 0) goto cleanup;
if (qemuDomainNamespaceSetupMemory(driver, vm, mem) < 0) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index fdc868912..4fe973c59 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3324,60 +3324,117 @@ qemuProcessNeedHugepagesPath(virDomainDefPtr def, }
+static bool +qemuProcessNeedMemoryBackingPath(virDomainDefPtr def, + virDomainMemoryDefPtr mem) +{ + size_t i; + size_t numaNodes; + + if (def->mem.source == VIR_DOMAIN_MEMORY_SOURCE_FILE || + def->mem.access != VIR_DOMAIN_MEMORY_ACCESS_DEFAULT) + return true; + + numaNodes = virDomainNumaGetNodeCount(def->numa); + for (i = 0; i < numaNodes; i++) { + if (virDomainNumaGetNodeMemoryAccessMode(def->numa, i) + != VIR_DOMAIN_MEMORY_ACCESS_DEFAULT) + return true; + } + + if (mem && + mem->model == VIR_DOMAIN_MEMORY_MODEL_DIMM && + (mem->access != VIR_DOMAIN_MEMORY_ACCESS_DEFAULT || + (mem->targetNode >= 0 && + virDomainNumaGetNodeMemoryAccessMode(def->numa, mem->targetNode) + != VIR_DOMAIN_MEMORY_ACCESS_DEFAULT))) + return true; + + return false; +} + +
Starting here...
+static int +qemuProcessBuildDestroyMemoryPathsImpl(virQEMUDriverPtr driver, + virDomainDefPtr def, + const char *path, + bool build)
Could just be qemuProcessBuildDestroyPath as it's one of many possible calls within (at least) the hugepage loop.
+{ + if (build) { + if (virFileExists(path)) + return 0; + + if (virFileMakePathWithMode(path, 0700) < 0) { + virReportSystemError(errno, + _("Unable to create %s"), + path); + return -1; + } + + if (qemuSecurityDomainSetPathLabel(driver->securityManager, + def, path) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unable to label %s"), path); + return -1; + } + } else { + if (virFileDeleteTree(path) < 0 && + errno != ENOENT) + VIR_WARN("Unable to remove hugepage path: %s (errno=%d)",
This error message would need to change to remove "hugepage"
+ path, errno); + } + + return 0; +} + +
... this seems separable too, modulo the change to use virFileDeleteTree which then would be a follow-up patch to be accepted on its own merits... The other option being a check to add some sort of isDir type logic or flag is input considering that hugepages wouldn't need to delete the path, right? Or should they?
As Dan points out in v1 - we do need to keep the $dir/libvirt/qemu though for other domains and we wouldn't want those subdirectories to get deleted from/for Hugepages before we do so for memory backing, right? I think the way you have this we'd delete those before getting to them in the logic, but my eyes are really tired right now.
int -qemuProcessBuildDestroyHugepagesPath(virQEMUDriverPtr driver, - virDomainObjPtr vm, - virDomainMemoryDefPtr mem, - bool build) +qemuProcessBuildDestroyMemoryPaths(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virDomainMemoryDefPtr mem, + bool build)
You could seemingly make the API name change separately too, IDC, but some do generally speaking of course...
{ virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); - char *hugepagePath = NULL; + char *path = NULL; size_t i; - bool shouldBuild = false; + bool shouldBuildHP = false; + bool shouldBuildMB = false; int ret = -1;
- if (build) - shouldBuild = qemuProcessNeedHugepagesPath(vm->def, mem); + if (build) { + shouldBuildHP = qemuProcessNeedHugepagesPath(vm->def, mem); + shouldBuildMB = qemuProcessNeedMemoryBackingPath(vm->def, mem); + }
- if (!build || shouldBuild) { + if (!build || shouldBuildHP) { for (i = 0; i < cfg->nhugetlbfs; i++) { - VIR_FREE(hugepagePath); - hugepagePath = qemuGetDomainHugepagePath(vm->def, &cfg->hugetlbfs[i]); + path = qemuGetDomainHugepagePath(vm->def, &cfg->hugetlbfs[i]);
- if (!hugepagePath) + if (!path) goto cleanup;
- if (build) { - if (virFileExists(hugepagePath)) { - ret = 0; - goto cleanup; - } - - if (virFileMakePathWithMode(hugepagePath, 0700) < 0) { - virReportSystemError(errno, - _("Unable to create %s"), - hugepagePath); - goto cleanup; - } + if (qemuProcessBuildDestroyMemoryPathsImpl(driver, vm->def, + path, build) < 0) + goto cleanup;
- if (qemuSecurityDomainSetPathLabel(driver->securityManager, - vm->def, hugepagePath) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("Unable to set huge path in security driver")); - goto cleanup; - } - } else { - if (rmdir(hugepagePath) < 0 && - errno != ENOENT) - VIR_WARN("Unable to remove hugepage path: %s (errno=%d)", - hugepagePath, errno); - } + VIR_FREE(path); } }
+ if (!build || shouldBuildMB) { + if (qemuGetMemoryBackingDomainPath(vm->def, cfg, &path) < 0) + goto cleanup; + + if (qemuProcessBuildDestroyMemoryPathsImpl(driver, vm->def, + path, build) < 0) + goto cleanup; + + VIR_FREE(path); + } + ret = 0; cleanup: - VIR_FREE(hugepagePath); + VIR_FREE(path); virObjectUnref(cfg); return ret; } @@ -5550,7 +5607,7 @@ qemuProcessPrepareHost(virQEMUDriverPtr driver, NULL) < 0) goto cleanup;
- if (qemuProcessBuildDestroyHugepagesPath(driver, vm, NULL, true) < 0) + if (qemuProcessBuildDestroyMemoryPaths(driver, vm, NULL, true) < 0) goto cleanup;
/* Ensure no historical cgroup for this VM is lying around bogus @@ -6254,7 +6311,7 @@ void qemuProcessStop(virQEMUDriverPtr driver, goto endjob; }
- qemuProcessBuildDestroyHugepagesPath(driver, vm, NULL, false); + qemuProcessBuildDestroyMemoryPaths(driver, vm, NULL, false);
vm->def->id = -1;
@@ -7112,7 +7169,7 @@ qemuProcessReconnect(void *opaque) goto cleanup; }
- if (qemuProcessBuildDestroyHugepagesPath(driver, obj, NULL, true) < 0) + if (qemuProcessBuildDestroyMemoryPaths(driver, obj, NULL, true) < 0) goto error;
if ((qemuDomainAssignAddresses(obj->def, priv->qemuCaps, diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h index 814b86d8a..cd9a72031 100644 --- a/src/qemu/qemu_process.h +++ b/src/qemu/qemu_process.h @@ -38,10 +38,10 @@ int qemuProcessStopCPUs(virQEMUDriverPtr driver, virDomainPausedReason reason, qemuDomainAsyncJob asyncJob);
-int qemuProcessBuildDestroyHugepagesPath(virQEMUDriverPtr driver, - virDomainObjPtr vm, - virDomainMemoryDefPtr mem, - bool build); +int qemuProcessBuildDestroyMemoryPaths(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virDomainMemoryDefPtr mem, + bool build);
void qemuProcessAutostartAll(virQEMUDriverPtr driver); void qemuProcessReconnectAll(virConnectPtr conn, virQEMUDriverPtr driver); diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa-memshared.args b/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa-memshared.args index 5700c3413..352819429 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa-memshared.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa-memshared.args @@ -10,10 +10,12 @@ QEMU_AUDIO_DRV=none \ -M pc \ -m 214 \ -smp 16,sockets=2,cores=4,threads=2 \ --object memory-backend-file,id=ram-node0,mem-path=/var/lib/libvirt/qemu/ram,\ +-object memory-backend-file,id=ram-node0,\ +mem-path=/var/lib/libvirt/qemu/ram/libvirt/qemu/-1-QEMUGuest1/ram-node0,\
And this is just where it really obvious what's happening - although it took me a bit to figure it all out, especially in my post KVM Forum travel haze.
Yes. This is the expected result and the reason why I wrote this patch. Okay, I'll try to separate out some changes and repost. Thanks. Michal
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- docs/news.xml | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/docs/news.xml b/docs/news.xml index 989b82aa5..29eaacb9a 100644 --- a/docs/news.xml +++ b/docs/news.xml @@ -51,6 +51,17 @@ </change> </section> <section title="Improvements"> + <change> + <summary> + qemu: Generate predictable paths for qemu memory backends + </summary> + <description> + In some cases management applications need to know + paths passed to memory-backend-file objects upfront. + Libvirt now generates predictable paths so applications + can prepare the files if they need to do so. + </description> + </change> </section> <section title="Bug fixes"> <change> -- 2.13.6
On 10/24/2017 07:41 AM, Michal Privoznik wrote:
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- docs/news.xml | 11 +++++++++++ 1 file changed, 11 insertions(+)
+1 for news.xml, but you'll need to be sure to update it when you get around to pushing this. Reviewed-by: John Ferlan <jferlan@redhat.com> John
On 11/02/2017 12:15 AM, John Ferlan wrote:
On 10/24/2017 07:41 AM, Michal Privoznik wrote:
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- docs/news.xml | 11 +++++++++++ 1 file changed, 11 insertions(+)
+1 for news.xml, but you'll need to be sure to update it when you get around to pushing this.
Yup. It'll be after the release so I will need to create whole new structure for the new release. Thanks. Michal
participants (2)
-
John Ferlan -
Michal Privoznik