On 26/09/18 19:17, Dario Faggioli wrote:
> On Tue, 2018-09-18 at 08:03 +0200, Juergen Gross wrote:
>> Add a new xl command "cpupool-set-parameters" and cpupool config file
>> support for setting per-cpupool generic parameters.
>>
>> Signed-off-by: Juergen Gross <jgr...@suse.com>
>>
> Seems good to me. Couple of questions.
>
> Question one: what about getting (and displaying, I guess in
> cpupoolinfo) the cpupool parameters?
>
>> --- a/tools/xl/xl_cpupool.c
>> +++ b/tools/xl/xl_cpupool.c
>> @@ -615,6 +625,35 @@ out:
>> return rc;
>> }
>>
>> +int main_cpupoolsetparameters(int argc, char **argv)
>> +{
>> + int opt;
>> + const char *pool;
>> + char *params;
>> + uint32_t poolid;
>> +
>> + SWITCH_FOREACH_OPT(opt, "", NULL, "cpupool-set-parameters", 2) {
>> + /* No options */
>> + }
>> +
>> + pool = argv[optind++];
>> + if (libxl_cpupool_qualifier_to_cpupoolid(ctx, pool, &poolid,
>> NULL) ||
>> + !libxl_cpupoolid_is_valid(ctx, poolid)) {
>> + fprintf(stderr, "unknown cpupool '%s'\n", pool);
>> + return EXIT_FAILURE;
>> + }
>> +
> Since we know that we, AFAIUI, never allow changing the parameters for
> a cpupool with domains in it already, shall we test this here, and bail
> early, with a specific error message, without even trying going down in
> Xen?
>
> I know it's sort-of duplicating checks with what's in the hypervisor,
> but I think it would be a common mistake, that it's worth trying to
> prevent/address specifically.
That's exactly what the PARAM_FLAG_RUNTIME is meant for. I could think
of parameters which might be changeable even with active domains in the
cpupool. So I wouldn't like to test that in the tools as we would need
to add the knowledge of each parameter to the tools.
Juergen
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
