Re: [PATCH v2 for Xen 4.6 4/4] xl: enabling XL to set per-VCPU parameters of a domain for RTDS scheduler

[Dario Faggioli <dario.faggioli@citrix.com>]

[-- Attachment #1.1: Type: text/plain, Size: 3100 bytes --]

On Mon, 2015-05-25 at 19:11 -0500, Chong Li wrote:
> Change main_sched_rtds and related output functions to support per-VCPU settings
> for xl sched-rtds tool.
> 
> Signed-off-by: Chong Li <chong.li@wustl.edu>
> Signed-off-by: Meng Xu <mengxu@cis.upenn.edu>
> Signed-off-by: Sisu Xi <xisisu@gmail.com>
> ---
>  tools/libxl/xl_cmdimpl.c | 261 +++++++++++++++++++++++++++++++++++++++++------
>  1 file changed, 230 insertions(+), 31 deletions(-)
> 
You must be changing tools/libxl/xl_cmdtable.c as well, or new options
won't work (at least they won't show up in the command's help).

> --- a/tools/libxl/xl_cmdimpl.c
> +++ b/tools/libxl/xl_cmdimpl.c

> @@ -6120,76 +6186,209 @@ int main_sched_rtds(int argc, char **argv)
>  {
>      const char *dom = NULL;
>      const char *cpupool = NULL;
> -    int period = 0; /* period is in microsecond */
> -    int budget = 0; /* budget is in microsecond */
> +
> +    int *vcpus = (int *)xmalloc(sizeof(int)); /* IDs of VCPUs that change */
> +    int *periods = (int *)xmalloc(sizeof(int)); /* period is in microsecond */
> +    int *budgets = (int *)xmalloc(sizeof(int)); /* budget is in microsecond */
> +    int vcpus_size = 1; /* size of vcpus array */
> +    int periods_size = 1; /* size of periods array */
> +    int budgets_size = 1; /* size of budgets array */
> +    int input_size = 0; /* number of the input param set (v, p, b) */
> +    bool flag_b = false;
> +    bool flag_p = false;
> +    bool flag_v = false;
>      bool opt_p = false;
>      bool opt_b = false;
> -    int opt, rc;
> +    bool opt_v = false;
> +    bool opt_o = false; /* get per-domain info instead of per-vcpu info */
> +    int opt, i;
> +    int rc = 0;
>      static struct option opts[] = {
>          {"domain", 1, 0, 'd'},
>          {"period", 1, 0, 'p'},
>          {"budget", 1, 0, 'b'},
> +        {"vcpu",1, 0, 'v'},
>          {"cpupool", 1, 0, 'c'},
> +        {"output", 1, 0, 'o'},
>          COMMON_LONG_OPTS,
>          {0, 0, 0, 0}
>      };
>
I don't like "-o/--output" as the name of the config switch for this.

Also, what is the semantic of passing -o, in case one has given
different parameters to each vcpus? What would the command print in that
case?

I think that, while it makes sense to have the interface in place for
the setting part, as a shortcut of setting all the parameters of all the
vcpus to the same values, for the getting and printing side of things,
it make much less sense.

I appreciate just now that this probably affect other bits of the
interface, as well as other interfaces, and I think we should handle it
consistently...

What do you think?
For example (although this belong to patch 3's review) what
libxl_domain_sched_params_get() does in the case I jus described?

Regards,
Dario

-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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