From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ian Campbell Subject: Re: [PATCH v2 14/18] libxl: introduce libxl_get_memory_static_max Date: Wed, 27 Aug 2014 03:09:09 +0100 Message-ID: <1409105349.28009.78.camel@citrix.com> References: <1406744639-28782-1-git-send-email-wei.liu2@citrix.com> <1406744639-28782-15-git-send-email-wei.liu2@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1406744639-28782-15-git-send-email-wei.liu2@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Wei Liu Cc: ian.jackson@eu.citrix.com, xen-devel@lists.xen.org List-Id: xen-devel@lists.xenproject.org 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) > +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. > @@ -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? Ian.