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

[Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>]

On Tue, Sep 01, 2015 at 01:04:11AM -0600, Jan Beulich wrote:
> >>> On 31.08.15 at 18:14, <konrad.wilk@xxxxxxxxxx> wrote:
> >> >> > --- 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:
> >> 
> >> Did you perhaps forget to adjust include/xlat.lst? I'm certain adding a 
> > prefix won't
> > 
> > Yes. And ran 'make distclean' before compiling.
> > 
> >> break everything, albeit iirc there's not going to be a 
> >> compat_xen_tmem_oid, 
> > but
> >> the xen_ prefix in such cases gets replaced by compat_.
> > 
> > Which is part of the problem - in 'xen_tmem_oid' the 'xen' parts got 
> > replaced.
> > And the 'compat_tmem_oid' gets generated but the tool is trying to find
> > 'compat_xen_tmem_oid' (at least that is the error) and can't find it now.
> 
> Perhaps that's because the structure definition is public/sysctl.h
> (note how the error message refers to public/tmem.h)? Since you
> move it in a later patch, could you put it into public/tmem.h right
> away?

There is no need for the compat layer with the patch
("tmem: Make the uint64_t oid[3] a proper structure: ") which
alters 'xen_sysctl_tmem_op':

+struct xen_tmem_oid {
+    uint64_t oid[3];
+};
+typedef struct xen_tmem_oid xen_tmem_oid_t;
+DEFINE_XEN_GUEST_HANDLE(xen_tmem_oid_t);
+
 struct xen_sysctl_tmem_op {
     uint32_t cmd;       /* IN: XEN_SYSCTL_TMEM_OP_* . */
     int32_t pool_id;    /* IN: 0 by default unless _SAVE_*, RESTORE_* .*/
@@ -746,7 +752,7 @@ struct xen_sysctl_tmem_op {
     uint32_t arg1;      /* IN: If not applicable to command use 0. */
     uint32_t arg2;      /* IN: If not applicable to command use 0. */
     uint32_t pad;       /* Padding so structure is the same under 32 and 64. */
-    uint64_t oid[3];    /* IN: If not applicable to command use 0s. */
+    xen_tmem_oid_t oid; /* IN: If not applicable to command use 0s. */
     XEN_GUEST_HANDLE_64(char) buf; /* IN/OUT: Buffer to save and restore ops. 
*/
 };
 typedef struct xen_sysctl_tmem_op xen_sysctl_tmem_op_t;

There is only need for the compat layer once I move the 'xen_tmem_oid_t' to
the tmem.h header file - and there is where the introduction in xlat.lst is 
done:

index 8cedee7..ace1d53 100644
--- a/xen/include/xlat.lst
+++ b/xen/include/xlat.lst
@@ -103,6 +103,7 @@
 !      sched_poll                      sched.h
 ?      sched_remote_shutdown           sched.h
 ?      sched_shutdown                  sched.h
+?      xen_tmem_oid                    tmem.h
 !      tmem_op                         tmem.h
 ?      t_buf                           trace.h
 ?      vcpu_get_physid                 vcpu.h

.. and which fails compilation.

I think I am confusing the matters by responding on the thread that
refers to the introduction of 'struct xen_tmem_oid' in sysctl (which works
fine btw), but I am tripping over problems with the next patch
('tmem: Use 'struct xen_tmem_oid' for every user')
which moves the 'struct xen_tmem_oid' in the tmem.h to be used by
both hypercalls (tmem.h and sysctl.h).

Attaching both patches.
> 
> Jan
>

Attachment: 0001-tmem-Make-the-uint64_t-oid-3-a-proper-structure-xen_.patch
Description: Text document

Attachment: 0002-tmem-Use-struct-xen_tmem_oid-for-every-user.patch
Description: Text document

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