Re: [libvirt PATCH 9/9] spec: Introduce arches_*
On Tue, Oct 6, 2020 at 12:35 PM Andrea Bolognani wrote: > > On Tue, 2020-10-06 at 08:15 -0400, Neal Gompa wrote: > > Then can we flip this conditional to %if 0%{?rhel} for the > > architecture list? As it is, it's unclear that the reason that *RHEL* > > is the less-capable variant. > > Are you thinking of something like > > %define arches_qemu_kvm %{arches_x86} %{power64} s390x %{arm} aarch64 > %if 0%{?rhel} > %define arches_qemu_kvm x86_64 %{power64} aarch64 s390x > %endif > > ? I can definitely go that route. > That's a way to do it. Another way to do it: %if 0%{?rhel} %define arches_qemu_kvm x86_64 %{power64} aarch64 s390x %else %define arches_qemu_kvm %{arches_x86} %{power64} s390x %{arm} aarch64 %endif But either way works. :) -- 真実はいつも一つ!/ Always, there's only one truth!
Re: [libvirt PATCH 9/9] spec: Introduce arches_*
On Tue, 2020-10-06 at 08:15 -0400, Neal Gompa wrote: > Then can we flip this conditional to %if 0%{?rhel} for the > architecture list? As it is, it's unclear that the reason that *RHEL* > is the less-capable variant. Are you thinking of something like %define arches_qemu_kvm %{arches_x86} %{power64} s390x %{arm} aarch64 %if 0%{?rhel} %define arches_qemu_kvm x86_64 %{power64} aarch64 s390x %endif ? I can definitely go that route. -- Andrea Bolognani / Red Hat / Virtualization
Re: [libvirt PATCH 9/9] spec: Introduce arches_*
On Tue, Oct 6, 2020 at 6:41 AM Andrea Bolognani wrote: > > On Mon, 2020-10-05 at 20:40 -0400, Neal Gompa wrote: > > On Mon, Oct 5, 2020 at 2:41 PM Andrea Bolognani wrote: > > > %if 0%{?fedora} > > > +%define arches_qemu_kvm %{arches_x86} %{power64} s390x %{arm} > > > aarch64 > > > %else > > > +%define arches_qemu_kvm x86_64 %{power64} aarch64 s390x > > > %endif > > > > This conditional is functionally irrelevant. The superset defined for > > Fedora does not change how things work for RHEL, and it'd be easier to > > just use the one architecture set. > > The difference I can see is that %{ix86} is not currently included in > %{arches_qemu_kvm} on RHEL, but with your change it would and, unlike > what happens for 32-bit ARM, RHEL packages are actually being built > on i686. > > Later on we have > > > > %define with_storage_gluster 0%{!?_without_storage_gluster:1} > > > +%ifnarch %{arches_qemu_kvm} > > > # gluster is only built where qemu driver is enabled on RHEL 8 > > > %if 0%{?rhel} >= 8 > > > %define with_storage_gluster 0 > > and AFAICT that would break with your proposed change, because we > would try to build with gluster support on i686 RHEL where gluster is > not actually available. > Then can we flip this conditional to %if 0%{?rhel} for the architecture list? As it is, it's unclear that the reason that *RHEL* is the less-capable variant. -- 真実はいつも一つ!/ Always, there's only one truth!
Re: [libvirt PATCH 9/9] spec: Introduce arches_*
On Mon, 2020-10-05 at 20:40 -0400, Neal Gompa wrote: > On Mon, Oct 5, 2020 at 2:41 PM Andrea Bolognani wrote: > > %if 0%{?fedora} > > +%define arches_qemu_kvm %{arches_x86} %{power64} s390x %{arm} > > aarch64 > > %else > > +%define arches_qemu_kvm x86_64 %{power64} aarch64 s390x > > %endif > > This conditional is functionally irrelevant. The superset defined for > Fedora does not change how things work for RHEL, and it'd be easier to > just use the one architecture set. The difference I can see is that %{ix86} is not currently included in %{arches_qemu_kvm} on RHEL, but with your change it would and, unlike what happens for 32-bit ARM, RHEL packages are actually being built on i686. Later on we have > > %define with_storage_gluster 0%{!?_without_storage_gluster:1} > > +%ifnarch %{arches_qemu_kvm} > > # gluster is only built where qemu driver is enabled on RHEL 8 > > %if 0%{?rhel} >= 8 > > %define with_storage_gluster 0 and AFAICT that would break with your proposed change, because we would try to build with gluster support on i686 RHEL where gluster is not actually available. -- Andrea Bolognani / Red Hat / Virtualization
Re: [libvirt PATCH 9/9] spec: Introduce arches_*
On Mon, Oct 5, 2020 at 2:41 PM Andrea Bolognani wrote: > > With this commit, all architecture lists that we base feature > enablement decisions on are defined within a few lines of each > other, increasing maintainability. > > Additionally, generic architecture lists that appear in the > conditions for multiple features are defined, so that repetition > is reduced. > > Note that a few checks (numactl, zfs, ceph) have been changed > from %ifarch to %ifnarch for consistency: while doing so, the > corresponding list of architectures has also been replaced with > the complement of the original one to ensure the overall behavior > would be preserved. > > Signed-off-by: Andrea Bolognani > --- > libvirt.spec.in | 48 ++-- > 1 file changed, 26 insertions(+), 22 deletions(-) > > diff --git a/libvirt.spec.in b/libvirt.spec.in > index b62b17ee80..32bc51b33c 100644 > --- a/libvirt.spec.in > +++ b/libvirt.spec.in > @@ -17,10 +17,22 @@ > %define _vpath_builddir %{_target_platform} > %endif > > +%define arches_64bitx86_64 %{power64} aarch64 s390x riscv64 > +%define arches_x86 %{ix86} x86_64 > + > +%define arches_systemtap_64bit %{arches_64bit} > +%define arches_dmidecode%{arches_x86} > +%define arches_xen %{arches_x86} > +%define arches_vbox %{arches_x86} > +%define arches_ceph %{arches_64bit} > +%define arches_zfs %{arches_x86} %{power64} %{arm} > +%define arches_numactl %{arches_x86} %{power64} aarch64 > +%define arches_numad%{arches_x86} %{power64} aarch64 > + > %if 0%{?fedora} > -%define qemu_kvm_arches %{ix86} x86_64 %{power64} s390x %{arm} > aarch64 > +%define arches_qemu_kvm %{arches_x86} %{power64} s390x %{arm} aarch64 > %else > -%define qemu_kvm_arches x86_64 %{power64} aarch64 s390x > +%define arches_qemu_kvm x86_64 %{power64} aarch64 s390x > %endif > This conditional is functionally irrelevant. The superset defined for Fedora does not change how things work for RHEL, and it'd be easier to just use the one architecture set. > # The hypervisor drivers that run in libvirtd > @@ -29,7 +41,7 @@ > %define with_libxl 0%{!?_without_libxl:1} > %define with_vbox 0%{!?_without_vbox:1} > > -%ifarch %{qemu_kvm_arches} > +%ifarch %{arches_qemu_kvm} > %define with_qemu_kvm %{with_qemu} > %else > %define with_qemu_kvm 0 > @@ -61,7 +73,7 @@ > %endif > > %define with_storage_gluster 0%{!?_without_storage_gluster:1} > -%ifnarch %{qemu_kvm_arches} > +%ifnarch %{arches_qemu_kvm} > # gluster is only built where qemu driver is enabled on RHEL 8 > %if 0%{?rhel} >= 8 > %define with_storage_gluster 0 > @@ -98,28 +110,20 @@ > > # Finally set the OS / architecture specific special cases > > -# Xen is available only on i386 x86_64 > -%ifnarch %{ix86} x86_64 > +# Architecture-dependent features > +%ifnarch %{arches_xen} > %define with_libxl 0 > %endif > - > -# vbox is available only on i386 x86_64 > -%ifnarch %{ix86} x86_64 > +%ifnarch %{arches_vbox} > %define with_vbox 0 > %endif > - > -# Numactl is not available on many non-x86 archs > -%ifarch s390x %{arm} riscv64 > +%ifnarch %{arches_numactl} > %define with_numactl 0 > %endif > - > -# zfs-fuse is not available on some architectures > -%ifarch s390x aarch64 riscv64 > +%ifnarch %{arches_zfs} > %define with_storage_zfs 0 > %endif > - > -# Ceph dropped support for 32-bit hosts > -%ifarch %{arm} %{ix86} > +%ifnarch %{arches_ceph} > %define with_storage_rbd 0 > %endif > > @@ -155,7 +159,7 @@ > %define with_sanlock 0%{!?_without_sanlock:1} > %endif > %if 0%{?rhel} > -%ifarch %{qemu_kvm_arches} > +%ifarch %{arches_qemu_kvm} > %define with_sanlock 0%{!?_without_sanlock:1} > %endif > %endif > @@ -179,12 +183,12 @@ > %if %{with_qemu} || %{with_lxc} > # numad is used to manage the CPU and memory placement dynamically, > # it's not available on many non-x86 architectures. > -%ifnarch s390x %{arm} riscv64 > +%ifnarch %{arches_numad} > %define with_numad0%{!?_without_numad:1} > %endif > %endif > > -%ifarch %{ix86} x86_64 > +%ifarch %{arches_dmidecode} > %define with_dmidecode 0%{!?_without_dmidecode:1} > %endif > > @@ -1256,7 +1260,7 @@ rm -f > $RPM_BUILD_ROOT%{_datadir}/augeas/lenses/tests/test_libvirtd_libxl.aug > # Copied into libvirt-docs subpackage eventually > mv $RPM_BUILD_ROOT%{_datadir}/doc/libvirt libvirt-docs > > -%ifarch %{power64} s390x x86_64 aarch64 riscv64 > +%ifarch %{arches_systemtap_64bit} > mv $RPM_BUILD_ROOT%{_datadir}/systemtap/tapset/libvirt_probes.stp \ > $RPM_BUILD_ROOT%{_datadir}/systemtap/tapset/libvirt_probes-64.stp > > -- > 2.26.2 > -- 真実はいつも一つ!/ Always, there's only one truth!
Re: [libvirt PATCH 9/9] spec: Introduce arches_*
On Mon, 2020-10-05 at 20:40 +0200, Andrea Bolognani wrote: > +++ b/libvirt.spec.in > @@ -17,10 +17,22 @@ > %define _vpath_builddir %{_target_platform} > %endif > > +%define arches_64bitx86_64 %{power64} aarch64 s390x riscv64 > +%define arches_x86 %{ix86} x86_64 > + > +%define arches_systemtap_64bit %{arches_64bit} > +%define arches_dmidecode%{arches_x86} > +%define arches_xen %{arches_x86} > +%define arches_vbox %{arches_x86} > +%define arches_ceph %{arches_64bit} > +%define arches_zfs %{arches_x86} %{power64} %{arm} > +%define arches_numactl %{arches_x86} %{power64} aarch64 > +%define arches_numad%{arches_x86} %{power64} aarch64 Pushing commit 5ebf0638972c5922d32e9819f2f979f3345bd9c2 Author: Neal Gompa Date: Sun Oct 4 22:16:57 2020 -0400 rpm: Enable Xen support on AArch64 Starting with Linux 5.9, Xen Dom0 works on commonly available AArch64 devices, such as the Raspberry Pi 4. Reference: https://xenproject.org/2020/09/29/xen-on-raspberry-pi-4-adventures/ Signed-off-by: Neal Gompa Reviewed-by: Andrea Bolognani caused a conflict with this patch, which luckily can be trivially addressed; in addition, the diff below should be squashed in. diff --git a/libvirt.spec.in b/libvirt.spec.in index 32bc51b33c..4bd68f6a9e 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -22,7 +22,7 @@ %define arches_systemtap_64bit %{arches_64bit} %define arches_dmidecode%{arches_x86} -%define arches_xen %{arches_x86} +%define arches_xen %{arches_x86} aarch64 %define arches_vbox %{arches_x86} %define arches_ceph %{arches_64bit} %define arches_zfs %{arches_x86} %{power64} %{arm} -- Andrea Bolognani / Red Hat / Virtualization
[libvirt PATCH 9/9] spec: Introduce arches_*
With this commit, all architecture lists that we base feature enablement decisions on are defined within a few lines of each other, increasing maintainability. Additionally, generic architecture lists that appear in the conditions for multiple features are defined, so that repetition is reduced. Note that a few checks (numactl, zfs, ceph) have been changed from %ifarch to %ifnarch for consistency: while doing so, the corresponding list of architectures has also been replaced with the complement of the original one to ensure the overall behavior would be preserved. Signed-off-by: Andrea Bolognani --- libvirt.spec.in | 48 ++-- 1 file changed, 26 insertions(+), 22 deletions(-) diff --git a/libvirt.spec.in b/libvirt.spec.in index b62b17ee80..32bc51b33c 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -17,10 +17,22 @@ %define _vpath_builddir %{_target_platform} %endif +%define arches_64bitx86_64 %{power64} aarch64 s390x riscv64 +%define arches_x86 %{ix86} x86_64 + +%define arches_systemtap_64bit %{arches_64bit} +%define arches_dmidecode%{arches_x86} +%define arches_xen %{arches_x86} +%define arches_vbox %{arches_x86} +%define arches_ceph %{arches_64bit} +%define arches_zfs %{arches_x86} %{power64} %{arm} +%define arches_numactl %{arches_x86} %{power64} aarch64 +%define arches_numad%{arches_x86} %{power64} aarch64 + %if 0%{?fedora} -%define qemu_kvm_arches %{ix86} x86_64 %{power64} s390x %{arm} aarch64 +%define arches_qemu_kvm %{arches_x86} %{power64} s390x %{arm} aarch64 %else -%define qemu_kvm_arches x86_64 %{power64} aarch64 s390x +%define arches_qemu_kvm x86_64 %{power64} aarch64 s390x %endif # The hypervisor drivers that run in libvirtd @@ -29,7 +41,7 @@ %define with_libxl 0%{!?_without_libxl:1} %define with_vbox 0%{!?_without_vbox:1} -%ifarch %{qemu_kvm_arches} +%ifarch %{arches_qemu_kvm} %define with_qemu_kvm %{with_qemu} %else %define with_qemu_kvm 0 @@ -61,7 +73,7 @@ %endif %define with_storage_gluster 0%{!?_without_storage_gluster:1} -%ifnarch %{qemu_kvm_arches} +%ifnarch %{arches_qemu_kvm} # gluster is only built where qemu driver is enabled on RHEL 8 %if 0%{?rhel} >= 8 %define with_storage_gluster 0 @@ -98,28 +110,20 @@ # Finally set the OS / architecture specific special cases -# Xen is available only on i386 x86_64 -%ifnarch %{ix86} x86_64 +# Architecture-dependent features +%ifnarch %{arches_xen} %define with_libxl 0 %endif - -# vbox is available only on i386 x86_64 -%ifnarch %{ix86} x86_64 +%ifnarch %{arches_vbox} %define with_vbox 0 %endif - -# Numactl is not available on many non-x86 archs -%ifarch s390x %{arm} riscv64 +%ifnarch %{arches_numactl} %define with_numactl 0 %endif - -# zfs-fuse is not available on some architectures -%ifarch s390x aarch64 riscv64 +%ifnarch %{arches_zfs} %define with_storage_zfs 0 %endif - -# Ceph dropped support for 32-bit hosts -%ifarch %{arm} %{ix86} +%ifnarch %{arches_ceph} %define with_storage_rbd 0 %endif @@ -155,7 +159,7 @@ %define with_sanlock 0%{!?_without_sanlock:1} %endif %if 0%{?rhel} -%ifarch %{qemu_kvm_arches} +%ifarch %{arches_qemu_kvm} %define with_sanlock 0%{!?_without_sanlock:1} %endif %endif @@ -179,12 +183,12 @@ %if %{with_qemu} || %{with_lxc} # numad is used to manage the CPU and memory placement dynamically, # it's not available on many non-x86 architectures. -%ifnarch s390x %{arm} riscv64 +%ifnarch %{arches_numad} %define with_numad0%{!?_without_numad:1} %endif %endif -%ifarch %{ix86} x86_64 +%ifarch %{arches_dmidecode} %define with_dmidecode 0%{!?_without_dmidecode:1} %endif @@ -1256,7 +1260,7 @@ rm -f $RPM_BUILD_ROOT%{_datadir}/augeas/lenses/tests/test_libvirtd_libxl.aug # Copied into libvirt-docs subpackage eventually mv $RPM_BUILD_ROOT%{_datadir}/doc/libvirt libvirt-docs -%ifarch %{power64} s390x x86_64 aarch64 riscv64 +%ifarch %{arches_systemtap_64bit} mv $RPM_BUILD_ROOT%{_datadir}/systemtap/tapset/libvirt_probes.stp \ $RPM_BUILD_ROOT%{_datadir}/systemtap/tapset/libvirt_probes-64.stp -- 2.26.2