Re: [Xen-devel] [PATCH] tools/libxl: include scheduler parameters in the output of xl list -l

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Wed, Jan 04, 2017 at 11:45:15AM +0000, Roger Pau Monne wrote:
> On Wed, Jan 04, 2017 at 11:35:54AM +0000, Wei Liu wrote:
> > On Wed, Jan 04, 2017 at 11:20:41AM +0000, Roger Pau Monne wrote:
> > > On Wed, Jan 04, 2017 at 11:08:51AM +0000, Wei Liu wrote:
> > > > On Wed, Jan 04, 2017 at 09:55:59AM +0000, Roger Pau Monne wrote:
> > > > > Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> > > > > Reported-by: Fatih Acar <fatih@xxxxxxxxx>
> > > > > ---
> > > > > Cc: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
> > > > > Cc: Wei Liu <wei.liu2@xxxxxxxxxx>
> > > > > ---
> > > > > This should be backported to all supported stable branches (4.6, 4.7, 
> > > > > 4.8).
> > > > > ---
> > > > >  tools/libxl/libxl.c | 10 ++++++++++
> > > > >  1 file changed, 10 insertions(+)
> > > > > 
> > > > > diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
> > > > > index 6fd4fe1..7aa6d41 100644
> > > > > --- a/tools/libxl/libxl.c
> > > > > +++ b/tools/libxl/libxl.c
> > > > > @@ -6925,6 +6925,16 @@ int 
> > > > > libxl_retrieve_domain_configuration(libxl_ctx *ctx, uint32_t domid,
> > > > >          }
> > > > >      }
> > > > >  
> > > > > +    /* Scheduler params */
> > > > > +    {
> > > > 
> > > > Please add the following:
> > > > 
> > > >  libxl_sched_params_dispose(&d_config->b_info.sched_params);
> > > >  libxl_sched_params_init(&d_config->b_info.sched_params);
> > > 
> > > libxl_domain_sched_params_get already calls libxl_sched_params_init which
> > > performs a memset plus sets the default values, so I don't see the need 
> > > to call
> > > _dispose and _init from libxl_retrieve_domain_configuration.
> > > 
> > 
> > At the very least please call _dispose. The _init call can be dropped.
> 
> If the _dispose call is really needed, shouldn't it be added to
> libxl_domain_sched_params_get? It seems asymmetric IMHO to do the _init call
> inside of libxl_domain_sched_params_get but not the _dispose one (which I 
> still
> think it's unneeded).
> 

Missed this email yesterday, sorry.

libxl_domain_sched_params_get was written before we codified how libxl
types should be used, so it is not exactly in line with newer code. It
is too late to change that now.

So yes, _dispose is needed, and it shouldn't be added to
_sched_params_get.

Wei.

> Roger.

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