Re: [libvirt] [PATCH v2] qemuBuildMemoryBackendStr: Don't crash if no hugetlbfs is mounted

[Peter Krempa]

On Mon, Sep 19, 2016 at 11:03:30 +0200, Michal Privoznik wrote:
> When trying to migrate a huge page enabled guest, I've noticed
> the following crash. Apparently, if no specific hugepages are
> requested:
> 
>   
> 
>   

[...]

> ---
> 
> 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_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.

'Callers must ensure that @hugetlbfs contains at least one entry.'
instead of the second sentence which doesn't really tell much.

Also add a description of the arguments since you are referring to them
in the comment.

> + *
> + * Returns 0 on success,
> + *-1 otherwise.

These will fit on a signe line.

> + * */

ACK with the above issues resolved.

Peter

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH v2] qemuBuildMemoryBackendStr: Don't crash if no hugetlbfs is mounted

[Michal Privoznik]

When trying to migrate a huge page enabled guest, I've noticed
the following crash. Apparently, if no specific hugepages are
requested:

  

  

and there are no hugepages configured on the destination, we try
to dereference a NULL pointer.

Program received signal SIGSEGV, Segmentation fault.
0x7fcc907fb20e in qemuGetHugepagePath (hugepage=0x0) at 
qemu/qemu_conf.c:1447
1447if (virAsprintf(&ret, "%s/libvirt/qemu", hugepage->mnt_dir) < 0)
(gdb) bt
#0  0x7fcc907fb20e in qemuGetHugepagePath (hugepage=0x0) at 
qemu/qemu_conf.c:1447
#1  0x7fcc907fb2f5 in qemuGetDefaultHugepath (hugetlbfs=0x0, nhugetlbfs=0) 
at qemu/qemu_conf.c:1466
#2  0x7fcc907b4afa 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  0x7fcc907b4f91 in qemuBuildMemoryCellBackendStr (def=0x7fcc70019070, 
qemuCaps=0x7fcc70004000, cfg=0x7fcc5c011800, cell=0, auto_nodeset=0x0, 
backendStr=0x7fcc70020360) at qemu/qemu_command.c:3413
#4  0x7fcc907c0406 in qemuBuildNumaArgStr (cfg=0x7fcc5c011800, 
def=0x7fcc70019070, cmd=0x7fcc700040c0, qemuCaps=0x7fcc70004000, 
auto_nodeset=0x0) at qemu/qemu_command.c:7470
#5  0x7fcc907c5fdf 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 
---

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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list