From mboxrd@z Thu Jan  1 00:00:00 1970
From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Subject: Re: [PATCH v3 07/11] tmem: Make the uint64_t oid[3] a
 proper structure: tmem_oid
Date: Mon, 31 Aug 2015 11:37:20 -0400
Message-ID: <20150831153720.GA9931@l.oracle.com>
References: <1440788000-32635-1-git-send-email-konrad.wilk@oracle.com>
	<1440788000-32635-8-git-send-email-konrad.wilk@oracle.com>
	<55E458E5020000780009E574@prv-mh.provo.novell.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
Received: from mail6.bemta3.messagelabs.com ([195.245.230.39])
	by lists.xen.org with esmtp (Exim 4.72)
	(envelope-from <konrad.wilk@oracle.com>) id 1ZWR9G-0003eE-0V
	for xen-devel@lists.xenproject.org; Mon, 31 Aug 2015 15:37:30 +0000
Content-Disposition: inline
In-Reply-To: <55E458E5020000780009E574@prv-mh.provo.novell.com>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Jan Beulich <JBeulich@suse.com>
Cc: andrew.cooper3@citrix.com, wei.liu2@citrix.com, xen-devel@lists.xenproject.org
List-Id: xen-devel@lists.xenproject.org

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
>