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

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



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

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

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel veillard com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/


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