3:24 a.m.
CI pipeline: https://gitlab.com/abologna/libvirt/-/pipelines/1292193155
Andrea Bolognani (4):
rpm: Drop weak dependency on ssh-proxy from client
rpm: Only Recommend ssh-proxy
rpm: Move dependency on ssh-proxy to QEMU driver
rpm: Drop with_ssh_proxy define
libvirt.spec.in | 23 ++---------------------
1 file changed, 2 insertions(+), 21 deletions(-)
--
2.45.0
3:24 a.m.
New subject: [PATCH 1/4] rpm: Drop weak dependency on ssh-proxy from client
The ssh-proxy feature works independently of the clients,
just like the NSS plugin does.
Moreover, ssh-proxy only works for local VMs, while clients
are routinely used to manage remote hypervisors.
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
libvirt.spec.in | 3 ---
1 file changed, 3 deletions(-)
diff --git a/libvirt.spec.in b/libvirt.spec.in
index f7c128d809..5cb19fa433 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -1025,9 +1025,6 @@ Summary: Client side utilities of the libvirt library
Requires: libvirt-libs = %{version}-%{release}
# Needed by virt-pki-validate script.
Requires: gnutls-utils
- %if %{with_ssh_proxy}
-Recommends: libvirt-ssh-proxy = %{version}-%{release}
- %endif
# Ensure smooth upgrades
Obsoletes: libvirt-bash-completion < 7.3.0
--
2.45.0
4:01 a.m.
New subject: [PATCH 1/4] rpm: Drop weak dependency on ssh-proxy from client
On Thu, May 16, 2024 at 10:24:19AM +0200, Andrea Bolognani wrote:
The ssh-proxy feature works independently of the clients,
just like the NSS plugin does.
Moreover, ssh-proxy only works for local VMs, while clients
are routinely used to manage remote hypervisors.
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
libvirt.spec.in | 3 ---
1 file changed, 3 deletions(-)
Yes, I guess this is ultimately pointless, as if the users picked
the libvirt-daemon-{qemu,kvm} convenience RPMs, they'll have the
SSH proxy installed regardless. If they didn't pick that and
selected individual RPMs, without libvirt-ssh-proxy, then we should
not be pulling it in anyway with the client.
Reviewed-by: Daniel P. Berrangé <berrange(a)redhat.com>
diff --git a/libvirt.spec.in b/libvirt.spec.in
index f7c128d809..5cb19fa433 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -1025,9 +1025,6 @@ Summary: Client side utilities of the libvirt library
Requires: libvirt-libs = %{version}-%{release}
# Needed by virt-pki-validate script.
Requires: gnutls-utils
- %if %{with_ssh_proxy}
-Recommends: libvirt-ssh-proxy = %{version}-%{release}
- %endif
# Ensure smooth upgrades
Obsoletes: libvirt-bash-completion < 7.3.0
--
2.45.0
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 :|
3:24 a.m.
New subject: [PATCH 2/4] rpm: Only Recommend ssh-proxy
The way things are implemented, installing the package not
only makes the feature available but also enables it.
Some admins might not want that to happen, so let's make the
dependency a weak one to offer them a way out.
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
libvirt.spec.in | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/libvirt.spec.in b/libvirt.spec.in
index 5cb19fa433..329b923e8f 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -905,7 +905,7 @@ Requires: libvirt-daemon-driver-nwfilter = %{version}-%{release}
Requires: libvirt-daemon-driver-secret = %{version}-%{release}
Requires: libvirt-daemon-driver-storage = %{version}-%{release}
%if %{with_ssh_proxy}
-Requires: libvirt-ssh-proxy = %{version}-%{release}
+Recommends: libvirt-ssh-proxy = %{version}-%{release}
%endif
Requires: qemu
@@ -936,7 +936,7 @@ Requires: libvirt-daemon-driver-nwfilter = %{version}-%{release}
Requires: libvirt-daemon-driver-secret = %{version}-%{release}
Requires: libvirt-daemon-driver-storage = %{version}-%{release}
%if %{with_ssh_proxy}
-Requires: libvirt-ssh-proxy = %{version}-%{release}
+Recommends: libvirt-ssh-proxy = %{version}-%{release}
%endif
Requires: qemu-kvm
--
2.45.0
3:50 a.m.
New subject: [PATCH 2/4] rpm: Only Recommend ssh-proxy
On Thu, May 16, 2024 at 10:24:20AM +0200, Andrea Bolognani wrote:
The way things are implemented, installing the package not
only makes the feature available but also enables it.
Some admins might not want that to happen, so let's make the
dependency a weak one to offer them a way out.
The whole reason for existance of the 'libvirt-daemon-{kvm,qemu}'
packages is to install the *entire* feature set relevant to QEMU/
KVM.
If admins want to select individual components, then pick the
actual sub-RPMs desire, and ignore libvirt-daemon{kvm,qemu}
meta packages.
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
libvirt.spec.in | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/libvirt.spec.in b/libvirt.spec.in
index 5cb19fa433..329b923e8f 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -905,7 +905,7 @@ Requires: libvirt-daemon-driver-nwfilter = %{version}-%{release}
Requires: libvirt-daemon-driver-secret = %{version}-%{release}
Requires: libvirt-daemon-driver-storage = %{version}-%{release}
%if %{with_ssh_proxy}
-Requires: libvirt-ssh-proxy = %{version}-%{release}
+Recommends: libvirt-ssh-proxy = %{version}-%{release}
%endif
Requires: qemu
@@ -936,7 +936,7 @@ Requires: libvirt-daemon-driver-nwfilter = %{version}-%{release}
Requires: libvirt-daemon-driver-secret = %{version}-%{release}
Requires: libvirt-daemon-driver-storage = %{version}-%{release}
%if %{with_ssh_proxy}
-Requires: libvirt-ssh-proxy = %{version}-%{release}
+Recommends: libvirt-ssh-proxy = %{version}-%{release}
%endif
Requires: qemu-kvm
--
2.45.0
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 :|
3:24 a.m.
New subject: [PATCH 3/4] rpm: Move dependency on ssh-proxy to QEMU driver
This way we can avoid repeating it twice.
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
libvirt.spec.in | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/libvirt.spec.in b/libvirt.spec.in
index 329b923e8f..0d6f15460d 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -810,6 +810,9 @@ 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
@@ -904,9 +907,6 @@ Requires: libvirt-daemon-driver-nodedev = %{version}-%{release}
Requires: libvirt-daemon-driver-nwfilter = %{version}-%{release}
Requires: libvirt-daemon-driver-secret = %{version}-%{release}
Requires: libvirt-daemon-driver-storage = %{version}-%{release}
- %if %{with_ssh_proxy}
-Recommends: libvirt-ssh-proxy = %{version}-%{release}
- %endif
Requires: qemu
%description daemon-qemu
@@ -935,9 +935,6 @@ Requires: libvirt-daemon-driver-nodedev = %{version}-%{release}
Requires: libvirt-daemon-driver-nwfilter = %{version}-%{release}
Requires: libvirt-daemon-driver-secret = %{version}-%{release}
Requires: libvirt-daemon-driver-storage = %{version}-%{release}
- %if %{with_ssh_proxy}
-Recommends: libvirt-ssh-proxy = %{version}-%{release}
- %endif
Requires: qemu-kvm
%description daemon-kvm
--
2.45.0
3:53 a.m.
New subject: [PATCH 3/4] rpm: Move dependency on ssh-proxy to QEMU driver
On Thu, May 16, 2024 at 10:24:21AM +0200, Andrea Bolognani wrote:
This way we can avoid repeating it twice.
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
libvirt.spec.in | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
This shouldn't be done. It will pull in the SSH proxy on every
install of the 'libvirt-ademon-driver-qemu' RPM which is not
desirable - this RPM is supposed to be providing the bare
minimum install with no optional extras.
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 :|
3:24 a.m.
New subject: [PATCH 4/4] rpm: Drop with_ssh_proxy define
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(a)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.0
4:03 a.m.
New subject: [PATCH 4/4] rpm: Drop with_ssh_proxy define
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
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
libvirt.spec.in | 15 +--------------
1 file changed, 1 insertion(+), 14 deletions(-)
Reviewed-by: Daniel P. Berrangé <berrange(a)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 :|
4:23 a.m.
New subject: [PATCH 4/4] rpm: Drop with_ssh_proxy define
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
4:26 a.m.
New subject: [PATCH 4/4] rpm: Drop with_ssh_proxy define
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 :|
4:34 a.m.
New subject: [PATCH 4/4] rpm: Drop with_ssh_proxy define
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
6:20 a.m.
New subject: [PATCH 4/4] rpm: Drop with_ssh_proxy define
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
220
days inactive
220
days old
12 comments
3 participants
participants (3)
-
Andrea Bolognani -
Daniel P. Berrangé -
Michal Prívozník