Re: [Xen-devel] [PATCH 5 of 5] blkif.h: Define and document the request number/size/segments extension

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 09.02.12 at 07:22, "Justin T. Gibbs" <justing@xxxxxxxxxxxxxxxx> wrote:

> On Feb 8, 2012, at 12:48 AM, Jan Beulich wrote:
> 
>>>>> On 07.02.12 at 14:49, Keir Fraser <keir.xen@xxxxxxxxx> wrote:
>>> On 07/02/2012 21:45, "Justin Gibbs" <justing@xxxxxxxxxxxxxxxx> wrote:
>>> 
>>>>> NAK. No backwards incompatible changes allowed in public headers.
>>>> 
>>>> Sorry for the slow reply on this.  I've been experimenting with ways to 
>>>> keep
>>>> legacy
>>>> source compatibility.  After trying lots of things, testing the impact on 
>>>> an
>>>> existing blkfront
>>>> and blkback implementation, I think the best two options are:
>>>> 
>>>> 1. Version the header file and require consumers to declare the interface
>>>> version
>>>>     they are using.  If the version isn't declared, the default, legacy,
>>>> "version 1.0" will
>>>>     be in effect.
>>>> 
>>>>     Positives:   No change in or constant naming conventions.  Data
>>>> structures and
>>>>                         constants for new features are properly hidden from
>>>> legacy implementations.
>>>>     Negatives: Messy #ifdefs
>>> 
>>> We already have this. See use of
>>> __XEN_INTERFACE_VERSION__/__XEN_LATEST_INTERFACE_VERSION__ in the public
>>> headers.
>> 
>> Hmm, I would think these should specifically not be used in the
>> io/ subtree - those aren't definitions of the interface to Xen, but
>> ones shared between the respective backends and frontends.
>> Each interface is (apart from its relying on the ring definitions)
>> entirely self contained.
>> 
>> Jan
> 
> 
> The versioning required allows a driver to declare, "I am compatible
> with any source compatibility breaking changes up to version X of
> the header file".  Declaring support for the latest version does
> not require that a driver implement the new extensions.  Just one
> constant needs to be renamed.  So I don't see this as really altering
> the interface between front and backends (i.e. it is not a "blkif2")

Sure. But pulling in header updates should not require *any* other
source changes, so long as __XEN_INTERFACE_VERSION__ isn't
bumped.

My point about not using __XEN_INTERFACE_VERSION__ under io/
is that these declare protocols that the hypervisor is completely
unaware of, whereas the constant really is meant to control
compatibility with hypervisor changes. In particular is this or any
other change to the protocols under io/ entirely unrelated to the
particular hypervisor version (and hence its interface revision).

It's also unclear to me why simply giving new constants new names
(instead of changing the meaning of existing ones) is such a big
deal - the most strait forward solution doubtlessly is not having
any conditionals in that header, and simply add new things with
new, unambiguous names.

Jan

> If the xen-compat.h behavior is that you can safely import the
> public headers so long as you do not bump __XEN_INTERFACE_VERSION__
> until after you have audited and adjusted for the #ifdef guarded
> changes in the header files, then that is exactly what is needed
> in blkif.h.
> 
> --
> Justin




_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel