Re: [Xen-devel] [PATCH v1] xen: fix HYPERVISOR_dm_op() prototype

[Sergey Dyasli <sergey.dyasli@citrix.com>]

On Tue, 2017-06-06 at 02:03 -0600, Jan Beulich wrote:
> > > > On 05.06.17 at 10:41, <sergey.dyasli@citrix.com> wrote:
> > 
> > --- a/arch/x86/include/asm/xen/hypercall.h
> > +++ b/arch/x86/include/asm/xen/hypercall.h
> > @@ -49,6 +49,7 @@
> >  #include <xen/interface/physdev.h>
> >  #include <xen/interface/platform.h>
> >  #include <xen/interface/xen-mca.h>
> > +#include <xen/interface/hvm/dm_op.h>
> 
> Why?
> 
> > @@ -474,7 +475,7 @@ HYPERVISOR_xenpmu_op(unsigned int op, void *arg)
> >  
> >  static inline int
> >  HYPERVISOR_dm_op(
> > -	domid_t dom, unsigned int nr_bufs, void *bufs)
> > +	domid_t dom, unsigned int nr_bufs, struct xen_dm_op_buf *bufs)
> 
> All you need above here is a forward declaration of the structure.
> We should really avoid forcing source files to include all sorts of
> headers without actually needing anything from them.

Thank you for the good suggestion. I will fix this in v2.

> 
> > --- a/include/xen/arm/hypercall.h
> > +++ b/include/xen/arm/hypercall.h
> > @@ -38,6 +38,7 @@
> >  #include <xen/interface/xen.h>
> >  #include <xen/interface/sched.h>
> >  #include <xen/interface/platform.h>
> > +#include <xen/interface/hvm/dm_op.h>
> 
> Same here.
> 
> > @@ -53,7 +54,8 @@ int HYPERVISOR_physdev_op(int cmd, void *arg);
> >  int HYPERVISOR_vcpu_op(int cmd, int vcpuid, void *extra_args);
> >  int HYPERVISOR_tmem_op(void *arg);
> >  int HYPERVISOR_vm_assist(unsigned int cmd, unsigned int type);
> > -int HYPERVISOR_dm_op(domid_t domid, unsigned int nr_bufs, void *bufs);
> > +int HYPERVISOR_dm_op(domid_t domid, unsigned int nr_bufs,
> > +		     struct xen_dm_op_buf *bufs);
> 
> How come you get away with changing a declaration without
> also changing the matching definition?

The definition of HYPERVISOR_dm_op() is in arch/arm/xen/hypercall.S

-- 
Thanks,
Sergey