[libvirt] [PATCH 2/6] Clarify the semantic of virDomainGetSchedulerParameters arguments
Matthias Bolte
matthias.bolte at googlemail.com
Wed May 18 16:28:47 UTC 2011
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
More information about the libvir-list
mailing list