[libvirt] [PATCH V2 0/2] libxl: user-specified domain config improvements
V2 of a small series to improve libxl driver's support for user-specified config https://www.redhat.com/archives/libvir-list/2014-September/msg01243.html Patches 1-4 of that series have been pushed. In patch 5, Michal suggested checking that the user-specified emulator is executable. Patch 1 of this series is a respin of patch 5 that includes Michal's suggestion. Patch 2 fixes an issue found while testing. Jim Fehlig (2): libxl: Support user-specified <emulator> libxl: fix double-free of libxl_domain_build_info src/libxl/libxl_conf.c | 48 +++++++++++++++++++++++++++++++++--------------- 1 file changed, 33 insertions(+), 15 deletions(-) -- 1.8.4.5
With the introduction of the libxlDomainGetEmulatorType function, it is trivial to support a user-specfied <emulator> in the libxl driver. This patch is based loosely on David Scott's old patch to do the same https://www.redhat.com/archives/libvir-list/2013-April/msg02119.html Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- V2: Check if user-specified emulator is executable. src/libxl/libxl_conf.c | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 09211f8..715895c 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -705,6 +705,28 @@ libxlMakeDomBuildInfo(virDomainDefPtr def, if (VIR_STRDUP(b_info->u.hvm.boot, bootorder) < 0) goto error; + if (def->emulator) { + if (!virFileExists(def->emulator)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("emulator '%s' not found"), + def->emulator); + goto error; + } + + if (!virFileIsExecutable(def->emulator)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("emulator '%s' is not executable"), + def->emulator); + goto error; + } + + VIR_FREE(b_info->device_model); + if (VIR_STRDUP(b_info->device_model, def->emulator) < 0) + goto error; + + b_info->device_model_version = libxlDomainGetEmulatorType(def); + } + if (def->nserials) { if (def->nserials > 1) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, -- 1.8.4.5
On error, libxlMakeDomBuildInfo() frees the caller-provided libxl_domain_build_info struct embedded in libxl_domain_config, causing a segfault Program received signal SIGSEGV, Segmentation fault. [Switching to Thread 0x7f9c13020700 (LWP 40988)] (gdb) bt 0 0x00007f9c162f95b4 in free () from /lib64/libc.so.6 1 0x00007f9c0d0965ad in libxl_bitmap_dispose () from /usr/lib64/libxenlight.so.4.4 2 0x00007f9c0d0a73bf in libxl_domain_build_info_dispose () from /usr/lib64/libxenlight.so.4.4 3 0x00007f9c0d0a7974 in libxl_domain_config_dispose () from /usr/lib64/libxenlight.so.4.4 4 0x00007f9c0d2e00c5 in libxlDomainStart (driver=0x7f9c0400e4e0, vm=0x7f9c0412b0d0, start_paused=false, restore_fd=-1) at libxl/libxl_domain.c:1323 5 0x00007f9c0d2e1d4b in libxlDomainCreateXML (conn=0x7f9c000009a0,...) at libxl/libxl_driver.c:660 Remove the call to libxl_domain_build_info_dispose() from libxlMakeDomBuildInfo(). On error, callers will dispose the libxl_domain_config object, which in turn disposes the build info. Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- This a new patch from V1, fixing an issue found while testing V2 of "libxl: Support user-specified <emulator>". src/libxl/libxl_conf.c | 32 ++++++++++++++------------------ 1 file changed, 14 insertions(+), 18 deletions(-) diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 715895c..e296ffc 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -640,7 +640,7 @@ libxlMakeDomBuildInfo(virDomainDefPtr def, b_info->max_vcpus = def->maxvcpus; if (libxl_cpu_bitmap_alloc(ctx, &b_info->avail_vcpus, def->maxvcpus)) - goto error; + return -1; libxl_bitmap_set_none(&b_info->avail_vcpus); for (i = 0; i < def->vcpus; i++) libxl_bitmap_set((&b_info->avail_vcpus), i); @@ -703,26 +703,26 @@ libxlMakeDomBuildInfo(virDomainDefPtr def, bootorder[def->os.nBootDevs] = '\0'; } if (VIR_STRDUP(b_info->u.hvm.boot, bootorder) < 0) - goto error; + return -1; if (def->emulator) { if (!virFileExists(def->emulator)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("emulator '%s' not found"), def->emulator); - goto error; + return -1; } if (!virFileIsExecutable(def->emulator)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("emulator '%s' is not executable"), def->emulator); - goto error; + return -1; } VIR_FREE(b_info->device_model); if (VIR_STRDUP(b_info->device_model, def->emulator) < 0) - goto error; + return -1; b_info->device_model_version = libxlDomainGetEmulatorType(def); } @@ -732,17 +732,17 @@ libxlMakeDomBuildInfo(virDomainDefPtr def, virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Only one serial device is supported by libxl")); - goto error; + return -1; } if (libxlMakeChrdevStr(def->serials[0], &b_info->u.hvm.serial) < 0) - goto error; + return -1; } if (def->nparallels) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Parallel devices are not supported by libxl")); - goto error; + return -1; } /* @@ -761,33 +761,29 @@ libxlMakeDomBuildInfo(virDomainDefPtr def, */ if (def->os.bootloader) { if (VIR_STRDUP(b_info->u.pv.bootloader, def->os.bootloader) < 0) - goto error; + return -1; } else if (def->os.kernel == NULL) { if (VIR_STRDUP(b_info->u.pv.bootloader, LIBXL_BOOTLOADER_PATH) < 0) - goto error; + return -1; } if (def->os.bootloaderArgs) { if (!(b_info->u.pv.bootloader_args = virStringSplit(def->os.bootloaderArgs, " \t\n", 0))) - goto error; + return -1; } if (VIR_STRDUP(b_info->u.pv.cmdline, def->os.cmdline) < 0) - goto error; + return -1; if (def->os.kernel) { /* libxl_init_build_info() sets VIR_STRDUP(kernel.path, "hvmloader") */ VIR_FREE(b_info->u.pv.kernel); if (VIR_STRDUP(b_info->u.pv.kernel, def->os.kernel) < 0) - goto error; + return -1; } if (VIR_STRDUP(b_info->u.pv.ramdisk, def->os.initrd) < 0) - goto error; + return -1; } return 0; - - error: - libxl_domain_build_info_dispose(b_info); - return -1; } static int -- 1.8.4.5
On 11.10.2014 00:03, Jim Fehlig wrote:
V2 of a small series to improve libxl driver's support for user-specified config
https://www.redhat.com/archives/libvir-list/2014-September/msg01243.html
Patches 1-4 of that series have been pushed. In patch 5, Michal suggested checking that the user-specified emulator is executable. Patch 1 of this series is a respin of patch 5 that includes Michal's suggestion. Patch 2 fixes an issue found while testing.
Jim Fehlig (2): libxl: Support user-specified <emulator> libxl: fix double-free of libxl_domain_build_info
src/libxl/libxl_conf.c | 48 +++++++++++++++++++++++++++++++++--------------- 1 file changed, 33 insertions(+), 15 deletions(-)
ACK Michal
Michal Privoznik wrote:
On 11.10.2014 00:03, Jim Fehlig wrote:
V2 of a small series to improve libxl driver's support for user-specified config
https://www.redhat.com/archives/libvir-list/2014-September/msg01243.html
Patches 1-4 of that series have been pushed. In patch 5, Michal suggested checking that the user-specified emulator is executable. Patch 1 of this series is a respin of patch 5 that includes Michal's suggestion. Patch 2 fixes an issue found while testing.
Jim Fehlig (2): libxl: Support user-specified <emulator> libxl: fix double-free of libxl_domain_build_info
src/libxl/libxl_conf.c | 48 +++++++++++++++++++++++++++++++++--------------- 1 file changed, 33 insertions(+), 15 deletions(-)
ACK
Is it ok to push these during 1.2.10 freeze? 2/2 is a bug fix... Apologies for not promptly pushing when ACK'ed, prior to the freeze. Regards, Jim
Jim Fehlig wrote:
Michal Privoznik wrote:
On 11.10.2014 00:03, Jim Fehlig wrote:
V2 of a small series to improve libxl driver's support for user-specified config
https://www.redhat.com/archives/libvir-list/2014-September/msg01243.html
Patches 1-4 of that series have been pushed. In patch 5, Michal suggested checking that the user-specified emulator is executable. Patch 1 of this series is a respin of patch 5 that includes Michal's suggestion. Patch 2 fixes an issue found while testing.
Jim Fehlig (2): libxl: Support user-specified <emulator> libxl: fix double-free of libxl_domain_build_info
src/libxl/libxl_conf.c | 48 +++++++++++++++++++++++++++++++++--------------- 1 file changed, 33 insertions(+), 15 deletions(-)
ACK
Is it ok to push these during 1.2.10 freeze? 2/2 is a bug fix...
Pushed these after confirmation from Laine on IRC. Regards, Jim
participants (2)
-
Jim Fehlig -
Michal Privoznik