Re: [Xen-devel] [PATCH v2 5/5] xen: use libxendevicemodel when available

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: Stefano Stabellini [mailto:sstabellini@xxxxxxxxxx]
> Sent: 06 March 2017 19:14
> To: Paul Durrant <Paul.Durrant@xxxxxxxxxx>
> Cc: 'Stefano Stabellini' <sstabellini@xxxxxxxxxx>; Anthony Perard
> <anthony.perard@xxxxxxxxxx>; xen-devel@xxxxxxxxxxxxxxxxxxxx; qemu-
> devel@xxxxxxxxxx
> Subject: RE: [PATCH v2 5/5] xen: use libxendevicemodel when available
> 
> On Mon, 6 Mar 2017, Paul Durrant wrote:
> > > -----Original Message-----
> > > From: Qemu-devel [mailto:qemu-devel-
> > > bounces+paul.durrant=citrix.com@xxxxxxxxxx] On Behalf Of Paul
> Durrant
> > > Sent: 06 March 2017 09:15
> > > To: 'Stefano Stabellini' <sstabellini@xxxxxxxxxx>
> > > Cc: Anthony Perard <anthony.perard@xxxxxxxxxx>; xen-
> > > devel@xxxxxxxxxxxxxxxxxxxx; qemu-devel@xxxxxxxxxx
> > > Subject: Re: [Qemu-devel] [PATCH v2 5/5] xen: use libxendevicemodel
> when
> > > available
> > >
> > > > -----Original Message-----
> > > > From: Stefano Stabellini [mailto:sstabellini@xxxxxxxxxx]
> > > > Sent: 03 March 2017 20:43
> > > > To: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> > > > Cc: Paul Durrant <Paul.Durrant@xxxxxxxxxx>; xen-
> > > devel@xxxxxxxxxxxxxxxxxxxx;
> > > > qemu-devel@xxxxxxxxxx; Anthony Perard
> <anthony.perard@xxxxxxxxxx>
> > > > Subject: RE: [PATCH v2 5/5] xen: use libxendevicemodel when available
> > > >
> > > > On Fri, 3 Mar 2017, Stefano Stabellini wrote:
> > > > > On Fri, 3 Mar 2017, Paul Durrant wrote:
> > > > > > > -----Original Message-----
> > > > > > > From: Stefano Stabellini [mailto:sstabellini@xxxxxxxxxx]
> > > > > > > Sent: 02 March 2017 22:50
> > > > > > > To: Paul Durrant <Paul.Durrant@xxxxxxxxxx>
> > > > > > > Cc: xen-devel@xxxxxxxxxxxxxxxxxxxx; qemu-devel@xxxxxxxxxx;
> > > Stefano
> > > > > > > Stabellini <sstabellini@xxxxxxxxxx>; Anthony Perard
> > > > > > > <anthony.perard@xxxxxxxxxx>
> > > > > > > Subject: Re: [PATCH v2 5/5] xen: use libxendevicemodel when
> > > available
> > > > > > >
> > > > > > > On Thu, 2 Mar 2017, Paul Durrant wrote:
> > > > > > > > This patch modifies the wrapper functions in xen_common.h to
> use
> > > > the
> > > > > > > > new xendevicemodel interface if it is available along with
> > > > compatibility
> > > > > > > > code to use the old libxenctrl interface if it is not.
> > > > > > > >
> > > > > > > > Signed-off-by: Paul Durrant <paul.durrant@xxxxxxxxxx>
> > > > > > > > ---
> > > > > > > > Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> > > > > > > > Cc: Anthony Perard <anthony.perard@xxxxxxxxxx>
> > > > > > > >
> > > > > > > > v2:
> > > > > > > > - Add a compat define for xenforeignmemory_close() since this
> is
> > > > now
> > > > > > > >   used.
> > > > > > > > ---
> > > > > > > >  include/hw/xen/xen_common.h | 115
> > > > > > > +++++++++++++++++++++++++++++++-------------
> > > > > > > >  xen-common.c                |   8 +++
> > > > > > > >  2 files changed, 90 insertions(+), 33 deletions(-)
> > > > > > > >
> > > > > > > > diff --git a/include/hw/xen/xen_common.h
> > > > > > > b/include/hw/xen/xen_common.h
> > > > > > > > index 31cf25f..48444e5 100644
> > > > > > > > --- a/include/hw/xen/xen_common.h
> > > > > > > > +++ b/include/hw/xen/xen_common.h
> > > > > > > > @@ -9,6 +9,7 @@
> > > > > > > >  #undef XC_WANT_COMPAT_EVTCHN_API
> > > > > > > >  #undef XC_WANT_COMPAT_GNTTAB_API
> > > > > > > >  #undef XC_WANT_COMPAT_MAP_FOREIGN_API
> > > > > > > > +#undef XC_WANT_COMPAT_DEVICEMODEL_API
> > > > > > > >
> > > > > > > >  #include <xenctrl.h>
> > > > > > > >  #include <xenstore.h>
> > > > > > > > @@ -26,48 +27,95 @@ extern xc_interface *xen_xc;
> > > > > > > >   * We don't support Xen prior to 4.2.0.
> > > > > > > >   */
> > > > > > > >
> > > > > > > > +#if CONFIG_XEN_CTRL_INTERFACE_VERSION < 490
> > > > > > > > +
> > > > > > > > +typedef xc_interface xendevicemodel_handle;
> > > > > > > > +
> > > > > > > > +#define xendevicemodel_open(l, f) xen_xc
> > > > > > > > +
> > > > > > > > +#define xendevicemodel_map_io_range_to_ioreq_server \
> > > > > > > > +    xc_hvm_map_io_range_to_ioreq_server
> > > > > > > > +#define
> xendevicemodel_unmap_io_range_from_ioreq_server \
> > > > > > > > +    xc_hvm_unmap_io_range_from_ioreq_server
> > > > > > > > +#define xendevicemodel_map_pcidev_to_ioreq_server \
> > > > > > > > +    xc_hvm_map_pcidev_to_ioreq_server
> > > > > > > > +#define xendevicemodel_unmap_pcidev_from_ioreq_server
> \
> > > > > > > > +    xc_hvm_unmap_pcidev_from_ioreq_server
> > > > > > > > +#define xendevicemodel_create_ioreq_server \
> > > > > > > > +    xc_hvm_create_ioreq_server
> > > > > > > > +#define xendevicemodel_destroy_ioreq_server \
> > > > > > > > +    xc_hvm_destroy_ioreq_server
> > > > > > > > +#define xendevicemodel_get_ioreq_server_info \
> > > > > > > > +    xc_hvm_get_ioreq_server_info
> > > > > > > > +#define xendevicemodel_set_ioreq_server_state \
> > > > > > > > +    xc_hvm_set_ioreq_server_state
> > > > > > > > +#define xendevicemodel_set_pci_intx_level \
> > > > > > > > +    xc_hvm_set_pci_intx_level
> > > > > > > > +#define xendevicemodel_set_pci_link_route \
> > > > > > > > +    xc_hvm_set_pci_link_route
> > > > > > > > +#define xendevicemodel_set_isa_irq_level \
> > > > > > > > +    xc_hvm_set_isa_irq_level
> > > > > > > > +#define xendevicemodel_inject_msi \
> > > > > > > > +    xc_hvm_inject_msi
> > > > > > > > +#define xendevicemodel_set_mem_type \
> > > > > > > > +    xc_hvm_set_mem_type
> > > > > > > > +#define xendevicemodel_track_dirty_vram \
> > > > > > > > +    xc_hvm_track_dirty_vram
> > > > > > > > +#define xendevicemodel_modified_memory \
> > > > > > > > +    xc_hvm_modified_memory
> > > > > > >
> > > > > > > It does build correctly now for Xen < 4.9, however it breaks 
> > > > > > > against
> > > > > > > xen-unstable:
> > > > > > >
> > > > > > >   ERROR: configure test passed without -Werror but failed with -
> > > Werror.
> > > > > > >          This is probably a bug in the configure script. The 
> > > > > > > failing
> command
> > > > > > >          will be at the bottom of config.log.
> > > > > > >          You can run configure with --disable-werror to bypass 
> > > > > > > this
> check.
> > > > > > >
> > > > > > > and config.log says:
> > > > > > >
> > > > > > >   config-temp/qemu-conf.c: In function 'main':
> > > > > > >   config-temp/qemu-conf.c:32:3: error: implicit declaration of
> function
> > > > > > > 'xc_hvm_set_mem_type' [-Werror=implicit-function-declaration]
> > > > > > >   config-temp/qemu-conf.c:32:3: error: nested extern declaration
> of
> > > > > > > 'xc_hvm_set_mem_type' [-Werror=nested-externs]
> > > > > > >   config-temp/qemu-conf.c:34:3: error: implicit declaration of
> function
> > > > > > > 'xc_hvm_inject_msi' [-Werror=implicit-function-declaration]
> > > > > > >   config-temp/qemu-conf.c:34:3: error: nested extern declaration
> of
> > > > > > > 'xc_hvm_inject_msi' [-Werror=nested-externs]
> > > > > > >   config-temp/qemu-conf.c:35:3: error: implicit declaration of
> function
> > > > > > > 'xc_hvm_create_ioreq_server' [-Werror=implicit-function-
> > > declaration]
> > > > > > >   config-temp/qemu-conf.c:35:3: error: nested extern declaration
> of
> > > > > > > 'xc_hvm_create_ioreq_server' [-Werror=nested-externs]
> > > > > > >
> > > > > > >
> > > > > > > With -DXC_WANT_COMPAT_DEVICEMODEL_API=1:
> > > > > > >
> > > > > > >   In file included from /local/qemu-
> > > > > > > upstream/include/hw/xen/xen_backend.h:4:0,
> > > > > > >                    from hw/block/xen_disk.c:27:
> > > > > > >   /local/qemu-upstream/include/hw/xen/xen_common.h: In
> function
> > > > > > > 'xen_set_mem_type':
> > > > > > >   /local/qemu-upstream/include/hw/xen/xen_common.h:78:5:
> error:
> > > > implicit
> > > > > > > declaration of function 'xc_hvm_set_mem_type' [-
> Werror=implicit-
> > > > function-
> > > > > > > declaration]
> > > > > > >
> > > > > >
> > > > > > Ah... Do you have Anthony's patch?
> > > > > >
> > > > > >
> > > >
> > >
> http://xenbits.xen.org/gitweb/?p=xen.git;a=commit;h=9970e98ace48574701
> > > > f7e2286fb67090481a3fec
> > > > > >
> > > > > > I suspect that may be the problem you're seeing above.
> > > > >
> > > > > Yes, I do have it. I am still having build troubles with v5 too. I am
> > > >
> > > > I meant v3
> > > >
> > > >
> > > > > attaching my build test script: feel free to use it to test the build 
> > > > > of
> > > > > a single QEMU branch against multiple Xen versions. It requires one
> Xen
> > > > > repository checked out and built for each Xen version. I have just
> > > > > added the Xen 4.9 tests, so they might be wrong somehow, but the
> > > others
> > > > > are known to work.
> > >
> > > Your script is lacking Anthony's fix. You need to patch your 4.9 configure
> > > clause accordingly. I.e. you need -
> > > I$(XEN_ROOT)/tools/libs/devicemodel/include and -
> > > L$(XEN_ROOT)/tools/libs/devicemodel in the --extra-cflags.
> >
> > Sorry, I meant the latter in --extra-ldflags and I notice you do actually 
> > have
> that. It's just the -I that's missing.
> 
> Thank you, you are right, that fixes the build against 4.9, but there
> are problems with all other versions:
> 
> [4.3, 4.4]
> 
> In file included from /local/qemu-
> upstream/include/hw/xen/xen_backend.h:4:0,
>                  from hw/block/xen_disk.c:27:
> /local/qemu-upstream/include/hw/xen/xen_common.h:42:5: error:
> unknown type name 'ioservid_t'
> /local/qemu-upstream/include/hw/xen/xen_common.h:49:49: error:
> unknown type name 'ioservid_t'
> /local/qemu-upstream/include/hw/xen/xen_common.h:58:49: error:
> unknown type name 'ioservid_t'
> /local/qemu-upstream/include/hw/xen/xen_common.h:66:49: error:
> unknown type name 'ioservid_t'
> /local/qemu-upstream/include/hw/xen/xen_common.h:74:49: error:
> unknown type name 'ioservid_t'
> /local/qemu-upstream/include/hw/xen/xen_common.h:82:49: error:
> unknown type name 'ioservid_t'
> /local/qemu-upstream/include/hw/xen/xen_common.h:90:49: error:
> unknown type name 'ioservid_t'
> /local/qemu-upstream/include/hw/xen/xen_common.h:96:49: error:
> unknown type name 'ioservid_t'
> make: *** [hw/block/xen_disk.o] Error 1
> 

That's one I'd not considered. I need to cater for an old enough xen where even 
the old ioreq server API is not available.

> 
> [4.5, 4.6, 4.7, 4.8]
> 
> In file included from /local/qemu-
> upstream/include/hw/xen/xen_backend.h:4:0,
>                  from hw/block/xen_disk.c:27:
> /local/qemu-upstream/include/hw/xen/xen_common.h: In function
> 'xendevicemodel_destroy_ioreq_server':
> /local/qemu-upstream/include/hw/xen/xen_common.h:92:5: error: implicit
> declaration of function 'xc_hvm__destroy_ioreq_server' [-Werror=implicit-
> function-declaration]
> /local/qemu-upstream/include/hw/xen/xen_common.h:92:5: error: nested
> extern declaration of 'xc_hvm__destroy_ioreq_server' [-Werror=nested-
> externs]
> cc1: all warnings being treated as errors

That's a typo (double _) which I've now found.

V4 coming up.

Thanks,

  Paul


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