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

On 03/30/2012 09:22 AM, Daniel P. Berrange wrote: > From: "Daniel P. Berrange" <berrange(a)redhat.com&gt; > > 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".

> 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?)

> +%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?

> + > +%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.

> > %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?

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?