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

["Justin T. Gibbs" <justing@xxxxxxxxxxxxxxxx>]

On Feb 9, 2012, at 2:25 AM, Jan Beulich wrote:

>>>> 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.

Perhaps my language wasn't clear.  The change to a blkif driver is would
only necessary if __XEN_INTERFACE_VERSION__ is bumped.  I was
trying to say that when the bump is made, the needed change would be
quite small.

> 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.

The confusion arrises because we'd need to, in order to keep the
terminology consistent, rename existing constants too.  More
specifically, a "request" in the legacy driver is a "logical device
operation".  I would like to retain that convention even if a logical
request consumes multiple ring entries.  However, if
BLKIF_MAX_SEGMENTS_PER_REQUEST comes to mean the
number of segments in the first ring entry of a request, then we
need to come up with another name for the maximum number of
segments in a "logical request".  Changing REQUEST to something
else (e.g. "OP") means even more surgery to existing drivers so
that they consistently use the new terminology.  So, in my alternative
proposal, I chose to shorten SEGMENTS to SEGS instead.  However,
this still leaves BLKIF_MAX_SEGMENTS_PER_REQUEST around
for folks to use incorrectly.

I'd rather just version BLKIF_MAX_SEGMENTS_PER_REQUEST.
I'll post a trial patch with this change shortly.

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