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

Richard W.M. Jones
2016-07-07T15:54:20Z

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

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

Rich.

--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-builder quickly builds VMs from scratch