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

Re: [libvirt] [PATCH] Refactor the libvirt RPM daemon pieces



On Fri, Mar 30, 2012 at 12:13:13PM -0400, Laine Stump wrote:
> On 03/30/2012 09:22 AM, Daniel P. Berrange wrote:
> > From: "Daniel P. Berrange" <berrange redhat com>
> >
> > NB: this is in response to a Fedora 17 beta blocker bug.
> > Currently gnome-boxes depends on 'libvirt' which pulls
> > in the default virtual network, which kills networking
> > if you install Fedora 17 inside a KVM guest.
> >
> > There are a number of flaws with our packaging of the libvirtd
> > daemon:
> >
> >  - Installing 'libvirt' does not install 'qemu-kvm' or 'xen'
> >    etc which are required to actually run the hypervisor in
> >    question
> >  - Installing 'libvirt' pulls in the default configuration
> >    files which may not be wanted & cause problems if installed
> >    inside a guest
> >  - It is not possible to explicitly required all the peices
> >    required to manage a specific hypervisor
> >
> > This change takes the 'libvirt' RPM and and changes it thus
> >
> >  - libvirt: just a virtual package with dep on libvirt-daemon and
> >    libvirt-daemon-configs
> >  - libvirt-daemon: the libvirt daemon and related pieces
> >  - libvirt-daemon-configs: the default network & filter configs
> >  - libvirt-docs: the website HTML
> >
> > We then introduce some more virtual (empty) packages
> >
> >  - libvirt-daemon-qemu: Deps on libvirt-daemon & 'qemu'
> >  - libvirt-daemon-kvm: Deps on libvirt-daemon & 'qemu-kvm'
> >  - libvirt-daemon-lxc: Deps on libvirt-daemon
> >  - libvirt-daemon-uml: Deps on libvirt-daemon
> >  - libvirt-daemon-xen: Deps on libvirt-daemon & 'xen'
> >
> >  - libvirt-qemu: Deps on libvirt-daemon-qemu & libvirt-daemon-configs
> >  - libvirt-kvm: Deps on libvirt-daemon-kvm & libvirt-daemon-configs
> >  - libvirt-lxc: Deps on libvirt-daemon-lxc & libvirt-daemon-configs
> >  - libvirt-uml: Deps on libvirt-daemon-uml & libvirt-daemon-configs
> >  - libvirt-xen: Deps on libvirt-daemon-xen & libvirt-daemon-configs
> >
> > My intent in the future is to turn on the driver modules by
> > default, at which time 'libvirt-daemon' will cease to include
> > any specific drivers, instead we'll get libvirt-daemon-driver-XXXX
> > packages for each driver. The libvirt-daemon-XXX packages will
> > then pull in each driver that they require.
> 
> I like the modularity of this package setup. There is one difference
> with the change described in the final paragraph, though, that will
> cause some surprises when it's done:
> 
> With the current setup, you can install the "libvirt" package, and it
> will just use/present whichever hypervisors happen to be installed on
> the machine. With the new setup, if you have qemu-kvm and lxc installed
> on the machine, then install "libvirt", you won't get the libvirt driver
> for either of these - you'll need to explicitly install libvirt-qemu and
> libvirt-lxc. This may cause surprises for people who already have
> bootstraps that just install "qemu-kvm" and "libvirt".

No, that's not the case. The behaviour of 'yum install libvirt' is
identical before & after. You won't get the 'libvirt-daemon-kvm'
package installed, but that's fine, because that's an empty
virtual package.


> > diff --git a/libvirt.spec.in b/libvirt.spec.in
> > index 67f1c33..743d43e 100644
> > --- a/libvirt.spec.in
> > +++ b/libvirt.spec.in
> > @@ -52,6 +52,14 @@
> >  %define with_libxl         0%{!?_without_libxl:%{server_drivers}}
> >  %define with_vmware        0%{!?_without_vmware:%{server_drivers}}
> >  
> > +%define with_qemu_tcg      %{with_qemu}
> 
> qemu_tcg wasn't mentioned anywhere in the documentation or commit log
> message. Was this intentional?
> 
> (Note after further reading - it looks like qemu_tcg is being used as an
> easier-to-differentiate synonym for "qemu" (i.e. software-only qemu), is
> that right?)

