[libvirt] [PATCH] xen: ensure /usr/sbin/xend exists before checking status

[Jim Fehlig]

Eric Blake wrote:
> On 04/28/2014 12:42 PM, Jim Fehlig wrote:
>   
>> With xend on the way out, installations may not even have
>> /usr/sbin/xend, which results in the following error when the
>> drivers are probed
>>
>> 2014-04-28 18:21:19.271+0000: 22129: error : virCommandWait:2426 :
>> internal error: Child process (/usr/sbin/xend status) unexpected exit
>> status 127: libvirt:  error : cannot execute binary /usr/sbin/xend:
>> No such file or directory
>>
>> Check for existence of /usr/sbin/xend before trying to run it with
>> the 'status' option.
>>
>> Signed-off-by: Jim Fehlig <jfehlig at suse.com>
>> ---
>>  src/libxl/libxl_driver.c | 19 ++++++++++++-------
>>  src/xen/xen_driver.c     | 13 ++++++++-----
>>  2 files changed, 20 insertions(+), 12 deletions(-)
>>     
>
>
>   
>> @@ -256,14 +255,20 @@ libxlDriverShouldLoad(bool privileged)
>>      }
>>  
>>      /* Don't load if legacy xen toolstack (xend) is in use */
>> -    cmd = virCommandNewArgList("/usr/sbin/xend", "status", NULL);
>> -    if (virCommandRun(cmd, &status) == 0 && status == 0) {
>> -        VIR_INFO("Legacy xen tool stack seems to be in use, disabling "
>> -                  "libxenlight driver.");
>> +    if (virFileExists("/usr/sbin/xend")) {
>> +        virCommandPtr cmd;
>> +
>> +        cmd = virCommandNewArgList("/usr/sbin/xend", "status", NULL);
>> +        if (virCommandRun(cmd, &status) == 0 && status == 0) {
>>     
>
> Pre-patch, passing &status to virCommandRun had the effect of avoiding a
> log message if the command ran with non-zero status.  But now that we
> are requiring the file to exist, and in particular, since we are already
> using NULL later on...
>
>   
>> +++ b/src/xen/xen_driver.c
>> @@ -315,13 +315,16 @@ xenUnifiedProbe(void)
>>  static bool
>>  xenUnifiedXendProbe(void)
>>  {
>> -    virCommandPtr cmd;
>>      bool ret = false;
>>  
>> -    cmd = virCommandNewArgList("/usr/sbin/xend", "status", NULL);
>> -    if (virCommandRun(cmd, NULL) == 0)
>> -        ret = true;
>> -    virCommandFree(cmd);
>> +    if (virFileExists("/usr/sbin/xend")) {
>> +        virCommandPtr cmd;
>> +
>> +        cmd = virCommandNewArgList("/usr/sbin/xend", "status", NULL);
>> +        if (virCommandRun(cmd, NULL) == 0)
>>     
>
> ...here, would it make sense to use virCommandRun(cmd, NULL) instead of
> checking status ourselves?

Yes, I think so.  No harm in a log message from virCommandRun on
non-zero exit.

>   If the intent is still to avoid a logged
> message when the status is non-zero, a comment in the code might be
> useful (for example, see how virStorageBackendQEMUImgBackingFormat in
> storage_backend.c has a comment).
>
> ACK with either the code simplification or the added comment.
>   

Code simplified and pushed.  Thanks for the review.

Regards,
Jim