When trying to migrate a huge page enabled guest, I've noticed the following crash. Apparently, if no specific hugepages are requested: <memoryBacking> <hugepages/> </memoryBacking> and there are no hugepages configured on the destination, we try to dereference a NULL pointer. Program received signal SIGSEGV, Segmentation fault. 0x00007fcc907fb20e in qemuGetHugepagePath (hugepage=0x0) at qemu/qemu_conf.c:1447 1447 if (virAsprintf(&ret, "%s/libvirt/qemu", hugepage->mnt_dir) < 0) (gdb) bt #0 0x00007fcc907fb20e in qemuGetHugepagePath (hugepage=0x0) at qemu/qemu_conf.c:1447 #1 0x00007fcc907fb2f5 in qemuGetDefaultHugepath (hugetlbfs=0x0, nhugetlbfs=0) at qemu/qemu_conf.c:1466 #2 0x00007fcc907b4afa in qemuBuildMemoryBackendStr (size=4194304, pagesize=0, guestNode=0, userNodeset=0x0, autoNodeset=0x0, def=0x7fcc70019070, qemuCaps=0x7fcc70004000, cfg=0x7fcc5c011800, backendType=0x7fcc95087228, backendProps=0x7fcc95087218, force=false) at qemu/qemu_command.c:3297 #3 0x00007fcc907b4f91 in qemuBuildMemoryCellBackendStr (def=0x7fcc70019070, qemuCaps=0x7fcc70004000, cfg=0x7fcc5c011800, cell=0, auto_nodeset=0x0, backendStr=0x7fcc70020360) at qemu/qemu_command.c:3413 #4 0x00007fcc907c0406 in qemuBuildNumaArgStr (cfg=0x7fcc5c011800, def=0x7fcc70019070, cmd=0x7fcc700040c0, qemuCaps=0x7fcc70004000, auto_nodeset=0x0) at qemu/qemu_command.c:7470 #5 0x00007fcc907c5fdf in qemuBuildCommandLine (driver=0x7fcc5c07b8a0, logManager=0x7fcc70003c00, def=0x7fcc70019070, monitor_chr=0x7fcc70004bb0, monitor_json=true, qemuCaps=0x7fcc70004000, migrateURI=0x7fcc700199c0 "defer", snapshot=0x0, vmop=VIR_NETDEV_VPORT_PROFILE_OP_MIGRATE_IN_START, standalone=false, enableFips=false, nodeset=0x0, nnicindexes=0x7fcc95087498, nicindexes=0x7fcc950874a0, domainLibDir=0x7fcc700047c0 "/var/lib/libvirt/qemu/domain-1-fedora") at qemu/qemu_command.c:9547 #6 0x00007fcc90807d37 in qemuProcessLaunch (conn=0x7fcc54000b10, driver=0x7fcc5c07b8a0, vm=0x7fcc70005320, asyncJob=QEMU_ASYNC_JOB_MIGRATION_IN, incoming=0x7fcc70015d70, snapshot=0x0, vmop=VIR_NETDEV_VPORT_PROFILE_OP_MIGRATE_IN_START, flags=4) at qemu/qemu_process.c:5238 #7 0x00007fcc90815233 in qemuMigrationPrepareAny (driver=0x7fcc5c07b8a0, dconn=0x7fcc54000b10, cookiein=0x7fcc70001ec0 "<qemu-migration>\n <name>fedora</name>\n <uuid>63840878-0deb-4095-97e6-fc444d9bc9fa</uuid>\n <hostname>lisa</hostname>\n <hostuuid>da1c2801-52f3-11cb-a9db-e905fe22010c</hostuuid>\n <feature name='lock"..., cookieinlen=328, cookieout=0x7fcc95087aa0, cookieoutlen=0x7fcc95087a90, def=0x7fcc950878f0, origname=0x0, st=0x0, protocol=0x7fcc908ab42f "tcp", port=49152, autoPort=true, listenAddress=0x0, nmigrate_disks=0, migrate_disks=0x0, nbdPort=0, compression=0x7fcc70000a30, flags=64) at qemu/qemu_migration.c:3752 #8 0x00007fcc908161ae in qemuMigrationPrepareDirect (driver=0x7fcc5c07b8a0, dconn=0x7fcc54000b10, cookiein=0x7fcc70001ec0 "<qemu-migration>\n <name>fedora</name>\n <uuid>63840878-0deb-4095-97e6-fc444d9bc9fa</uuid>\n <hostname>lisa</hostname>\n <hostuuid>da1c2801-52f3-11cb-a9db-e905fe22010c</hostuuid>\n <feature name='lock"..., cookieinlen=328, cookieout=0x7fcc95087aa0, cookieoutlen=0x7fcc95087a90, uri_in=0x0, uri_out=0x7fcc700008c0, def=0x7fcc950878f0, origname=0x0, listenAddress=0x0, nmigrate_disks=0, migrate_disks=0x0, nbdPort=0, compression=0x7fcc70000a30, flags=64) at qemu/qemu_migration.c:4100 #9 0x00007fcc908673f3 in qemuDomainMigratePrepare3Params (dconn=0x7fcc54000b10, params=0x7fcc70000930, nparams=1, cookiein=0x7fcc70001ec0 "<qemu-migration>\n <name>fedora</name>\n <uuid>63840878-0deb-4095-97e6-fc444d9bc9fa</uuid>\n <hostname>lisa</hostname>\n <hostuuid>da1c2801-52f3-11cb-a9db-e905fe22010c</hostuuid>\n <feature name='lock"..., cookieinlen=328, cookieout=0x7fcc95087aa0, cookieoutlen=0x7fcc95087a90, uri_out=0x7fcc700008c0, flags=64) at qemu/qemu_driver.c:12204 #10 0x00007fcc9d5499da in virDomainMigratePrepare3Params (dconn=0x7fcc54000b10, params=0x7fcc70000930, nparams=1, cookiein=0x7fcc70001ec0 "<qemu-migration>\n <name>fedora</name>\n <uuid>63840878-0deb-4095-97e6-fc444d9bc9fa</uuid>\n <hostname>lisa</hostname>\n <hostuuid>da1c2801-52f3-11cb-a9db-e905fe22010c</hostuuid>\n <feature name='lock"..., cookieinlen=328, cookieout=0x7fcc95087aa0, cookieoutlen=0x7fcc95087a90, uri_out=0x7fcc700008c0, flags=64) at libvirt-domain.c:5141 #11 0x000055c71ce1c9f5 in remoteDispatchDomainMigratePrepare3Params (server=0x55c71f0a0cc0, client=0x55c71f0bef20, msg=0x55c71f0ae460, rerr=0x7fcc95087b80, args=0x7fcc70000900, ret=0x7fcc70000af0) at remote.c:5117 #12 0x000055c71cdfbd9e in remoteDispatchDomainMigratePrepare3ParamsHelper (server=0x55c71f0a0cc0, client=0x55c71f0bef20, msg=0x55c71f0ae460, rerr=0x7fcc95087b80, args=0x7fcc70000900, ret=0x7fcc70000af0) at remote_dispatch.h:7820 #13 0x00007fcc9d5cd810 in virNetServerProgramDispatchCall (prog=0x55c71f0ad070, server=0x55c71f0a0cc0, client=0x55c71f0bef20, msg=0x55c71f0ae460) at rpc/virnetserverprogram.c:437 #14 0x00007fcc9d5cd371 in virNetServerProgramDispatch (prog=0x55c71f0ad070, server=0x55c71f0a0cc0, client=0x55c71f0bef20, msg=0x55c71f0ae460) at rpc/virnetserverprogram.c:307 #15 0x000055c71ce35903 in virNetServerProcessMsg (srv=0x55c71f0a0cc0, client=0x55c71f0bef20, prog=0x55c71f0ad070, msg=0x55c71f0ae460) at rpc/virnetserver.c:148 #16 0x000055c71ce359c3 in virNetServerHandleJob (jobOpaque=0x55c71f0aef30, opaque=0x55c71f0a0cc0) at rpc/virnetserver.c:169 #17 0x00007fcc9d47a6f3 in virThreadPoolWorker (opaque=0x55c71f0ae540) at util/virthreadpool.c:167 #18 0x00007fcc9d479c95 in virThreadHelper (data=0x55c71f0ae560) at util/virthread.c:206 #19 0x00007fcc9aabd494 in start_thread () from /lib64/libpthread.so.0 #20 0x00007fcc9a8025dd in clone () from /lib64/libc.so.6 Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- src/qemu/qemu_command.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 038c38f..0bafc3f 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3294,6 +3294,12 @@ qemuBuildMemoryBackendStr(unsigned long long size, if (!(mem_path = qemuGetHugepagePath(&cfg->hugetlbfs[i]))) goto cleanup; } else { + if (!cfg->nhugetlbfs) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("hugetlbfs filesystem is not mounted " + "or disabled by administrator config")); + goto cleanup; + } if (!(mem_path = qemuGetDefaultHugepath(cfg->hugetlbfs, cfg->nhugetlbfs))) goto cleanup; -- 2.8.4

