As a general rule, we use defines for features that can only be
enabled on a subset of the platforms that we target, and we
don't offer fine-grained control over every single possible
meson configuration knob at the RPM level.
In the case of ssh-proxy, we are enabling it everywhere already,
so having a define for it is unnecessary.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
---
libvirt.spec.in | 15 +--------------
1 file changed, 1 insertion(+), 14 deletions(-)
diff --git a/libvirt.spec.in b/libvirt.spec.in
index 0d6f15460d..b6f9bf86f3 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -91,7 +91,6 @@
# Other optional features
%define with_numactl 0%{!?_without_numactl:1}
%define with_userfaultfd_sysctl 0%{!?_without_userfaultfd_sysctl:1}
-%define with_ssh_proxy 0%{!?_without_ssh_proxy:1}
# A few optional bits off by default, we enable later
%define with_fuse 0
@@ -810,9 +809,7 @@ Summary: QEMU driver plugin for the libvirtd daemon
Requires: libvirt-daemon-common = %{version}-%{release}
Requires: libvirt-daemon-log = %{version}-%{release}
Requires: libvirt-libs = %{version}-%{release}
- %if %{with_ssh_proxy}
Recommends: libvirt-ssh-proxy = %{version}-%{release}
- %endif
Requires: /usr/bin/qemu-img
# For image compression
Requires: gzip
@@ -1104,14 +1101,12 @@ Requires: libvirt-daemon-driver-network = %{version}-%{release}
Libvirt plugin for NSS for translating domain names into IP addresses.
%endif
-%if %{with_ssh_proxy}
%package ssh-proxy
Summary: Libvirt SSH proxy
Requires: libvirt-libs = %{version}-%{release}
%description ssh-proxy
Allows SSH into domains via VSOCK without need for network.
-%endif
%if %{with_mingw32}
%package -n mingw32-libvirt
@@ -1304,12 +1299,6 @@ exit 1
%define arg_userfaultfd_sysctl -Duserfaultfd_sysctl=disabled
%endif
-%if %{with_ssh_proxy}
- %define arg_ssh_proxy -Dssh_proxy=enabled
-%else
- %define arg_ssh_proxy -Dssh_proxy=disabled
-%endif
-
%define when %(date +"%%F-%%T")
%define where %(hostname)
%define who %{?packager}%{!?packager:Unknown}
@@ -1391,7 +1380,7 @@ export SOURCE_DATE_EPOCH=$(stat --printf='%Y' %{_specdir}/libvirt.spec)
-Dtls_priority=%{tls_priority} \
-Dsysctl_config=enabled \
%{?arg_userfaultfd_sysctl} \
- %{?arg_ssh_proxy} \
+ -Dssh_proxy=enabled \
%{?enable_werror} \
-Dexpensive_tests=enabled \
-Dinit_script=systemd \
@@ -2447,11 +2436,9 @@ exit 0
%{_libdir}/libnss_libvirt.so.2
%{_libdir}/libnss_libvirt_guest.so.2
- %if %{with_ssh_proxy}
%files ssh-proxy
%config(noreplace) %{_sysconfdir}/ssh/ssh_config.d/30-libvirt-ssh-proxy.conf
%{_libexecdir}/libvirt-ssh-proxy
- %endif
%if %{with_lxc}
%files login-shell
--
2.45.0On Thu, May 16, 2024 at 10:24:22AM +0200, Andrea Bolognani wrote: > As a general rule, we use defines for features that can only be > enabled on a subset of the platforms that we target, and we > don't offer fine-grained control over every single possible > meson configuration knob at the RPM level. > > In the case of ssh-proxy, we are enabling it everywhere already, > so having a define for it is unnecessary. The only reason for a conditional would be if some older distro lacks support for this SSH proxy'ing feature. Assuming RHEL-9 / Ubuntu 22.04 have it, then this is indeed redundant > > Signed-off-by: Andrea Bolognani <abologna@redhat.com> > --- > libvirt.spec.in | 15 +-------------- > 1 file changed, 1 insertion(+), 14 deletions(-) Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On Thu, May 16, 2024 at 10:03:02AM GMT, Daniel P. Berrangé wrote: > On Thu, May 16, 2024 at 10:24:22AM +0200, Andrea Bolognani wrote: > > As a general rule, we use defines for features that can only be > > enabled on a subset of the platforms that we target, and we > > don't offer fine-grained control over every single possible > > meson configuration knob at the RPM level. > > > > In the case of ssh-proxy, we are enabling it everywhere already, > > so having a define for it is unnecessary. > > The only reason for a conditional would be if some older distro lacks > support for this SSH proxy'ing feature. Assuming RHEL-9 / Ubuntu 22.04 > have it, then this is indeed redundant We don't need to worry about Ubuntu in the spec file ;) IIUC requirements are mostly on the guest OS side, and on the host OS side we just need the ssh ProxyCommand feature which would have been available since forever. Michal, can you please confirm that this is accurate? -- Andrea Bolognani / Red Hat / Virtualization
On 5/16/24 11:23, Andrea Bolognani wrote: > On Thu, May 16, 2024 at 10:03:02AM GMT, Daniel P. Berrangé wrote: >> On Thu, May 16, 2024 at 10:24:22AM +0200, Andrea Bolognani wrote: >>> As a general rule, we use defines for features that can only be >>> enabled on a subset of the platforms that we target, and we >>> don't offer fine-grained control over every single possible >>> meson configuration knob at the RPM level. >>> >>> In the case of ssh-proxy, we are enabling it everywhere already, >>> so having a define for it is unnecessary. >> >> The only reason for a conditional would be if some older distro lacks >> support for this SSH proxy'ing feature. Assuming RHEL-9 / Ubuntu 22.04 >> have it, then this is indeed redundant > > We don't need to worry about Ubuntu in the spec file ;) > > IIUC requirements are mostly on the guest OS side, and on the host OS > side we just need the ssh ProxyCommand feature which would have been > available since forever. Michal, can you please confirm that this is > accurate? > Indeed. I just wanted to give distro maintainers ability to fine tune what features they enable. But I guess it makes sens to have this always enabled. Michal
On Thu, May 16, 2024 at 02:23:13AM -0700, Andrea Bolognani wrote: > On Thu, May 16, 2024 at 10:03:02AM GMT, Daniel P. Berrangé wrote: > > On Thu, May 16, 2024 at 10:24:22AM +0200, Andrea Bolognani wrote: > > > As a general rule, we use defines for features that can only be > > > enabled on a subset of the platforms that we target, and we > > > don't offer fine-grained control over every single possible > > > meson configuration knob at the RPM level. > > > > > > In the case of ssh-proxy, we are enabling it everywhere already, > > > so having a define for it is unnecessary. > > > > The only reason for a conditional would be if some older distro lacks > > support for this SSH proxy'ing feature. Assuming RHEL-9 / Ubuntu 22.04 > > have it, then this is indeed redundant > > We don't need to worry about Ubuntu in the spec file ;) Heh. Face palm. > IIUC requirements are mostly on the guest OS side, and on the host OS > side we just need the ssh ProxyCommand feature which would have been > available since forever. Michal, can you please confirm that this is > accurate? We need ProxyUseFdpass too, and I wasn't sure how far back that one went. With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On Thu, May 16, 2024 at 10:26:28AM GMT, Daniel P. Berrangé wrote: > On Thu, May 16, 2024 at 02:23:13AM -0700, Andrea Bolognani wrote: > > IIUC requirements are mostly on the guest OS side, and on the host OS > > side we just need the ssh ProxyCommand feature which would have been > > available since forever. Michal, can you please confirm that this is > > accurate? > > We need ProxyUseFdpass too, and I wasn't sure how far back that one > went. According to [1] it was introduced in OpenSSH 6.5 (2014), so we should be good. [1] https://www.openssh.com/releasenotes.html -- Andrea Bolognani / Red Hat / Virtualization
© 2016 - 2024 Red Hat, Inc.