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

[Wei Liu <wei.liu2@xxxxxxxxxx>]

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.

Wei.

> Paulina

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