[libvirt PATCH] spec: libvirt-daemon: Add optional dependency on *-client

Jiri Denemark posted 1 patch 1 year, 5 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/9ef91dc433754274752791b97f399c4cc977c6d4.1667492658.git.jdenemar@redhat.com
libvirt.spec.in | 6 ++++++
1 file changed, 6 insertions(+)
[libvirt PATCH] spec: libvirt-daemon: Add optional dependency on *-client
Posted by Jiri Denemark 1 year, 5 months ago
The libvirt-daemon subpackage contains libvirt-guests.sh script (used by
libvirt-guests service), which requires virsh to actually work. But
since dynamic libraries were separated from libvirt-client to
libvirt-libs more than 6 years ago, libvirt-daemon no longer requires
virsh to be installed. So unless libvirt-client is explicitly installed
(either manually or by installing the libvirt meta package),
libvirt-guests will not work.

Just adding libvirt-client as a dependency of libvirt-daemon would go
against the original idea behind splitting libvirt-client: users may not
want to install or use any client binaries on the host where the daemon
runs (either they just use various language bindings or access the
daemon remotely). To solve this we could possibly turn libvirt-daemon
into an empty package and separate the daemons and libvirt-guests into
subpackages to make sure we support both use cases, but marking
libvirt-client as Recommended for libvirt-daemon does the same job in a
much simpler way.

https://bugzilla.redhat.com/show_bug.cgi?id=2136591

Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
---
 libvirt.spec.in | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/libvirt.spec.in b/libvirt.spec.in
index 0bbcdb8956..450f50f5b5 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -441,6 +441,12 @@ Summary: Server side daemon and supporting files for libvirt library
 # The client side, i.e. shared libs are in a subpackage
 Requires: %{name}-libs = %{version}-%{release}
 
+# The libvirt-guests.sh script requires virsh from libvirt-client subpackage,
+# but not every deployment wants to use libvirt-guests service. Using
+# Recommends here will install libvirt-client by default (if available), but
+# RPM won't complain if the package is unavailable, masked, or removed later.
+Recommends: %{name}-client = %{version}-%{release}
+
 # netcat is needed on the server side so that clients that have
 # libvirt < 6.9.0 can connect, but newer versions will prefer
 # virt-ssh-helper. Making this a Recommends means that it gets
-- 
2.38.1
Re: [libvirt PATCH] spec: libvirt-daemon: Add optional dependency on *-client
Posted by Martin Kletzander 1 year, 5 months ago
On Thu, Nov 03, 2022 at 05:24:18PM +0100, Jiri Denemark wrote:
>The libvirt-daemon subpackage contains libvirt-guests.sh script (used by
>libvirt-guests service), which requires virsh to actually work. But
>since dynamic libraries were separated from libvirt-client to
>libvirt-libs more than 6 years ago, libvirt-daemon no longer requires
>virsh to be installed. So unless libvirt-client is explicitly installed
>(either manually or by installing the libvirt meta package),
>libvirt-guests will not work.
>
>Just adding libvirt-client as a dependency of libvirt-daemon would go
>against the original idea behind splitting libvirt-client: users may not
>want to install or use any client binaries on the host where the daemon
>runs (either they just use various language bindings or access the
>daemon remotely). To solve this we could possibly turn libvirt-daemon
>into an empty package and separate the daemons and libvirt-guests into
>subpackages to make sure we support both use cases, but marking
>libvirt-client as Recommended for libvirt-daemon does the same job in a
>much simpler way.
>

Or you could just move the libvirt-guests files to libvirt-client
package since they couldn't work without it anyway.

>https://bugzilla.redhat.com/show_bug.cgi?id=2136591
>
>Signed-off-by: Jiri Denemark <jdenemar@redhat.com>

Reviewed-by: Martin Kletzander <mkletzan@redhat.com>

