Re: [Xen-devel] [PATCH v2 6/9] tmem/libxc: Squash XEN_SYSCTL_TMEM_OP_[SET|SAVE]..

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 30/09/16 19:11, Konrad Rzeszutek Wilk wrote:
> diff --git a/xen/common/tmem_control.c b/xen/common/tmem_control.c
> index fc20a9f..151d8ef 100644
> --- a/xen/common/tmem_control.c
> +++ b/xen/common/tmem_control.c
> @@ -258,43 +258,56 @@ static int tmemc_list(domid_t cli_id, 
> tmem_cli_va_param_t buf, uint32_t len,
>      return 0;
>  }
>  
> -static int __tmemc_set_var(struct client *client, uint32_t subop, uint32_t 
> arg1)
> +static int __tmemc_set_client_info(struct client *client,
> +                                   XEN_GUEST_HANDLE(xen_tmem_client_t) buf)
>  {
>      domid_t cli_id = client->cli_id;
>      uint32_t old_weight;
> +    xen_tmem_client_t info = { };
>  
> -    switch (subop)
> +    ASSERT(client);

This ASSERT() is useless after have already dereferenced client to get
cli_id.  Best to defer the initialisation of the domid and keep the
assert like this.

> @@ -303,13 +316,35 @@ static int tmemc_set_var(domid_t cli_id, uint32_t 
> subop, uint32_t arg1)
>      {
>          client = tmem_client_from_cli_id(cli_id);
>          if ( client )
> -            ret = __tmemc_set_var(client, subop, arg1);
> +            ret = __tmemc_set_client_info(client, info);
>      }
>      return ret;
>  }
>  
> -static int tmemc_save_subop(int cli_id, uint32_t pool_id,
> -                        uint32_t subop, tmem_cli_va_param_t buf, uint32_t 
> arg1)
> +static int tmemc_get_client_info(int cli_id,
> +                                 XEN_GUEST_HANDLE(xen_tmem_client_t) info)
> +{
> +    struct client *client = tmem_client_from_cli_id(cli_id);
> +
> +    if ( client )
> +    {
> +        if ( copy_to_guest(info, &client->info, 1) )
> +            return  -EFAULT;
> +    }
> +    else
> +    {
> +            static const xen_tmem_client_t generic = {
> +                                                 .version = 
> TMEM_SPEC_VERSION,
> +                                                 .maxpools = 
> MAX_POOLS_PER_DOMAIN };

There is some weird alignment/tabs going on here.

With these fixed, Acked-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel