[PATCH] vstorage: remove build time checks for runtime binaries

[Michal Privoznik]

On 1/19/21 7:34 AM, Nikolay Shirokovskiy wrote:
> Accoring to current agreement mentioned in list recently [1]. Now
> vstorage driver will be build in default devs environment and also can
> be included into CI. This also closes quite old abandoned thread on
> alternative checks for binaries in case of this same driver [2].
> 
> [1] https://www.redhat.com/archives/libvir-list/2021-January/msg00750.html
> [2] https://www.redhat.com/archives/libvir-list/2020-July/msg00697.html
> 
> Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy at virtuozzo.com>
> ---
>   meson.build                            | 22 ++--------------------
>   src/storage/storage_backend_vstorage.c |  4 ++--
>   2 files changed, 4 insertions(+), 22 deletions(-)
> 
> diff --git a/meson.build b/meson.build
> index b5277b4..e3e7ff7 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -1957,26 +1957,8 @@ if conf.has('WITH_LIBVIRTD')
>     endif
>   
>     if not get_option('storage_vstorage').disabled()
> -    vstorage_enable = true
> -
> -    foreach name : ['vstorage', 'vstorage-mount', 'umount']
> -      set_variable(
> -        '@0 at _prog'.format(name.underscorify()),
> -        find_program(name, required: get_option('storage_vstorage'), dirs: libvirt_sbin_path)
> -      )
> -      if not get_variable('@0 at _prog'.format(name.underscorify())).found()
> -        vstorage_enable = false
> -      endif
> -    endforeach
> -
> -    if vstorage_enable
> -      use_storage = true
> -      conf.set('WITH_STORAGE_VSTORAGE', 1)
> -      foreach name : ['vstorage', 'vstorage-mount', 'umount']
> -        path = get_variable('@0 at _prog'.format(name.underscorify())).path()
> -        conf.set_quoted(name.to_upper(), path)
> -      endforeach
> -    endif
> +    use_storage = true
> +    conf.set('WITH_STORAGE_VSTORAGE', 1)
>     endif
>   
>     if not get_option('storage_zfs').disabled()
> diff --git a/src/storage/storage_backend_vstorage.c b/src/storage/storage_backend_vstorage.c
> index 6cff9f1..7c67407 100644
> --- a/src/storage/storage_backend_vstorage.c
> +++ b/src/storage/storage_backend_vstorage.c
> @@ -65,7 +65,7 @@ virStorageBackendVzPoolStart(virStoragePoolObjPtr pool)
>   
>       mode = g_strdup_printf("%o", def->target.perms.mode);
>   
> -    cmd = virCommandNewArgList(VSTORAGE_MOUNT,
> +    cmd = virCommandNewArgList("vstorage-mount",
>                                  "-c", def->source.name,
>                                  def->target.path,
>                                  "-m", mode,


These two hunks look good, although I have slight preference to keep 
VSTORAGE_MOUNT and maybe just:

   #define VSTORAGE_MOUNT "vstorage-mount"

somewhere in storage_backend_vstorage.c; but that is just cosmetics and 
we have existing examples of your approach too:

   libvirt.git $ git grep "virCommandNew.*(\"" | wc -l
   27


> @@ -129,7 +129,7 @@ virStorageBackendVzPoolStop(virStoragePoolObjPtr pool)
>       if ((rc = virStorageBackendVzIsMounted(pool)) != 1)
>           return rc;
>   
> -    cmd = virCommandNewArgList(UMOUNT, def->target.path, NULL);
> +    cmd = virCommandNewArgList("umount", def->target.path, NULL);
>       return virCommandRun(cmd, NULL);
>   }
>   
> 

A-ha! So if FS driver is disabled, then UMOUNT is undefined? Well, an 
empty string, whatever. If it is so then:

Reviewed-by: Michal Privoznik <mprivozn at redhat.com>

Michal