Re: Oldest supported Xen version in upstream QEMU (Was: Re: [Minios-devel] [PATCH v2 0/15+5+5] Begin to disentangle libxenctrl and provide some stable libraries)

[Stefano Stabellini <stefano.stabellini@eu.citrix.com>]

On Thu, 24 Sep 2015, Ian Campbell wrote:
> On Thu, 2015-09-24 at 20:33 +0100, Stefano Stabellini wrote:
> > On Thu, 24 Sep 2015, Ian Campbell wrote:
> > > On Wed, 2015-09-23 at 18:36 +0100, Stefano Stabellini wrote:
> > > > On Wed, 23 Sep 2015, Ian Campbell wrote:
> > > > > On Tue, 2015-09-22 at 22:31 +0100, Stefano Stabellini wrote: 
> > > > > > The oldest Xen version I build-test for every pull request is
> > Xen 4.0.0,
> > > 
> > > I setup a build trees for 4.0 thru 4.6 yesterday to test this, what
> > a
> > > pain 4.1 and 4.0 are to build with a modern gcc! (Mostly newer
> > compiler
> > > warnings and mostly, but not all, fixes which I could just backport
> > > from newer Xen, the exceptions were a couple of things which were
> > > removed before they needed to be fixed)
> > > 
> > > > > > I think it is very reasonable to remove anything older than
> > that.
> > > > > > I am OK with removing Xen 4.0.0 too, but I would like a
> > warning to be
> > > > > > sent ahead of time to qemu-devel to see if anybody complains.
> > > > > 
> > > > > There is not much point in removing <=3.4 support and keeping
> > 4.0, since
> > > > > 4.0.0 was the last one which used a plain int as a handle,
> > anything older
> > > > > than 4.0.0 is trivial if 4.0.0 is supported.
> > > > > 
> > > > > One approach I am considering in order to keep 4.0.0 support
> > and earlier
> > > > > was to turn the "int fd" for <=4.0 into a pointer by having the
> > open
> > > > > wrapper do malloc(sizeof int) and the using wrappers do
> > xc_foo(*handle).
> > > > > 
> > > > > This way all the different variants take pointers and we have
> > less hoops to
> > > > > jump through to typedef everything in the correct way for each
> > variant.
> > > > > 
> > > > > If you would rather avoid doing that then I think dropping
> > 4.0.0 support
> > > > > would be the way to go and I'll send a mail to qemu-devel.
> > > >  
> > > > I would rather drop 4.0 support.
> > > 
> > > Supporting 4.0 didn't turn out quite as ugly as I had feared.
> > > 
> > > So before I send an email to qemu-devel to propose dropping 4.0
> > what do
> > > you think of the following which handles the evtchn case, there is
> > a
> > > similar patch for gnttab and a (yet to be written) patch for the
> > > foreign memory mapping case.
> > > 
> > > The relevant bit for this discussion is the 4.0.0 definition of
> > > xenevtchn_open in xen_common.h, the rest is just adjusting it to
> > use
> > > the API of the new library (for reasons explained in the commit
> > > message).
> > 
> > I think that it is OK in principle.
> > 
> > 
> > > diff --git a/include/hw/xen/xen_common.h
> > b/include/hw/xen/xen_common.h
> > > index 5923290..5700c1b 100644
> > > --- a/include/hw/xen/xen_common.h
> > > +++ b/include/hw/xen/xen_common.h
> > > @@ -39,17 +39,37 @@ static inline void *xc_map_foreign_bulk(int
> > xc_handle, uint32_t dom, int prot,
> > >  #if CONFIG_XEN_CTRL_INTERFACE_VERSION < 410
> > >  
> > >  typedef int XenXC;
> > > -typedef int XenEvtchn;
> > > +typedef int xenevtchn_handle;
> > >  typedef int XenGnttab;
> >  
> > ...
> > 
> > > @@ -108,17 +128,20 @@ static inline void xs_close(struct xs_handle
> > *xsh)
> > >  #else
> > >  
> > >  typedef xc_interface *XenXC;
> > > -typedef xc_evtchn *XenEvtchn;
> > > +typedef xc_evtchn xenevtchn_handle;
> > >  typedef xc_gnttab *XenGnttab;
> > >  
> > 
> > There is no reasons why we couldn't have a small compat shim on Xen >
> > 4.6 too, so I would change the definition of XenEvtchn for newer
> > versions of Xen and avoid some of the renaming in this patch to reduce
> > the changes.
> > 
> > For example, why not define xc_evtchn_fd as xenevtchn_fd for Xen > 4.6?
> > So that we don't need to go and rename all the call sites?
> 
> The idea was that the code would use the new stable API names from the
> stable libraries going forward, rather than using a shim to turn the
> stable APIs back into the old ones.

I don't think that is very important from QEMU's point of view, using a
shim is just fine, especially if it reduces the patch size to 5 lines of
code :-)