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

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



On Fri, Mar 18, 2011 at 11:33:20AM +0000, 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);
> 
> 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. 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

  ACK too even though this was pushed already, I agree :-)

Daniel


-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel veillard com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/


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