Re: [Xen-devel] [PATCH v3 2/2] qdisk - hw/block/xen_disk: grant copy implementation

[Paulina Szubarczyk <paulinaszubarczyk@xxxxxxxxx>]

On 07/15/2016 07:11 PM, Anthony PERARD wrote:
> On Fri, Jul 15, 2016 at 12:15:45PM +0100, Wei Liu wrote:
> > On Fri, Jul 15, 2016 at 12:28:48PM +0200, Paulina Szubarczyk wrote:
> > > On 07/14/2016 12:37 PM, Wei Liu wrote:
> > > > On Wed, Jun 22, 2016 at 10:38:53AM +0200, Paulina Szubarczyk wrote:
> > > > > diff --git a/configure b/configure
> > > > > index e41876a..355d3fa 100755
> > > > > --- a/configure
> > > > > +++ b/configure
> > > > > @@ -1843,7 +1843,7 @@ fi
> > > > >   # xen probe
> > > > >
> > > > >   if test "$xen" != "no" ; then
> > > > > -  xen_libs="-lxenstore -lxenctrl -lxenguest"
> > > > > +  xen_libs="-lxenstore -lxenctrl -lxenguest -lxengnttab"
> > > > First thing, -lxengnttab should be in xen_stable_libs.
> > > Do I understand correctly that I should add a new variable
> > > "xen_stable_libs"? I could not find it in the qemu tree used anywhere else.
> > Hmm... there is already one in upstream QEMU -- which means you're
> > perhaps using qemu-xen tree.
> >
> > I think all new development should happen on upstream qemu, not in our
> > qemu-xen tree.
> >
> > > > The probing needs to be more sophisticated.
> > > >
> > > > You need to probe the new function your added as well. Just a few lines
> > > > below xen_stable_libs there is a section for hand-coded probing source
> > > > code, which you would need to modify.
> > > >
> > > > Assuming your gnttab change will be merged into 4.8 (the release under
> > > > development at the moment), you need to have a separate program for it.
> > > I will add that.
> > >
> > > > After you've done proper probing, you will know which version of Xen
> > > > this qemu is compiling against.  And then, there should be some fallback
> > > > mechanism to compile and run this qemu with older version of xen. This
> > > > is not too hard because you can guard your code with feature flag or
> > > > ifdef (please consult Stefan and Anthony which method to use).
> > > >
> > > > Feel free to ask questions. I will try my best to explain.
> > > >
> > > > > +    blkdev->feature_grant_copy =
> > > > > +                (xengnttab_grant_copy(blkdev->xendev.gnttabdev, 0, NULL) == 0);
> > > > This is a bit problematic. As this patch stands, it won't compile on
> > > > older version of Xen because there is no such function there.
> > > There is a variable CONFIG_XEN_CTRL_INTERFACE_VERSION holding current
> > > version of the Xen control library this qemu is configured with. It is set
> > > from the configure file. The feature could be guarded with ifdef by a new
> > > variable CONFIG_XEN_LIBS_INTERFACE_VERSION or they could be unified to
> > > CONFIG_XEN_TOOLS_INTERFACE_VERSION to not fill the same value twice.
> > Another way is to provide a stub for this function to always return 0.
> >
> > Please wait for Stefano and Anthony to see which method they prefer.
> I think using CONFIG_XEN_CTRL_INTERFACE_VERSION is fine. With maybe a
> stub of xengnttab_grant_copy() in xen_common.h.
I will add the stub but the structure xengnttab_grant_copy_segment need 
to be repeated in the xen_common.h again, it is also not defined in the 
earlier versions.
Paulina

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