Re: [libvirt] [PATCH 1/2] node_memory: Do not fail if there is parameter unsupported
ping
On 2012年11月22日 11:14, Osier Yang wrote:
> It makes no sense to fail the whole command if there is parameter
> unsupported by the kernel. This patch fixes it by ignoring the
> unsupported parameter for getMemoryParameters, and ignoring the
> unsupported parameter for setMemoryParameters too if there are
> more than one parameters to set, otherwise error out.
I'm not sure I agree with these semantics.
> *
> - * Get all node memory parameters. On input, @nparams gives the
> size
> - * of the @params array; on output, @nparams gives how many slots
> were
> - * filled with parameter information, which might be less but will
> - * not exceed the input value.
> + * Get all node memory parameters (parameter unsupported by OS
> will be
> + * ignored). On input, @nparams gives the size of the @params
s/ignored/omitted/
But is this really right, or shouldn't we at least be able to
fake things? If the OS doesn't support tuning a variable, then
we should return a hard-coded value for the default setting
of that variable (in relation to newer kernels that DO support
tuning), rather than omitting it altogether.
> array;
> + * on output, @nparams gives how many slots were filled with
> parameter
> + * information, which might be less but will not exceed the input
> value.
> *
> * As a special case, calling with @params as NULL and @nparams
> as 0 on
> * input will cause @nparams on output to contain the number of
> parameters
> @@ -6811,7 +6811,9 @@ error:
> * value nparams of virDomainGetSchedulerType)
> * @flags: extra flags; not used yet, so callers should always
> pass 0
> *
> - * Change all or a subset of the node memory tunables.
> + * Change all or a subset of the node memory tunables. Tunable
> + * unsupported by OS wll be ignored if @nparams is not 1,
w/wll/will/
I disagree. I think we should continue to error out on unrecognized
tunables; furthermore, we should error out up front (if the user
passes an array of 3 elements, where only the 2nd element is
unsupported, then we should NOT modify the first element; that is,
we should verify that all the elements are settable before making
any modification). It is a disservice to the user to ignore their
request for a change.
And if you go with my approach that getting parameters should
return a hard-coded value rather than omitting unsupported
tunables, then the user should be allowed to pass the default
value of that tunable; the only thing they cannot do is
pass a non-default value.