thr3ads.net - Libguestfs - Re: [Libguestfs] [PATCH] mltools, options: support --allow-discards when decrypting LUKS devices [Jan 2020]

If this information is useful, please help other people find it:
Share via:

Jan Synacek

2020-Jan-21 14:07 UTC

[Libguestfs] [PATCH 0/1] WIP: Support LUKS-encrypted partitions

The following patch attempts to implement sparsification of
LUKS-encrypted partitions. It uses lsblk to pair the underlying LUKS
block device with its mapped name. Also, --allow-discards was added
by default to luks_open().

There are several potential issues that I can think of:

1) If and entire device is encrypted (not just one of more partitions),
the lsblk trick might not work.

2) The --allow-discards is needed to be able to run fstrim on a
decrypted partition. I *think* that it's safe to be added
unconditionally, but I'm not sure. It might be better to just add
another luks_open() variant that uses the option.

3) As it is right now, lsblk is called for every crypto_LUKS device to
see if a corresponding mapping had been created. I *think* it's good
enough, but keeping a list of (blkdev, mapname) in the daemon memory
and adding an API call to retrieve it might be better.

Comments and pointers on how to proceed further are appreciated.

Jan Synacek (1):
  WIP: sparsify: Support LUKS-encrypted partitions

 daemon/listfs.ml | 18 +++++++++++++++---
 daemon/luks.c    |  1 +
 2 files changed, 16 insertions(+), 3 deletions(-)

-- 
2.24.1

Jan Synacek

2020-Jan-21 14:07 UTC

head link

[Libguestfs] [PATCH 1/1] WIP: sparsify: Support LUKS-encrypted partitions

---
 daemon/listfs.ml | 18 +++++++++++++++---
 daemon/luks.c    |  1 +
 2 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/daemon/listfs.ml b/daemon/listfs.ml
index bf4dca6d4..48880f2e5 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
+
   let devices = List.filter is_not_partitioned_device devices in
   let ret = List.filter_map check_with_vfs_type devices in
 
@@ -144,9 +146,19 @@ 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]
+  )
 
   (* A single btrfs device can turn into many volumes. *)
   else if vfs_type = "btrfs" then (
diff --git a/daemon/luks.c b/daemon/luks.c
index d631cb100..1ffeaf293 100644
--- a/daemon/luks.c
+++ b/daemon/luks.c
@@ -110,6 +110,7 @@ luks_open (const char *device, const char *key, const char
*mapname,
   ADD_ARG (argv, i, "-d");
   ADD_ARG (argv, i, tempfile);
   if (readonly) ADD_ARG (argv, i, "--readonly");
+  ADD_ARG (argv, i, "--allow-discards");
   ADD_ARG (argv, i, "luksOpen");
   ADD_ARG (argv, i, device);
   ADD_ARG (argv, i, mapname);
-- 
2.24.1

Richard W.M. Jones

2020-Jan-21 14:53 UTC

head link

Re: [Libguestfs] [PATCH 0/1] WIP: Support LUKS-encrypted partitions

On Tue, Jan 21, 2020 at 03:07:11PM +0100, Jan Synacek
wrote:> The following patch attempts to implement sparsification of
> LUKS-encrypted partitions. It uses lsblk to pair the underlying LUKS
> block device with its mapped name. Also, --allow-discards was added
> by default to luks_open().
> 
> There are several potential issues that I can think of:
> 
> 1) If and entire device is encrypted (not just one of more partitions),
> the lsblk trick might not work.
>
> 2) The --allow-discards is needed to be able to run fstrim on a
> decrypted partition. I *think* that it's safe to be added
> unconditionally,
My concerns about making --allow-discards unconditional would be:

* If old versions of cryptsetup supported it at all.

The option was added in cryptsetup 1.4 in Oct 2011, so that's not an
issue.

* If it breaks cryptsetup in any situation.
>From a casual look at libdevmapper it seems like some devices don'tsupport discards.  libdevmapper issues a log message and actually
retries in certain situations, but I'm not sure if that applies to
luksOpen.

* If people opening luks partitions would want to disallow discards.

Not sure.
> but I'm not sure. It might be better to just add
> another luks_open() variant that uses the option.
We can add optional flags to existing APIs.  This is better than
adding new APIs.  Adding a flag is probably the safest choice since it
punts the decision to the caller and it won't break existing API
users.

