[PATCH] storage: find vstorage-mount binary in runtime

[Nikolay Shirokovskiy]

On 14.07.2020 11:50, Daniel P. Berrangé wrote:
> On Tue, Jul 14, 2020 at 10:26:00AM +0300, Nikolay Shirokovskiy wrote:
>> This allows us to use CI for vstorage driver without installing Virtuozzo
>> Storage packages. This way we can leave aside license considerations.
>>
>> By the way we need to change configure defaults from 'check' to 'no' otherwise
>> vstorage driver will be build on any system with umount binary which is not
>> expected I guess.
>>
>> Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy at virtuozzo.com>
>> ---
>>  m4/virt-storage-vstorage.m4            | 17 +----------------
>>  src/storage/storage_backend_vstorage.c |  9 ++++++++-
>>  2 files changed, 9 insertions(+), 17 deletions(-)
>>
>> diff --git a/m4/virt-storage-vstorage.m4 b/m4/virt-storage-vstorage.m4
>> index e3b3bb4..cf0a543 100644
>> --- a/m4/virt-storage-vstorage.m4
>> +++ b/m4/virt-storage-vstorage.m4
>> @@ -21,30 +21,19 @@ dnl
>>  AC_DEFUN([LIBVIRT_STORAGE_ARG_VSTORAGE], [
>>    LIBVIRT_ARG_WITH_FEATURE([STORAGE_VSTORAGE],
>>                             [Virtuozzo Storage backend for the storage driver],
>> -                           [check])
>> +                           [no])
>>  ])
> 
> Why was this changed to "no".  It means we'll never enable the storage
> driver without an explicit --with-storage-vstorage arg passed

But with "check" we will have this driver built on any system with umount
as I mentioned in commit message. I guess this is not desired given the
driver actually needs some more binaries that are not usually installed.

Nikolay

> 
>>  
>>  AC_DEFUN([LIBVIRT_STORAGE_CHECK_VSTORAGE], [
>>    if test "$with_storage_vstorage" = "yes" ||
>>       test "$with_storage_vstorage" = "check"; then
>> -    AC_PATH_PROG([VSTORAGE], [vstorage], [], [$LIBVIRT_SBIN_PATH])
>> -    AC_PATH_PROG([VSTORAGE_MOUNT], [vstorage-mount], [], [$LIBVIRT_SBIN_PATH])
>>      AC_PATH_PROG([UMOUNT], [umount], [], [$LIBVIRT_SBIN_PATH])
>>  
>>      if test "$with_storage_vstorage" = "yes"; then
>> -      if test -z "$VSTORAGE" || test -z "$VSTORAGE_MOUNT"; then
>> -        AC_MSG_ERROR([We need vstorage and vstorage-mount tool for Vstorage storage driver]);
>> -      fi
>>        if test -z "$UMOUNT" ; then
>>          AC_MSG_ERROR([We need umount for Vstorage storage driver]);
>>        fi
>>      else
>> -      if test -z "$VSTORAGE" ; then
>> -        with_storage_vstorage=no
>> -      fi
>> -      if test -z "$VSTORAGE_MOUNT" ; then
>> -        with_storage_vstorage=no
>> -      fi
>>        if test -z "$UMOUNT" ; then
>>          with_storage_vstorage=no
>>        fi
>> @@ -57,10 +46,6 @@ AC_DEFUN([LIBVIRT_STORAGE_CHECK_VSTORAGE], [
>>      if test "$with_storage_vstorage" = "yes" ; then
>>        AC_DEFINE_UNQUOTED([WITH_STORAGE_VSTORAGE], 1,
>>                           [whether Vstorage backend for storage driver is enabled])
>> -      AC_DEFINE_UNQUOTED([VSTORAGE], ["$VSTORAGE"],
>> -                         [Location or name of the vstorage client tool])
>> -      AC_DEFINE_UNQUOTED([VSTORAGE_MOUNT], ["$VSTORAGE_MOUNT"],
>> -                         [Location or name of the vstorage mount tool])
>>        AC_DEFINE_UNQUOTED([UMOUNT], ["$UMOUNT"],
>>                           [Location or name of the umount programm])
>>      fi
>> diff --git a/src/storage/storage_backend_vstorage.c b/src/storage/storage_backend_vstorage.c
>> index 6cff9f1..ea3bfaa 100644
>> --- a/src/storage/storage_backend_vstorage.c
>> +++ b/src/storage/storage_backend_vstorage.c
>> @@ -44,9 +44,16 @@ virStorageBackendVzPoolStart(virStoragePoolObjPtr pool)
>>      g_autofree char *grp_name = NULL;
>>      g_autofree char *usr_name = NULL;
>>      g_autofree char *mode = NULL;
>> +    g_autofree char *mount_bin = NULL;
>>      g_autoptr(virCommand) cmd = NULL;
>>      int ret;
>>  
>> +    if (!(mount_bin = virFindFileInPath("vstorage-mount"))) {
>> +        virReportError(VIR_ERR_INTERNAL_ERROR,
>> +                       "%s", _("unable to find vstorage-mount"));
>> +        return -1;
>> +    }
>> +
>>      /* Check the permissions */
>>      if (def->target.perms.mode == (mode_t)-1)
>>          def->target.perms.mode = VIR_STORAGE_DEFAULT_POOL_PERM_MODE;
>> @@ -65,7 +72,7 @@ virStorageBackendVzPoolStart(virStoragePoolObjPtr pool)
>>  
>>      mode = g_strdup_printf("%o", def->target.perms.mode);
>>  
>> -    cmd = virCommandNewArgList(VSTORAGE_MOUNT,
>> +    cmd = virCommandNewArgList(mount_bin,
>>                                 "-c", def->source.name,
>>                                 def->target.path,
>>                                 "-m", mode,
>> -- 
>> 1.8.3.1
>>
> 
> Regards,
> Daniel
>