Martin Kletzander
2020-Oct-07 07:25 UTC
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
Martin Kletzander
2020-Oct-09 12:29 UTC
Re: [Libguestfs] [PATCH common v2 4/4] options: Ignore errors from guestfs_luks_uuid.
On Wed, Oct 07, 2020 at 09:25:15AM +0200, Martin Kletzander wrote:>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. >Oops, unfortunately that broke the php bindings =D But I'm guessing that again just needs a different include path. I only noticed it now when building Fedora 32 with all the build dependencies installed.>>-- >>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>_______________________________________________ >Libguestfs mailing list >Libguestfs@redhat.com >https://www.redhat.com/mailman/listinfo/libguestfs
Martin Kletzander
2020-Oct-09 12:32 UTC
Re: [Libguestfs] [PATCH common v2 4/4] options: Ignore errors from guestfs_luks_uuid.
On Fri, Oct 09, 2020 at 02:29:02PM +0200, Martin Kletzander wrote:>Oops, unfortunately that broke the php bindings =D But I'm guessing that again >just needs a different include path. I only noticed it now when building Fedora >32 with all the build dependencies installed. >Sorry, stupid me, this should have been a reply to the other thread related to the golang build breakage.
Reasonably Related Threads
- Re: [PATCH common v2 4/4] options: Ignore errors from guestfs_luks_uuid.
- Re: [PATCH common v2 4/4] options: Ignore errors from guestfs_luks_uuid.
- [PATCH common v2 4/4] options: Ignore errors from guestfs_luks_uuid.
- Re: [PATCH common v2 4/4] options: Ignore errors from guestfs_luks_uuid.
- [PATCH common 3/4] options: Ignore errors from guestfs_luks_uuid.