Re: [Libguestfs] [libnbd PATCH] api: Add LIBNBD_SHUTDOWN_IMMEDIATE flag

2020-09-17 Thread Eric Blake
On 9/17/20 8:27 AM, Eric Blake wrote: I think for the reasons you've outlined here it's fine to change this from cmd_flags to shutdown_flags.  And the new flag looks useful. Patch looks good, so ACK. I'm leaning towards having the flag value be 0x1 (that is, outside the range of cmd

Re: [Libguestfs] [libnbd PATCH v2 4/5] api: Add STRICT_FLAGS to set_strict_mode

2020-09-17 Thread Richard W.M. Jones
On Thu, Sep 17, 2020 at 08:38:02AM -0500, Eric Blake wrote: > On 9/17/20 8:32 AM, Richard W.M. Jones wrote: > >On Fri, Sep 11, 2020 at 04:49:55PM -0500, Eric Blake wrote: > >>The next strict knob: allow the user to pass unknown flags across the > >>wire (this is different than passing a known flag

Re: [Libguestfs] [libnbd PATCH v2 4/5] api: Add STRICT_FLAGS to set_strict_mode

2020-09-17 Thread Eric Blake
On 9/17/20 8:32 AM, Richard W.M. Jones wrote: On Fri, Sep 11, 2020 at 04:49:55PM -0500, Eric Blake wrote: The next strict knob: allow the user to pass unknown flags across the wire (this is different than passing a known flag at the wrong time). It is interesting to note that NBD only permits

Re: [Libguestfs] [libnbd PATCH v2 4/5] api: Add STRICT_FLAGS to set_strict_mode

2020-09-17 Thread Richard W.M. Jones
On Fri, Sep 11, 2020 at 04:49:55PM -0500, Eric Blake wrote: > The next strict knob: allow the user to pass unknown flags across the > wire (this is different than passing a known flag at the wrong time). > > It is interesting to note that NBD only permits 16 bits of flags, but > we have a

Re: [Libguestfs] [libnbd PATCH v2 5/5] api: Add STRICT_BOUNDS/ZERO_SIZE to nbd_set_strict_mode

2020-09-17 Thread Richard W.M. Jones
ACK series, thanks. 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

Re: [Libguestfs] [libnbd PATCH] api: Add LIBNBD_SHUTDOWN_IMMEDIATE flag

2020-09-17 Thread Eric Blake
On 9/17/20 8:22 AM, Richard W.M. Jones wrote: On Fri, Sep 11, 2020 at 09:31:11AM -0500, Eric Blake wrote: As mentioned in commits 176fc4ea and 609c25f0, our original plan in adding a flags argument to nbd_shutdown was to let us specify different behaviors at the libnbd level, rather than NBD

Re: [Libguestfs] [libnbd PATCH v2 3/5] api: Add nbd_set_strict_mode

2020-09-17 Thread Richard W.M. Jones
On Fri, Sep 11, 2020 at 04:49:54PM -0500, Eric Blake wrote: > @@ -451,7 +464,7 @@ test whether this is the case with > L."; > >"get_tls", { > default_call with > -args = []; ret = REnum (tls_enum); > +args = []; ret = REnum tls_enum; > may_set_error = false; >

Re: [Libguestfs] [libnbd PATCH v2 1/5] api: Add xxx_MASK constant for each Flags type

2020-09-17 Thread Richard W.M. Jones
On Fri, Sep 11, 2020 at 04:49:52PM -0500, Eric Blake wrote: > Document and make it easier for applications to determine which > bitmask values were supported at compile time. Also, generating > bitmask values in hex tends to be more legible than in decimal, as it > makes it obvious that the

Re: [Libguestfs] [libnbd PATCH v2 2/5] generator: Refactor filtering of accepted OFlags

2020-09-17 Thread Richard W.M. Jones
On Fri, Sep 11, 2020 at 04:49:53PM -0500, Eric Blake wrote: > Rather than having to open-code the list of accepted command flags in > the unlocked version of each command, we can store that information in > the generator to produce the check directly in the public API. Also a nice simplification

Re: [Libguestfs] [libnbd PATCH] api: Add LIBNBD_SHUTDOWN_IMMEDIATE flag

2020-09-17 Thread Richard W.M. Jones
On Fri, Sep 11, 2020 at 09:31:11AM -0500, Eric Blake wrote: > As mentioned in commits 176fc4ea and 609c25f0, our original plan in > adding a flags argument to nbd_shutdown was to let us specify > different behaviors at the libnbd level, rather than NBD protocol > flags (for that, the user has

[Libguestfs] [PATCH v3 8/8] lib/canonical-name.c: Hide EINVAL error from underlying API call.

2020-09-17 Thread Richard W.M. Jones
When guestfs_lvm_canonical_lv_name was called with a /dev/dm* or /dev/mapper* name which was not an LV then a noisy error would be printed. This would typically have happened with encrypted disks, and now happens very noticably when inspecting Windows BitLocker- encrypted guests. Using the

[Libguestfs] [PATCH v3 0/8] Windows BitLocker support.

2020-09-17 Thread Richard W.M. Jones
As discussed in the emails today, this is the third version addressing most points from the v1/v2 review. You will need to pair this with the changes in libguestfs-common from this series: https://www.redhat.com/archives/libguestfs/2020-September/msg00050.html Rich.

[Libguestfs] [PATCH v3 2/8] fish: Update documentation to refer to cryptsetup-open/close and BitLocker.

2020-09-17 Thread Richard W.M. Jones
--- fish/guestfish.pod | 30 ++ 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/fish/guestfish.pod b/fish/guestfish.pod index c4fdd17b7..9f086f110 100644 --- a/fish/guestfish.pod +++ b/fish/guestfish.pod @@ -857,34 +857,40 @@ it, eg: Libguestfs has

[Libguestfs] [PATCH v3 3/8] daemon: Rewrite list-filesystems implementation imperatively.

2020-09-17 Thread Richard W.M. Jones
Simple refactoring to make the code clearer, should have no other effect. --- daemon/listfs.ml | 64 1 file changed, 32 insertions(+), 32 deletions(-) diff --git a/daemon/listfs.ml b/daemon/listfs.ml index 903f363d0..3b71471ae 100644 ---

[Libguestfs] [PATCH v3 7/8] daemon: lvm_canonical_lv_name: Return EINVAL if called with non-LV.

2020-09-17 Thread Richard W.M. Jones
Previously callers were unable to distinguish a regular error (like an I/O error) from the case where you call this API on something which is valid but not a logical volume. Set errno to a known value in this case. --- daemon/lvm.c | 2 +- generator/actions_core.ml | 3 ++- 2 files

[Libguestfs] [PATCH v3 4/8] daemon: Ignore BitLocker disks in list-filesystems API.

2020-09-17 Thread Richard W.M. Jones
--- daemon/listfs.ml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/daemon/listfs.ml b/daemon/listfs.ml index 3b71471ae..a4c917d12 100644 --- a/daemon/listfs.ml +++ b/daemon/listfs.ml @@ -141,8 +141,8 @@ and check_with_vfs_type ret device = else if String.is_suffix

[Libguestfs] [PATCH v3 6/8] daemon: Search device-mapper devices for list-filesystems API.

2020-09-17 Thread Richard W.M. Jones
In case any bare filesystems were decrypted using cryptsetup-open, they would appear as /dev/mapper/name devices. Since list-filesystems did not consider those when searching for filesystems, the unencrypted filesystems would not be returned. Note that previously this worked for LUKS because the

[Libguestfs] [PATCH v3 1/8] New APIs: cryptsetup-open and cryptsetup-close.

2020-09-17 Thread Richard W.M. Jones
This commit deprecates luks-open/luks-open-ro/luks-close for the more generic sounding names cryptsetup-open/cryptsetup-close, which also correspond directly to the cryptsetup commands. The optional cryptsetup-open readonly flag is used to replace the functionality of luks-open-ro. The optional

[Libguestfs] [PATCH v3 5/8] daemon: Reimplement list_dm_devices API in OCaml.

2020-09-17 Thread Richard W.M. Jones
Simple refactoring. The only annoying point is requiring an extra module because of OCaml module dependency restrictions. --- .gitignore| 1 + daemon/Makefile.am| 3 ++ daemon/lvm.c | 76 --- daemon/lvm_dm.ml |

Re: [Libguestfs] [PATCH v2 1/7] New APIs: cryptsetup-open and cryptsetup-close.

2020-09-17 Thread Richard W.M. Jones
On Thu, Sep 17, 2020 at 12:10:03PM +0200, Pino Toscano wrote: > At least in my (non extensive) tests with cryptsetup, it seems it can > detect the right format even without --type=format or the luksOpen/etc > aliases. I had a look into this some more and in fact "cryptsetup open" does not infer

Re: [Libguestfs] [PATCH v2 7/7] lib/canonical-name.c: Hide errors.

2020-09-17 Thread Richard W.M. Jones
On Thu, Sep 17, 2020 at 12:12:43PM +0200, Pino Toscano wrote: > On Monday, 7 September 2020 11:44:00 CEST Richard W.M. Jones wrote: > > Fixes “XXX” comment. This turns out to be necessary in order to > > suppress a warning when inspecting Windows BitLocker-encrypted guests. > > > > The warning

Re: [Libguestfs] [PATCH v2 1/7] New APIs: cryptsetup-open and cryptsetup-close.

2020-09-17 Thread Richard W.M. Jones
[Apologies for missing your earlier reply when I reposted the patch series for v2] On Thu, Sep 17, 2020 at 12:10:03PM +0200, Pino Toscano wrote: > Regarding the deprecated functions: wouldn't it be better to have them > in the library, rather than in the daemon? The machinery needed in the >

Re: [Libguestfs] [PATCH v2 7/7] lib/canonical-name.c: Hide errors.

2020-09-17 Thread Pino Toscano
On Monday, 7 September 2020 11:44:00 CEST Richard W.M. Jones wrote: > Fixes “XXX” comment. This turns out to be necessary in order to > suppress a warning when inspecting Windows BitLocker-encrypted guests. > > The warning (which still appears in debugging output even with this > patch) is: > >

Re: [Libguestfs] [PATCH v2 1/7] New APIs: cryptsetup-open and cryptsetup-close.

2020-09-17 Thread Pino Toscano
On Monday, 7 September 2020 11:43:54 CEST Richard W.M. Jones wrote: > This commit deprecates luks-open/luks-open-ro/luks-close for the more > generic sounding names cryptsetup-open/cryptsetup-close, which also > correspond directly to the cryptsetup commands. > > The optional cryptsetup-open