Re: [libvirt] [PATCH 03/12] libxl: Earlier detection of not running on Xen

On Fri, Aug 30, 2013 at 03:46:49PM -0600, Jim Fehlig wrote: > Detect early on in libxl driver initialization if the driver > should be loaded at all, avoiding needless initialization steps > that only have to be undone later. While at it, move th > detection to a helper function to improve readability. > > After detecting that the driver should be loaded, subsequent > failures such as initializing the log stream, allocating libxl > ctx, etc. should be treated as failure to initialize the driver. > > Signed-off-by: Jim Fehlig <jfehlig(a)suse.com&gt; > --- > src/libxl/libxl_driver.c | 62 +++++++++++++++++++++++++++++++----------------- > 1 file changed, 40 insertions(+), 22 deletions(-) > > diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c > index ff4f6be..759fed5 100644 > --- a/src/libxl/libxl_driver.c > +++ b/src/libxl/libxl_driver.c > @@ -775,34 +775,54 @@ libxlStateCleanup(void) > return 0; > } > > -static int > -libxlStateInitialize(bool privileged, > - virStateInhibitCallback callback ATTRIBUTE_UNUSED, > - void *opaque ATTRIBUTE_UNUSED) > +static bool > +libxlDriverShouldLoad(bool privileged) > { > - const libxl_version_info *ver_info; > - char *log_file = NULL; > + bool ret = false; > virCommandPtr cmd; > - int status, ret = 0; > - unsigned int free_mem; > - char ebuf[1024]; > + int status; > > - /* Disable libxl driver if non-root */ > + /* Don't load if non-root */ > if (!privileged) { > VIR_INFO("Not running privileged, disabling libxenlight driver"); > - return 0; > + return ret; > + } > + > + /* Don't load if not running on a Xen control domain (dom0) */ > + if (!virFileExists("/proc/xen/capabilities")) { > + VIR_INFO("No Xen capabilities detected, probably not running " > + "in a Xen Dom0. Disabling libxenlight driver"); > + > + return ret; > } > > - /* Disable driver if legacy xen toolstack (xend) is in use */ > + /* Don't load if legacy xen toolstack (xend) is in use */ > Indentation typo > cmd = virCommandNewArgList("/usr/sbin/xend", "status", NULL); > if (virCommandRun(cmd, &status) == 0 && status == 0) { > VIR_INFO("Legacy xen tool stack seems to be in use, disabling " > "libxenlight driver."); > - virCommandFree(cmd); > - return 0; > + } else { > + ret = true; > } > virCommandFree(cmd); > > + return ret; > +} > + > +static int > +libxlStateInitialize(bool privileged, > + virStateInhibitCallback callback ATTRIBUTE_UNUSED, > + void *opaque ATTRIBUTE_UNUSED) > +{ > + const libxl_version_info *ver_info; > + char *log_file = NULL; > + int ret = 0; > + unsigned int free_mem; > + char ebuf[1024]; > + > + if (!libxlDriverShouldLoad(privileged)) > + return 0; > + > if (VIR_ALLOC(libxl_driver) < 0) > return -1; > > @@ -883,21 +903,20 @@ libxlStateInitialize(bool privileged, > 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; > + VIR_ERROR(_("Failed to create logger for libxenlight")); > + goto error; > } > > 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; > + VIR_ERROR(_("Failed to initialize libxenlight context")); > + goto error; > } > > if ((ver_info = libxl_get_version_info(libxl_driver->ctx)) == NULL) { > - VIR_INFO("cannot version information from libxenlight, disabling driver"); > - goto fail; > + VIR_ERROR(_("Failed to get version information from libxenlight")); > For the errors here, it is preferrable to use virReportError() which will in turn call VIR_ERROR anywway.