[Libguestfs] [PATCH 02/12] generator: Convert relevant arguments from Device to Mountable

[Matthew Booth]

On Fri, 2013-02-08 at 12:29 +0000, Richard W.M. Jones wrote:
> On Fri, Feb 08, 2013 at 11:57:03AM +0000, Matthew Booth wrote:
> > On Fri, 2013-02-08 at 11:42 +0000, Richard W.M. Jones wrote:
> > > On Thu, Feb 07, 2013 at 03:57:48PM +0000, Matthew Booth wrote:
> > > > This change updates the api style of all apis which should take Mountable
> > > > descriptions rather than block devices. It also updates the documentation
> > > > accordingly, but doesn't implement any functional changes.
> > > 
> > > [...]
> > > > +=head2 MOUNTABLES
> > > 
> > > I don't think we should document Mountable at all, at least not at
> > > this stage.  The reason is twofold: (a) People shouldn't be trying to
> > > construct these strings, except for device/partition names which is
> > > existing practice.  More importantly, (b) we may want to change or
> > > replace the btrfsvol strings in future if we decide that we got it
> > > wrong (or if btrfs changes something), and we can only do that if we
> > > can be sure that people aren't trying to construct them from scratch.
> > > 
> > > So I would only ack this patch if we removed two things: the
> > > 'btrfsvol:' example that appears in one of the function descriptions,
> > > and the whole MOUNTABLES section in the documentation.
> > 
> > I don't think we should remove it from the documentation. If we include
> > this patch it will form part of the API, so it should be documented. The
> > format is also designed to be extensible, so changing it in the future
> > isn't a problem. btrfsvol need never be removed because it serves an
> > actual need. If we discover a better way to do this in the future, or a
> > more general way that solves many needs, we can add new mountables type
> > without breaking the api.
> 
> I didn't mean to suggest that we cannot change the format in future,
> but the question is whether it should form a part of the API (or if we
> should tell people to treat any colon-prefixed strings as opaque).
> 
> The reason is this: suppose we discover that "btrfsvol:" is wrong for
> some reason.  We add a new prefix (eg. "btrfsvol2:").  But if
> "btrfsvol:" is part of the API then we have to support both formats
> forever, even if it turns out that the old format has some sort of
> unresolvable difficulty in correct usage.  If it's not part of the API
> we can just forget about "btrfsvol:" since no API will return it, so
> it can never be passed back to us.
> 
> The other question is if callers would ever need to construct
> "btrfsvol:" strings.  That's unclear.  You can already use the subvol=
> mount option to mount btrfs subvolumes, but that's complicated for the
> caller because they have to deal with a specific filesystem type [ZFS,
> the only other filesystem I know about that has subvolumes, doesn't
> even use the standard 'mount' command, so that's not a good example].
> "btrfsvol:" is not any better -- callers still need to know about a
> specific filesystem.
> 
> It may be better to add an OString "subvolume" option to mount to
> prevent people needing to construct "btrfsvol:" strings themselves,
> and to my mind that would mean we could leave the colon-prefixed
> strings undocumented (at least, for now - this doesn't stop us from
> adding formal documentation later).

I don't think this works in practise. The format of these strings are
extremely simple to work out. If somebody writes a program which
generates them, then we change them, we've unnecessarily broken that
program.

The argument about discovering btrfsvol: is wrong is the same argument
you can make about the entire libguestfs api guarantee. It's possible
it's wrong, but it doesn't matter because we can leave it in place and
add another one.

We could possibly add a subvolume optional argument to mount, but that's
a separate discussion. If btrfsvol: strings are exposed and people can
construct them, they are already effectively part of the API.

Matt