Re: [PATCH v1 08/12] tmem: Handle 'struct tmem_info' as a seperate field in the

[Konrad Rzeszutek Wilk <konrad@kernel.org>]

On Wed, Sep 28, 2016 at 06:56:40AM -0600, Jan Beulich wrote:
> >>> On 28.09.16 at 11:42, <konrad.wilk@oracle.com> wrote:
> > Note: We still have to do this awkward 'guest_handle_cast'
> > otherwise it will not compile on ARM - which defines _two_
> > of these macros (__guest_handle_64_xen_sysctl_tmem_client_t
> > and __guest_handle_xen_sysctl_tmem_client_t). And if cast is
> > not used then a compile error comes up as we use the wrong one.
> 
> This seems suspicious, but it's hard to judge without knowing what
> exactly the errors were.

tmem_control.c: In function ‘tmem_control’:
tmem_control.c:426:9: error: incompatible type for argument 2 of
‘tmemc_set_client_info’
         ret = tmemc_set_client_info(op->cli_id, op->u.client);
         ^
tmem_control.c:302:12: note: expected
‘__guest_handle_xen_sysctl_tmem_client_t’ but argument is of type
‘__guest_handle_64_xen_sysctl_tmem_client_t’
 static int tmemc_set_client_info(domid_t cli_id,
            ^
tmem_control.c:432:9: error: incompatible type for argument 2 of
‘tmemc_get_client_info’
         ret = tmemc_get_client_info(op->cli_id, op->u.client);
         ^

> 
> > --- a/xen/common/tmem_control.c
> > +++ b/xen/common/tmem_control.c
> > @@ -258,7 +258,7 @@ 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,
> > +static int __tmemc_set_var(struct client *client,
> >                             XEN_GUEST_HANDLE_PARAM(xen_sysctl_tmem_client_t) buf)
> >  {
> >      domid_t cli_id = client->cli_id;
> > @@ -267,11 +267,6 @@ static int __tmemc_set_var(struct client *client, uint32_t subop,
> >  
> >      ASSERT(client);
> >  
> > -    if ( subop != XEN_SYSCTL_TMEM_OP_SET_CLIENT_INFO )
> > -    {
> > -        tmem_client_warn("tmem: unknown subop %d for tmemc_set_var\n", subop);
> > -        return -1;
> > -    }
> >      if ( copy_from_guest(&info, buf, 1) )
> >          return -EFAULT;
> 
> The adjustments above look pretty unrelated to the purpose of the
> patch, but well - you're the maintainer of this code.

<smacks himself in the head> That is indeed wrong. This should have
been done in the previous patch: "tmem/libxc: Squash
XEN_SYSCTL_TMEM_OP_[SET|SAVE].."


> 
> Jan
> 

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