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

[Roger Pau Monne <roger.pau@xxxxxxxxxx>]

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).

Roger.

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