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