[libvirt] [PATCH 3/6] Clarify that virDomainSetSchedulerParameters(Flags) can take subset

[Matthias Bolte]

2011/5/18 Eric Blake <eblake at redhat.com>:
> On 05/18/2011 04:21 AM, Matthias Bolte wrote:
>> Add invalid argument checks for params and nparams to the public API
>> and remove the from the drivers (e.g. xend).
>>
>> Add subset handling to libxl and test drivers.
>
>> @@ -5092,6 +5092,12 @@ virDomainSetSchedulerParameters(virDomainPtr domain,
>>          virDispatchError(NULL);
>>          return -1;
>>      }
>> +
>> +    if (params == NULL || nparams < 0) {
>
> So this documents that nparams should be in the closed range [1, max]...
>
>> @@ -2683,20 +2683,19 @@ static int testDomainSetSchedulerParams(virDomainPtr domain,
>>          goto cleanup;
>>      }
>>
>> -    if (nparams != 1) {
>> -        testError(VIR_ERR_INVALID_ARG, "nparams");
>> -        goto cleanup;
>> -    }
>> -    if (STRNEQ(params[0].field, "weight")) {
>> -        testError(VIR_ERR_INVALID_ARG, "field");
>> -        goto cleanup;
>> -    }
>> -    if (params[0].type != VIR_DOMAIN_SCHED_FIELD_UINT) {
>> -        testError(VIR_ERR_INVALID_ARG, "type");
>> -        goto cleanup;
>> +    for (i = 0; i < nparams; i++) {
>> +        if (STRNEQ(params[i].field, "weight")) {
>
> Hmm, this change lets us pass an array of two separate weight changes,
> the last one wins, whereas prepatch the maximum was 1, so you could not
> pass two duplicate weights.

Reminds me that I wanted to add a comment about duplicate parameters
to the setter functions.

>From a practical point of view this change didn't alter the behavior
of the test driver as the weight is hardcoded to 50 anyway :)

>> +++ b/src/xen/xen_hypervisor.c
>> @@ -1364,10 +1364,9 @@ xenHypervisorSetSchedulerParameters(virDomainPtr domain,
>>          return -1;
>>      }
>>
>> -    if ((nparams == 0) || (params == NULL)) {
>> -        virXenErrorFunc(VIR_ERR_INVALID_ARG, __FUNCTION__,
>> -                        "Noparameters given", 0);
>> -        return(-1);
>> +    if (nparams == 0) {
>> +        /* nothing to do, exit early */
>> +        return 0;
>
> Given the public check that nparams is not less than 1, this is now dead
> code.

Yep, I missed to removed that check when changing the check from
nparams < 0 to nparams <= 0.

Matthias