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@xxxxxxxxxxxx | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list