Re: [PATCH] Refactor the libvirt RPM daemon pieces

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Mar 30, 2012 at 04:40:09PM -0600, Eric Blake wrote:
> On 03/30/2012 10:53 AM, Daniel P. Berrange wrote:
> > From: "Daniel P. Berrange" <berrange@xxxxxxxxxx>
> > 
> > There are a number of flaws with our packaging of the libvirtd
> > daemon:
> > 
> 
> > +
> > +%if %{with_libvirtd}
> > +%package daemon
> > +Summary: Server side daemon and supporting files for libvirt library
> > +Group: Development/Libraries
> > +
> > +# All runtime requirements for the libvirt package (runtime requrements
> > +# for subpackages are listed later in those subpackages)
> > +
> > +# The client side, i.e. shared libs and virsh are in a subpackage
> > +Requires: %{name}-client = %{version}-%{release}
> > +
> > +# Used by many of the drivers, so turn it on whenever the
> > +# daemon is present
> > +%if %{with_libvirtd}
> 
> You've now nested %if %{with_libvirtd} twice; this could be simplified,
> but cleanups can be saved for another day if we are under the gun to get
> this out now (is there a cppi equivalent for spec files to help show
> nesting?  Can you even indent %if directives for sanity in a spec file?)

Yes, i'll kill that nesting.

> 
> > +# for modprobe of pci devices
> ...
> 
> > +# For glusterfs
> > +%if 0%{?fedora} >= 11
> > +Requires: glusterfs-client >= 2.0.1
> > +%endif
> > +%endif
> > +%if %{with_qemu}
> > +# From QEMU RPMs
> > +Requires: /usr/bin/qemu-img
> 
> Should this Requires: be pushed down to libvirt-daemon-qemu...

This is used by the storage driver, which is shared amongst
xen, qemu, libxl, lxc & uml hypervisor drivers.

> 
> > +# For image compression
> > +Requires: gzip
> > +Requires: bzip2
> > +Requires: lzop
> > +Requires: xz
> > +%else
> > +%if %{with_xen}
> > +# From Xen RPMs
> > +Requires: /usr/sbin/qcow-create
> 
> and this one to libvirt-daemon-kvm?

Likewise, shared.

> > +# For virConnectGetSysinfo
> > +Requires: dmidecode
> > +%endif
> > +# For service management
> > +%if %{with_systemd}
> > +Requires(post): systemd-units
> > +Requires(post): systemd-sysv
> > +Requires(preun): systemd-units
> > +Requires(postun): systemd-units
> 
> These four requires deal with services, but we already pushed services
> into libvirt-daemon-config-network; I'm guessing these should be
> relocated to that section.
> 
> Or maybe I'm misunderstanding - is it that these enable libvirtd to run
> as a service, but libvirtd will do nothing to cause interference (like
> trying to set up the 'default' NAT network at 192.168.122.0) unless you
> also have the config files?  In that case, what you did looks okay.

I only pushed the install of the network XML into the daemon-config-network
RPM. There is still a '%post daemon' section which deals with the init
scrpits which is what these requires are for.

> > +%endif
> > +%if %{with_numad}
> > +Requires: numad
> > +%endif
> > +
> > +%description daemon
> > +Server side daemon required to manage the virtualization capabilities
> > +of recent versions of Linux. Requires a hypervisor specific sub-RPM
> > +for specific drivers.
> 
> It looks weird to have the %description so far down from the %package
> with all the Requires in between.  But that's cosmetic.

This is layout normal practice - it just looks a bit wierd because we
have soooo many requires.

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

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list


[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]