[libvirt] [PATCHv2.5] specfile: fix make rpm when with_driver_modules is 1

Commit ff76566 moved around things in the specfiles to put driver-specific files into their appropriate sub-packages (when with_driver_modules == 1), but accidentally changed things so that the deamon-driver-network and daemon-config-network files were only included in a package when with_driver_modules == 0. This broke "make rpm" on fedora (where with_driver_modules == 1). This patch follows the pattern (already used for the files in other sub-modules) of duplicating the files for the main package (!with_driver_modules) and the sub-package (with_driver_modules). --- Changes from V1: realized that %{_datadir} is /usr/share, so %{_datadir}/libvirt/networks/default.xml should be installed as a part of daemon-driver-network (or the main package, if not building with driver modules). libvirt.spec.in | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/libvirt.spec.in b/libvirt.spec.in index 849ec80..015e627 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -1894,11 +1894,6 @@ exit 0 %endif %endif # ! %{with_driver_modules} - %if %{with_network} -%files daemon-config-network -%defattr(-, root, root) - %endif - %if %{with_nwfilter} %files daemon-config-nwfilter %defattr(-, root, root) @@ -1915,6 +1910,14 @@ exit 0 %if %{with_network} %files daemon-driver-network %defattr(-, root, root) +%dir %attr(0700, root, root) %{_sysconfdir}/libvirt/qemu/ +%dir %attr(0700, root, root) %{_sysconfdir}/libvirt/qemu/networks/ +%dir %attr(0700, root, root) %{_sysconfdir}/libvirt/qemu/networks/autostart +%dir %{_datadir}/libvirt/networks/ +%{_datadir}/libvirt/networks/default.xml +%ghost %dir %{_localstatedir}/run/libvirt/network/ +%dir %attr(0700, root, root) %{_localstatedir}/lib/libvirt/network/ +%dir %attr(0755, root, root) %{_localstatedir}/lib/libvirt/dnsmasq/ %{_libdir}/%{name}/connection-driver/libvirt_driver_network.so %endif -- 1.8.3.1

Laine Stump wrote:
Commit ff76566 moved around things in the specfiles to put driver-specific files into their appropriate sub-packages (when with_driver_modules == 1), but accidentally changed things so that the deamon-driver-network and daemon-config-network files were only included in a package when with_driver_modules == 0. This broke "make rpm" on fedora (where with_driver_modules == 1).
This patch follows the pattern (already used for the files in other sub-modules) of duplicating the files for the main package (!with_driver_modules) and the sub-package (with_driver_modules). ---
Changes from V1: realized that %{_datadir} is /usr/share, so %{_datadir}/libvirt/networks/default.xml should be installed as a part of daemon-driver-network (or the main package, if not building with driver modules).
libvirt.spec.in | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/libvirt.spec.in b/libvirt.spec.in index 849ec80..015e627 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -1894,11 +1894,6 @@ exit 0 %endif %endif # ! %{with_driver_modules}
- %if %{with_network} -%files daemon-config-network -%defattr(-, root, root) - %endif -
So daemon-config-network has no files, but should this be removed? I noticed daemon-qemu, daemon-kvm, daemon-lxc, etc. define a %files section even though they have no files. BTW, what is the purpose of daemon-config-network? Is it just to provide the %post scriptlet in some update cases, e.g. updating packages from pre to post with_driver_modules?
%if %{with_nwfilter} %files daemon-config-nwfilter %defattr(-, root, root) @@ -1915,6 +1910,14 @@ exit 0 %if %{with_network} %files daemon-driver-network %defattr(-, root, root) +%dir %attr(0700, root, root) %{_sysconfdir}/libvirt/qemu/ +%dir %attr(0700, root, root) %{_sysconfdir}/libvirt/qemu/networks/ +%dir %attr(0700, root, root) %{_sysconfdir}/libvirt/qemu/networks/autostart +%dir %{_datadir}/libvirt/networks/ +%{_datadir}/libvirt/networks/default.xml +%ghost %dir %{_localstatedir}/run/libvirt/network/ +%dir %attr(0700, root, root) %{_localstatedir}/lib/libvirt/network/ +%dir %attr(0755, root, root) %{_localstatedir}/lib/libvirt/dnsmasq/ %{_libdir}/%{name}/connection-driver/libvirt_driver_network.so %endif
Definitely ACK to this part. Regards, Jim

