[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



On 30.08.2013 23:46, 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

s/th/the/

> 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 suse com>
> ---
>  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 off.

>      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"));
> +        goto error;
>      }
>      libxl_driver->version = (ver_info->xen_version_major * 1000000) +
>              (ver_info->xen_version_minor * 1000);
> @@ -956,7 +975,6 @@ libxlStateInitialize(bool privileged,
>  
>  error:
>      ret = -1;
> -fail:
>      VIR_FREE(log_file);
>      if (libxl_driver)
>          libxlDriverUnlock(libxl_driver);
> 

ACK with those two nits fixed.

Michal


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]