>---
> libvirt.spec.in | 6 ++++++
> 1 file changed, 6 insertions(+)
>
>diff --git a/libvirt.spec.in b/libvirt.spec.in
>index 0bbcdb8956..450f50f5b5 100644
>--- a/libvirt.spec.in
>+++ b/libvirt.spec.in
>@@ -441,6 +441,12 @@ Summary: Server side daemon and supporting files for libvirt library
> # The client side, i.e. shared libs are in a subpackage
> Requires: %{name}-libs = %{version}-%{release}
>
>+# The libvirt-guests.sh script requires virsh from libvirt-client subpackage,
>+# but not every deployment wants to use libvirt-guests service. Using
>+# Recommends here will install libvirt-client by default (if available), but
>+# RPM won't complain if the package is unavailable, masked, or removed later.
>+Recommends: %{name}-client = %{version}-%{release}
>+
> # netcat is needed on the server side so that clients that have
> # libvirt < 6.9.0 can connect, but newer versions will prefer
> # virt-ssh-helper. Making this a Recommends means that it gets
>-- 
>2.38.1
>
Re: [libvirt PATCH] spec: libvirt-daemon: Add optional dependency on *-client
Posted by Jim Fehlig 1 year, 5 months ago
On 11/4/22 09:22, Martin Kletzander wrote:
> On Thu, Nov 03, 2022 at 05:24:18PM +0100, Jiri Denemark wrote:
>> The libvirt-daemon subpackage contains libvirt-guests.sh script (used by
>> libvirt-guests service), which requires virsh to actually work. But
>> since dynamic libraries were separated from libvirt-client to
>> libvirt-libs more than 6 years ago, libvirt-daemon no longer requires
>> virsh to be installed. So unless libvirt-client is explicitly installed
>> (either manually or by installing the libvirt meta package),
>> libvirt-guests will not work.
>>
>> Just adding libvirt-client as a dependency of libvirt-daemon would go
>> against the original idea behind splitting libvirt-client: users may not
>> want to install or use any client binaries on the host where the daemon
>> runs (either they just use various language bindings or access the
>> daemon remotely). To solve this we could possibly turn libvirt-daemon
>> into an empty package and separate the daemons and libvirt-guests into
>> subpackages to make sure we support both use cases, but marking
>> libvirt-client as Recommended for libvirt-daemon does the same job in a
>> much simpler way.
>>
> 
> Or you could just move the libvirt-guests files to libvirt-client
> package since they couldn't work without it anyway.

This actually seems like a better approach, especially in the context of modular 
daemons.

Regards,
Jim
Re: [libvirt PATCH] spec: libvirt-daemon: Add optional dependency on *-client
Posted by Jiri Denemark 1 year, 5 months ago
On Fri, Nov 04, 2022 at 10:09:31 -0600, Jim Fehlig wrote:
> On 11/4/22 09:22, Martin Kletzander wrote:
> > On Thu, Nov 03, 2022 at 05:24:18PM +0100, Jiri Denemark wrote:
> >> The libvirt-daemon subpackage contains libvirt-guests.sh script (used by
> >> libvirt-guests service), which requires virsh to actually work. But
> >> since dynamic libraries were separated from libvirt-client to
> >> libvirt-libs more than 6 years ago, libvirt-daemon no longer requires
> >> virsh to be installed. So unless libvirt-client is explicitly installed
> >> (either manually or by installing the libvirt meta package),
> >> libvirt-guests will not work.
> >>
> >> Just adding libvirt-client as a dependency of libvirt-daemon would go
> >> against the original idea behind splitting libvirt-client: users may not
> >> want to install or use any client binaries on the host where the daemon
> >> runs (either they just use various language bindings or access the
> >> daemon remotely). To solve this we could possibly turn libvirt-daemon
> >> into an empty package and separate the daemons and libvirt-guests into
> >> subpackages to make sure we support both use cases, but marking
> >> libvirt-client as Recommended for libvirt-daemon does the same job in a
> >> much simpler way.
> >>
> > 
> > Or you could just move the libvirt-guests files to libvirt-client
> > package since they couldn't work without it anyway.
> 
> This actually seems like a better approach, especially in the context of modular 
> daemons.

