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

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