Re: [Xen-devel] [PATCH v2 14/18] libxl: introduce libxl_get_memory_static_max

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Wed, Aug 27, 2014 at 03:09:09AM +0100, Ian Campbell wrote:
> On Wed, 2014-07-30 at 19:23 +0100, Wei Liu wrote:
> > +/* type determines which node to return:
> > + *   1 "target"
> > + *   2 "static-max"
> 
> You've just invented enums ;-)
> 
> Either use an actual enum, or a bool flag (since this is internal only I
> don't mind which)
> 

Bool flag it is.

> > +int libxl_get_memory_target(libxl_ctx *ctx, uint32_t domid, uint32_t 
> > *out_target)
> > +{
> > +    GC_INIT(ctx);
> > +    int rc;
> > +
> > +    rc = libxl__get_memory_target(gc, domid, out_target, 1);
> 
> FWIW for trivial helper wrappers like this where the wrapper does
> nothing I think it can be acceptable to bend the usual rule about
> internal things taking a gc a bit. Although that can just be saving
> problems for later when someone adds some new magic to a wrapper.
> 

I would rather keep the code as it is now.

> > @@ -884,6 +891,7 @@ int libxl_domain_core_dump(libxl_ctx *ctx, uint32_t 
> > domid,
> >  int libxl_domain_setmaxmem(libxl_ctx *ctx, uint32_t domid, uint32_t 
> > target_memkb);
> >  int libxl_set_memory_target(libxl_ctx *ctx, uint32_t domid, int32_t 
> > target_memkb, int relative, int enforce);
> >  int libxl_get_memory_target(libxl_ctx *ctx, uint32_t domid, uint32_t 
> > *out_target);
> > +int libxl_get_memory_static_max(libxl_ctx *ctx, uint32_t domid, uint32_t 
> > *out_static_max);
> 
> Do you need this to be public, or did you just do it for consistency?
> 

I did it for consistency, in case it's useful to library users. If you
would rather not expose this I can make it internal.

Wei.

> Ian.

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