Libguestfs - Sep 2022 - [PATCH libguestfs-common] options: Don't attempt to scan LVs if "lvm2" feature is not available

Since we added support for transparent decryption of LUKS in
commit a232e62dcf ("fish: '-i' option automatically handles
whole-disk
encryption") we have always assumed that the "lvm2" feature is
available and so we could call guestfs_vg_activate_all.  That API
would have failed in the unlikely event that the feature is not
available, preventing opening any (even unencrypted) disk.

The code has changed greatly since then but the basic problem remains.
Test for the "lvm2" feature, otherwise avoid trying to scan LVs.

Reported-by: Feng Li
Fixes: a232e62dcf
---
 options/decrypt.c | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/options/decrypt.c b/options/decrypt.c
index 6fc7760e3..19fe93ce2 100644
--- a/options/decrypt.c
+++ b/options/decrypt.c
@@ -202,8 +202,8 @@ decrypt_mountables (guestfs_h *g, const char * const
*mountables,
 void
 inspect_do_decrypt (guestfs_h *g, struct key_store *ks)
 {
+  const char *lvm2_feature[] = { "lvm2", NULL };
   CLEANUP_FREE_STRING_LIST char **partitions = guestfs_list_partitions (g);
-  CLEANUP_FREE_STRING_LIST char **lvs = NULL;
   bool need_rescan;
 
   if (partitions == NULL)
@@ -211,13 +211,17 @@ inspect_do_decrypt (guestfs_h *g, struct key_store *ks)
 
   need_rescan = decrypt_mountables (g, (const char * const *)partitions, ks);
 
-  if (need_rescan) {
-    if (guestfs_lvm_scan (g, 1) == -1)
+  if (guestfs_feature_available (g, (char **) lvm2_feature) > 0) {
+    CLEANUP_FREE_STRING_LIST char **lvs = NULL;
+
+    if (need_rescan) {
+      if (guestfs_lvm_scan (g, 1) == -1)
+        exit (EXIT_FAILURE);
+    }
+
+    lvs = guestfs_lvs (g);
+    if (lvs == NULL)
       exit (EXIT_FAILURE);
+    decrypt_mountables (g, (const char * const *)lvs, ks);
   }
-
-  lvs = guestfs_lvs (g);
-  if (lvs == NULL)
-    exit (EXIT_FAILURE);
-  decrypt_mountables (g, (const char * const *)lvs, ks);
 }
-- 
2.37.0.rc2

On 09/27/22 14:10, Richard W.M. Jones wrote:
> Since we added support for transparent decryption of LUKS in
> commit a232e62dcf ("fish: '-i' option automatically handles
whole-disk
> encryption") we have always assumed that the "lvm2" feature
is
> available and so we could call guestfs_vg_activate_all.  That API
> would have failed in the unlikely event that the feature is not
> available, preventing opening any (even unencrypted) disk.
> 
> The code has changed greatly since then but the basic problem remains.
> Test for the "lvm2" feature, otherwise avoid trying to scan LVs.
> 
> Reported-by: Feng Li
> Fixes: a232e62dcf
> ---
>  options/decrypt.c | 20 ++++++++++++--------
>  1 file changed, 12 insertions(+), 8 deletions(-)
> 
> diff --git a/options/decrypt.c b/options/decrypt.c
> index 6fc7760e3..19fe93ce2 100644
> --- a/options/decrypt.c
> +++ b/options/decrypt.c
> @@ -202,8 +202,8 @@ decrypt_mountables (guestfs_h *g, const char * const
*mountables,
>  void
>  inspect_do_decrypt (guestfs_h *g, struct key_store *ks)
>  {
> +  const char *lvm2_feature[] = { "lvm2", NULL };
>    CLEANUP_FREE_STRING_LIST char **partitions = guestfs_list_partitions
(g);
> -  CLEANUP_FREE_STRING_LIST char **lvs = NULL;
>    bool need_rescan;
>  
>    if (partitions == NULL)
> @@ -211,13 +211,17 @@ inspect_do_decrypt (guestfs_h *g, struct key_store
*ks)
>  
>    need_rescan = decrypt_mountables (g, (const char * const *)partitions,
ks);
>  
> -  if (need_rescan) {
> -    if (guestfs_lvm_scan (g, 1) == -1)
> +  if (guestfs_feature_available (g, (char **) lvm2_feature) > 0) {
> +    CLEANUP_FREE_STRING_LIST char **lvs = NULL;
> +
> +    if (need_rescan) {
> +      if (guestfs_lvm_scan (g, 1) == -1)
> +        exit (EXIT_FAILURE);
> +    }
> +
> +    lvs = guestfs_lvs (g);
> +    if (lvs == NULL)
>        exit (EXIT_FAILURE);
> +    decrypt_mountables (g, (const char * const *)lvs, ks);
>    }
> -
> -  lvs = guestfs_lvs (g);
> -  if (lvs == NULL)
> -    exit (EXIT_FAILURE);
> -  decrypt_mountables (g, (const char * const *)lvs, ks);
>  }
> 
Reviewed-by: Laszlo Ersek <lersek at redhat.com>

Thanks for fixing this!
Laszlo