Re: [PATCH V5 31/32] libxl: update domain configuration when updating memory targets

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

On Tue, May 20, 2014 at 03:32:04PM +0100, Ian Campbell wrote:
> On Tue, 2014-05-13 at 22:54 +0100, Wei Liu wrote:
> > Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> > ---
> >  tools/libxl/libxl.c |   36 ++++++++++++++++++++++++++++++++++++
> >  1 file changed, 36 insertions(+)
> > 
> > diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
> > index b578a5c..f93096b 100644
> > --- a/tools/libxl/libxl.c
> > +++ b/tools/libxl/libxl.c
> > @@ -3529,6 +3529,25 @@ out:
> >  
> >  /******************************************************************************/
> >  
> > +/* Macro to load / store domain configuration. They must use in pair.
> > + * Load macro defines d_config that can be used by other code. Store
> > + * macro will dispose d_config.
> > + */
> 
> I was asking for a helper earlier, I suppose this could be useful there
> too.
> 
> TBH I think a helper fn and callback arrangement would be preferable to
> magic macros (macros only when there is no alternative I think). I
> suppose that would result in a plethora of little callback helpers
> though. Hrm, what do you think?
> 

Yes. The downside is we need to have many helpers. But I think it is a
better approach.

> > +#define LOAD_DOMAIN_CONFIG(domid)                                       \
> > +    libxl_domain_config d_config;                                       \
> > +    libxl_domain_config_init(&d_config);                                \
> > +    rc = libxl_load_domain_configuration(CTX, (domid), &d_config);      \
> > +    if (rc)                                                             \
> > +        goto out;                                                       \
> > +
> > +#define STORE_DOMAIN_CONFIG(domid)                                      \
> > +    rc = libxl_store_domain_configuration(CTX, (domid), &d_config);     \
> > +    if (rc) {                                                           \
> > +        libxl_domain_config_dispose(&d_config);                         \
> > +        goto out;                                                       \
> > +    }                                                                   \
> > +    libxl_domain_config_dispose(&d_config);                             \
> 
>      rc = 
>      libxl_domain_config_dispose
>      if (rc)
>          goto out;
> 
> would work here I think, since dispose doesn't clobber rc.
> 

Yes, you're right.

Wei.