Re: [PATCH 2 of 4 v3] blkif.h: Provide more complete documentation of the blkif interface

[Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>]

On Wed, Feb 22, 2012 at 11:14:31AM +0000, Ian Campbell wrote:
> On Tue, 2012-02-21 at 21:36 +0000, Konrad Rzeszutek Wilk wrote:
> [...]
> > > With that in mind, I believe that all of the required information
> > > about discard is still present in blkif.h, but perhaps presented
> > > differently or moved to different sections.  Can you be more
> > > specific about the information that you feel is missing here and
> > > in the other places you noted below?
> > 
> > The original statement about how the backend would determine how
> > to expose this information.
> 
> Please can you quote the actual text of the statements which you think
> are missing and/or should be retained so we know precisely what you are
> talking about.

Ah, it is in the Notes section. However the information that is not present in 4) is

	The discard-alignment parameter indicates how many bytes
	the beginning of the partition is offset from the internal allocation unit's
	natural alignment. Do not confuse this with natural disk alignment offset.

[granted we don't expose the natural disk alignment offset right now, so maybe
that comment is useless]

and 6):
	Devices that support discard functionality may
	internally allocate space using units that are bigger than the logical block
	size.  The discard-granularity parameter indicates the size of the internal
	allocation unit in bytes if reported by the device. Otherwise the
	discard-granularity will be set to match the device's physical block size.
	It is the minimum size you can discard.

[though parts of that comment are there]

> 
> > > The comment block here mirrors the language of all the other feature
> > > flags.  Do you believe they should be changed in some way too?
> > 
> > Well, the desire (mine) is to include as much details (or even more) in the
> > spec so that it can be read as a novel if desired.
> > 
> > But I will defer to Ian - if he is OK then this shouldn't gate
> > the acceptance of this patch which is a step forward.
> 
> Perhaps details which aren't part of the formal spec, e.g.
> implementation tips, details of various OSes implementation choices etc
> could be kept elsewhere, e.g. on the wiki?

On the Document day when I started documenting the PSE != PAT and the page
pinning procedure - I had no idea where to stick that explanation of what goes behind
the scene and how to properly do it so Ian Jackson suggested in the header of
the function. That is an implementation tip of how to properly do it
so you don't shoot yourself in the foot.

Where should something similar for blkback be so it can grow along with the source?

> 
> The problem with including these sorts of things in the spec is that you
> then have to get all pedantic about which statements are normative and
> which are just quirks or details of a particular valid implementation of
> the spec etc.

I cheerish when folks are enthuastic to be pedantic about it so at the end
the result has all t's crossed and i's dotted. Perhaps we should stick
in the document a line saying: "BEWARE: PATCHES TO THIS AREA ARE GOING TO BE
REVIEWED EXTREMELY CLOSE. SO GET YOUR ASBESTOS UNDERWEAR ON!"

> 
> Ian.