I think installing system services as part of libvirt-client is even
stranger than requiring libvirt-client as a dependency of the daemon
package.

It would make more sense to split the daemon package, separate the
monolithic daemon, proxy, and libvirt-guests so that one can only
install the parts that are actually needed when using modular daemons,
but I think it's quite an overkill for several reasons. First, the
monolithic daemon is supposed to completely disappear at some point. And
the ancient libvirt-guests service should really be replaced by a
solution implemented by the daemons themselves. And not only because the
libvirt-guests service may cause a modular daemon to be started when a
host is being shut down in case no domain is running.

Jirka
Re: [libvirt PATCH] spec: libvirt-daemon: Add optional dependency on *-client
Posted by Jim Fehlig 1 year, 5 months ago
On 11/7/22 04:32, Jiri Denemark wrote:
> On Fri, Nov 04, 2022 at 10:09:31 -0600, Jim Fehlig wrote:
>> On 11/4/22 09:22, Martin Kletzander wrote:
>>> On Thu, Nov 03, 2022 at 05:24:18PM +0100, Jiri Denemark wrote:
>>>> The libvirt-daemon subpackage contains libvirt-guests.sh script (used by
>>>> libvirt-guests service), which requires virsh to actually work. But
>>>> since dynamic libraries were separated from libvirt-client to
>>>> libvirt-libs more than 6 years ago, libvirt-daemon no longer requires
>>>> virsh to be installed. So unless libvirt-client is explicitly installed
>>>> (either manually or by installing the libvirt meta package),
>>>> libvirt-guests will not work.
>>>>
>>>> Just adding libvirt-client as a dependency of libvirt-daemon would go
>>>> against the original idea behind splitting libvirt-client: users may not
>>>> want to install or use any client binaries on the host where the daemon
>>>> runs (either they just use various language bindings or access the
>>>> daemon remotely). To solve this we could possibly turn libvirt-daemon
>>>> into an empty package and separate the daemons and libvirt-guests into
>>>> subpackages to make sure we support both use cases, but marking
>>>> libvirt-client as Recommended for libvirt-daemon does the same job in a
>>>> much simpler way.
>>>>
>>>
>>> Or you could just move the libvirt-guests files to libvirt-client
>>> package since they couldn't work without it anyway.
>>
>> This actually seems like a better approach, especially in the context of modular
>> daemons.
> 
> I think installing system services as part of libvirt-client is even
> stranger than requiring libvirt-client as a dependency of the daemon
> package.

Nod. Neither is a good solution.

> It would make more sense to split the daemon package, separate the
> monolithic daemon, proxy, and libvirt-guests so that one can only
> install the parts that are actually needed when using modular daemons,

I've taken an initial stab at that

https://listman.redhat.com/archives/libvir-list/2022-November/235924.html

> but I think it's quite an overkill for several reasons. First, the
> monolithic daemon is supposed to completely disappear at some point.

In the meantime they could coexist a bit nicer with something like the above.

> And the ancient libvirt-guests service should really be replaced by a
> solution implemented by the daemons themselves. And not only because the
> libvirt-guests service may cause a modular daemon to be started when a
> host is being shut down in case no domain is running.

Agreed, but IMO this is a separate problem to solve.

