This requires to define the binary name in code because we compile
src/util/viriscsi.c unconditionally.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
---
 meson.build         | 22 ++++++++++++++++------
 src/util/viriscsi.h |  2 ++
 2 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/meson.build b/meson.build
index ed4afd2052..7eeaa689f9 100644
--- a/meson.build
+++ b/meson.build
@@ -824,7 +824,6 @@ endforeach
 optional_programs = [
   'augparse',
   'flake8',
-  'iscsiadm',
   'pdwtags',
 ]
 
@@ -1742,11 +1741,22 @@ if conf.has('WITH_LIBVIRTD')
     error('Need glusterfs (libgfapi) for gluster storage driver')
   endif
 
-  if not get_option('storage_iscsi').disabled() and iscsiadm_prog.found()
-    use_storage = true
-    conf.set('WITH_STORAGE_ISCSI', 1)
-  elif get_option('storage_iscsi').enabled()
-    error('We need iscsiadm for iSCSI storage driver')
+  if not get_option('storage_iscsi').disabled()
+    iscsi_enable = true
+    iscsiadm_prog = find_program('iscsiadm', required: false, dirs: libvirt_sbin_path)
+
+    if not iscsiadm_prog.found()
+      if get_option('storage_iscsi').enabled()
+        error('We need iscsiadm for iSCSI storage driver')
+      else
+        iscsi_enable = false
+      endif
+    endif
+
+    if iscsi_enable
+      use_storage = true
+      conf.set('WITH_STORAGE_ISCSI', 1)
+    endif
   endif
 
   if not get_option('storage_iscsi_direct').disabled() and libiscsi_dep.found()
diff --git a/src/util/viriscsi.h b/src/util/viriscsi.h
index 6d17d139eb..fa7ff280c2 100644
--- a/src/util/viriscsi.h
+++ b/src/util/viriscsi.h
@@ -23,6 +23,8 @@
 
 #include "internal.h"
 
+#define ISCSIADM "iscsiadm"
+
 char *
 virISCSIGetSession(const char *devpath,
                    bool probe)
-- 
2.31.1

On Tue, May 25, 2021 at 10:00:43AM +0200, Pavel Hrdina wrote:
> +  if not get_option('storage_iscsi').disabled()
> +    iscsi_enable = true
> +    iscsiadm_prog = find_program('iscsiadm', required: false, dirs: libvirt_sbin_path)
> +
> +    if not iscsiadm_prog.found()
> +      if get_option('storage_iscsi').enabled()
> +        error('We need iscsiadm for iSCSI storage driver')
> +      else
> +        iscsi_enable = false
> +      endif
> +    endif

I believe this could be rewritten as

  if not get_option('storage_iscsi').disabled()
    iscsi_enable = true
    iscsiadm_prog = find_program('iscsiadm', required:
get_option('storage_iscsi'), dirs: libvirt_sbin_path)

    if not iscsiadm_prog.found()
      iscsi_enable = false
    endif

I see this pattern used elsewhere in the file and it's more concise
while achieving the same result AFAICT.

-- 
Andrea Bolognani / Red Hat / Virtualization

On Tue, May 25, 2021 at 02:36:04AM -0700, Andrea Bolognani wrote:
> On Tue, May 25, 2021 at 10:00:43AM +0200, Pavel Hrdina wrote:
> > +  if not get_option('storage_iscsi').disabled()
> > +    iscsi_enable = true
> > +    iscsiadm_prog = find_program('iscsiadm', required: false, dirs: libvirt_sbin_path)
> > +
> > +    if not iscsiadm_prog.found()
> > +      if get_option('storage_iscsi').enabled()
> > +        error('We need iscsiadm for iSCSI storage driver')
> > +      else
> > +        iscsi_enable = false
> > +      endif
> > +    endif
>
> I believe this could be rewritten as
>
>   if not get_option('storage_iscsi').disabled()
>     iscsi_enable = true
>     iscsiadm_prog = find_program('iscsiadm', required: get_option('storage_iscsi'), dirs: libvirt_sbin_path)
>
>     if not iscsiadm_prog.found()
>       iscsi_enable = false
>     endif
>
> I see this pattern used elsewhere in the file and it's more concise
> while achieving the same result AFAICT.

I just realized that you rewrite this check again in 6/8, so no
matter what it looks like in this commit

  Reviewed-by: Andrea Bolognani <abologna@redhat.com>

-- 
Andrea Bolognani / Red Hat / Virtualization