[libvirt] [PATCH 20/27] libvirt.spec.in: remove most storage conditionals

[Daniel P. Berrange]

On Thu, May 05, 2016 at 01:11:45PM +0200, Michal Privoznik wrote:
> On 04.05.2016 18:17, Daniel P. Berrange wrote:
> > Both RHEL and Fedora build with the storage driver and
> > most of its sub-drivers enabled at all times.
> > 
> > Signed-off-by: Daniel P. Berrange <berrange at redhat.com>
> > ---
> >  libvirt.spec.in | 86 +++++++++------------------------------------------------
> >  1 file changed, 13 insertions(+), 73 deletions(-)
> > 
> > diff --git a/libvirt.spec.in b/libvirt.spec.in
> > index 69c6af4..829cdd1 100644
> > --- a/libvirt.spec.in
> > +++ b/libvirt.spec.in
> > @@ -57,11 +57,6 @@
> >  %define with_hyperv        0%{!?_without_hyperv:1}
> >  
> >  # Then the secondary host drivers, which run inside libvirtd
> > -%define with_storage_fs       0%{!?_without_storage_fs:1}
> > -%define with_storage_lvm      0%{!?_without_storage_lvm:1}
> > -%define with_storage_iscsi    0%{!?_without_storage_iscsi:1}
> > -%define with_storage_disk     0%{!?_without_storage_disk:1}
> > -%define with_storage_mpath    0%{!?_without_storage_mpath:1}
> >  %if 0%{?fedora} || 0%{?rhel} >= 7
> >      %define with_storage_rbd      0%{!?_without_storage_rbd:1}
> >  %else
> > @@ -211,13 +206,6 @@
> >  
> >  %define with_nodedev 1
> >  
> > -%if %{with_storage_fs} || %{with_storage_mpath} || %{with_storage_iscsi} || %{with_storage_lvm} || %{with_storage_disk}
> > -    %define with_storage 1
> > -%else
> > -    %define with_storage 0
> > -%endif
> > -
> > -
> >  # Force QEMU to run as non-root
> >  %define qemu_user  qemu
> >  %define qemu_group  qemu
> > @@ -367,10 +355,8 @@ BuildRequires: polkit-devel >= 0.112
> >  %else
> >  BuildRequires: polkit-devel >= 0.93
> >  %endif
> > -%if %{with_storage_fs}
> >  # For mount/umount in FS driver
> >  BuildRequires: util-linux
> > -%endif
> >  %if %{with_qemu}
> >  # From QEMU RPMs
> >  BuildRequires: /usr/bin/qemu-img
> > @@ -380,22 +366,14 @@ BuildRequires: /usr/bin/qemu-img
> >  BuildRequires: /usr/sbin/qcow-create
> >      %endif
> >  %endif
> > -%if %{with_storage_lvm}
> >  # For LVM drivers
> >  BuildRequires: lvm2
> > -%endif
> > -%if %{with_storage_iscsi}
> >  # For ISCSI driver
> >  BuildRequires: iscsi-initiator-utils
> > -%endif
> > -%if %{with_storage_disk}
> >  # For disk driver
> >  BuildRequires: parted-devel
> > -%endif
> > -%if %{with_storage_mpath} || %{with_storage_disk}
> >  # For Multipath support
> >  BuildRequires: device-mapper-devel
> > -%endif
> >  %if %{with_storage_rbd}
> >      %if 0%{?rhel} >= 7
> >  BuildRequires: librados2-devel
> > @@ -435,12 +413,10 @@ BuildRequires: audit-libs-devel
> >  # we need /usr/sbin/dtrace
> >  BuildRequires: systemtap-sdt-devel
> >  
> > -%if %{with_storage_fs}
> >  # For mount/umount in FS driver
> >  BuildRequires: util-linux
> >  # For showmount in FS driver (netfs discovery)
> >  BuildRequires: nfs-utils
> > -%endif
> >  
> >  # Communication with the firewall and polkit daemons use DBus
> >  BuildRequires: dbus-devel
> > @@ -622,56 +598,44 @@ The secret driver plugin for the libvirtd daemon, providing
> >  an implementation of the secret key APIs.
> >  
> >  
> > -%if %{with_storage}
> >  %package daemon-driver-storage
> >  Summary: Storage driver plugin for the libvirtd daemon
> >  Group: Development/Libraries
> >  Requires: libvirt-daemon = %{version}-%{release}
> > -    %if %{with_storage_fs}
> >  Requires: nfs-utils
> >  # For mkfs
> >  Requires: util-linux
> >  # For glusterfs
> > -        %if 0%{?fedora}
> > +%if 0%{?fedora}
> >  Requires: glusterfs-client >= 2.0.1
> > -        %endif
> > -    %endif
> > -    %if %{with_storage_lvm}
> > +%endif
> >  # For LVM drivers
> >  Requires: lvm2
> > -    %endif
> > -    %if %{with_storage_iscsi}
> >  # For ISCSI driver
> >  Requires: iscsi-initiator-utils
> > -    %endif
> > -    %if %{with_storage_disk}
> >  # For disk driver
> >  Requires: parted
> >  Requires: device-mapper
> > -    %endif
> > -    %if %{with_storage_mpath}
> >  # For multipath support
> >  Requires: device-mapper
> > -    %endif
> > -    %if %{with_storage_sheepdog}
> > +%if %{with_storage_sheepdog}
> >  # For Sheepdog support
> >  Requires: sheepdog
> 
> This is interesting. I did not have sheepdog installed on my system and
> rpmbuild failed for me at configure phase because sheepdog was enabled
> but not installed. I wonder how this could have ever worked.
> s/Requires/BuildRequires/ please. Or should we set some default in our
> configure script? Third option would be to have both Requires and
> BuildRequires in the spec file.

We should have both Requires + BuildRequires, because we don't link to
any libs - we just rely on the CLI tools, so would not get any automatic
dep on the binary package.

Since we had a missing BR, it seems we were silently not building
sheepdog support, despite requiring it at install time :-)

Anyway, I'll fix this in a separate patch, since its a pre-existing
bug


Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|