[Libguestfs] [PATCH v2 2/2] sparsify: support LUKS-encrypted partitions

[Pino Toscano]

On Monday, 27 January 2020 13:12:36 CET Jan Synacek wrote:
> ---
>  daemon/listfs.ml          | 18 +++++++++++++++---
>  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, 26 insertions(+), 10 deletions(-)

The changes here ought to be split in different patches:
1) adding the optional parameter to luks_open
2) the adaptation of inspect_do_decrypt to the changes in common (which
   will be updated together)
3) the rest

The changes for #1 and #2 seems OK, so it will be easy to merge them.

> diff --git a/daemon/listfs.ml b/daemon/listfs.ml
> index bf4dca6d4..4f1af474a 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
> @@ -144,9 +145,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]
> +  )

I'm slightly skeptic about these changes: a LUKS device can contain
different things: a single filesystem, a device with a partition table
and filesystems, a LV, etc. Hence, we cannot assume the /dev/mapper
name will be a filesystem.

>    if (readonly) ADD_ARG (argv, i, "--readonly");
> +  if (allowdiscards) ADD_ARG (argv, i, "--allow-discards");

According to its release notes, cryptsetup added this option in 2.0.0:
it seems ~2y old, however non-that-old distros already have it.
So IMHO there is no need to add an existance check for this option,
which can be added later only in case we need to support older
versions.

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