To add new opt arguments, add them to the second list (currently []
for luks_open).  See for example:

https://github.com/libguestfs/libguestfs/blob/a754cd43078e43f1a2b5d10e54b684c70c5525d7/generator/actions_core.ml#L213

Because the existing API does not have optional arguments you must add
‘once_had_no_optargs = true’ so that the generator adds the backwards
compatibility API.
> 3) As it is right now, lsblk is called for every crypto_LUKS device to
> see if a corresponding mapping had been created. I *think* it's good
> enough, but keeping a list of (blkdev, mapname) in the daemon memory
> and adding an API call to retrieve it might be better.
I'm fairly sure this _isn't_ a good plan since other APIs would update
and invalidate this cache.  Do the simple thing.  If it's slow then we
can fix that later.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
libguestfs lets you edit virtual machines.  Supports shell scripting,
bindings from many languages.  http://libguestfs.org

Richard W.M. Jones

2020-Jan-21 14:56 UTC

head link

Re: [Libguestfs] [PATCH 1/1] WIP: sparsify: Support LUKS-encrypted partitions

On Tue, Jan 21, 2020 at 03:07:12PM +0100, Jan Synacek
wrote:> ---
>  daemon/listfs.ml | 18 +++++++++++++++---
>  daemon/luks.c    |  1 +
>  2 files changed, 16 insertions(+), 3 deletions(-)
> 
> diff --git a/daemon/listfs.ml b/daemon/listfs.ml
> index bf4dca6d4..48880f2e5 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
> +
>    let devices = List.filter is_not_partitioned_device devices in
>    let ret = List.filter_map check_with_vfs_type devices in
>  
> @@ -144,9 +146,19 @@ 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
As a matter of style I'd put the "let" on a new line.
> +          Some [mnt, Blkid.vfs_type mnt]
> +  )
>  
>    (* A single btrfs device can turn into many volumes. *)
>    else if vfs_type = "btrfs" then (
> diff --git a/daemon/luks.c b/daemon/luks.c
> index d631cb100..1ffeaf293 100644
> --- a/daemon/luks.c
> +++ b/daemon/luks.c
> @@ -110,6 +110,7 @@ luks_open (const char *device, const char *key, const
char *mapname,
>    ADD_ARG (argv, i, "-d");
>    ADD_ARG (argv, i, tempfile);
>    if (readonly) ADD_ARG (argv, i, "--readonly");
> +  ADD_ARG (argv, i, "--allow-discards");
>    ADD_ARG (argv, i, "luksOpen");
>    ADD_ARG (argv, i, device);
>    ADD_ARG (argv, i, mapname);
Seems fine except for considering if --allow-discards should
be a flag (boolean optarg) for the luks_open API.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine.  Supports Linux and Windows.
http://people.redhat.com/~rjones/virt-df/

Richard W.M. Jones

2020-Jan-22 08:09 UTC

head link

Re: [Libguestfs] [PATCH 0/1] WIP: Support LUKS-encrypted partitions

On Tue, Jan 21, 2020 at 02:53:32PM +0000, Richard W.M. Jones
wrote:>
https://github.com/libguestfs/libguestfs/blob/a754cd43078e43f1a2b5d10e54b684c70c5525d7/generator/actions_core.ml#L213
This one is probably a clearer example:

https://github.com/libguestfs/libguestfs/blob/a754cd43078e43f1a2b5d10e54b684c70c5525d7/generator/actions_core.ml#L657

Don't forgot we need once_had_no_optargs = true so that we don't break
existing callers.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine.  Supports Linux and Windows.
http://people.redhat.com/~rjones/virt-df/

Jan Synacek

2020-Jan-22 09:14 UTC

head link

[Libguestfs] [PATCH] mltools, options: support --allow-discards when decrypting LUKS devices

---
 mltools/tools_utils-c.c | 8 ++++----
 mltools/tools_utils.ml  | 6 +++---
 mltools/tools_utils.mli | 2 +-
 options/decrypt.c       | 5 +++--
 options/inspect.c       | 2 +-
 options/options.h       | 2 +-
 6 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/mltools/tools_utils-c.c b/mltools/tools_utils-c.c
index 6c43b8d..1dcebc4 100644
--- a/mltools/tools_utils-c.c
+++ b/mltools/tools_utils-c.c
@@ -36,7 +36,7 @@
 
 #include "options.h"
 
-extern value guestfs_int_mllib_inspect_decrypt (value gv, value gpv, value
keysv);
+extern value guestfs_int_mllib_inspect_decrypt (value gv, value gpv, value
keysv, value allowdiscards);
 extern value guestfs_int_mllib_set_echo_keys (value unitv);
 extern value guestfs_int_mllib_set_keys_from_stdin (value unitv);
 extern value guestfs_int_mllib_rfc3339_date_time_string (value unitv);
@@ -46,9 +46,9 @@ int echo_keys = 0;
 int keys_from_stdin = 0;
 
 value
-guestfs_int_mllib_inspect_decrypt (value gv, value gpv, value keysv)
+guestfs_int_mllib_inspect_decrypt (value gv, value gpv, value keysv, value
allowdiscards)
 {
-  CAMLparam3 (gv, gpv, keysv);
+  CAMLparam4 (gv, gpv, keysv, allowdiscards);
   CAMLlocal2 (elemv, v);
   guestfs_h *g = (guestfs_h *) (intptr_t) Int64_val (gpv);
   struct key_store *ks = NULL;
@@ -86,7 +86,7 @@ guestfs_int_mllib_inspect_decrypt (value gv, value gpv, value
keysv)
     keysv = Field (keysv, 1);
   }
 
-  inspect_do_decrypt (g, ks);
+  inspect_do_decrypt (g, ks, Int_val (allowdiscards));
 
   CAMLreturn (Val_unit);
 }
diff --git a/mltools/tools_utils.ml b/mltools/tools_utils.ml
index 1271802..cb94125 100644
--- a/mltools/tools_utils.ml
+++ b/mltools/tools_utils.ml
@@ -29,7 +29,7 @@ and key_store_key    | KeyString of string
   | KeyFileName of string
 
-external c_inspect_decrypt : Guestfs.t -> int64 -> (string *
key_store_key) list -> unit = "guestfs_int_mllib_inspect_decrypt"
+external c_inspect_decrypt : Guestfs.t -> int64 -> (string *
key_store_key) list -> bool -> unit =
"guestfs_int_mllib_inspect_decrypt"
 external c_set_echo_keys : unit -> unit =
"guestfs_int_mllib_set_echo_keys" "noalloc"
 external c_set_keys_from_stdin : unit -> unit =
"guestfs_int_mllib_set_keys_from_stdin" "noalloc"
 external c_rfc3339_date_time_string : unit -> string =
"guestfs_int_mllib_rfc3339_date_time_string"
@@ -650,7 +650,7 @@ let is_btrfs_subvolume g fs      if g#last_errno () =
Guestfs.Errno.errno_EINVAL then false
     else raise exn
 
-let inspect_decrypt g ks +let inspect_decrypt g ?(allow_discards = false) ks   
(* Turn the keys in the key_store into a simpler struct, so it is possible
    * to read it using the C API.
    *)
@@ -664,7 +664,7 @@ let inspect_decrypt g ks     * function.
    *)
   c_inspect_decrypt g#ocaml_handle (Guestfs.c_pointer g#ocaml_handle)
