> -----Original Message----- > From: Anthony PERARD <anthony.per...@citrix.com> > Sent: 29 November 2019 12:46 > To: Durrant, Paul <pdurr...@amazon.com> > Cc: xen-devel@lists.xenproject.org; George Dunlap > <george.dun...@citrix.com>; Ian Jackson <ian.jack...@eu.citrix.com>; Wei > Liu <w...@xen.org>; Andrew Cooper <andrew.coop...@citrix.com>; George Dunlap > <george.dun...@eu.citrix.com>; Jan Beulich <jbeul...@suse.com>; Julien > Grall <jul...@xen.org>; Konrad Rzeszutek Wilk <konrad.w...@oracle.com>; > Stefano Stabellini <sstabell...@kernel.org>; Marek Marczykowski-Górecki > <marma...@invisiblethingslab.com>; Volodymyr Babchuk > <volodymyr_babc...@epam.com>; Roger Pau Monné <roger....@citrix.com> > Subject: Re: [PATCH-for-4.13 v5] Rationalize max_grant_frames and > max_maptrack_frames handling > > On Thu, Nov 28, 2019 at 04:52:24PM +0000, Paul Durrant wrote: > > diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h > > index 49b56fa1a3..a2a5d321c5 100644 > > --- a/tools/libxl/libxl.h > > +++ b/tools/libxl/libxl.h > > @@ -364,8 +364,8 @@ > > */ > > #define LIBXL_HAVE_BUILDINFO_GRANT_LIMITS 1 > > > > -#define LIBXL_MAX_GRANT_FRAMES_DEFAULT 32 > > -#define LIBXL_MAX_MAPTRACK_FRAMES_DEFAULT 1024 > > +#define LIBXL_MAX_GRANT_FRAMES_DEFAULT -1 > > +#define LIBXL_MAX_MAPTRACK_FRAMES_DEFAULT -1 > > > > /* > > * LIBXL_HAVE_BUILDINFO_* indicates that libxl_domain_build_info has > > diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl > > index 0546d7865a..63e29bb2fb 100644 > > --- a/tools/libxl/libxl_types.idl > > +++ b/tools/libxl/libxl_types.idl > > @@ -511,8 +511,8 @@ libxl_domain_build_info = > Struct("domain_build_info",[ > > > > ("vnuma_nodes", Array(libxl_vnode_info, "num_vnuma_nodes")), > > > > - ("max_grant_frames", uint32, {'init_val': > 'LIBXL_MAX_GRANT_FRAMES_DEFAULT'}), > > - ("max_maptrack_frames", uint32, {'init_val': > 'LIBXL_MAX_MAPTRACK_FRAMES_DEFAULT'}), > > + ("max_grant_frames", integer, {'init_val': > 'LIBXL_MAX_GRANT_FRAMES_DEFAULT'}), > > + ("max_maptrack_frames", integer, {'init_val': > 'LIBXL_MAX_MAPTRACK_FRAMES_DEFAULT'}), > > That's a change in the libxl API, could you add a LIBX_HAVE_* macro? >
Is it really, in practice? > > > > ("device_model_version", libxl_device_model_version), > > ("device_model_stubdomain", libxl_defbool), > > diff --git a/tools/libxl/libxlu_cfg.c b/tools/libxl/libxlu_cfg.c > > index 72815d25dd..cafc632fc1 100644 > > --- a/tools/libxl/libxlu_cfg.c > > +++ b/tools/libxl/libxlu_cfg.c > > @@ -268,8 +268,9 @@ int xlu_cfg_replace_string(const XLU_Config *cfg, > const char *n, > > return 0; > > } > > > > -int xlu_cfg_get_long(const XLU_Config *cfg, const char *n, > > - long *value_r, int dont_warn) { > > +int xlu_cfg_get_bounded_long(const XLU_Config *cfg, const char *n, > > + long min, long max, long *value_r, > > + int dont_warn) { > > long l; > > XLU_ConfigSetting *set; > > int e; > > @@ -303,10 +304,31 @@ int xlu_cfg_get_long(const XLU_Config *cfg, const > char *n, > > cfg->config_source, set->lineno, n); > > return EINVAL; > > } > > + if (l < min) { > > + if (!dont_warn) > > + fprintf(cfg->report, > > + "%s:%d: warning: value `%ld' is smaller than > minimum bound '%ld'\n", > > + cfg->config_source, set->lineno, l, min); > > + return EINVAL; > > + } > > + if (l > max) { > > + if (!dont_warn) > > + fprintf(cfg->report, > > + "%s:%d: warning: value `%ld' is greater than > maximum bound '%ld'\n", > > + cfg->config_source, set->lineno, l, max); > > + return EINVAL; > > + } > > I'm not sure what was the intention with the new function > xlu_cfg_get_bounded_long(), but I don't think libxlu is the right place > for it. That function is only going to make it harder for users to find > mistakes in the config file. If `n' value is out of bound, it will only > get ignored, and xl will keep going. I think xlu_cfg should only be a > parser (and can check for syntax error). > > Can you move that function to xl? > I can, but why is this not considered useful in libxl? The call returns failure for an out-of-bounds check. If xl currently chooses to treat EINVAL as ENOENT then that's xl's bug to deal with. Paul > Thanks, > > -- > Anthony PERARD _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel