Re: [Xen-devel] [PATCH v4 1/8] xen: import ring.h from xen

[Juergen Gross <jgross@xxxxxxxx>]

On 29/03/17 01:54, Stefano Stabellini wrote:
> On Tue, 28 Mar 2017, Juergen Gross wrote:
>> On 28/03/17 00:48, Stefano Stabellini wrote:
>>> On Mon, 27 Mar 2017, Juergen Gross wrote:
>>>> On 24/03/17 18:37, Stefano Stabellini wrote:
>>>>> On Fri, 24 Mar 2017, Juergen Gross wrote:
>>>>>> On 23/03/17 19:22, Stefano Stabellini wrote:
>>>>>>> On Thu, 23 Mar 2017, Paolo Bonzini wrote:
>>>>>>>> On 23/03/2017 14:55, Juergen Gross wrote:
>>>>>>>>> On 23/03/17 14:00, Greg Kurz wrote:
>>>>>>>>>> On Mon, 20 Mar 2017 11:19:05 -0700
>>>>>>>>>> Stefano Stabellini <sstabellini@xxxxxxxxxx> wrote:
>>>>>>>>>>
>>>>>>>>>>> Do not use the ring.h header installed on the system. Instead, 
>>>>>>>>>>> import
>>>>>>>>>>> the header into the QEMU codebase. This avoids problems when QEMU is
>>>>>>>>>>> built against a Xen version too old to provide all the ring macros.
>>>>>>>>>>>
>>>>>>>>>>> Signed-off-by: Stefano Stabellini <stefano@xxxxxxxxxxx>
>>>>>>>>>>> Reviewed-by: Greg Kurz <groug@xxxxxxxx>
>>>>>>>>>>> CC: anthony.perard@xxxxxxxxxx
>>>>>>>>>>> CC: jgross@xxxxxxxx
>>>>>>>>>>> ---
>>>>>>>>>>> NB: The new macros have not been committed to Xen yet. Do not apply 
>>>>>>>>>>> this
>>>>>>>>>>> patch until they do.
>>>>>>>>>>> ---
>>>>>>>>>>
>>>>>>>>>> Looking at your other series for the kernel part of this feature:
>>>>>>>>>>
>>>>>>>>>> https://lkml.org/lkml/2017/3/22/761
>>>>>>>>>>
>>>>>>>>>> I realize that the ring.h header from Xen also exists in the kernel 
>>>>>>>>>> tree... 
>>>>>>>>>>
>>>>>>>>>> Shouldn't all the code that can be used in both kernel and userspace 
>>>>>>>>>> go to a
>>>>>>>>>> header file under include/uapi in the kernel tree ? And then we 
>>>>>>>>>> would import
>>>>>>>>>> it under include/standard-headers/linux in the QEMU tree and we 
>>>>>>>>>> could keep it
>>>>>>>>>> in sync using scripts/update-linux-headers.sh.
>>>>>>>>>>
>>>>>>>>>> Cc'ing Paolo for insights.
>>>>>>>>>
>>>>>>>>> As Xen isn't part of the kernel we don't want that. You can use and/or
>>>>>>>>> build qemu with xen-9pfs backend support on an old Linux kernel 
>>>>>>>>> without
>>>>>>>>> the related frontend.
>>>>>>>>
>>>>>>>> As long as the header changes rarely, I guess it's fine not to go
>>>>>>>> through update-linux-headers.sh.
>>>>>>>
>>>>>>> Very rarely, last time ring.h was changed was 2015, and to introduce a
>>>>>>> new macro (which we don't necessarily need in QEMU).
>>>>>>>
>>>>>>>
>>>>>>>>> OTOH I don't see the advantage of not using the headers from Xen. This
>>>>>>>>> is working for qdisk and pvusb backends and for all the Xen libraries.
>>>>>>>>> Do you expect the 9pfs backend to be used for a qemu version built
>>>>>>>>> against a Xen version not supporting that backend?
>>>>>>>
>>>>>>> Yes, I think that is entirely possible: Xen and QEMU versions can mix
>>>>>>> and match.
>>>>>>>
>>>>>>> Keeping in mind that the 9pfs backend has actually no build dependencies
>>>>>>> on Xen, except for these new ring.h macros, we have the following
>>>>>>> options:
>>>>>>>
>>>>>>> 1) we build the 9pfs backend only for Xen >= 4.9, because of the new
>>>>>>>    macros in ring.h that we need
>>>>>>
>>>>>> Right. You have sent 9pfs support patches for Xen tools. So obviously
>>>>>> you need a proper Xen version to use 9pfs. Why not build qemu against
>>>>>> it? Do you really expect a new Xen being used with an old qemu while
>>>>>> wanting to use new features? That makes no sense for me.
>>>>>  
>>>>> Tools support is needed to setup the frontend/backend connection as
>>>>> usual, but that's not a requirement for building the 9pfs backend. In
>>>>> fact, the backend doesn't need any tools support for it to work. The
>>>>> macro themselves are just a convenience - the backend would work just
>>>>> fine without them. Why restrict the QEMU build gratuitously?
>>>>
>>>> You are duplicating a header without any real benefit I can see. This
>>>> is adding future work for keeping both versions of the header in sync.
>>>>
>>>> In which scenario would you want qemu to support xen-9pfs without being
>>>> built against a Xen version supporting xen-9pfs?
>>>>
>>>> I am not completely against copying the header, I just don't see an
>>>> advantage for any distro or user in doing it.
>>>
>>> I understand your point of view, and honestly it wouldn't be a problem
>>> doing it the way you suggested either. However, I think that going
>>> forward it will be less of a maintenance pain to keep ring.h in sync,
>>> compared to maintaining a versioned build dependency between Xen and
>>> QEMU for the compilation of one PV backend. We do have version checks
>>> in QEMU for Xen compatibility, but not for PV backends or the xenpv
>>> machine yet.
>>
>> For the pvUSB backend I just used a mandatory macro from the header for
>> the #ifdef. The backend will signal support when it was defined during
>> build and will refuse initialization otherwise. Xen tools are able to
>> recoginze qemu support of the backend by looking into Xenstore.
> 
> 
> What do you think of the following:
> 
> diff --git a/hw/9pfs/Makefile.objs b/hw/9pfs/Makefile.objs
> index cab5e94..42530b8 100644
> --- a/hw/9pfs/Makefile.objs
> +++ b/hw/9pfs/Makefile.objs
> @@ -5,6 +5,8 @@ common-obj-y += coth.o cofs.o codir.o cofile.o
>  common-obj-y += coxattr.o 9p-synth.o
>  common-obj-$(CONFIG_OPEN_BY_HANDLE) +=  9p-handle.o
>  common-obj-y += 9p-proxy.o
> -common-obj-$(CONFIG_XEN) += xen-9p-backend.o
> +ifeq ($(shell test $(CONFIG_XEN_CTRL_INTERFACE_VERSION) -ge 40900; echo 
> $$?),0)
> +common-obj-y += xen-9p-backend.o
> +endif

What about:

XEN_9PFS = $(shell test $(CONFIG_XEN_CTRL_INTERFACE_VERSION) -ge 40900
&& echo y)
common-obj-$(XEN_9PFS) += xen-9p-backend.o


Juergen

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