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
Description: This is a digitally signed message part.