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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



2011/5/18 Eric Blake <eblake@xxxxxxxxxx>:
> 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

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]