[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



On Tue, Oct 06, 2020 at 03:06:54PM +0100, Richard W.M. Jones wrote:
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.


Glad I understood it then, ACK to that.

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

Attachment: signature.asc
Description: PGP signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]