[PATCH 0/2] qemu: Create base hugepages path on memory hotplug

*** BLURB HERE *** Michal Prívozník (2): qemu: Separate out hugepages basedir making qemu: Create base hugepages path on memory hotplug src/qemu/qemu_conf.c | 27 +++++++++++++++++++++++++++ src/qemu/qemu_conf.h | 3 +++ src/qemu/qemu_driver.c | 17 +---------------- src/qemu/qemu_process.c | 4 ++++ 4 files changed, 35 insertions(+), 16 deletions(-) -- 2.35.1

During its initialization, the QEMU driver iterates over hugetlbfs mount points, creating the driver specific path in each of them ($prefix/libvirt/qemu). This path is created with very wide mode (0777) because per-domain directories are then created under it. Separate this code into a function so that it can be re-used. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_conf.c | 27 +++++++++++++++++++++++++++ src/qemu/qemu_conf.h | 3 +++ src/qemu/qemu_driver.c | 17 +---------------- 3 files changed, 31 insertions(+), 16 deletions(-) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 4f59e5fb07..c20fec26ba 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -1576,3 +1576,30 @@ qemuGetMemoryBackingPath(virQEMUDriver *driver, *memPath = g_strdup_printf("%s/%s", domainPath, alias); return 0; } + + +int +qemuMkdirBaseHugepage(virQEMUDriver *driver, + virHugeTLBFS *hugepage) +{ + + g_autofree char *hugepagePath = NULL; + + hugepagePath = qemuGetBaseHugepagePath(driver, hugepage); + + if (!hugepagePath) + return -1; + + if (g_mkdir_with_parents(hugepagePath, 0777) < 0) { + virReportSystemError(errno, + _("unable to create hugepage path %s"), + hugepagePath); + return -1; + } + + if (driver->privileged && + virFileUpdatePerm(hugepage->mnt_dir, 0, S_IXGRP | S_IXOTH) < 0) + return -1; + + return 0; +} diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index c40c452f58..f9314e23a3 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -359,3 +359,6 @@ int qemuGetMemoryBackingPath(virQEMUDriver *driver, const virDomainDef *def, const char *alias, char **memPath); + +int qemuMkdirBaseHugepage(virQEMUDriver *driver, + virHugeTLBFS *hugepage); diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 40d23b5723..744661f0f7 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -837,22 +837,7 @@ qemuStateInitialize(bool privileged, * it, since we can't assume the root mount point has permissions that * will let our spawned QEMU instances use it. */ for (i = 0; i < cfg->nhugetlbfs; i++) { - g_autofree char *hugepagePath = NULL; - - hugepagePath = qemuGetBaseHugepagePath(qemu_driver, &cfg->hugetlbfs[i]); - - if (!hugepagePath) - goto error; - - if (g_mkdir_with_parents(hugepagePath, 0777) < 0) { - virReportSystemError(errno, - _("unable to create hugepage path %s"), - hugepagePath); - goto error; - } - if (privileged && - virFileUpdatePerm(cfg->hugetlbfs[i].mnt_dir, - 0, S_IXGRP | S_IXOTH) < 0) + if (qemuMkdirBaseHugepage(qemu_driver, &cfg->hugetlbfs[i]) < 0) goto error; } -- 2.35.1

On Wed, Oct 12, 2022 at 12:37:58PM +0200, Michal Privoznik wrote:
During its initialization, the QEMU driver iterates over hugetlbfs mount points, creating the driver specific path in each of them ($prefix/libvirt/qemu). This path is created with very wide mode (0777) because per-domain directories are then created under it.
Separate this code into a function so that it can be re-used.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_conf.c | 27 +++++++++++++++++++++++++++ src/qemu/qemu_conf.h | 3 +++ src/qemu/qemu_driver.c | 17 +---------------- 3 files changed, 31 insertions(+), 16 deletions(-)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 4f59e5fb07..c20fec26ba 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -1576,3 +1576,30 @@ qemuGetMemoryBackingPath(virQEMUDriver *driver, *memPath = g_strdup_printf("%s/%s", domainPath, alias); return 0; } + + +int +qemuMkdirBaseHugepage(virQEMUDriver *driver, + virHugeTLBFS *hugepage)
Apart from virDomainHugepage we don't have another naming precedent Hugepage functions, hence I'd like to propose qemuHugepageMakeBasedir instead. Reviewed-by: Erik Skultety <eskultet@redhat.com>

Users can play all sorts of games with mount points. For instance, they can unmount and mount back a hugetlbfs and only after that attempt to hotplug memory. This has an unfortunate consequence though. During memory hotplug, when qemuProcessBuildDestroyMemoryPaths() is called the path is created with very restrictive mode (0700) because under the hood g_mkdir_with_parents(path, 0700) is called. Therefore, create the driver generic portion of the path separately. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2134009 Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_process.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index d78e91efed..9420201466 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4015,6 +4015,10 @@ qemuProcessBuildDestroyMemoryPaths(virQEMUDriver *driver, if (!path) return -1; + if (build && + qemuMkdirBaseHugepage(driver, &cfg->hugetlbfs[i]) < 0) + return -1; + if (qemuProcessBuildDestroyMemoryPathsImpl(driver, vm, path, build) < 0) return -1; -- 2.35.1

On Wed, Oct 12, 2022 at 12:37:59PM +0200, Michal Privoznik wrote:
Users can play all sorts of games with mount points. For instance, they can unmount and mount back a hugetlbfs and only after that attempt to hotplug memory.
This has an unfortunate consequence though. During memory hotplug, when qemuProcessBuildDestroyMemoryPaths() is called the path is created with very restrictive mode (0700) because under the hood g_mkdir_with_parents(path, 0700) is called.
Therefore, create the driver generic portion of the path separately.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2134009 Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- Reviewed-by: Erik Skultety <eskultet@redhat.com>
participants (2)
-
Erik Skultety
-
Michal Privoznik