Re: [Xen-devel] [PATCH v1 1/5] xen/libcx/tmem: Replace TMEM_RESTORE_NEW with XEN_SYSCTL_TMEM_OP_SET_POOLS

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 19.03.17 at 14:41, <konrad@xxxxxxxxxx> wrote:
> @@ -1908,7 +1908,7 @@ long do_tmem_op(tmem_cli_op_t uops)
>      /* Acquire write lock for all commands at first. */
>      write_lock(&tmem_rwlock);
>  
> -    if ( op.cmd == TMEM_CONTROL )
> +    if ( op.cmd == TMEM_CONTROL || op.cmd == TMEM_RESTORE_NEW )

May I suggest making this a switch()?

> --- a/xen/common/tmem_control.c
> +++ b/xen/common/tmem_control.c
> @@ -402,6 +402,52 @@ static int tmemc_get_pool(int cli_id,
>      return rc ? : idx;
>  }
>  
> +static int tmemc_set_pools(int cli_id,
> +                           XEN_GUEST_HANDLE(xen_tmem_pool_info_t) pools,
> +                           uint32_t len)
> +{
> +    unsigned int i, idx;
> +    int rc = 0;
> +    unsigned int nr = len / sizeof(xen_tmem_pool_info_t);
> +    struct client *client = tmem_client_from_cli_id(cli_id);
> +
> +    if ( len % sizeof(xen_tmem_pool_info_t) )
> +        return -EINVAL;
> +
> +    if ( nr > MAX_POOLS_PER_DOMAIN )
> +        return -E2BIG;
> +
> +    if ( !guest_handle_okay(pools, nr) )
> +        return -EINVAL;
> +
> +    if ( !client )
> +    {
> +        client = client_create(cli_id);
> +        if ( !client )
> +            return -ENOMEM;
> +    }
> +    for ( idx = 0, i = 0; i < nr; i++ )
> +    {
> +        xen_tmem_pool_info_t pool;
> +
> +        if ( __copy_from_guest_offset(&pool, pools, i, 1 ) )
> +            return -EFAULT;
> +
> +        rc = do_tmem_new_pool(cli_id, pool.id, pool.flags.raw,
> +                              pool.uuid[0], pool.uuid[1]);
> +        if ( rc < 0 )
> +            break;
> +
> +        pool.id = rc;
> +        if ( __copy_to_guest_offset(pools, idx, &pool, 1) )
> +            return -EFAULT;
> +        idx++;

idx and i are being incremented in lock step afaics - no need to have
two loop variables.

> --- a/xen/include/public/sysctl.h
> +++ b/xen/include/public/sysctl.h
> @@ -770,6 +770,7 @@ DEFINE_XEN_GUEST_HANDLE(xen_sysctl_psr_cat_op_t);
>  #define XEN_SYSCTL_TMEM_OP_SET_CLIENT_INFO        6
>  #define XEN_SYSCTL_TMEM_OP_GET_POOLS              7
>  #define XEN_SYSCTL_TMEM_OP_QUERY_FREEABLE_MB      8
> +#define XEN_SYSCTL_TMEM_OP_SET_POOLS              9
>  #define XEN_SYSCTL_TMEM_OP_SAVE_BEGIN             10
>  #define XEN_SYSCTL_TMEM_OP_SAVE_GET_NEXT_PAGE     19
>  #define XEN_SYSCTL_TMEM_OP_SAVE_GET_NEXT_INV      20

I guess you also want to adjust the comment next to the "pool"
union member in struct xen_sysctl_tmem_op. I'm also not sure
what to best do about the unused n_pages field - I'd suggest to
at least check it's zero.

Jan


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