Re: [Libguestfs] [PATCH 02/12] generator: Convert relevant arguments from Device to Mountable
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).
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW