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

Re: [libvirt] [PATCH V6] Add libxenlight driver



Daniel P. Berrange wrote:
> On Thu, Mar 17, 2011 at 09:17:58PM -0600, Jim Fehlig wrote:
>   
>> diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c
>> index 7d08df3..bec9577 100644
>> --- a/src/xen/xen_driver.c
>> +++ b/src/xen/xen_driver.c
>> @@ -50,6 +50,7 @@
>>  #include "uuid.h"
>>  #include "fdstream.h"
>>  #include "files.h"
>> +#include "command.h"
>>  
>>  #define VIR_FROM_THIS VIR_FROM_XEN
>>  
>> @@ -229,6 +230,26 @@ xenUnifiedProbe (void)
>>      return 0;
>>  }
>>  
>> +#ifdef WITH_LIBXL
>> +static int
>> +xenUnifiedXendProbe (void)
>> +{
>> +    virCommandPtr cmd;
>> +    int status;
>> +    int ret = 0;
>> +
>> +    if ((cmd = virCommandNew("/usr/sbin/xend")) != NULL) {
>> +        virCommandAddArg(cmd, "status");
>> +        if (virCommandRun(cmd, &status) == 0) {
>> +            if (status == 0)
>> +                ret = 1;
>> +        }
>> +        virCommandFree(cmd);
>> +    }
>> +    return ret;
>> +}
>>     
>
> It isn't neccessary to check the return status of CommandNew(),
> since if it got OOM, the subsequent API calls with diagnose
> that correctly. You can also add args at the same time.
>
>    virCommandPtr cmd = virCommandNewArgList("/usr/sbin/xend", "status", NULL);
>    if (virCommandRun(cmd, &status) == 0 && status == 0)
>        ret = 1;
>    virCommandFree(cmd);
>   

Ah, thanks for the tip.

> There was another case of this code pattern earlier which can
> be simplified too
>
>   
>> @@ -292,6 +313,13 @@ xenUnifiedOpen (virConnectPtr conn, virConnectAuthPtr auth, int flags)
>>          }
>>      }
>>  
>> +#ifdef WITH_LIBXL
>> +    /* Decline xen:// URI if xend is not running and libxenlight
>> +     * driver is potentially available. */
>> +    if (!xenUnifiedXendProbe())
>> +        return VIR_DRV_OPEN_DECLINED;
>> +#endif
>> +
>>      /* We now know the URI is definitely for this driver, so beyond
>>       * here, don't return DECLINED, always use ERROR */
>>  
>>     
>
> With this change to virCommandPtr usage, I will happily ACK this
> new driver.

Great, thanks!  I've made the changes you suggested and pushed the new
driver :-).

>  I think it is at the stage now where its better to
> have the code in tree, and let people send small incrementall
> followup patches to deal with any further bugs that appear
>   

Yes, I agree,  Markus already has patches lined up to add functionality,
so I'll no longer be holding up his work.

Thanks to everyone for helping review and test the driver!

Regards,
Jim


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