* [PATCH] tools/libxl: include scheduler parameters in the output of xl list -l
@ 2017-01-04 9:55 Roger Pau Monne
2017-01-04 11:08 ` Wei Liu
0 siblings, 1 reply; 6+ messages in thread
From: Roger Pau Monne @ 2017-01-04 9:55 UTC (permalink / raw)
To: xen-devel; +Cc: Wei Liu, Ian Jackson, Roger Pau Monne
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reported-by: Fatih Acar <fatih@gandi.net>
---
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>
---
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 */
+ {
+ rc = libxl_domain_sched_params_get(ctx, domid,
+ &d_config->b_info.sched_params);
+ if (rc) {
+ LOGD(ERROR, domid, "Fail to get scheduler parameters");
+ goto out;
+ }
+ }
+
out:
if (lock) libxl__unlock_domain_userdata(lock);
CTX_UNLOCK;
--
2.10.1 (Apple Git-78)
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] tools/libxl: include scheduler parameters in the output of xl list -l
2017-01-04 9:55 [PATCH] tools/libxl: include scheduler parameters in the output of xl list -l Roger Pau Monne
@ 2017-01-04 11:08 ` Wei Liu
2017-01-04 11:20 ` Roger Pau Monne
0 siblings, 1 reply; 6+ messages in thread
From: Wei Liu @ 2017-01-04 11:08 UTC (permalink / raw)
To: Roger Pau Monne; +Cc: xen-devel, Ian Jackson, Wei Liu
On Wed, Jan 04, 2017 at 09:55:59AM +0000, Roger Pau Monne wrote:
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> Reported-by: Fatih Acar <fatih@gandi.net>
> ---
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> Cc: Wei Liu <wei.liu2@citrix.com>
> ---
> 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);
Also I would like to move this block before device handling.
> + rc = libxl_domain_sched_params_get(ctx, domid,
> + &d_config->b_info.sched_params);
> + if (rc) {
> + LOGD(ERROR, domid, "Fail to get scheduler parameters");
> + goto out;
> + }
> + }
> +
> out:
> if (lock) libxl__unlock_domain_userdata(lock);
> CTX_UNLOCK;
> --
> 2.10.1 (Apple Git-78)
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] tools/libxl: include scheduler parameters in the output of xl list -l
2017-01-04 11:08 ` Wei Liu
@ 2017-01-04 11:20 ` Roger Pau Monne
2017-01-04 11:35 ` Wei Liu
0 siblings, 1 reply; 6+ messages in thread
From: Roger Pau Monne @ 2017-01-04 11:20 UTC (permalink / raw)
To: Wei Liu; +Cc: xen-devel, Ian Jackson
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@citrix.com>
> > Reported-by: Fatih Acar <fatih@gandi.net>
> > ---
> > Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> > Cc: Wei Liu <wei.liu2@citrix.com>
> > ---
> > 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.
> Also I would like to move this block before device handling.
I can certainly do that.
Roger.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] tools/libxl: include scheduler parameters in the output of xl list -l
2017-01-04 11:20 ` Roger Pau Monne
@ 2017-01-04 11:35 ` Wei Liu
2017-01-04 11:45 ` Roger Pau Monne
0 siblings, 1 reply; 6+ messages in thread
From: Wei Liu @ 2017-01-04 11:35 UTC (permalink / raw)
To: Roger Pau Monne; +Cc: xen-devel, Wei Liu, Ian Jackson
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@citrix.com>
> > > Reported-by: Fatih Acar <fatih@gandi.net>
> > > ---
> > > Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> > > Cc: Wei Liu <wei.liu2@citrix.com>
> > > ---
> > > 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.
> > Also I would like to move this block before device handling.
>
> I can certainly do that.
>
> Roger.
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] tools/libxl: include scheduler parameters in the output of xl list -l
2017-01-04 11:35 ` Wei Liu
@ 2017-01-04 11:45 ` Roger Pau Monne
2017-01-05 10:44 ` Wei Liu
0 siblings, 1 reply; 6+ messages in thread
From: Roger Pau Monne @ 2017-01-04 11:45 UTC (permalink / raw)
To: Wei Liu; +Cc: xen-devel, Ian Jackson
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@citrix.com>
> > > > Reported-by: Fatih Acar <fatih@gandi.net>
> > > > ---
> > > > Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> > > > Cc: Wei Liu <wei.liu2@citrix.com>
> > > > ---
> > > > 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@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] tools/libxl: include scheduler parameters in the output of xl list -l
2017-01-04 11:45 ` Roger Pau Monne
@ 2017-01-05 10:44 ` Wei Liu
0 siblings, 0 replies; 6+ messages in thread
From: Wei Liu @ 2017-01-05 10:44 UTC (permalink / raw)
To: Roger Pau Monne; +Cc: xen-devel, Wei Liu, Ian Jackson
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@citrix.com>
> > > > > Reported-by: Fatih Acar <fatih@gandi.net>
> > > > > ---
> > > > > Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> > > > > Cc: Wei Liu <wei.liu2@citrix.com>
> > > > > ---
> > > > > 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@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-01-05 10:44 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-01-04 9:55 [PATCH] tools/libxl: include scheduler parameters in the output of xl list -l Roger Pau Monne
2017-01-04 11:08 ` Wei Liu
2017-01-04 11:20 ` Roger Pau Monne
2017-01-04 11:35 ` Wei Liu
2017-01-04 11:45 ` Roger Pau Monne
2017-01-05 10:44 ` Wei Liu
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.