[Libguestfs] [PATCH 1/3] mllib: add checking for btrfs subvolume

[Pino Toscano]

On Thursday 07 July 2016 16:54:20 Richard W.M. Jones wrote:
> On Thu, Jul 07, 2016 at 06:04:14PM +0300, Maxim Perevedentsev wrote:
> > This is needed to skip btrfs subvolumes from output
> > of list_filesystems where device is needed.
> > ---
> >  mllib/common_utils.ml  | 10 ++++++++++
> >  mllib/common_utils.mli |  3 +++
> >  2 files changed, 13 insertions(+)
> > 
> > diff --git a/mllib/common_utils.ml b/mllib/common_utils.ml
> > index 77b9acd..30fc5cd 100644
> > --- a/mllib/common_utils.ml
> > +++ b/mllib/common_utils.ml
> > @@ -922,3 +922,13 @@ let inspect_mount_root g ?mount_opts_fn root =
> >  
> >  let inspect_mount_root_ro =
> >    inspect_mount_root ~mount_opts_fn:(fun _ -> "ro")
> > +
> > +let is_btrfs_subvolume g fs =
> > +  let device = g#mountable_device fs in
> > +  let subvol =
> > +    try
> > +      g#mountable_subvolume fs
> > +    with Guestfs.Error msg as exn ->
> > +      if g#last_errno () = Guestfs.Errno.errno_EINVAL then ""
> > +      else raise exn in
> > +  device <> "" && subvol <> ""
> 
> Firstly I think device <> "" is always true.
> 
> Secondly it wasn't obvious to me until I thought about it that you're
> testing if subvol is not equal to the value ("") returned three lines
> earlier.
> 
> How about this, which is type safe and a lot simpler:
> 
> let is_btrfs_subvolume g fs =
>   let device = g#mountable_device fs in
>   try
>     ignore (g#mountable_subvolume fs); true
>   with Guestfs.Error msg as exn ->
>     if g#last_errno () = Guestfs.Errno.errno_EINVAL then false
>     else raise exn

This is what I was writing too, but you were faster :)
I think this will fail to build as "device" is unused, so the first
line should be:

  (* Make sure we can determine the device of the mountable. *)
  ignore (g#mountable_device fs);

> Apart from that, this is just a translation of fish/options.c:
> display_mountpoints_on_failure into OCaml, so ACK if that was fixed.

Indeed, LGTM with the above fix.

Thanks,
-- 
Pino Toscano
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part.
URL: <http://listman.redhat.com/archives/libguestfs/attachments/20160707/b869e121/attachment.sig>