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

[Eric Blake]

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?  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.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 604 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20140428/6903a7ec/attachment-0001.sig>