Re: [libvirt] [PATCH] libxl: fix libvirtd segfault

Functional ack; this patch fixes the crash on my system.

On Fri, Aug 09, 2013 at 05:58:45PM -0600, Jim Fehlig wrote: > Commit d72ef888 introduced a bug in the libxl driver that will > segfault libvirtd if libxl reports an error message, e.g. when > attempting to initialize the driver on a non-Xen system. I > assumed it was valid to pass a NULL logger to libxl_ctx_alloc(), > but that is not the case since any errors associated with the ctx > that are emitted by libxl will dereference the logger and crash > libvirtd. > > Errors associated with the libxl driver-wide ctx could be useful > for debugging anyway, so create a 'libxl-driver.log' to capture > these errors. > --- > src/libxl/libxl_conf.h | 3 +++ > src/libxl/libxl_driver.c | 30 ++++++++++++++++++++++++++++-- > 2 files changed, 31 insertions(+), 2 deletions(-) > > diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h > index 78133b9..9d72b72 100644 > --- a/src/libxl/libxl_conf.h > +++ b/src/libxl/libxl_conf.h > @@ -57,6 +57,9 @@ struct _libxlDriverPrivate { > virDomainXMLOptionPtr xmlopt; > unsigned int version; > > + /* log stream for driver-wide libxl ctx */ > + FILE *logger_file; > + xentoollog_logger *logger; > /* libxl ctx for driver wide ops; getVersion, getNodeInfo, ... */ > libxl_ctx *ctx; > > diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c > index cc9e772..9dc7261 100644 > --- a/src/libxl/libxl_driver.c > +++ b/src/libxl/libxl_driver.c > @@ -1180,6 +1180,9 @@ libxlStateCleanup(void) > virObjectUnref(libxl_driver->xmlopt); > virObjectUnref(libxl_driver->domains); > libxl_ctx_free(libxl_driver->ctx); > + xtl_logger_destroy(libxl_driver->logger); > + if (libxl_driver->logger_file) > + VIR_FORCE_FCLOSE(libxl_driver->logger_file); > > virObjectUnref(libxl_driver->reservedVNCPorts); > > @@ -1229,6 +1232,7 @@ libxlStateInitialize(bool privileged, > void *opaque ATTRIBUTE_UNUSED) > { > const libxl_version_info *ver_info; > + char *log_file = NULL; > virCommandPtr cmd; > int status, ret = 0; > unsigned int free_mem; > @@ -1308,6 +1312,17 @@ libxlStateInitialize(bool privileged, > goto error; > } > > + if (virAsprintf(&log_file, "%s/libxl-driver.log", libxl_driver->logDir) < 0) > + goto error; > + > + if ((libxl_driver->logger_file = fopen(log_file, "a")) == NULL) { > + virReportSystemError(errno, > + _("failed to create logfile %s"), > + log_file); > + goto error; > + } > + VIR_FREE(log_file); > + > /* read the host sysinfo */ > if (privileged) > libxl_driver->hostsysinfo = virSysinfoRead(); > @@ -1316,8 +1331,18 @@ libxlStateInitialize(bool privileged, > if (!libxl_driver->domainEventState) > goto error; > > - if (libxl_ctx_alloc(&libxl_driver->ctx, LIBXL_VERSION, 0, NULL)) { > - VIR_INFO("cannot initialize libxenlight context, probably not running in a Xen Dom0, disabling driver"); > + libxl_driver->logger = > + (xentoollog_logger *)xtl_createlogger_stdiostream(libxl_driver->logger_file, XTL_DEBUG, 0); > + if (!libxl_driver->logger) { > + VIR_INFO("cannot create logger for libxenlight, disabling driver"); > + goto fail; > + } > + > + if (libxl_ctx_alloc(&libxl_driver->ctx, > + LIBXL_VERSION, 0, > + libxl_driver->logger)) { > + VIR_INFO("cannot initialize libxenlight context, probably not running " > + "in a Xen Dom0, disabling driver"); > goto fail; > } > > @@ -1383,6 +1408,7 @@ libxlStateInitialize(bool privileged, > error: > ret = -1; > fail: > + VIR_FREE(log_file); > if (libxl_driver) > libxlDriverUnlock(libxl_driver); > libxlStateCleanup(); >