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

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

 



2011/5/18 Daniel Veillard <veillard@xxxxxxxxxx>:
> 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

--
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]