Re: [PATCH v5 3/7] libxl/libxl_domain_info: Log if domain not found.

[Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>]

On Fri, Apr 03, 2015 at 11:12:15PM +0100, Ian Murray wrote:
> On 03/04/15 21:02, Konrad Rzeszutek Wilk wrote:
> > If we cannot find the domain - log an error (and still
> > continue returning an error).
> Forgive me if I am misunderstanding the effect of this patch (I tried to
> find the original rationale but failed). If the effect is that commands
> such as xl domid will cause a log entry when the specified domain
> doesn't exist, I would suggest that's going to be a problem for people

It would.
> that use that or similar commands to tell if a domain is present or
> still alive. I use it as part of a back-up script to make sure a domain
> shutdown before the script continues. I suspect many other people will
> be doing something similar.

But won't 'xl domid' give you an return 0 if it exists and 1 if it does not?

Ah it does this (if it can't find the domain):

6195         fprintf(stderr, "Can't get domid of domain name '%s', maybe this domain does not exist.\n", domname);
6196         return 1;                                                               


If you are using 'xl list <domid>' it also prints:

4739         if (rc == ERROR_DOMAIN_NOTFOUND) {                                      
4740             fprintf(stderr, "Error: Domain \'%s\' does not exist.\n",           
4741                 argv[optind]);                                                  
4742             return -rc;                                  

(Previously it would also print this).

Either way the data is already presented to the user. With this
patch it is presented twice - which is repetitive.


Ian C, thoughts? Just ditch this patch? (The patchset can go in without
this one).

> 
> Apologies if I have the wrong end of the stick!

There is never an wrong end!
> 
> Thanks,
> 
> Ian.
> 
> 
> >
> > Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> > Acked-by: Ian Campbell <ian.campbell@citrix.com>
> > ---
> >  tools/libxl/libxl.c | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
> > index c0e9cfe..8753e27 100644
> > --- a/tools/libxl/libxl.c
> > +++ b/tools/libxl/libxl.c
> > @@ -698,8 +698,10 @@ int libxl_domain_info(libxl_ctx *ctx, libxl_dominfo *info_r,
> >          LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "getting domain info list");
> >          return ERROR_FAIL;
> >      }
> > -    if (ret==0 || xcinfo.domain != domid) return ERROR_DOMAIN_NOTFOUND;
> > -
> > +    if (ret==0 || xcinfo.domain != domid) {
> > +        LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "Domain %d not found!", domid);
> > +        return ERROR_DOMAIN_NOTFOUND;
> > +    }
> >      if (info_r)
> >          xcinfo2xlinfo(ctx, &xcinfo, info_r);
> >      return 0;
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel