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 exnThis 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
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ Libguestfs mailing list [email protected] https://www.redhat.com/mailman/listinfo/libguestfs
