Use better detection of hugetlbfs mount points. Yes, there can be
multiple mount points each serving different huge page size.
Since we already have ability to override the mount point in the
qemu.conf file, this crazy backward compatibility code is brought in.
Now we allow multiple mount points, so the "hugetlbfs_mount" option
must take an list of strings (mount points). But previously, it was
just a string, so we must accept both types now.
Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx>
---
Notes:
Yes, the libvirtd_qemu.aug change is missing here. The reason is prosaic: I
don't know how to make augeas accept both array of string and a single string
for a configuration variable. BTW: the security_drivers which does exactly what
I'm doing here, accepts only single string in the aug file too.
src/Makefile.am | 12 ++---
src/qemu/qemu.conf | 9 +++-
src/qemu/qemu_command.c | 20 +++++---
src/qemu/qemu_conf.c | 124 ++++++++++++++++++++++++++++++++++++++++++-----
src/qemu/qemu_conf.h | 9 +++-
src/qemu/qemu_driver.c | 39 +++++++--------
src/qemu/qemu_process.c | 21 +++++---
tests/qemuxml2argvtest.c | 10 ++--
8 files changed, 183 insertions(+), 61 deletions(-)
diff --git a/src/Makefile.am b/src/Makefile.am
index 982f63d..2444d51 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -1209,10 +1209,10 @@ libvirt_driver_qemu_impl_la_CFLAGS = \
$(AM_CFLAGS)
libvirt_driver_qemu_impl_la_LDFLAGS = $(AM_LDFLAGS)
libvirt_driver_qemu_impl_la_LIBADD = $(CAPNG_LIBS) \
- $(GNUTLS_LIBS) \
- $(LIBNL_LIBS) \
- $(LIBXML_LIBS) \
- $(NULL)
+ $(GNUTLS_LIBS) \
+ $(LIBNL_LIBS) \
+ $(LIBXML_LIBS) \
+ libvirt_util.la
libvirt_driver_qemu_impl_la_SOURCES = $(QEMU_DRIVER_SOURCES)
conf_DATA += qemu/qemu.conf
@@ -1995,8 +1995,8 @@ endif WITH_DRIVER_MODULES
BUILT_SOURCES += libvirt_probes.h libvirt_probes.stp libvirt_functions.stp
if WITH_QEMU
-libvirt_driver_qemu_la_LIBADD += libvirt_qemu_probes.lo
-nodist_libvirt_driver_qemu_la_SOURCES = libvirt_qemu_probes.h
+libvirt_driver_qemu_impl_la_LIBADD += libvirt_qemu_probes.lo libvirt_probes.lo
+libvirt_driver_qemu_impl_la_SOURCES += libvirt_qemu_probes.h
BUILT_SOURCES += libvirt_qemu_probes.h
endif WITH_QEMU
diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf
index 18ce2a8..a079b93 100644
--- a/src/qemu/qemu.conf
+++ b/src/qemu/qemu.conf
@@ -330,7 +330,14 @@
# unspecified here, determination of a host mount point in /proc/mounts
# will be attempted. Specifying an explicit mount overrides detection
# of the same in /proc/mounts. Setting the mount point to "" will
-# disable guest hugepage backing.
+# disable guest hugepage backing. If desired, multiple mount points can
+# be specified at once, separated by comma and enclosed in square
+# brackets, for example:
+#
+# hugetlbfs_mount = ["/dev/hugepages2M", "/dev/hugepages1G"]
+#
+# The size of huge page served by specific mount point is determined by
+# libvirt at the daemon startup.
#
# NB, within this mount point, guests will create memory backing files
# in a location of $MOUNTPOINT/libvirt/qemu
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index fe207a4..091447a 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -7333,14 +7333,12 @@ qemuBuildCommandLine(virConnectPtr conn,
def->mem.max_balloon = VIR_DIV_UP(def->mem.max_balloon, 1024) * 1024;
virCommandAddArgFormat(cmd, "%llu", def->mem.max_balloon / 1024);
if (def->mem.hugepage_backed) {
- if (!cfg->hugetlbfsMount) {
+ char *mem_path;
+
+ if (!cfg->nhugetlbfs) {
virReportError(VIR_ERR_INTERNAL_ERROR,
- "%s", _("hugetlbfs filesystem is not mounted"));
- goto error;
- }
- if (!cfg->hugepagePath) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- "%s", _("hugepages are disabled by administrator config"));
+ "%s", _("hugetlbfs filesystem is not mounted "
+ "or disabled by administrator config"));
goto error;
}
if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_MEM_PATH)) {
@@ -7349,8 +7347,14 @@ qemuBuildCommandLine(virConnectPtr conn,
def->emulator);
goto error;
}
+
+ if (!(mem_path = qemuGetDefaultHugepath(cfg->hugetlbfs,
+ cfg->nhugetlbfs)))
+ goto error;
+
virCommandAddArgList(cmd, "-mem-prealloc", "-mem-path",
- cfg->hugepagePath, NULL);
+ mem_path, NULL);
+ VIR_FREE(mem_path);
}
if (def->mem.locked && !virQEMUCapsGet(qemuCaps, QEMU_CAPS_MLOCK)) {
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index e62bec0..bed9680 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -230,19 +230,17 @@ virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool privileged)
cfg->migrationPortMin = QEMU_MIGRATION_PORT_MIN;
cfg->migrationPortMax = QEMU_MIGRATION_PORT_MAX;
-#if defined HAVE_MNTENT_H && defined HAVE_GETMNTENT_R
- /* For privileged driver, try and find hugepage mount automatically.
+ /* For privileged driver, try and find hugetlbfs mounts automatically.
* Non-privileged driver requires admin to create a dir for the
- * user, chown it, and then let user configure it manually */
+ * user, chown it, and then let user configure it manually. */
if (privileged &&
- !(cfg->hugetlbfsMount = virFileFindMountPoint("hugetlbfs"))) {
- if (errno != ENOENT) {
- virReportSystemError(errno, "%s",
- _("unable to find hugetlbfs mountpoint"));
+ virFileFindHugeTLBFS(&cfg->hugetlbfs, &cfg->nhugetlbfs) < 0) {
+ /* This however is not implemented on all platforms. */
+ virErrorPtr err = virGetLastError();
+ if (err && err->code != VIR_ERR_NO_SUPPORT)
goto error;
- }
}
-#endif
+
if (VIR_STRDUP(cfg->bridgeHelperName, "/usr/libexec/qemu-bridge-helper") < 0)
goto error;
@@ -293,8 +291,11 @@ static void virQEMUDriverConfigDispose(void *obj)
VIR_FREE(cfg->spicePassword);
VIR_FREE(cfg->spiceSASLdir);
- VIR_FREE(cfg->hugetlbfsMount);
- VIR_FREE(cfg->hugepagePath);
+ while (cfg->nhugetlbfs) {
+ cfg->nhugetlbfs--;
+ VIR_FREE(cfg->hugetlbfs[cfg->nhugetlbfs].mnt_dir);
+ }
+ VIR_FREE(cfg->hugetlbfs);
VIR_FREE(cfg->bridgeHelperName);
VIR_FREE(cfg->saveImageFormat);
@@ -307,6 +308,26 @@ static void virQEMUDriverConfigDispose(void *obj)
}
+static int
+virQEMUDriverConfigHugeTLBFSInit(virHugeTLBFSPtr hugetlbfs,
+ const char *path,
+ bool deflt)
+{
+ int ret = -1;
+
+ if (VIR_STRDUP(hugetlbfs->mnt_dir, path) < 0)
+ goto cleanup;
+
+ if (virFileGetHugepageSize(path, &hugetlbfs->size) < 0)
+ goto cleanup;
+
+ hugetlbfs->deflt = deflt;
+ ret = 0;
+ cleanup:
+ return ret;
+}
+
+
int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
const char *filename)
{
@@ -555,7 +576,59 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
GET_VALUE_BOOL("auto_dump_bypass_cache", cfg->autoDumpBypassCache);
GET_VALUE_BOOL("auto_start_bypass_cache", cfg->autoStartBypassCache);
- GET_VALUE_STR("hugetlbfs_mount", cfg->hugetlbfsMount);
+ /* Some crazy backcompat. Back in the old days, this was just a pure
+ * string. We must continue supporting it. These days however, this may be
+ * an array of strings. */
+ p = virConfGetValue(conf, "hugetlbfs_mount");
+ if (p) {
+ /* There already might be something autodetected. Avoid leaking it. */
+ while (cfg->nhugetlbfs) {
+ cfg->nhugetlbfs--;
+ VIR_FREE(cfg->hugetlbfs[cfg->nhugetlbfs].mnt_dir);
+ }
+ VIR_FREE(cfg->hugetlbfs);
+
+ if (p->type == VIR_CONF_LIST) {
+ size_t len = 0;
+ virConfValuePtr pp = p->list;
+
+ /* Calc length and check items */
+ while (pp) {
+ if (pp->type != VIR_CONF_STRING) {
+ virReportError(VIR_ERR_CONF_SYNTAX, "%s",
+ _("hugetlbfs_mount must be a list of strings"));
+ goto cleanup;
+ }
+ len++;
+ pp = pp->next;
+ }
+
+ if (len && VIR_ALLOC_N(cfg->hugetlbfs, len) < 0)
+ goto cleanup;
+ cfg->nhugetlbfs = len;
+
+ pp = p->list;
+ len = 0;
+ while (pp) {
+ if (virQEMUDriverConfigHugeTLBFSInit(&cfg->hugetlbfs[len],
+ pp->str, !len) < 0)
+ goto cleanup;
+ len++;
+ pp = pp->next;
+ }
+ } else {
+ CHECK_TYPE("hugetlbfs_mount", VIR_CONF_STRING);
+ if (STRNEQ(p->str, "")) {
+ if (VIR_ALLOC_N(cfg->hugetlbfs, 1) < 0)
+ goto cleanup;
+ cfg->nhugetlbfs = 1;
+ if (virQEMUDriverConfigHugeTLBFSInit(&cfg->hugetlbfs[0],
+ p->str, true) < 0)
+ goto cleanup;
+ }
+ }
+ }
+
GET_VALUE_STR("bridge_helper", cfg->bridgeHelperName);
GET_VALUE_BOOL("mac_filter", cfg->macFilter);
@@ -1402,3 +1475,30 @@ qemuTranslateSnapshotDiskSourcePool(virConnectPtr conn ATTRIBUTE_UNUSED,
_("Snapshots are not yet supported with 'pool' volumes"));
return -1;
}
+
+char *
+qemuGetHugepagePath(virHugeTLBFSPtr hugepage)
+{
+ char *ret;
+
+ if (virAsprintf(&ret, "%s/libvirt/qemu", hugepage->mnt_dir) < 0)
+ return NULL;
+
+ return ret;
+}
+
+char *
+qemuGetDefaultHugepath(virHugeTLBFSPtr hugetlbfs,
+ size_t nhugetlbfs)
+{
+ size_t i;
+
+ for (i = 0; i < nhugetlbfs; i++)
+ if (hugetlbfs[i].deflt)
+ break;
+
+ if (i == nhugetlbfs)
+ i = 0;
+
+ return qemuGetHugepagePath(&hugetlbfs[i]);
+}
diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
index 78b08e5..c3c9d6c 100644
--- a/src/qemu/qemu_conf.h
+++ b/src/qemu/qemu_conf.h
@@ -45,6 +45,7 @@
# include "qemu_capabilities.h"
# include "virclosecallbacks.h"
# include "virhostdev.h"
+# include "virfile.h"
# ifdef CPU_SETSIZE /* Linux */
# define QEMUD_CPUMASK_LEN CPU_SETSIZE
@@ -126,8 +127,9 @@ struct _virQEMUDriverConfig {
int webSocketPortMin;
int webSocketPortMax;
- char *hugetlbfsMount;
- char *hugepagePath;
+ virHugeTLBFSPtr hugetlbfs;
+ size_t nhugetlbfs;
+
char *bridgeHelperName;
bool macFilter;
@@ -311,4 +313,7 @@ int qemuTranslateDiskSourcePool(virConnectPtr conn,
int qemuTranslateSnapshotDiskSourcePool(virConnectPtr conn,
virDomainSnapshotDiskDefPtr def);
+char * qemuGetHugepagePath(virHugeTLBFSPtr hugepage);
+char * qemuGetDefaultHugepath(virHugeTLBFSPtr hugetlbfs,
+ size_t nhugetlbfs);
#endif /* __QEMUD_CONF_H */
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index b85d909..87f2e5a 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -577,11 +577,11 @@ qemuStateInitialize(bool privileged,
char *driverConf = NULL;
virConnectPtr conn = NULL;
char ebuf[1024];
- char *membase = NULL;
- char *mempath = NULL;
virQEMUDriverConfigPtr cfg;
uid_t run_uid = -1;
gid_t run_gid = -1;
+ char *hugepagePath = NULL;
+ size_t i;
if (VIR_ALLOC(qemu_driver) < 0)
return -1;
@@ -753,37 +753,33 @@ qemuStateInitialize(bool privileged,
/* If hugetlbfs is present, then we need to create a sub-directory within
* it, since we can't assume the root mount point has permissions that
- * will let our spawned QEMU instances use it.
- *
- * NB the check for '/', since user may config "" to disable hugepages
- * even when mounted
- */
- if (cfg->hugetlbfsMount &&
- cfg->hugetlbfsMount[0] == '/') {
- if (virAsprintf(&membase, "%s/libvirt",
- cfg->hugetlbfsMount) < 0 ||
- virAsprintf(&mempath, "%s/qemu", membase) < 0)
+ * will let our spawned QEMU instances use it. */
+ for (i = 0; i < cfg->nhugetlbfs; i++) {
+ hugepagePath = qemuGetHugepagePath(&cfg->hugetlbfs[i]);
+
+ if (!hugepagePath)
goto error;
- if (virFileMakePath(mempath) < 0) {
+ if (virFileMakePathWithMode(hugepagePath, 1777) < 0) {
virReportSystemError(errno,
- _("unable to create hugepage path %s"), mempath);
+ _("unable to create hugepage path %s"),
+ hugepagePath);
goto error;
}
if (cfg->privileged) {
- if (virFileUpdatePerm(membase, 0, S_IXGRP | S_IXOTH) < 0)
+ if (virFileUpdatePerm(cfg->hugetlbfs[i].mnt_dir,
+ 0, S_IXGRP | S_IXOTH) < 0)
goto error;
- if (chown(mempath, cfg->user, cfg->group) < 0) {
+ if (chown(hugepagePath, cfg->user, cfg->group) < 0) {
virReportSystemError(errno,
_("unable to set ownership on %s to %d:%d"),
- mempath, (int) cfg->user,
+ hugepagePath,
+ (int) cfg->user,
(int) cfg->group);
goto error;
}
}
- VIR_FREE(membase);
-
- cfg->hugepagePath = mempath;
+ VIR_FREE(hugepagePath);
}
if (!(qemu_driver->closeCallbacks = virCloseCallbacksNew()))
@@ -844,8 +840,7 @@ qemuStateInitialize(bool privileged,
error:
virObjectUnref(conn);
VIR_FREE(driverConf);
- VIR_FREE(membase);
- VIR_FREE(mempath);
+ VIR_FREE(hugepagePath);
qemuStateCleanup();
return -1;
}
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 3d10732..7626cef 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3791,12 +3791,21 @@ int qemuProcessStart(virConnectPtr conn,
}
virDomainAuditSecurityLabel(vm, true);
- if (cfg->hugepagePath && vm->def->mem.hugepage_backed) {
- if (virSecurityManagerSetHugepages(driver->securityManager,
- vm->def, cfg->hugepagePath) < 0) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- "%s", _("Unable to set huge path in security driver"));
- goto cleanup;
+ if (vm->def->mem.hugepage_backed) {
+ for (i = 0; i < cfg->nhugetlbfs; i++) {
+ char *hugepagePath = qemuGetHugepagePath(&cfg->hugetlbfs[i]);
+
+ if (!hugepagePath)
+ goto cleanup;
+
+ if (virSecurityManagerSetHugepages(driver->securityManager,
+ vm->def, hugepagePath) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ "%s", _("Unable to set huge path in security driver"));
+ VIR_FREE(hugepagePath);
+ goto cleanup;
+ }
+ VIR_FREE(hugepagePath);
}
}
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 28436f2..1a5a4b0 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -524,12 +524,14 @@ mymain(void)
VIR_FREE(driver.config->stateDir);
if (VIR_STRDUP_QUIET(driver.config->stateDir, "/nowhere") < 0)
return EXIT_FAILURE;
- VIR_FREE(driver.config->hugetlbfsMount);
- if (VIR_STRDUP_QUIET(driver.config->hugetlbfsMount, "/dev/hugepages") < 0)
+ VIR_FREE(driver.config->hugetlbfs);
+ if (VIR_ALLOC_N(driver.config->hugetlbfs, 1) < 0)
return EXIT_FAILURE;
- VIR_FREE(driver.config->hugepagePath);
- if (VIR_STRDUP_QUIET(driver.config->hugepagePath, "/dev/hugepages/libvirt/qemu") < 0)
+ driver.config->nhugetlbfs = 1;
+ if (VIR_STRDUP(driver.config->hugetlbfs[0].mnt_dir, "/dev/hugepages") < 0)
return EXIT_FAILURE;
+ driver.config->hugetlbfs[0].size = 2048;
+ driver.config->hugetlbfs[0].deflt = true;
driver.config->spiceTLS = 1;
if (VIR_STRDUP_QUIET(driver.config->spicePassword, "123456") < 0)
return EXIT_FAILURE;
--
1.8.5.5
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list