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/ ___ Libguestfs mailing list Libguestfs@redhat.com https://www.redhat.com/mailman/listinfo/libguestfs
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't support 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 ___ Libguestfs mailing list Libguestfs@redhat.com https://www.redhat.com/mailman/listinfo/libguestfs
[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 ___ Libguestfs mailing list Libguestfs@redhat.com https://www.redhat.com/mailman/listinfo/libguestfs
[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 ___ Libguestfs mailing list Libguestfs@redhat.com https://www.redhat.com/mailman/listinfo/libguestfs
Re: [Libguestfs] [PATCH] nbdkit: fix condition in probe_filter
On Mon, Jan 20, 2020 at 02:05:41PM +0100, Tomáš Golembiovský wrote: > The tests assume probe_filter returns true if the filter is available > (and not the other way around). > > Signed-off-by: Tomáš Golembiovský > --- > v2v/nbdkit.ml | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/v2v/nbdkit.ml b/v2v/nbdkit.ml > index 77d2a506..00122bec 100644 > --- a/v2v/nbdkit.ml > +++ b/v2v/nbdkit.ml > @@ -142,7 +142,7 @@ let common_create ?bandwidth plugin_name plugin_args > plugin_env = > let cmd = >sprintf "%s nbdkit --dump-plugin --filter=%s null >/dev/null" >env_as_string filter_name in > -Sys.command cmd <> 0 > +Sys.command cmd == 0 >in FYI: https://github.com/libguestfs/nbdkit/commit/daaac81a83e3347f6a9c95679f2556952a0a5ade Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-p2v converts physical machines to virtual machines. Boot with a live CD or over the network (PXE) and turn machines into KVM guests. http://libguestfs.org/virt-v2v ___ Libguestfs mailing list Libguestfs@redhat.com https://www.redhat.com/mailman/listinfo/libguestfs
Re: [Libguestfs] [PATCH] vCenter: pass user name to nbdkit curl plugin
On Mon, Jan 20, 2020 at 04:54:35PM +0100, Tomáš Golembiovský wrote: > Signed-off-by: Tomáš Golembiovský > --- > v2v/vCenter.ml | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/v2v/vCenter.ml b/v2v/vCenter.ml > index 89c5579b..d9bf12c1 100644 > --- a/v2v/vCenter.ml > +++ b/v2v/vCenter.ml > @@ -79,7 +79,7 @@ let rec map_source ?bandwidth ?password_file dcPath uri > server path = > >let nbdkit = > Nbdkit.create_curl ?bandwidth ?cookie:session_cookie ~password ~sslverify > - https_url in > + ?user:uri.uri_user https_url in >let qemu_uri = Nbdkit.run nbdkit in Thanks - pushed. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com Fedora Windows cross-compiler. Compile Windows programs, test, and build Windows installers. Over 100 libraries supported. http://fedoraproject.org/wiki/MinGW ___ Libguestfs mailing list Libguestfs@redhat.com https://www.redhat.com/mailman/listinfo/libguestfs
Re: [Libguestfs] [PATCH] nbdkit: fix condition in probe_filter
On Mon, Jan 20, 2020 at 02:05:41PM +0100, Tomáš Golembiovský wrote: > The tests assume probe_filter returns true if the filter is available > (and not the other way around). > > Signed-off-by: Tomáš Golembiovský > --- > v2v/nbdkit.ml | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/v2v/nbdkit.ml b/v2v/nbdkit.ml > index 77d2a506..00122bec 100644 > --- a/v2v/nbdkit.ml > +++ b/v2v/nbdkit.ml > @@ -142,7 +142,7 @@ let common_create ?bandwidth plugin_name plugin_args > plugin_env = > let cmd = >sprintf "%s nbdkit --dump-plugin --filter=%s null >/dev/null" >env_as_string filter_name in > -Sys.command cmd <> 0 > +Sys.command cmd == 0 >in Thanks - now I'm wonder how this worked at all before ... I've pushed this. 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/ ___ Libguestfs mailing list Libguestfs@redhat.com https://www.redhat.com/mailman/listinfo/libguestfs
