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

On Tue, Jul 14, 2020 at 11:55:47AM +0300, Nikolay Shirokovskiy wrote: > > > 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(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 > > 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. The core rule for configure checks is that users should never have to pass any --with args to enable use of features that their host OS has. So we need some mechanism to correctly enable the driver. I'd suggest that we in fact get rid of this entire check, and just make the storage driver be directly configured based on $with_vz, beucase if you have the hypervisor driver enabled, you want the storage driver to match.