On Mon, Apr 19, 2021 at 06:01:42PM +0200, Andrea Bolognani wrote:
> On Mon, 2021-04-19 at 13:35 +0100, Daniel P. Berrangé wrote:
> > On Fri, Apr 16, 2021 at 09:12:45PM +0200, Pavel Hrdina wrote:
> > > - if not get_option('storage_iscsi').disabled() and iscsiadm_prog.found()
> > > + if not get_option('storage_iscsi').disabled()
> > > use_storage = true
> > > conf.set('WITH_STORAGE_ISCSI', 1)
> >
> > So this now enables iSCSI even on platforms which don't support iscsiadm
> > tools.
> >
> > > + use_storage = true
> > > + conf.set('WITH_STORAGE_LVM', 1)
> >
> > And enables LVM on all platforms, even though this is Linux only..
> >
> > Overall this patch makes it so that our meson rules don't
> > "do the right thing" as a default behaviour, and just enable
> > everything whether you have it installed or not.
>
> Can't we address that by implementing explicit OS checks?
For some of the that's easy, if they're obviously Linux or FreeBSD
specific. Others though the platform supportability is not so
clear, at least not unless someone wants to check the tools and
what platforms they genuinely claim to support.
> Right now we're using "the iscsiadm command happens to be installed
> on the build host" as a proxy for "the iscsiadm command will be
> available on the deployment host", which is not a bad approximation
> but is not quite right either.
Both approaches are approximations - checking the binary may miss out
platforms where the feature is possible but not installed by the user.
Checking the OS platform may miss out platforms where libvirt devs
didn't realize there was support for the 3rd party tool. The former
was more the favoured approach as it removes libvirt devs from being
the problem, and makes the users' be the problem :-)
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
