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
[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
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
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 inAs 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
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#L213This 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
[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
[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
[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
[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
[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
[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
[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
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
Maybe Matching Threads
- [PATCH 0/1] WIP: Support LUKS-encrypted partitions
- [PATCH 1/1] sparsify: support LUKS-encrypted partitions
- Re: [PATCH 1/1] sparsify: support LUKS-encrypted partitions
- [PATCH 1/1] WIP: sparsify: Support LUKS-encrypted partitions
- [PATCH 0/1] sparsify: support LUKS-encrypted partitions