Libguestfs - Sep 2022 - [PATCH libguestfs] daemon: Make vg_scan and lvm_scan no-ops if no LVM feature

If the LVM ("lvm2") feature is not available, these calls would fail.
Really they ought to be part of the "lvm2" optgroup which would cause
the generator to call reply_with_unavailable_feature and generate the
correct ENOTSUP error.  When vgscan was originally added in 2010 it
was not added to the optgroup, and when lvm_scan was later added in
2018 and deprecating vgscan, the same mistake was copied.

Before this commit they will try to run the lvm pvscan command which
will fail returning some other error (instead of ENOTSUP).

Fix this by turning the calls into no-ops if the LVM feature is not
available, since scanning for LVM objects when there is no LVM can be
safely turned into a no-op.

See also
https://listman.redhat.com/archives/libguestfs/2022-September/thread.html#29908

Reported-by: Laszlo Ersek
Fixes: 55dfcb2211 ("New API: lvm_scan, deprecate vgscan")
Fixes: 9752039e52 ("New API: vgscan")
---
 daemon/lvm.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/daemon/lvm.c b/daemon/lvm.c
index 72c59c3a1b..c273946d34 100644
--- a/daemon/lvm.c
+++ b/daemon/lvm.c
@@ -651,6 +651,13 @@ do_lvm_scan (int activate)
   const char *argv[MAX_ARGS];
   size_t i = 0;
 
+  /* Historically this call was never added to the "lvm2" optgroup.
+   * Rather than changing that and have the small risk of breaking
+   * callers, just make it into a no-op if LVM is not available.
+   */
+  if (optgroup_lvm2_available () == 0)
+    return 0;
+
   ADD_ARG (argv, i, "lvm");
   ADD_ARG (argv, i, "pvscan");
   ADD_ARG (argv, i, "--cache");
-- 
2.37.0.rc2

On 09/27/22 14:10, Richard W.M. Jones wrote:
> If the LVM ("lvm2") feature is not available, these calls would
fail.
> Really they ought to be part of the "lvm2" optgroup which would
cause
> the generator to call reply_with_unavailable_feature and generate the
> correct ENOTSUP error.  When vgscan was originally added in 2010 it
> was not added to the optgroup, and when lvm_scan was later added in
> 2018 and deprecating vgscan, the same mistake was copied.
> 
> Before this commit they will try to run the lvm pvscan command which
> will fail returning some other error (instead of ENOTSUP).
> 
> Fix this by turning the calls into no-ops if the LVM feature is not
> available, since scanning for LVM objects when there is no LVM can be
> safely turned into a no-op.
> 
> See also
>
https://listman.redhat.com/archives/libguestfs/2022-September/thread.html#29908
> 
> Reported-by: Laszlo Ersek
> Fixes: 55dfcb2211 ("New API: lvm_scan, deprecate vgscan")
> Fixes: 9752039e52 ("New API: vgscan")
> ---
>  daemon/lvm.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/daemon/lvm.c b/daemon/lvm.c
> index 72c59c3a1b..c273946d34 100644
> --- a/daemon/lvm.c
> +++ b/daemon/lvm.c
> @@ -651,6 +651,13 @@ do_lvm_scan (int activate)
>    const char *argv[MAX_ARGS];
>    size_t i = 0;
>  
> +  /* Historically this call was never added to the "lvm2"
optgroup.
> +   * Rather than changing that and have the small risk of breaking
> +   * callers, just make it into a no-op if LVM is not available.
> +   */
> +  if (optgroup_lvm2_available () == 0)
> +    return 0;
> +
>    ADD_ARG (argv, i, "lvm");
>    ADD_ARG (argv, i, "pvscan");
>    ADD_ARG (argv, i, "--cache");
> 
Acked-by: Laszlo Ersek <lersek at redhat.com>

Thanks!
Laszlo