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

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(a)virtuozzo.com&gt; > --- > 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

> > 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