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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list