-    keys_as_list
+    keys_as_list allow_discards
 
 let with_timeout op timeout ?(sleep = 2) fn    let start_t = Unix.gettimeofday
() in
diff --git a/mltools/tools_utils.mli b/mltools/tools_utils.mli
index ab70f58..309a033 100644
--- a/mltools/tools_utils.mli
+++ b/mltools/tools_utils.mli
@@ -194,7 +194,7 @@ val inspect_mount_root_ro : Guestfs.guestfs -> string
-> unit
 val is_btrfs_subvolume : Guestfs.guestfs -> string -> bool
 (** Checks if a filesystem is a btrfs subvolume. *)
 
-val inspect_decrypt : Guestfs.guestfs -> key_store -> unit
+val inspect_decrypt : Guestfs.guestfs -> ?allow_discards:bool ->
key_store -> unit
 (** Simple implementation of decryption: look for any [crypto_LUKS]
     partitions and decrypt them, then rescan for VGs.  This only works
     for Fedora whole-disk encryption. *)
diff --git a/options/decrypt.c b/options/decrypt.c
index 683cf5e..0f24a7a 100644
--- a/options/decrypt.c
+++ b/options/decrypt.c
@@ -71,7 +71,7 @@ make_mapname (const char *device, char *mapname, size_t len)
  * encryption schemes.
  */
 void