Yes, this is to let us deal with RHEL where we only provide
qemu-kvm, not the rest of the TCG based binaries.


> > +%package docs
> > +Summary: Documentation for libvirt library and daemon
> > +Group: Development/Libraries
> > +
> > +%description docs
> > +Copy of the libvirt website documentation
> > +
> > +%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.
> 
> Functionally it probably makes no difference, but I think it would be
> better if "%description daemon" was put next to "%package daemon" rather
> than having the %package and %description of docs in between. Or is this
> grouped in some other way that I missed?

Yes, that ordering is a mistake I've fixed.

> > +
> > +%package daemon-configs
> > +Summary: Default configuration files for the libvirtd daemon
> > +Group: Development/Libraries
> > +
> > +Requires: libvirt-daemon = %{version}-%{release}
> > +
> > +%description daemon-configs
> > +Default configuration files for setting up NAT based networking
> > +and guest network traffic filtering.
> > +%endif
> > +
> > +# XXX when we turn on driver modules, we will need to
> > +# create daemon-drv-XXX sub-RPMs and add them as deps
> > +# to all of the following  daemon-XXX RPMs
> > +
> > +%if %{with_qemu_tcg}
> > +%package daemon-qemu
> > +Summary: Server side daemon & driver required to run QEMU guests
> > +Group: Development/Libraries
> > +
> > +Requires: libvirt-daemon = %{version}-%{release}
> > +Requires: qemu
> > +%endif
> > +
> > +%if %{with_qemu_kvm}
> > +%package daemon-kvm
> > +Summary: Server side daemon & driver required to run QEMU guests
> > +Group: Development/Libraries
> > +
> > +Requires: libvirt-daemon = %{version}-%{release}
> > +Requires: qemu-kvm
> > +%endif
> > +
> > +%if %{with_qemu_tcg}
> > +%description daemon-qemu
> > +Server side daemon and driver required to manage the virtualization
> > +capabilities of the QEMU TCG emulators
> > +%endif
> > +
> > +%if %{with_qemu_kvm}
> > +%description daemon-kvm
> > +Server side daemon and driver required to manage the virtualization
> > +capabilities of the QEMU KVM hypervisor
> > +%endif
> 
> These sections also seem to be mixed up. But maybe I'm expecting a
> different kind of grouping than you're using.

This was an artifact of an intermediate idea I tried. I'll move it back
to the natural ordering.


> >  
> >  %description lock-sanlock
> >  Includes the Sanlock lock manager plugin for the QEMU
> > @@ -811,8 +996,7 @@ autoreconf -if
> >             %{with_packager_version} \
> >             --with-qemu-user=%{qemu_user} \
> >             --with-qemu-group=%{qemu_group} \
> > -           %{init_scripts} \
> > -           --with-remote-pid-file=%{_localstatedir}/run/libvirtd.pid
> > +           %{init_scripts}
> 
> What's the story with this bit? Is that option included in some other
> way that I missed now? Or is it unneeded? Should it be in this patch, or
> is it addressing a separate issue?

Opps, that should have been separate. --with-remote-pid-file was deleted
from configure long ago.


> Aside from the above comments everything seems to make sense (for as far
> as I understand specfiles :-/), but the real proof I guess will be in
> examining the results of builds, and what is pulled in when particular
> packages are installed.
> 
> One worry about this from the point of view of Fedora is that final beta
> deadline is past, and we basically have one chance to get this right. Is
> there any way to do this in two steps such that the 1st would be
> sufficient to solve Fedora's problem, but be much less intrusive? Or
> would the extra step itself just add a bunch more chances for something
> to go wrong in the 2nd step, with no real simplification gain in the
> first step?

I don't think there's a good way todo it in two separate steps - in fact
it would likely make it even harder to ensure a correct upgrade paths.


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


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