[libvirt] [PATCH 2/6] Clarify the semantic of virDomainGetSchedulerParameters arguments

[Matthias Bolte]

2011/5/18 Daniel Veillard <veillard at redhat.com>:
> On Wed, May 18, 2011 at 12:21:09PM +0200, Matthias Bolte wrote:
>> params and nparams are essential and cannot be NULL. Check this in
>> libvirt.c and remove redundant checks from the drivers (e.g. xend).
>>
>> Instead of enforcing that nparams must point to exact same value as
>> returned by virDomainGetSchedulerType relax this to a lower bound
>> check. This is what some drivers (e.g. xen hypervisor and esx)
>> already did. Other drivers (e.g. xend) didn't check nparams at all
>> and assumed that there is enough space in params.
>>
>> Unify the behavior in all drivers to a lower bound check and update
>> nparams to the number of valid values in params on success.
> [...]
>> - * Get the scheduler parameters, the @params array will be filled with the
>> - * values.
>> + * Get all scheduler parameters, the @params array will be filled with the
>> + * values and @nparams will be updated to the number of valid elements in
>> + * @params.
>>   *
>>   * Returns -1 in case of error, 0 in case of success.
>>   */
>> @@ -5041,6 +5042,12 @@ virDomainGetSchedulerParameters(virDomainPtr domain,
>>          virDispatchError(NULL);
>>          return -1;
>>      }
>> +
>> +    if (params == NULL || nparams == NULL || *nparams < 0) {
>
>  Hum, what would *nparams == 0 mean ? The arry pointer has to be
> allocated anyway.
> Maybe we should use *nparams <= 0 here

True, I changed that before pushing the whole series.

>> +        virLibDomainError(VIR_ERR_INVALID_ARG, __FUNCTION__);
>> +        goto error;
>> +    }
>> +
>>      conn = domain->conn;
>>
>>      if (conn->driver->domainGetSchedulerParameters) {
>> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
>> index 6b2d806..b85c743 100644
>> --- a/src/libxl/libxl_driver.c
>> +++ b/src/libxl/libxl_driver.c
>> @@ -2466,7 +2466,7 @@ libxlDomainGetSchedulerParameters(virDomainPtr dom, virSchedParameterPtr params,
>>          goto cleanup;
>>      }
>>
>> -    if (*nparams != XEN_SCHED_CREDIT_NPARAM) {
>> +    if (*nparams < XEN_SCHED_CREDIT_NPARAM) {
>>          libxlError(VIR_ERR_INVALID_ARG, "%s", _("Invalid parameter count"));
>>          goto cleanup;
>>      }
>> @@ -2494,6 +2494,7 @@ libxlDomainGetSchedulerParameters(virDomainPtr dom, virSchedParameterPtr params,
>>          goto cleanup;
>>      }
>>
>> +    *nparams = XEN_SCHED_CREDIT_NPARAM;
>>      ret = 0;
>>
>>  cleanup:
>> diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
>> index 2bb592d..a65299b 100644
>> --- a/src/lxc/lxc_driver.c
>> +++ b/src/lxc/lxc_driver.c
>> @@ -2234,7 +2234,7 @@ static int lxcGetSchedulerParameters(virDomainPtr domain,
>>      if (driver->cgroup == NULL)
>>          return -1;
>>
>> -    if ((*nparams) != 1) {
>> +    if (*nparams < 1) {
>
>  which is what we are doing here and in all the drivers.
>
> ACK, with that nit
>
> Daniel

Matthias