On Wed, Jan 22, 2020 at 10:54 AM Richard W.M. Jones <rjones@redhat.com> wrote:
On Wed, Jan 22, 2020 at 10:16:14AM +0100, Jan Synacek wrote:
> From: Jan Synacek <jan.synacek@redhat.com>
>
> ---
>  daemon/listfs.ml          | 19 ++++++++++++++++---
>  daemon/luks.c             |  9 +++++----
>  generator/actions_core.ml |  3 ++-
>  gobject/Makefile.inc      |  2 ++
>  inspector/inspector.c     |  2 +-
>  sparsify/in_place.ml      |  2 +-
>  6 files changed, 27 insertions(+), 10 deletions(-)
>
> diff --git a/daemon/listfs.ml b/daemon/listfs.ml
> index bf4dca6d4..a618513e8 100644
> --- a/daemon/listfs.ml
> +++ b/daemon/listfs.ml
> @@ -19,6 +19,7 @@
>  open Printf

>  open Std_utils
> +open Utils

>  (* Enumerate block devices (including MD, LVM, LDM and partitions) and use
>   * vfs-type to check for filesystems on devices.  Some block devices cannot
> @@ -30,6 +31,7 @@ let rec list_filesystems () =

>    (* Devices. *)
>    let devices = Devsparts.list_devices () in
> +

Did you mean to add a blank line here?

No, I didn't notice. I'll fix it.

>    let devices = List.filter is_not_partitioned_device devices in
>    let ret = List.filter_map check_with_vfs_type devices in

> @@ -144,9 +146,20 @@ and check_with_vfs_type device =
>    else if String.is_suffix vfs_type "_member" then
>      None

> -  (* Ignore LUKS-encrypted partitions.  These are also containers, as above. *)
> -  else if vfs_type = "crypto_LUKS" then
> -    None
> +  (* If a LUKS-encrypted partition had been opened, include the corresponding
> +   * device mapper filesystem path. *)
> +  else if vfs_type = "crypto_LUKS" then (
> +    let out = command "lsblk" ["-n"; "-l"; "-o"; "NAME"; device] in
> +      (* Example output: #lsblk -n -l -o NAME /dev/sda5
> +       * sda5
> +       * lukssda5
> +       *)
> +      match String.trimr @@ snd @@  String.split "\n" out with
> +      | "" -> None
> +      | part ->
> +        let mnt = Mountable.of_path @@ "/dev/mapper/" ^ part in
> +          Some [mnt, Blkid.vfs_type mnt]

Now Some doesn't line up with let :-/

Will fix.
 
>    { defaults with
>      name = "luks_open"; added = (1, 5, 1);
> -    style = RErr, [String (Device, "device"); String (Key, "key"); String (PlainString, "mapname")], [];
> +    style = RErr, [String (Device, "device"); String (Key, "key"); String (PlainString, "mapname")], [OBool "allowdiscards"];
> +    once_had_no_optargs = true;
>      optional = Some "luks";
>      shortdesc = "open a LUKS-encrypted block device";
>      longdesc = "\

This is fine.

The rest of this patch is fine.

I'm losing track of what order these patches would be applied
in order to preserve git bisection.  Maybe submit the whole series
in one go for version 2?

The first to apply should be the one that patches the common/ subrepo. I'll submit them all at once next time.

And again, I'm sorry for the mess with the incorrect submissions. I didn't notice that my patches had a wrong address in them.
 
Thank you for the review!
--
Jan Synacek
Software Engineer, Red Hat