On Mon, Dec 16, 2013 at 09:30:31AM -0700, Jim Fehlig wrote:
Laine Stump wrote:
Commit ff76566 moved around things in the specfiles to put driver-specific files into their appropriate sub-packages (when with_driver_modules == 1), but accidentally changed things so that the deamon-driver-network and daemon-config-network files were only included in a package when with_driver_modules == 0. This broke "make rpm" on fedora (where with_driver_modules == 1).
This patch follows the pattern (already used for the files in other sub-modules) of duplicating the files for the main package (!with_driver_modules) and the sub-package (with_driver_modules). ---
Changes from V1: realized that %{_datadir} is /usr/share, so %{_datadir}/libvirt/networks/default.xml should be installed as a part of daemon-driver-network (or the main package, if not building with driver modules).
libvirt.spec.in | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/libvirt.spec.in b/libvirt.spec.in index 849ec80..015e627 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -1894,11 +1894,6 @@ exit 0 %endif %endif # ! %{with_driver_modules}
- %if %{with_network} -%files daemon-config-network -%defattr(-, root, root) - %endif -
So daemon-config-network has no files, but should this be removed? I noticed daemon-qemu, daemon-kvm, daemon-lxc, etc. define a %files section even though they have no files.
BTW, what is the purpose of daemon-config-network? Is it just to provide the %post scriptlet in some update cases, e.g. updating packages from pre to post with_driver_modules?
It exists to ensure that the default network gets defined. 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 :|

On 12/16/2013 06:30 PM, Jim Fehlig wrote:
Laine Stump wrote:
Commit ff76566 moved around things in the specfiles to put driver-specific files into their appropriate sub-packages (when with_driver_modules == 1), but accidentally changed things so that the deamon-driver-network and daemon-config-network files were only included in a package when with_driver_modules == 0. This broke "make rpm" on fedora (where with_driver_modules == 1).
This patch follows the pattern (already used for the files in other sub-modules) of duplicating the files for the main package (!with_driver_modules) and the sub-package (with_driver_modules). ---
Changes from V1: realized that %{_datadir} is /usr/share, so %{_datadir}/libvirt/networks/default.xml should be installed as a part of daemon-driver-network (or the main package, if not building with driver modules).
libvirt.spec.in | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/libvirt.spec.in b/libvirt.spec.in index 849ec80..015e627 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -1894,11 +1894,6 @@ exit 0 %endif %endif # ! %{with_driver_modules}
- %if %{with_network} -%files daemon-config-network -%defattr(-, root, root) - %endif -
So daemon-config-network has no files, but should this be removed?
No (or rather "I don't know for sure), I just forgot to put it back in when I updated my previous patch. Once again demonstrating that I should never push anything without review, even if it's to fix a broken build :-)
I noticed daemon-qemu, daemon-kvm, daemon-lxc, etc. define a %files section even though they have no files.
BTW, what is the purpose of daemon-config-network? Is it just to provide the %post scriptlet in some update cases, e.g. updating packages from pre to post with_driver_modules?
daemon-config-network is only there to allow a simple choice of installing the default network or not. And since the default network is necessarily different for each installation (due to different (random) uuid and mac address), it isn't installed with %files, but with the %post script. Using %post instead of %files also makes it easy to avoid overwriting an existing default network config.
%if %{with_nwfilter} %files daemon-config-nwfilter %defattr(-, root, root) @@ -1915,6 +1910,14 @@ exit 0 %if %{with_network} %files daemon-driver-network %defattr(-, root, root) +%dir %attr(0700, root, root) %{_sysconfdir}/libvirt/qemu/ +%dir %attr(0700, root, root) %{_sysconfdir}/libvirt/qemu/networks/ +%dir %attr(0700, root, root) %{_sysconfdir}/libvirt/qemu/networks/autostart +%dir %{_datadir}/libvirt/networks/ +%{_datadir}/libvirt/networks/default.xml +%ghost %dir %{_localstatedir}/run/libvirt/network/ +%dir %attr(0700, root, root) %{_localstatedir}/lib/libvirt/network/ +%dir %attr(0755, root, root) %{_localstatedir}/lib/libvirt/dnsmasq/ %{_libdir}/%{name}/connection-driver/libvirt_driver_network.so %endif
Definitely ACK to this part.
Okay, I've pushed just that hunk. Thanks for helping me understand this and get it (hopefully) right!

Laine Stump wrote:
On 12/16/2013 06:30 PM, Jim Fehlig wrote:
BTW, what is the purpose of daemon-config-network? Is it just to provide the %post scriptlet in some update cases, e.g. updating packages from pre to post with_driver_modules?
daemon-config-network is only there to allow a simple choice of installing the default network or not. And since the default network is necessarily different for each installation (due to different (random) uuid and mac address), it isn't installed with %files, but with the %post script. Using %post instead of %files also makes it easy to avoid overwriting an existing default network config.
Ah, cool - makes sense. Thanks for the explanation! Regards, Jim
participants (3)
-
Daniel P. Berrange
-
Jim Fehlig
-
Laine Stump