Re: [libvirt] RPM spec file patch
On 11/16/2011 11:19 PM, Chris Picton wrote:
On Wed, 2011-11-16 at 15:31 -0700, Eric Blake wrote:
On 11/15/2011 11:42 PM, Chris Picton wrote:
Hi
Please accept the following patch to the rpm spec file.
Thanks for the report. However, I'm not yet quite convinced that your
proposed patch is the best approach.
I meant to revisit this in time for 0.9.8, but obviously slipped on that
front. Oh well, it's now on my list for 0.9.9.
The following may be a better way of expressing the dependencies
1) set sane defaults for variables near the top
%define with_openvz0%{!?_without_openvz:%{server_drivers}}
2) override these with platform specific values later on (but taking
cognisance of the user supplied options
%if 0%{?rhel}
%define with_openvz 0%{?_with_openvz:1}
...
%endif
This is already used in some cases where certain features are turned on
on specific platforms:
%if 0%{?fedora} = 13 || 0%{?rhel} = 6
%define with_dtrace 0%{!?_without_dtrace:1}
%endif
So the bulk of the changes would be updating the spec file to replace
%define with_xxx 0
with
%define with_xxx 0{?_with_xxx:1}
Yes, that looks like a reasonable approach.
--
Eric Blake ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] RPM spec file patch
Hi
Please accept the following patch to the rpm spec file.
It allows me to enable specific options (like openvz) at the build
comand line, even if they have been disabled by OS feature selection.
eg for an openvz build on centos 6
rpmbuild -bb \
--define 'rhel 6' \
--without dtrace \
--without sanlock \
--without netcf \
--with openvz \
libvirt.spec
Regards
Chris
(Not subscribed to the mailing list)
diff -uNr libvirt-0.9.7.orig/libvirt.spec.in libvirt-0.9.7/libvirt.spec.in
--- libvirt-0.9.7.orig/libvirt.spec.in 2011-11-08 08:52:03.0 +0200
+++ libvirt-0.9.7/libvirt.spec.in 2011-11-16 08:40:15.0 +0200
@@ -232,6 +232,47 @@
%define with_rhel5 0
%endif
+# Force enable specific features if called on the command line
+%{?_with_audit:%define with_audit 1}
+%{?_with_avahi:%define with_avahi 1}
+%{?_with_capng:%define with_capng 1}
+%{?_with_cgconfig:%define with_cgconfig 1}
+%{?_with_dtrace:%define with_dtrace 1}
+%{?_with_esx:%define with_esx 1}
+%{?_with_hal:%define with_hal 1}
+%{?_with_hyperv:%define with_hyperv 1}
+%{?_with_libnl:%define with_libnl 1}
+%{?_with_libpcap:%define with_libpcap 1}
+%{?_with_libvirtd:%define with_libvirtd 1}
+%{?_with_libxl:%define with_libxl 1}
+%{?_with_lxc:%define with_lxc 1}
+%{?_with_macvtap:%define with_macvtap 1}
+%{?_with_netcf:%define with_netcf 1}
+%{?_with_network:%define with_network 1}
+%{?_with_numactl:%define with_numactl 1}
+%{?_with_nwfilter:%define with_nwfilter 1}
+%{?_with_openvz:%define with_openvz 1}
+%{?_with_phyp:%define with_phyp 1}
+%{?_with_polkit:%define with_polkit 1}
+%{?_with_python:%define with_python 1}
+%{?_with_qemu:%define with_qemu 1}
+%{?_with_rhel5:%define with_rhel5 1}
+%{?_with_sanlock:%define with_sanlock 1}
+%{?_with_sasl:%define with_sasl 1}
+%{?_with_selinux:%define with_selinux 1}
+%{?_with_storage_disk:%define with_storage_disk 1}
+%{?_with_storage_fs:%define with_storage_fs 1}
+%{?_with_storage_iscsi:%define with_storage_iscsi 1}
+%{?_with_storage_lvm:%define with_storage_lvm 1}
+%{?_with_storage_mpath:%define with_storage_mpath 1}
+%{?_with_udev:%define with_udev 1}
+%{?_with_uml:%define with_uml 1}
+%{?_with_vbox:%define with_vbox 1}
+%{?_with_vmware:%define with_vmware 1}
+%{?_with_xen:%define with_xen 1}
+%{?_with_xenapi:%define with_xenapi 1}
+%{?_with_yajl:%define with_yajl 1}
+
Summary: Library providing a simple virtualization API
Name: libvirt
Version: @VERSION@
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] RPM spec file patch
On 11/15/2011 11:42 PM, Chris Picton wrote:
Hi
Please accept the following patch to the rpm spec file.
It allows me to enable specific options (like openvz) at the build
comand line, even if they have been disabled by OS feature selection.
eg for an openvz build on centos 6
rpmbuild -bb \
--define 'rhel 6' \
--without dtrace \
--without sanlock \
--without netcf \
--with openvz \
libvirt.spec
Regards
Thanks for the report. However, I'm not yet quite convinced that your
proposed patch is the best approach. I'll explain, focusing on just the
nwfilter macros as an example.
First, some background, to make sure I'm analyzing things correctly (and
in part, documenting things I learned so that I can refer to this mail
later :).
https://docs.fedoraproject.org/en-US/Fedora_Draft_Documentation/0.1/html-single/RPM_Guide/index.html#id570231
was handy in seeing how spec file macros are expanded. I temporarily
added this line to libvirt.spec, then ran variations of 'rpmbuild -bp
libvirt.spec' to see what effect things have:
%{error: with %{_with_nwfilter}, without %{_without_nwfilter}}
It was pretty easy to learn that:
rpmbuild --without a --with b
is short-hand for
rpmbuild --define '_without_a --without-a1' --define '_with_b --with-b'
Note that these are macros with a leading underscore, and that the
definition chosen happens to be one that can be reused as a typical
./configure argument. But we later want to make decisions on
dependencies based on which commands were enabled via rpmbuild or
earlier %define, and checking both _without_nwfilter and _with_nwfilter
at every decision point is tedious, so we create a new macro,
with_nwfilter, whose contents are guaranteed to be a numeric string that
is usable as an %if expression (the value of the string is important
only as 0 vs. non-zero).
Now, notice that the existing spec file starts with:
%define with_nwfilter 0%{!?_without_nwfilter:0}
which guarantees with_nwfilter will be defined, either to 0 (if
_without_nwfilter is defined) or 00 (if _without_nwfilter was not
defined). At first glance, that seems a bit fishy - shouldn't we be
defining it to 0 or 1, instead of 0 or 0? But reading the comment makes
it clearer (this is setting a default to off, which is enabled later
based on other conditions); and indeed, the next use is:
%if %{with_qemu}
%define with_nwfilter 0%{!?_without_nwfilter:%{server_drivers}}
%endif
Aha. If %{with_qemu} is set, then we redefine with_nwfilter; this time
to the value 0 if _without_nwfilter is set, or to 0%{server_drivers}
(which in turn is 00 or 01) if _without_nwfilter is missing. If
%{with_qemu} is clear, then we leave with_nwfilter at 0.
After that point, your patch would then add:
%{?_with_nwfilter:%define with_nwfilter 1}
which says that if _with_nwfilter is defined, then redefine
with_nwfilter once more, this time to 1.
Still, that's a lot of logic to go through three %defines before we get
to the final value. Also, it looks like our spec file does not make it
easy to override client_only, which controls several of the defaults (of
course, it is okay to hardcode client_only to 1 on the architectures
where we do not support libvirtd, but allowing the user to override it
to 0 on platforms where we normally build the server would be handy).
Perhaps it would be simpler to do something like this, which 1) fixes
client_only to allow user override on appropriate platforms, and 2) sets
default values based on inspecting _both_ with and without variants.
This is not a complete patch, so much as a request for further
discussion. The idea is to demonstrate how defining the defaults for
with_libvirtd and with_polkit should consider the _with_ variant in
addition to the _without_ variant; again with the end result being 0 to
disable, nonzero (whether 010, 011, or 001) to enable.
Thoughts?
diff --git i/libvirt.spec.in w/libvirt.spec.in
index 89f710c..d085feb 100644
--- i/libvirt.spec.in
+++ w/libvirt.spec.in
@@ -11,7 +11,7 @@
# A client only build will create a libvirt.so only containing
# the generic RPC driver, and test driver and no libvirtd
# Default to a full server + client build
-%define client_only0
+%{!?client_only:%define client_only0}
# Now turn off server build in certain cases
@@ -34,7 +34,7 @@
# of any particular OS
# First the daemon itself
-%define with_libvirtd 0%{!?_without_libvirtd:%{server_drivers}}
+%define with_libvirtd
0%{!?_without_libvirtd:%{server_drivers}%{?_with_libvirtd:1}}
%define with_avahi 0%{!?_without_avahi:%{server_drivers}}
# Then the hypervisor drivers that run on local host
@@ -64,7 +64,7 @@
%define with_selinux 0%{!?_without_selinux:%{server_drivers}}
# A few optional bits off by default, we enable later
-%define with_polkit0%{!?_without_polkit:0}
+%define with_polkit0%{!?_without_polkit:0}%{?_with_polkit:1}
%define with_capng 0%{!?_without_capng:0}
%define with_netcf
Re: [libvirt] RPM spec file patch
On Wed, 2011-11-16 at 15:31 -0700, Eric Blake wrote:
On 11/15/2011 11:42 PM, Chris Picton wrote:
Hi
Please accept the following patch to the rpm spec file.
Thanks for the report. However, I'm not yet quite convinced that your
proposed patch is the best approach. I'll explain, focusing on just the
nwfilter macros as an example.
[snip]
Perhaps it would be simpler to do something like this, which 1) fixes
client_only to allow user override on appropriate platforms, and 2) sets
default values based on inspecting _both_ with and without variants.
This is not a complete patch, so much as a request for further
discussion. The idea is to demonstrate how defining the defaults for
with_libvirtd and with_polkit should consider the _with_ variant in
addition to the _without_ variant; again with the end result being 0 to
disable, nonzero (whether 010, 011, or 001) to enable.
In my case, I was trying to override the behaviour of the spec file
which is to (in the case of with_openvz)
1) set sane defaults for variables near the top
%define with_openvz0%{!?_without_openvz:%{server_drivers}}
2) override these with platform specific values later on
%if 0%{?rhel}
%define with_openvz 0
...
%endif
3) use the overridden values. ignoring the user-supplied options
In your patch, it still does not change the above behaviour, as the
defaults are set near the top, but still force overridden further down
The following may be a better way of expressing the dependencies
1) set sane defaults for variables near the top
%define with_openvz0%{!?_without_openvz:%{server_drivers}}
2) override these with platform specific values later on (but taking
cognisance of the user supplied options
%if 0%{?rhel}
%define with_openvz 0%{?_with_openvz:1}
...
%endif
This is already used in some cases where certain features are turned on
on specific platforms:
%if 0%{?fedora} = 13 || 0%{?rhel} = 6
%define with_dtrace 0%{!?_without_dtrace:1}
%endif
So the bulk of the changes would be updating the spec file to replace
%define with_xxx 0
with
%define with_xxx 0{?_with_xxx:1}
And then also updating for the client_only settings as shown below as
well
Regards
Chris
diff --git i/libvirt.spec.in w/libvirt.spec.in
index 89f710c..d085feb 100644
--- i/libvirt.spec.in
+++ w/libvirt.spec.in
@@ -11,7 +11,7 @@
# A client only build will create a libvirt.so only containing
# the generic RPC driver, and test driver and no libvirtd
# Default to a full server + client build
-%define client_only0
+%{!?client_only:%define client_only0}
# Now turn off server build in certain cases
@@ -34,7 +34,7 @@
# of any particular OS
# First the daemon itself
-%define with_libvirtd 0%{!?_without_libvirtd:%{server_drivers}}
+%define with_libvirtd
0%{!?_without_libvirtd:%{server_drivers}%{?_with_libvirtd:1}}
%define with_avahi 0%{!?_without_avahi:%{server_drivers}}
# Then the hypervisor drivers that run on local host
@@ -64,7 +64,7 @@
%define with_selinux 0%{!?_without_selinux:%{server_drivers}}
# A few optional bits off by default, we enable later
-%define with_polkit0%{!?_without_polkit:0}
+%define with_polkit0%{!?_without_polkit:0}%{?_with_polkit:1}
%define with_capng 0%{!?_without_capng:0}
%define with_netcf 0%{!?_without_netcf:0}
%define with_udev 0%{!?_without_udev:0}
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
