Re: [PATCH v1 01/10] libxl: libxl-json format and internal functions to get / set it

[Wei Liu <wei.liu2@citrix.com>]

On Wed, Jul 16, 2014 at 05:11:22PM +0100, Ian Campbell wrote:
> On Thu, 2014-07-10 at 15:32 +0100, Wei Liu wrote:
> 
> > diff --git a/tools/libxl/libxl_internal.c b/tools/libxl/libxl_internal.c
> > index 81f8985..dc47177 100644
> > --- a/tools/libxl/libxl_internal.c
> > +++ b/tools/libxl/libxl_internal.c
> > @@ -381,6 +381,62 @@ out:
> >      return rc;
> >  }
> >  
> > +int libxl__get_domain_configuration(libxl__gc *gc, uint32_t domid,
> > +                                    libxl_domain_config *d_config)
> > +{
> > +    uint8_t *data = NULL;
> > +    int rc, len;
> > +
> > +    rc = libxl_userdata_retrieve(CTX, domid, "libxl-json", &data, &len);
> > +    if (rc) {
> > +        LOGEV(ERROR, rc,
> > +              "failed to retrieve domain configuration for domain %d", domid);
> > +        rc = ERROR_FAIL;
> 
> rc here is already a valid libxl ERROR_* here, isn't it? No need to
> override in that case. You have this several times in this series..
> 

OK.

> > +        goto out;
> > +    }
> > +
> > +    if (len == 0) {
> > +        LOGE(ERROR, "configuration data stream empty for domain %d", domid);
> > +        rc = ERROR_FAIL;
> 
> This one on the other hand does need setting explicitly.
> 
> How could this happen though? Ian J is in favour of adding more error

The file is accidently overwritten, failure on last write etc. Bad
things happen.

> codes instead of constantly reusing ERROR_FAIL. Here it would seem that
> depending on whether/how this is supposed to happen either a
> ERROR_INTERNAL_MELTDOWN (i.e. everythig has gone wrong) ish thing or
> ERROR_NO_DOMAIN_CONFIG would be appropriate.
> 

OK. Will figure out a new error code.

Wei.

> Ian.