Re: [Libguestfs] [PATCH common v2 4/4] options: Ignore errors from guestfs_luks_uuid.

On Tue, Oct 06, 2020 at 03:25:20PM +0200, Martin Kletzander wrote: > On Mon, Sep 07, 2020 at 10:41:20AM +0100, Richard W.M. Jones wrote: > >For BitLocker disks cryptsetup does not (yet? ever?) support reading > >UUIDs and this function will fail. This does not matter here so just > >ignore the error. > > > >Note there is no error message, cryptsetup simply returns with a bad > >exit code: > > > >><rescue> cryptsetup luksUUID /dev/sda2 > >><rescue> echo $? > >1 > > > >Updates commit bb4a2dc17a78b53437896d4215ae82df8e11b788. > >--- > >options/decrypt.c | 15 ++++++++++++++- > >1 file changed, 14 insertions(+), 1 deletion(-) > > > >diff --git a/options/decrypt.c b/options/decrypt.c > >index 8eb24bc..6b1c0a8 100644 > >--- a/options/decrypt.c > >+++ b/options/decrypt.c > >@@ -25,6 +25,7 @@ > > > >#include <stdio.h> > >#include <stdlib.h> > >+#include <stdbool.h> > >#include <string.h> > >#include <libintl.h> > >#include <error.h> > >@@ -82,11 +83,23 @@ inspect_do_decrypt (guestfs_h *g, struct key_store *ks) > > CLEANUP_FREE char *type = guestfs_vfs_type (g, partitions[i]); > > if (type && > > (STREQ (type, "crypto_LUKS") || STREQ (type, "BitLocker"))) { > >+ bool is_bitlocker = STREQ (type, "BitLocker"); > > char mapname[32]; > > make_mapname (partitions[i], mapname, sizeof mapname); > > > >#ifdef GUESTFS_HAVE_LUKS_UUID > >- CLEANUP_FREE char *uuid = guestfs_luks_uuid (g, partitions[i]); > >+ CLEANUP_FREE char *uuid; > >+ if (!is_bitlocker) > >+ uuid = guestfs_luks_uuid (g, partitions[i]); > >+ else { > >+ /* This may fail for Windows BitLocker disks because > >+ * cryptsetup luksUUID cannot read a UUID (unclear if > >+ * this is a limitation of the format or cryptsetup). > >+ */ > >+ guestfs_push_error_handler (g, NULL, NULL); > >+ uuid = guestfs_luks_uuid (g, partitions[i]); > > I have yet to look at the libguestfs patches, but I do not completely understand > what is the reason for calling "guestfs_luks_uuid" when you know it will fail. > Or is there a case when it might be useful to get the result? Yes I don't know what I was thinking there. It's difficult to revisit and revise patches months later. It would be better as simply: diff --git a/options/decrypt.c b/options/decrypt.c index 8eb24bc..ca2a0bb 100644 --- a/options/decrypt.c +++ b/options/decrypt.c @@ -25,6 +25,7 @@ #include <stdio.h> #include <stdlib.h> +#include <stdbool.h> #include <string.h> #include <libintl.h> #include <error.h> @@ -82,11 +83,19 @@ inspect_do_decrypt (guestfs_h *g, struct key_store *ks) CLEANUP_FREE char *type = guestfs_vfs_type (g, partitions[i]); if (type && (STREQ (type, "crypto_LUKS") || STREQ (type, "BitLocker"))) { + bool is_bitlocker = STREQ (type, "BitLocker"); char mapname[32]; make_mapname (partitions[i], mapname, sizeof mapname); #ifdef GUESTFS_HAVE_LUKS_UUID - CLEANUP_FREE char *uuid = guestfs_luks_uuid (g, partitions[i]); + CLEANUP_FREE char *uuid = NULL; + + /* This fails for Windows BitLocker disks because cryptsetup + * luksUUID cannot read a UUID (unclear if this is a limitation + * of the format or cryptsetup). + */ + if (!is_bitlocker) + uuid = guestfs_luks_uuid (g, partitions[i]); #else const char *uuid = NULL; #endif 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