Regards,
Jim
Re: [libvirt PATCH] spec: libvirt-daemon: Add optional dependency on *-client
Posted by Martin Kletzander 1 year, 5 months ago
On Fri, Nov 04, 2022 at 10:09:31AM -0600, Jim Fehlig wrote:
>On 11/4/22 09:22, Martin Kletzander wrote:
>> On Thu, Nov 03, 2022 at 05:24:18PM +0100, Jiri Denemark wrote:
>>> The libvirt-daemon subpackage contains libvirt-guests.sh script (used by
>>> libvirt-guests service), which requires virsh to actually work. But
>>> since dynamic libraries were separated from libvirt-client to
>>> libvirt-libs more than 6 years ago, libvirt-daemon no longer requires
>>> virsh to be installed. So unless libvirt-client is explicitly installed
>>> (either manually or by installing the libvirt meta package),
>>> libvirt-guests will not work.
>>>
>>> Just adding libvirt-client as a dependency of libvirt-daemon would go
>>> against the original idea behind splitting libvirt-client: users may not
>>> want to install or use any client binaries on the host where the daemon
>>> runs (either they just use various language bindings or access the
>>> daemon remotely). To solve this we could possibly turn libvirt-daemon
>>> into an empty package and separate the daemons and libvirt-guests into
>>> subpackages to make sure we support both use cases, but marking
>>> libvirt-client as Recommended for libvirt-daemon does the same job in a
>>> much simpler way.
>>>
>>
>> Or you could just move the libvirt-guests files to libvirt-client
>> package since they couldn't work without it anyway.
>
>This actually seems like a better approach, especially in the context of modular
>daemons.
>

Unfortunately, now that I think about it, that would have to be dealt with a
little bit more so that people don't see the libvirt-guests being removed.
Using "Provides" would not help, but in the combination with the Recommends it
could, theoretically work.  I'll let Jiri think about it because he had a bunch
of ideas and reasoning behind this decision.

>Regards,
>Jim
>
Re: [libvirt PATCH] spec: libvirt-daemon: Add optional dependency on *-client
Posted by Andrea Bolognani 1 year, 5 months ago
On Mon, Nov 07, 2022 at 10:18:14AM +0100, Martin Kletzander wrote:
> On Fri, Nov 04, 2022 at 10:09:31AM -0600, Jim Fehlig wrote:
> > On 11/4/22 09:22, Martin Kletzander wrote:
> > > On Thu, Nov 03, 2022 at 05:24:18PM +0100, Jiri Denemark wrote:
> > > > The libvirt-daemon subpackage contains libvirt-guests.sh script (used by
> > > > libvirt-guests service), which requires virsh to actually work. But
> > > > since dynamic libraries were separated from libvirt-client to
> > > > libvirt-libs more than 6 years ago, libvirt-daemon no longer requires
> > > > virsh to be installed. So unless libvirt-client is explicitly installed
> > > > (either manually or by installing the libvirt meta package),
> > > > libvirt-guests will not work.
> > > >
> > > > Just adding libvirt-client as a dependency of libvirt-daemon would go
> > > > against the original idea behind splitting libvirt-client: users may not
> > > > want to install or use any client binaries on the host where the daemon
> > > > runs (either they just use various language bindings or access the
> > > > daemon remotely). To solve this we could possibly turn libvirt-daemon
> > > > into an empty package and separate the daemons and libvirt-guests into
> > > > subpackages to make sure we support both use cases, but marking
> > > > libvirt-client as Recommended for libvirt-daemon does the same job in a
> > > > much simpler way.
> > >
> > > Or you could just move the libvirt-guests files to libvirt-client
> > > package since they couldn't work without it anyway.
> >
> > This actually seems like a better approach, especially in the context of modular
> > daemons.
>
> Unfortunately, now that I think about it, that would have to be dealt with a
> little bit more so that people don't see the libvirt-guests being removed.
> Using "Provides" would not help, but in the combination with the Recommends it
> could, theoretically work.  I'll let Jiri think about it because he had a bunch
> of ideas and reasoning behind this decision.

libvirt-guests is a service, so it doesn't feel like it would fit
well in the libvirt-client package.

I think the Recommends is the right way to go, and to be honest even
a hard Depends wouldn't feel unreasonable: the main reason to split
the client bits from the server bits is so that *clients* can be very
lightweight, but the server is going to be heavyweight by definition
and going out of our way to avoid the extra ~1MB feels unnecessary.
virsh is *the* tool to manage, inspect and debug a virtualization
server, so why wouldn't you want to have it handy anyway?

Either as-is or with the Recommends upgraded to a Depends

  Reviewed-by: Andrea Bolognani <abologna@redhat.com>

-- 
Andrea Bolognani / Red Hat / Virtualization