Re: [Xen-devel] [PATCH 1 of 1 v5] libxl: introduce LIBXL_DOMAIN_TYPE_INVALID

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Wed, 2012-06-06 at 11:47 +0100, Ian Jackson wrote:
> Dario Faggioli writes ("[PATCH 1 of 1 v5] libxl: introduce 
> LIBXL_DOMAIN_TYPE_INVALID"):
> > To avoid recent gcc complaining about:
> > libxl.c: In function âlibxl_primary_console_execâ:
> > libxl.c:1233:9: error: case value â4294967295â not in enumerated type 
> > âlibxl_domain_typeâ [-Werror=switch]
> ...
> > +    if (type == LIBXL_DOMAIN_TYPE_INVALID) {
> > +        LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
> > +                   "invalid domain type for domain %d", domid);
> > +        rc = ERROR_INVAL;
> > +        goto remus_fail;
> 
> This is not an expected error condition, is it ?  ERROR_INVAL is for
> libxl being passed impromper parameters.  So I think this should be
> ERROR_FAIL.

Also perhaps the logging should be done in libxl__domain_type? Unless
there are too many legitimate occasions where INVALID might come up?

> 
> > @@ -692,11 +699,20 @@ int libxl_domain_suspend(libxl_ctx *ctx,
> >      int debug = info != NULL && info->flags & XL_SUSPEND_DEBUG;
> >      int rc = 0;
> >  
> > +    if (type == LIBXL_DOMAIN_TYPE_INVALID) {
> > +        LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
> > +                   "invalid domain type for domain %d", domid);
> > +        rc = ERROR_INVAL;
> > +        goto suspend_fail;
> > +    }
> 
> Is it possible for you to leave this part alone ?  It's moved about a
> lot in my suspend/resume series.
> 
> I will fix it up at the end of my series.
> 
> Thanks,
> Ian.



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel