rpm: Simplify qemu+kvm conditionals and eliminate duplication

[PATCH] rpm: Simplify qemu+kvm conditionals and eliminate duplication

Posted by Neal Gompa 3 years, 7 months ago

The conditionals for enabling qemu+kvm were unnecessarily complex.
In practice, there were far fewer cases where the functionality would
be disabled than what the conditional logic expressed, and this change
simplifies it to the realistic set of options.

Signed-off-by: Neal Gompa <ngompa13@gmail.com>
---
 libvirt.spec.in | 49 +++++++++++++++++++++++--------------------------
 1 file changed, 23 insertions(+), 26 deletions(-)

diff --git a/libvirt.spec.in b/libvirt.spec.in
index c4a7c30737..6940066de9 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -20,31 +20,12 @@
 
 %define with_qemu_tcg      %{with_qemu}
 
-%define qemu_kvm_arches %{ix86} x86_64
-
-%if 0%{?fedora}
-    %define qemu_kvm_arches %{ix86} x86_64 %{power64} s390x %{arm} aarch64
-%endif
-
-%if 0%{?rhel}
-    %define with_qemu_tcg 0
-    %define qemu_kvm_arches x86_64 %{power64} aarch64 s390x
-%endif
-
 # On RHEL 7 and older macro _vpath_builddir is not defined.
 %if 0%{?rhel} && 0%{?rhel} <= 7
     %define _vpath_builddir %{_target_platform}
 %endif
 
-%ifarch %{qemu_kvm_arches}
-    %define with_qemu_kvm      %{with_qemu}
-%else
-    %define with_qemu_kvm      0
-%endif
-
-%if ! %{with_qemu_tcg} && ! %{with_qemu_kvm}
-    %define with_qemu 0
-%endif
+%define with_qemu_kvm      %{with_qemu}
 
 # Then the hypervisor drivers that run outside libvirtd, in libvirt.so
 %define with_openvz        0%{!?_without_openvz:1}
@@ -61,12 +42,6 @@
 %endif
 
 %define with_storage_gluster 0%{!?_without_storage_gluster:1}
-%ifnarch %{qemu_kvm_arches}
-    # gluster is only built where qemu driver is enabled on RHEL 8
-    %if 0%{?rhel} >= 8
-        %define with_storage_gluster 0
-    %endif
-%endif
 
 %define with_numactl          0%{!?_without_numactl:1}
 
@@ -97,6 +72,11 @@
 
 # Finally set the OS / architecture specific special cases
 
+# KVM is available on most architectures
+%ifnarch %{ix86} x86_64 %{power64} s390x %{arm} aarch64
+    %define with_qemu_kvm 0
+%endif
+
 # Xen is available only on i386 x86_64 ia64
 %ifnarch %{ix86} x86_64 ia64
     %define with_libxl 0
@@ -122,6 +102,23 @@
     %define with_storage_rbd 0
 %endif
 
+# RHEL does not ship qemu-tcg
+%if 0%{?rhel}
+    %define with_qemu_tcg 0
+%endif
+
+# In the event that qemu-tcg and qemu-kvm are unavailable, don't ship qemu
+%if ! %{with_qemu_tcg} && ! %{with_qemu_kvm}
+    %define with_qemu 0
+%endif
+
+%if ! %{with_qemu_kvm}
+    # gluster is only built where qemu driver is enabled on RHEL 8
+    %if 0%{?rhel} >= 8
+        %define with_storage_gluster 0
+    %endif
+%endif
+
 # RHEL doesn't ship OpenVZ, VBox, PowerHypervisor,
 # VMware, libxenlight (Xen 4.1 and newer),
 # or HyperV.
-- 
2.26.2

Re: [PATCH] rpm: Simplify qemu+kvm conditionals and eliminate duplication

Posted by Neal Gompa 3 years, 6 months ago

On Thu, Sep 24, 2020 at 4:05 PM Neal Gompa <ngompa13@gmail.com> wrote:
>
> The conditionals for enabling qemu+kvm were unnecessarily complex.
> In practice, there were far fewer cases where the functionality would
> be disabled than what the conditional logic expressed, and this change
> simplifies it to the realistic set of options.
>
> Signed-off-by: Neal Gompa <ngompa13@gmail.com>
> ---
>  libvirt.spec.in | 49 +++++++++++++++++++++++--------------------------
>  1 file changed, 23 insertions(+), 26 deletions(-)
>
> diff --git a/libvirt.spec.in b/libvirt.spec.in
> index c4a7c30737..6940066de9 100644
> --- a/libvirt.spec.in
> +++ b/libvirt.spec.in
> @@ -20,31 +20,12 @@
>
>  %define with_qemu_tcg      %{with_qemu}
>
> -%define qemu_kvm_arches %{ix86} x86_64
> -
> -%if 0%{?fedora}
> -    %define qemu_kvm_arches %{ix86} x86_64 %{power64} s390x %{arm} aarch64
> -%endif
> -
> -%if 0%{?rhel}
> -    %define with_qemu_tcg 0
> -    %define qemu_kvm_arches x86_64 %{power64} aarch64 s390x
> -%endif
> -
>  # On RHEL 7 and older macro _vpath_builddir is not defined.
>  %if 0%{?rhel} && 0%{?rhel} <= 7
>      %define _vpath_builddir %{_target_platform}
>  %endif
>
> -%ifarch %{qemu_kvm_arches}
> -    %define with_qemu_kvm      %{with_qemu}
> -%else
> -    %define with_qemu_kvm      0
> -%endif
> -
> -%if ! %{with_qemu_tcg} && ! %{with_qemu_kvm}
> -    %define with_qemu 0
> -%endif
> +%define with_qemu_kvm      %{with_qemu}
>
>  # Then the hypervisor drivers that run outside libvirtd, in libvirt.so
>  %define with_openvz        0%{!?_without_openvz:1}
> @@ -61,12 +42,6 @@
>  %endif
>
>  %define with_storage_gluster 0%{!?_without_storage_gluster:1}
> -%ifnarch %{qemu_kvm_arches}
> -    # gluster is only built where qemu driver is enabled on RHEL 8
> -    %if 0%{?rhel} >= 8
> -        %define with_storage_gluster 0
> -    %endif
> -%endif
>
>  %define with_numactl          0%{!?_without_numactl:1}
>
> @@ -97,6 +72,11 @@
>
>  # Finally set the OS / architecture specific special cases
>
> +# KVM is available on most architectures
> +%ifnarch %{ix86} x86_64 %{power64} s390x %{arm} aarch64
> +    %define with_qemu_kvm 0
> +%endif
> +
>  # Xen is available only on i386 x86_64 ia64
>  %ifnarch %{ix86} x86_64 ia64
>      %define with_libxl 0
> @@ -122,6 +102,23 @@
>      %define with_storage_rbd 0
>  %endif
>
> +# RHEL does not ship qemu-tcg
> +%if 0%{?rhel}
> +    %define with_qemu_tcg 0
> +%endif
> +
> +# In the event that qemu-tcg and qemu-kvm are unavailable, don't ship qemu
> +%if ! %{with_qemu_tcg} && ! %{with_qemu_kvm}
> +    %define with_qemu 0
> +%endif
> +
> +%if ! %{with_qemu_kvm}
> +    # gluster is only built where qemu driver is enabled on RHEL 8
> +    %if 0%{?rhel} >= 8
> +        %define with_storage_gluster 0
> +    %endif
> +%endif
> +
>  # RHEL doesn't ship OpenVZ, VBox, PowerHypervisor,
>  # VMware, libxenlight (Xen 4.1 and newer),
>  # or HyperV.
> --
> 2.26.2
>

Anyone seen this yet?


-- 
真実はいつも一つ！/ Always, there's only one truth!

Re: [PATCH] rpm: Simplify qemu+kvm conditionals and eliminate duplication

Posted by Pavel Hrdina 3 years, 6 months ago

On Thu, Sep 24, 2020 at 04:04:38PM -0400, Neal Gompa wrote:
> The conditionals for enabling qemu+kvm were unnecessarily complex.
> In practice, there were far fewer cases where the functionality would
> be disabled than what the conditional logic expressed, and this change
> simplifies it to the realistic set of options.
> 
> Signed-off-by: Neal Gompa <ngompa13@gmail.com>
> ---
>  libvirt.spec.in | 49 +++++++++++++++++++++++--------------------------
>  1 file changed, 23 insertions(+), 26 deletions(-)
> 
> diff --git a/libvirt.spec.in b/libvirt.spec.in
> index c4a7c30737..6940066de9 100644
> --- a/libvirt.spec.in
> +++ b/libvirt.spec.in
> @@ -20,31 +20,12 @@
>  
>  %define with_qemu_tcg      %{with_qemu}
>  
> -%define qemu_kvm_arches %{ix86} x86_64
> -
> -%if 0%{?fedora}
> -    %define qemu_kvm_arches %{ix86} x86_64 %{power64} s390x %{arm} aarch64
> -%endif
> -
> -%if 0%{?rhel}
> -    %define with_qemu_tcg 0
> -    %define qemu_kvm_arches x86_64 %{power64} aarch64 s390x
> -%endif
> -
>  # On RHEL 7 and older macro _vpath_builddir is not defined.
>  %if 0%{?rhel} && 0%{?rhel} <= 7
>      %define _vpath_builddir %{_target_platform}
>  %endif
>  
> -%ifarch %{qemu_kvm_arches}
> -    %define with_qemu_kvm      %{with_qemu}
> -%else
> -    %define with_qemu_kvm      0
> -%endif
> -
> -%if ! %{with_qemu_tcg} && ! %{with_qemu_kvm}
> -    %define with_qemu 0
> -%endif
> +%define with_qemu_kvm      %{with_qemu}

I would move this line above the rhel-7 _vpath_builddir workaround to
have it next to with_qemu_tcg.

>  # Then the hypervisor drivers that run outside libvirtd, in libvirt.so
>  %define with_openvz        0%{!?_without_openvz:1}
> @@ -61,12 +42,6 @@
>  %endif
>  
>  %define with_storage_gluster 0%{!?_without_storage_gluster:1}
> -%ifnarch %{qemu_kvm_arches}
> -    # gluster is only built where qemu driver is enabled on RHEL 8
> -    %if 0%{?rhel} >= 8
> -        %define with_storage_gluster 0
> -    %endif
> -%endif
>  
>  %define with_numactl          0%{!?_without_numactl:1}
>  
> @@ -97,6 +72,11 @@
>  
>  # Finally set the OS / architecture specific special cases
>  
> +# KVM is available on most architectures
> +%ifnarch %{ix86} x86_64 %{power64} s390x %{arm} aarch64
> +    %define with_qemu_kvm 0
> +%endif
> +
>  # Xen is available only on i386 x86_64 ia64
>  %ifnarch %{ix86} x86_64 ia64
>      %define with_libxl 0
> @@ -122,6 +102,23 @@
>      %define with_storage_rbd 0
>  %endif
>  
> +# RHEL does not ship qemu-tcg
> +%if 0%{?rhel}
> +    %define with_qemu_tcg 0
> +%endif
> +
> +# In the event that qemu-tcg and qemu-kvm are unavailable, don't ship qemu
> +%if ! %{with_qemu_tcg} && ! %{with_qemu_kvm}
> +    %define with_qemu 0
> +%endif
> +
> +%if ! %{with_qemu_kvm}
> +    # gluster is only built where qemu driver is enabled on RHEL 8
> +    %if 0%{?rhel} >= 8
> +        %define with_storage_gluster 0
> +    %endif
> +%endif
> +
>  # RHEL doesn't ship OpenVZ, VBox, PowerHypervisor,
>  # VMware, libxenlight (Xen 4.1 and newer),
>  # or HyperV.

There is one more place with qemu_kvm_arches:

%if 0%{?rhel}
    %ifarch %{qemu_kvm_arches}
        %define with_sanlock 0%{!?_without_sanlock:1}
    %endif
%endif

Otherwise looks good.

Pavel