Re: [PATCH v3 07/11] tmem: Make the uint64_t oid[3] a proper structure: tmem_oid

[Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>]

On Mon, Aug 31, 2015 at 05:38:45AM -0600, Jan Beulich wrote:
> >>> On 28.08.15 at 20:53, <konrad.wilk@oracle.com> wrote:
> > @@ -2714,7 +2719,7 @@ long do_tmem_op(tmem_cli_op_t uops)
> >              write_unlock(&tmem_rwlock);
> >              read_lock(&tmem_rwlock);
> >  
> > -            oidp = (struct oid *)&op.u.gen.oid[0];
> > +            oidp = (struct tmem_oid *)&op.u.gen.oid[0];
> 
> AIUI this is going to go away later anyway, but generally I think it
> would be better to hide explicit casts like this by using container_of()
> when possible.

I am not sure if it will be possible as the structure that contains
the tmem_oid is an anonymous structure within an union:

struct tmem_op {
    blah
    union {
	struct {
		tmem_oid_t oid;
	} gen;
    } u;
}

And the 'container_of' macro looks to require only one level of
nesting.
> 
> > --- a/xen/include/public/sysctl.h
> > +++ b/xen/include/public/sysctl.h
> > @@ -737,6 +737,12 @@ DEFINE_XEN_GUEST_HANDLE(xen_sysctl_psr_cat_op_t);
> >  #define XEN_SYSCTL_TMEM_OP_RESTORE_PUT_PAGE       32
> >  #define XEN_SYSCTL_TMEM_OP_RESTORE_FLUSH_PAGE     33
> >  
> > +struct tmem_oid {
> > +    uint64_t oid[3];
> > +};
> > +typedef struct tmem_oid tmem_oid_t;
> > +DEFINE_XEN_GUEST_HANDLE(tmem_oid_t);
> 
> I know this is going to be a boring mechanical thing, but I'd really
> like to see this to be xen_tmem_oid (and alike), especially since
> you intend to also use the type for th non-tools part of the
> interface.

This throws a wrench in the compat autogeneration tool.

I keep on getting:

Fields of 'compat_xen_tmem_oid' not found in 'compat/tmem.h'
and it failing to generate compat/.xlat/tmem.h file. Sticking an prefix of
xen to the 'common/compat/tmem_xen.c' or just leaving it as is did not help:


 #include <xen/hypercall.h>
 #include <compat/tmem.h>
 
-#define xen_tmem_op tmem_op
-/*CHECK_tmem_op;*/
-#undef xen_tmem_op
+#define xen_xen_tmem_oid xen_tmem_oid
+CHECK_xen_tmem_oid;
+#undef xen_xen_tmem_oid

Thoughts?
> 
> Jan
>