-inspect_do_decrypt (guestfs_h *g, struct key_store *ks)
+inspect_do_decrypt (guestfs_h *g, struct key_store *ks, int allowdiscards)
 {
   CLEANUP_FREE_STRING_LIST char **partitions = guestfs_list_partitions (g);
   if (partitions == NULL)
@@ -101,7 +101,8 @@ inspect_do_decrypt (guestfs_h *g, struct key_store *ks)
          * is set?  This might break 'mount_ro'.
          */
         guestfs_push_error_handler (g, NULL, NULL);
-        r = guestfs_luks_open (g, partitions[i], keys[j], mapname);
+        r = guestfs_luks_open_opts (g, partitions[i], keys[j], mapname,
+			            GUESTFS_LUKS_OPEN_OPTS_ALLOWDISCARDS, allowdiscards, -1);
         guestfs_pop_error_handler (g);
         if (r == 0)
           goto opened;
diff --git a/options/inspect.c b/options/inspect.c
index 3de6d70..1dc9fef 100644
--- a/options/inspect.c
+++ b/options/inspect.c
@@ -70,7 +70,7 @@ inspect_mount_handle (guestfs_h *g, struct key_store *ks)
   if (live)
     error (EXIT_FAILURE, 0, _("don’t use --live and -i options
together"));
 
-  inspect_do_decrypt (g, ks);
+  inspect_do_decrypt (g, ks, 0);
 
   char **roots = guestfs_inspect_os (g);
   if (roots == NULL)
diff --git a/options/options.h b/options/options.h
index 9b78302..a63b468 100644
--- a/options/options.h
+++ b/options/options.h
@@ -137,7 +137,7 @@ struct key_store {
 extern void parse_config (void);
 
 /* in decrypt.c */
-extern void inspect_do_decrypt (guestfs_h *g, struct key_store *ks);
+extern void inspect_do_decrypt (guestfs_h *g, struct key_store *ks, int
allowdiscards);
 
 /* in domain.c */
 extern int add_libvirt_drives (guestfs_h *g, const char *guest);
-- 
2.24.1

Jan Synacek

2020-Jan-22 09:16 UTC

head link

[Libguestfs] [PATCH 0/1] sparsify: support LUKS-encrypted partitions

The following patch implements sparsification of LUKS-encrypted
partitions. It uses lsblk to pair the underlying LUKS block device with
its mapped name. To sparsify a LUKS-encrypted device, it needs to be
opened using --allow-discards. Support for the argument was submitted as
a separate patch [1].

[1] https://www.redhat.com/archives/libguestfs/2020-January/msg00158.html

Jan Synacek (1):
  sparsify: support LUKS-encrypted partitions

 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(-)

-- 
2.24.1

Jan Synacek

2020-Jan-22 09:16 UTC

head link

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

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
+
   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]
+  )
 
   (* A single btrfs device can turn into many volumes. *)
   else if vfs_type = "btrfs" then (
diff --git a/daemon/luks.c b/daemon/luks.c
index d631cb100..306b2dcfb 100644
--- a/daemon/luks.c
+++ b/daemon/luks.c
@@ -83,7 +83,7 @@ remove_temp (char *tempfile)
 
 static int
 luks_open (const char *device, const char *key, const char *mapname,
-           int readonly)
+           int readonly, int allowdiscards)
 {
   /* Sanity check: /dev/mapper/mapname must not exist already.  Note
    * that the device-mapper control device (/dev/mapper/control) is
@@ -110,6 +110,7 @@ luks_open (const char *device, const char *key, const char
*mapname,
   ADD_ARG (argv, i, "-d");
   ADD_ARG (argv, i, tempfile);
   if (readonly) ADD_ARG (argv, i, "--readonly");
+  if (allowdiscards) ADD_ARG (argv, i, "--allow-discards");
   ADD_ARG (argv, i, "luksOpen");
   ADD_ARG (argv, i, device);
   ADD_ARG (argv, i, mapname);
@@ -130,15 +131,15 @@ luks_open (const char *device, const char *key, const char
*mapname,
 }
 
 int
-do_luks_open (const char *device, const char *key, const char *mapname)
+do_luks_open (const char *device, const char *key, const char *mapname, int
allowdiscards)
 {
-  return luks_open (device, key, mapname, 0);
+  return luks_open (device, key, mapname, 0, allowdiscards);
 }
 
 int
 do_luks_open_ro (const char *device, const char *key, const char *mapname)
 {
-  return luks_open (device, key, mapname, 1);
+  return luks_open (device, key, mapname, 1, 0);
 }
 
 int
diff --git a/generator/actions_core.ml b/generator/actions_core.ml
index cb7e8dcd0..662b63289 100644
--- a/generator/actions_core.ml
+++ b/generator/actions_core.ml
@@ -5631,7 +5631,8 @@ group scan." };
 
   { 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 = "\
diff --git a/gobject/Makefile.inc b/gobject/Makefile.inc
index 067f861a9..a7b856bee 100644
--- a/gobject/Makefile.inc
+++ b/gobject/Makefile.inc
@@ -86,6 +86,7 @@ guestfs_gobject_headers= \
   include/guestfs-gobject/optargs-is_fifo.h \
   include/guestfs-gobject/optargs-is_file.h \
   include/guestfs-gobject/optargs-is_socket.h \
+  include/guestfs-gobject/optargs-luks_open.h \
   include/guestfs-gobject/optargs-md_create.h \
   include/guestfs-gobject/optargs-mke2fs.h \
   include/guestfs-gobject/optargs-mkfs.h \
@@ -179,6 +180,7 @@ guestfs_gobject_sources= \
   src/optargs-is_fifo.c \
   src/optargs-is_file.c \
   src/optargs-is_socket.c \
+  src/optargs-luks_open.c \
   src/optargs-md_create.c \
   src/optargs-mke2fs.c \
   src/optargs-mkfs.c \
diff --git a/inspector/inspector.c b/inspector/inspector.c
index fa8e721ff..db322a19a 100644
--- a/inspector/inspector.c
+++ b/inspector/inspector.c
@@ -298,7 +298,7 @@ main (int argc, char *argv[])
    * the -i option) because it can only handle a single root.  So we
    * use low-level APIs.
    */
-  inspect_do_decrypt (g, ks);
+  inspect_do_decrypt (g, ks, 0);
 
   free_key_store (ks);
 
diff --git a/sparsify/in_place.ml b/sparsify/in_place.ml
index 7da83dafd..ade3c6843 100644
--- a/sparsify/in_place.ml
+++ b/sparsify/in_place.ml
@@ -62,7 +62,7 @@ let run disk format ignores zeroes ks      error ~exit_code:3
(f_"discard/trim is not supported");
 
   (* Decrypt the disks. *)
-  inspect_decrypt g ks;
+  inspect_decrypt g ~allow_discards:true ks;
 
   (* Discard non-ignored filesystems that we are able to mount, and
    * selected swap partitions.
-- 
2.24.1

Jan Synacek

2020-Jan-22 09:20 UTC

head link

[Libguestfs] [PATCH 0/1] sparsify: support LUKS-encrypted partitions

The following patch implements sparsification of LUKS-encrypted
partitions. It uses lsblk to pair the underlying LUKS block device with
its mapped name. To sparsify a LUKS-encrypted device, it needs to be
opened using --allow-discards. Support for the argument was submitted as
a separate patch [1].

[1] https://www.redhat.com/archives/libguestfs/2020-January/msg00158.html

Jan Synacek (1):
  sparsify: support LUKS-encrypted partitions

 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(-)

-- 
2.24.1

Jan Synacek

2020-Jan-22 09:20 UTC

head link

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

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
+
   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]
+  )
 
   (* A single btrfs device can turn into many volumes. *)
   else if vfs_type = "btrfs" then (
diff --git a/daemon/luks.c b/daemon/luks.c
index d631cb100..306b2dcfb 100644
--- a/daemon/luks.c
+++ b/daemon/luks.c
@@ -83,7 +83,7 @@ remove_temp (char *tempfile)
 
 static int
 luks_open (const char *device, const char *key, const char *mapname,
-           int readonly)
+           int readonly, int allowdiscards)
 {
   /* Sanity check: /dev/mapper/mapname must not exist already.  Note
    * that the device-mapper control device (/dev/mapper/control) is
@@ -110,6 +110,7 @@ luks_open (const char *device, const char *key, const char
*mapname,
   ADD_ARG (argv, i, "-d");
   ADD_ARG (argv, i, tempfile);
   if (readonly) ADD_ARG (argv, i, "--readonly");
+  if (allowdiscards) ADD_ARG (argv, i, "--allow-discards");
   ADD_ARG (argv, i, "luksOpen");
   ADD_ARG (argv, i, device);
   ADD_ARG (argv, i, mapname);
@@ -130,15 +131,15 @@ luks_open (const char *device, const char *key, const char
*mapname,
 }
 
 int
-do_luks_open (const char *device, const char *key, const char *mapname)
+do_luks_open (const char *device, const char *key, const char *mapname, int
allowdiscards)
 {
-  return luks_open (device, key, mapname, 0);
+  return luks_open (device, key, mapname, 0, allowdiscards);
 }
 
 int
 do_luks_open_ro (const char *device, const char *key, const char *mapname)
 {
-  return luks_open (device, key, mapname, 1);
+  return luks_open (device, key, mapname, 1, 0);
 }
 
 int
diff --git a/generator/actions_core.ml b/generator/actions_core.ml
index cb7e8dcd0..662b63289 100644
--- a/generator/actions_core.ml
+++ b/generator/actions_core.ml
@@ -5631,7 +5631,8 @@ group scan." };
 
   { 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 = "\
diff --git a/gobject/Makefile.inc b/gobject/Makefile.inc
index 067f861a9..a7b856bee 100644
--- a/gobject/Makefile.inc
+++ b/gobject/Makefile.inc
@@ -86,6 +86,7 @@ guestfs_gobject_headers= \
   include/guestfs-gobject/optargs-is_fifo.h \
   include/guestfs-gobject/optargs-is_file.h \
   include/guestfs-gobject/optargs-is_socket.h \
+  include/guestfs-gobject/optargs-luks_open.h \
   include/guestfs-gobject/optargs-md_create.h \
   include/guestfs-gobject/optargs-mke2fs.h \
   include/guestfs-gobject/optargs-mkfs.h \
@@ -179,6 +180,7 @@ guestfs_gobject_sources= \
   src/optargs-is_fifo.c \
   src/optargs-is_file.c \
   src/optargs-is_socket.c \
+  src/optargs-luks_open.c \
   src/optargs-md_create.c \
   src/optargs-mke2fs.c \
   src/optargs-mkfs.c \
diff --git a/inspector/inspector.c b/inspector/inspector.c
index fa8e721ff..db322a19a 100644
--- a/inspector/inspector.c
+++ b/inspector/inspector.c
@@ -298,7 +298,7 @@ main (int argc, char *argv[])
    * the -i option) because it can only handle a single root.  So we
    * use low-level APIs.
    */
-  inspect_do_decrypt (g, ks);
+  inspect_do_decrypt (g, ks, 0);
 
   free_key_store (ks);
 
diff --git a/sparsify/in_place.ml b/sparsify/in_place.ml
index 7da83dafd..ade3c6843 100644
--- a/sparsify/in_place.ml
+++ b/sparsify/in_place.ml
@@ -62,7 +62,7 @@ let run disk format ignores zeroes ks      error ~exit_code:3
(f_"discard/trim is not supported");
 
   (* Decrypt the disks. *)
-  inspect_decrypt g ks;
+  inspect_decrypt g ~allow_discards:true ks;
 
   (* Discard non-ignored filesystems that we are able to mount, and
    * selected swap partitions.
-- 
2.24.1

Jan Synacek

2020-Jan-22 09:21 UTC

head link

[Libguestfs] [PATCH 0/1] sparsify: support LUKS-encrypted partitions

The following patch implements sparsification of LUKS-encrypted
partitions. It uses lsblk to pair the underlying LUKS block device with
its mapped name. To sparsify a LUKS-encrypted device, it needs to be
opened using --allow-discards. Support for the argument was submitted as
a separate patch [1].

[1] https://www.redhat.com/archives/libguestfs/2020-January/msg00158.html

Jan Synacek (1):
  sparsify: support LUKS-encrypted partitions

 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(-)

-- 
2.24.1

Jan Synacek

2020-Jan-22 09:21 UTC

head link

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

---
 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
+
   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]
+  )
 
   (* A single btrfs device can turn into many volumes. *)
   else if vfs_type = "btrfs" then (
diff --git a/daemon/luks.c b/daemon/luks.c
index d631cb100..306b2dcfb 100644
--- a/daemon/luks.c
+++ b/daemon/luks.c
@@ -83,7 +83,7 @@ remove_temp (char *tempfile)
 
 static int
 luks_open (const char *device, const char *key, const char *mapname,
-           int readonly)
+           int readonly, int allowdiscards)
 {
   /* Sanity check: /dev/mapper/mapname must not exist already.  Note
    * that the device-mapper control device (/dev/mapper/control) is
@@ -110,6 +110,7 @@ luks_open (const char *device, const char *key, const char
*mapname,
   ADD_ARG (argv, i, "-d");
   ADD_ARG (argv, i, tempfile);
   if (readonly) ADD_ARG (argv, i, "--readonly");
+  if (allowdiscards) ADD_ARG (argv, i, "--allow-discards");
   ADD_ARG (argv, i, "luksOpen");
   ADD_ARG (argv, i, device);
   ADD_ARG (argv, i, mapname);
@@ -130,15 +131,15 @@ luks_open (const char *device, const char *key, const char
*mapname,
 }
 
 int
-do_luks_open (const char *device, const char *key, const char *mapname)
+do_luks_open (const char *device, const char *key, const char *mapname, int
allowdiscards)
 {
-  return luks_open (device, key, mapname, 0);
+  return luks_open (device, key, mapname, 0, allowdiscards);
 }
 
 int
 do_luks_open_ro (const char *device, const char *key, const char *mapname)
 {
-  return luks_open (device, key, mapname, 1);
+  return luks_open (device, key, mapname, 1, 0);
 }
 
 int
diff --git a/generator/actions_core.ml b/generator/actions_core.ml
index cb7e8dcd0..662b63289 100644
--- a/generator/actions_core.ml
+++ b/generator/actions_core.ml
@@ -5631,7 +5631,8 @@ group scan." };
 
   { 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 = "\
diff --git a/gobject/Makefile.inc b/gobject/Makefile.inc
index 067f861a9..a7b856bee 100644
--- a/gobject/Makefile.inc
+++ b/gobject/Makefile.inc
@@ -86,6 +86,7 @@ guestfs_gobject_headers= \
   include/guestfs-gobject/optargs-is_fifo.h \
   include/guestfs-gobject/optargs-is_file.h \
   include/guestfs-gobject/optargs-is_socket.h \
+  include/guestfs-gobject/optargs-luks_open.h \
   include/guestfs-gobject/optargs-md_create.h \
   include/guestfs-gobject/optargs-mke2fs.h \
   include/guestfs-gobject/optargs-mkfs.h \
@@ -179,6 +180,7 @@ guestfs_gobject_sources= \
   src/optargs-is_fifo.c \
   src/optargs-is_file.c \
   src/optargs-is_socket.c \
+  src/optargs-luks_open.c \
   src/optargs-md_create.c \
   src/optargs-mke2fs.c \
   src/optargs-mkfs.c \
diff --git a/inspector/inspector.c b/inspector/inspector.c
index fa8e721ff..db322a19a 100644
--- a/inspector/inspector.c
+++ b/inspector/inspector.c
@@ -298,7 +298,7 @@ main (int argc, char *argv[])
    * the -i option) because it can only handle a single root.  So we
    * use low-level APIs.
    */
-  inspect_do_decrypt (g, ks);
+  inspect_do_decrypt (g, ks, 0);
 
   free_key_store (ks);
 
diff --git a/sparsify/in_place.ml b/sparsify/in_place.ml
index 7da83dafd..ade3c6843 100644
--- a/sparsify/in_place.ml
+++ b/sparsify/in_place.ml
@@ -62,7 +62,7 @@ let run disk format ignores zeroes ks      error ~exit_code:3
(f_"discard/trim is not supported");
 
   (* Decrypt the disks. *)
-  inspect_decrypt g ks;
+  inspect_decrypt g ~allow_discards:true ks;
 
   (* Discard non-ignored filesystems that we are able to mount, and
    * selected swap partitions.
-- 
2.24.1

Richard W.M. Jones

2020-Jan-22 09:50 UTC

head link

Re: [Libguestfs] [PATCH] mltools, options: support --allow-discards when decrypting LUKS devices

On Wed, Jan 22, 2020 at 10:14:38AM +0100, Jan Synacek
wrote:> -val inspect_decrypt : Guestfs.guestfs -> key_store -> unit
> +val inspect_decrypt : Guestfs.guestfs -> ?allow_discards:bool ->
key_store -> unit
>
>  (** Simple implementation of decryption: look for any [crypto_LUKS]
>      partitions and decrypt them, then rescan for VGs.  This only works
>      for Fedora whole-disk encryption. *)
Documentation here needs a short explanation of what the
new allow_discards parameter does, and what the default is.
> diff --git a/options/decrypt.c b/options/decrypt.c
> index 683cf5e..0f24a7a 100644
> --- a/options/decrypt.c
> +++ b/options/decrypt.c
> @@ -71,7 +71,7 @@ make_mapname (const char *device, char *mapname, size_t
len)
>   * encryption schemes.
>   */
>  void
> -inspect_do_decrypt (guestfs_h *g, struct key_store *ks)
> +inspect_do_decrypt (guestfs_h *g, struct key_store *ks, int allowdiscards)
>  {
>    CLEANUP_FREE_STRING_LIST char **partitions = guestfs_list_partitions
(g);
>    if (partitions == NULL)
> @@ -101,7 +101,8 @@ inspect_do_decrypt (guestfs_h *g, struct key_store *ks)
>           * is set?  This might break 'mount_ro'.
>           */
>          guestfs_push_error_handler (g, NULL, NULL);
> -        r = guestfs_luks_open (g, partitions[i], keys[j], mapname);
> +        r = guestfs_luks_open_opts (g, partitions[i], keys[j], mapname,
> +			            GUESTFS_LUKS_OPEN_OPTS_ALLOWDISCARDS, allowdiscards, -1);
Obviously this means this patch depends on the API change :-)

[...]>  /* in decrypt.c */
> -extern void inspect_do_decrypt (guestfs_h *g, struct key_store *ks);
> +extern void inspect_do_decrypt (guestfs_h *g, struct key_store *ks, int
allowdiscards);
> -  inspect_do_decrypt (g, ks);
> +  inspect_do_decrypt (g, ks, 0);
Kind of wonder if we want to use a C bool here instead of an int.

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
http://libguestfs.org/virt-builder.1.html

Possibly Parallel Threads

Search for more possibly parallel threads

Libguestfs - Jan 2020 - Re: [PATCH] mltools, options: support --allow-discards when decrypting LUKS devices

[Libguestfs] [PATCH 0/1] WIP: Support LUKS-encrypted partitions

[Libguestfs] [PATCH 1/1] WIP: sparsify: Support LUKS-encrypted partitions

Re: [Libguestfs] [PATCH 0/1] WIP: Support LUKS-encrypted partitions

Re: [Libguestfs] [PATCH 1/1] WIP: sparsify: Support LUKS-encrypted partitions

Re: [Libguestfs] [PATCH 0/1] WIP: Support LUKS-encrypted partitions

[Libguestfs] [PATCH] mltools, options: support --allow-discards when decrypting LUKS devices

[Libguestfs] [PATCH 0/1] sparsify: support LUKS-encrypted partitions

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

[Libguestfs] [PATCH 0/1] sparsify: support LUKS-encrypted partitions

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

[Libguestfs] [PATCH 0/1] sparsify: support LUKS-encrypted partitions

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

Re: [Libguestfs] [PATCH] mltools, options: support --allow-discards when decrypting LUKS devices

Possibly Parallel Threads