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> --- src/Makefile.am | 1 + src/qemu/qemu_command.c | 20 ++++---- src/qemu/qemu_conf.c | 122 +++++++++++++++++++++++++++++++++++++++++------ src/qemu/qemu_conf.h | 9 +++- src/qemu/qemu_driver.c | 39 +++++++-------- src/qemu/qemu_process.c | 21 +++++--- tests/qemuxml2argvtest.c | 10 ++-- 7 files changed, 166 insertions(+), 56 deletions(-) diff --git a/src/Makefile.am b/src/Makefile.am index 982f63d..85e61c2 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -1212,6 +1212,7 @@ libvirt_driver_qemu_impl_la_LIBADD = $(CAPNG_LIBS) \ $(GNUTLS_LIBS) \ $(LIBNL_LIBS) \ $(LIBXML_LIBS) \ + libvirt_util.la \ $(NULL) libvirt_driver_qemu_impl_la_SOURCES = $(QEMU_DRIVER_SOURCES) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 8062510..b14ce83 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..cf5ce97 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -230,19 +230,13 @@ 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")); - goto error; - } - } -#endif + virFileFindHugeTLBFS(&cfg->hugetlbfs, &cfg->nhugetlbfs) < 0) + goto error; + if (VIR_STRDUP(cfg->bridgeHelperName, "/usr/libexec/qemu-bridge-helper") < 0) goto error; @@ -293,8 +287,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 +304,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 +572,57 @@ 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 && 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) { + if (VIR_REALLOC_N(cfg->hugetlbfs, len) < 0) + goto cleanup; + } else { + VIR_FREE(cfg->hugetlbfs); + } + 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 (p && p->str) { + if (VIR_REALLOC_N(cfg->hugetlbfs, 1) < 0) + goto cleanup; + cfg->nhugetlbfs = 1; + if (virQEMUDriverConfigHugeTLBFSInit(&cfg->hugetlbfs[0], + p->str, true) < 0) + goto cleanup; + } else { + VIR_FREE(cfg->hugetlbfs); + cfg->nhugetlbfs = 0; + } + } + GET_VALUE_STR("bridge_helper", cfg->bridgeHelperName); GET_VALUE_BOOL("mac_filter", cfg->macFilter); @@ -1402,3 +1469,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 33541d3..90be700 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 8a6b384..16d03d8 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