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

[Jim Fehlig]

Daniel P. Berrange wrote:
> 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 at 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 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.
>   

I'll change these to virReportError().  Is it generally preferable to
use virReportError() over VIR_ERROR?  If so, I'll send a followup patch
to replace any remaining uses of VIR_ERROR.

Regards,
Jim