On 19.09.2016 08:32, Peter Krempa wrote: Okay, That i = 0 assignment does not state array size, it just picks the first element in the array if no explicitly set default huge page size is found. Well, after my 2nd patch, there's no other caller than a fixed one. And I think qemuGetDefaultHugepath is really designed to be just a simple function to fetch caller the path to huge pages. But okay, I can discard 2/2 and basically move out its internals to various other functions (like qemuGetDefaultHugepath). Michal

Now that we have two same implementations for getting path for huge pages backed guest memory, lets merge them into one function. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- src/qemu/qemu_command.c | 63 ++++--------------------------------------------- src/qemu/qemu_conf.c | 50 +++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_conf.h | 4 ++++ 3 files changed, 59 insertions(+), 58 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 0bafc3f..051a0bc 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3275,35 +3275,9 @@ qemuBuildMemoryBackendStr(unsigned long long size, if (!(props = virJSONValueNewObject())) return -1; - if (pagesize || hugepage) { - if (pagesize) { - /* Now lets see, if the huge page we want to use is even mounted - * and ready to use */ - for (i = 0; i < cfg->nhugetlbfs; i++) { - if (cfg->hugetlbfs[i].size == pagesize) - break; - } - - if (i == cfg->nhugetlbfs) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Unable to find any usable hugetlbfs mount for %llu KiB"), - pagesize); - goto cleanup; - } - - if (!(mem_path = qemuGetHugepagePath(&cfg->hugetlbfs[i]))) - goto cleanup; - } else { - if (!cfg->nhugetlbfs) { - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("hugetlbfs filesystem is not mounted " - "or disabled by administrator config")); - goto cleanup; - } - if (!(mem_path = qemuGetDefaultHugepath(cfg->hugetlbfs, - cfg->nhugetlbfs))) - goto cleanup; - } + if (pagesize) { + if (qemuGetHupageMemPath(cfg, pagesize, &mem_path) < 0) + goto cleanup; *backendType = "memory-backend-file"; @@ -7272,7 +7246,6 @@ qemuBuildMemPathStr(virQEMUDriverConfigPtr cfg, { const long system_page_size = virGetSystemPageSizeKB(); char *mem_path = NULL; - size_t i = 0; /* * No-op if hugepages were not requested. @@ -7287,13 +7260,6 @@ qemuBuildMemPathStr(virQEMUDriverConfigPtr cfg, if (def->mem.hugepages[0].size == system_page_size) return 0; - if (!cfg->nhugetlbfs) { - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("hugetlbfs filesystem is not mounted " - "or disabled by administrator config")); - return -1; - } - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_MEM_PATH)) { virReportError(VIR_ERR_INTERNAL_ERROR, _("hugepage backing not supported by '%s'"), @@ -7301,27 +7267,8 @@ qemuBuildMemPathStr(virQEMUDriverConfigPtr cfg, return -1; } - if (!def->mem.hugepages[0].size) { - if (!(mem_path = qemuGetDefaultHugepath(cfg->hugetlbfs, - cfg->nhugetlbfs))) - return -1; - } else { - for (i = 0; i < cfg->nhugetlbfs; i++) { - if (cfg->hugetlbfs[i].size == def->mem.hugepages[0].size) - break; - } - - if (i == cfg->nhugetlbfs) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Unable to find any usable hugetlbfs " - "mount for %llu KiB"), - def->mem.hugepages[0].size); - return -1; - } - - if (!(mem_path = qemuGetHugepagePath(&cfg->hugetlbfs[i]))) - return -1; - } + if (qemuGetHupageMemPath(cfg, def->mem.hugepages[0].size, &mem_path) < 0) + return -1; virCommandAddArgList(cmd, "-mem-prealloc", "-mem-path", mem_path, NULL); VIR_FREE(mem_path); diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index dad8334..5a97e96 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -1465,3 +1465,53 @@ qemuGetDefaultHugepath(virHugeTLBFSPtr hugetlbfs, return qemuGetHugepagePath(&hugetlbfs[i]); } + + +/** + * qemuGetHupageMemPath: Construct HP enabled memory backend path + * + * If no specific hugepage size is requested (@pagesize is zero) + * the default hugepage size is used). + * The resulting path is stored at @memPath. + * + * Returns 0 on success, + * -1 otherwise. + */ +int +qemuGetHupageMemPath(virQEMUDriverConfigPtr cfg, + unsigned long long pagesize, + char **memPath) +{ + size_t i = 0; + + if (!cfg->nhugetlbfs) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("hugetlbfs filesystem is not mounted " + "or disabled by administrator config")); + return -1; + } + + if (!pagesize) { + if (!(*memPath = qemuGetDefaultHugepath(cfg->hugetlbfs, + cfg->nhugetlbfs))) + return -1; + } else { + for (i = 0; i < cfg->nhugetlbfs; i++) { + if (cfg->hugetlbfs[i].size == pagesize) + break; + } + + if (i == cfg->nhugetlbfs) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unable to find any usable hugetlbfs " + "mount for %llu KiB"), + pagesize); + return -1; + } + + if (!(*memPath = qemuGetHugepagePath(&cfg->hugetlbfs[i]))) + return -1; + } + + return 0; +} diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index d8232cc..d32689a 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -335,4 +335,8 @@ int qemuTranslateSnapshotDiskSourcePool(virConnectPtr conn, char * qemuGetHugepagePath(virHugeTLBFSPtr hugepage); char * qemuGetDefaultHugepath(virHugeTLBFSPtr hugetlbfs, size_t nhugetlbfs); + +int qemuGetHupageMemPath(virQEMUDriverConfigPtr cfg, + unsigned long long pagesize, + char **memPath); #endif /* __QEMUD_CONF_H */ -- 2.8.4

When trying to migrate a huge page enabled guest, I've noticed the following crash. Apparently, if no specific hugepages are requested: <memoryBacking> <hugepages/> </memoryBacking> and there are no hugepages configured on the destination, we try to dereference a NULL pointer. Program received signal SIGSEGV, Segmentation fault. 0x00007fcc907fb20e in qemuGetHugepagePath (hugepage=0x0) at qemu/qemu_conf.c:1447 1447 if (virAsprintf(&ret, "%s/libvirt/qemu", hugepage->mnt_dir) < 0) (gdb) bt #0 0x00007fcc907fb20e in qemuGetHugepagePath (hugepage=0x0) at qemu/qemu_conf.c:1447 #1 0x00007fcc907fb2f5 in qemuGetDefaultHugepath (hugetlbfs=0x0, nhugetlbfs=0) at qemu/qemu_conf.c:1466 #2 0x00007fcc907b4afa in qemuBuildMemoryBackendStr (size=4194304, pagesize=0, guestNode=0, userNodeset=0x0, autoNodeset=0x0, def=0x7fcc70019070, qemuCaps=0x7fcc70004000, cfg=0x7fcc5c011800, backendType=0x7fcc95087228, backendProps=0x7fcc95087218, force=false) at qemu/qemu_command.c:3297 #3 0x00007fcc907b4f91 in qemuBuildMemoryCellBackendStr (def=0x7fcc70019070, qemuCaps=0x7fcc70004000, cfg=0x7fcc5c011800, cell=0, auto_nodeset=0x0, backendStr=0x7fcc70020360) at qemu/qemu_command.c:3413 #4 0x00007fcc907c0406 in qemuBuildNumaArgStr (cfg=0x7fcc5c011800, def=0x7fcc70019070, cmd=0x7fcc700040c0, qemuCaps=0x7fcc70004000, auto_nodeset=0x0) at qemu/qemu_command.c:7470 #5 0x00007fcc907c5fdf in qemuBuildCommandLine (driver=0x7fcc5c07b8a0, logManager=0x7fcc70003c00, def=0x7fcc70019070, monitor_chr=0x7fcc70004bb0, monitor_json=true, qemuCaps=0x7fcc70004000, migrateURI=0x7fcc700199c0 "defer", snapshot=0x0, vmop=VIR_NETDEV_VPORT_PROFILE_OP_MIGRATE_IN_START, standalone=false, enableFips=false, nodeset=0x0, nnicindexes=0x7fcc95087498, nicindexes=0x7fcc950874a0, domainLibDir=0x7fcc700047c0 "/var/lib/libvirt/qemu/domain-1-fedora") at qemu/qemu_command.c:9547 Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- diff to v1: - added a comment to qemuGetDefaultHugepath() as requested in the review src/qemu/qemu_command.c | 6 ++++++ src/qemu/qemu_conf.c | 10 ++++++++++ 2 files changed, 16 insertions(+) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 038c38f..0bafc3f 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3294,6 +3294,12 @@ qemuBuildMemoryBackendStr(unsigned long long size, if (!(mem_path = qemuGetHugepagePath(&cfg->hugetlbfs[i]))) goto cleanup; } else { + if (!cfg->nhugetlbfs) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("hugetlbfs filesystem is not mounted " + "or disabled by administrator config")); + goto cleanup; + } if (!(mem_path = qemuGetDefaultHugepath(cfg->hugetlbfs, cfg->nhugetlbfs))) goto cleanup; diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index dad8334..901ad23 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -1450,6 +1450,16 @@ qemuGetHugepagePath(virHugeTLBFSPtr hugepage) return ret; } + +/** + * qemuGetDefaultHugepath: + * + * Get default hugepages path for memory backend. Does not work + * well with @nhugetlbfs == 0 or @hugetlbfs == NULL. + * + * Returns 0 on success, + * -1 otherwise. + * */ char * qemuGetDefaultHugepath(virHugeTLBFSPtr hugetlbfs, size_t nhugetlbfs) -- 2.8.4