The former is ridiculously tiny and doesn't have any use on
its own, so it hardly warrants the overhead of an additional
package; the latter is also very small and, just like virsh,
is something that you likely want to have available on any
virtualization host to help with management and debugging
tasks.

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
---
 libvirt.spec.in | 28 ++++------------------------
 1 file changed, 4 insertions(+), 24 deletions(-)

diff --git a/libvirt.spec.in b/libvirt.spec.in
index f9af330186..80a12a307a 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -901,7 +901,6 @@ Requires: %{name}-libs = %{version}-%{release}
 Requires: gettext
 # Needed by virt-pki-validate script.
 Requires: gnutls-utils
-Requires: %{name}-bash-completion = %{version}-%{release}
 
 %description client
 The client binaries needed to access the virtualization
@@ -918,20 +917,6 @@ Requires: cyrus-sasl-gssapi
 %description libs
 Shared libraries for accessing the libvirt daemon.
 
-%package admin
-Summary: Set of tools to control libvirt daemon
-Requires: %{name}-libs = %{version}-%{release}
-Requires: %{name}-bash-completion = %{version}-%{release}
-
-%description admin
-The client side utilities to control the libvirt daemon.
-
-%package bash-completion
-Summary: Bash completion script
-
-%description bash-completion
-Bash completion script stub.
-
 %if %{with_wireshark}
 %package wireshark
 Summary: Wireshark dissector plugin for libvirt RPC transactions
@@ -1872,10 +1857,12 @@ exit 0
 
 %files client
 %{_mandir}/man1/virsh.1*
+%{_mandir}/man1/virt-admin.1*
 %{_mandir}/man1/virt-xml-validate.1*
 %{_mandir}/man1/virt-pki-validate.1*
 %{_mandir}/man1/virt-host-validate.1*
 %{_bindir}/virsh
+%{_bindir}/virt-admin
 %{_bindir}/virt-xml-validate
 %{_bindir}/virt-pki-validate
 %{_bindir}/virt-host-validate
@@ -1886,8 +1873,9 @@ exit 0
 %{_datadir}/systemtap/tapset/libvirt_qemu_probes*.stp
 %endif
 
+%{_datadir}/bash-completion/completions/vsh
 %{_datadir}/bash-completion/completions/virsh
-
+%{_datadir}/bash-completion/completions/virt-admin
 
 %{_unitdir}/libvirt-guests.service
 %config(noreplace) %{_sysconfdir}/sysconfig/libvirt-guests
@@ -1911,14 +1899,6 @@ exit 0
 
 %{_datadir}/libvirt/test-screenshot.png
 
-%files admin
-%{_mandir}/man1/virt-admin.1*
-%{_bindir}/virt-admin
-%{_datadir}/bash-completion/completions/virt-admin
-
-%files bash-completion
-%{_datadir}/bash-completion/completions/vsh
-
 %if %{with_wireshark}
 %files wireshark
 %{wireshark_plugindir}/libvirt.so
-- 
2.26.3

On 4/15/21 11:20 AM, Andrea Bolognani wrote:
> The former is ridiculously tiny and doesn't have any use on
> its own, so it hardly warrants the overhead of an additional
> package; the latter is also very small and, just like virsh,
> is something that you likely want to have available on any
> virtualization host to help with management and debugging
> tasks.
> 
> Signed-off-by: Andrea Bolognani <abologna@redhat.com>
> ---
>   libvirt.spec.in | 28 ++++------------------------
>   1 file changed, 4 insertions(+), 24 deletions(-)
> 
> diff --git a/libvirt.spec.in b/libvirt.spec.in
> index f9af330186..80a12a307a 100644
> --- a/libvirt.spec.in
> +++ b/libvirt.spec.in
> @@ -901,7 +901,6 @@ Requires: %{name}-libs = %{version}-%{release}
>   Requires: gettext
>   # Needed by virt-pki-validate script.
>   Requires: gnutls-utils
> -Requires: %{name}-bash-completion = %{version}-%{release}
>   
>   %description client
>   The client binaries needed to access the virtualization
> @@ -918,20 +917,6 @@ Requires: cyrus-sasl-gssapi
>   %description libs
>   Shared libraries for accessing the libvirt daemon.
>   
> -%package admin
> -Summary: Set of tools to control libvirt daemon
> -Requires: %{name}-libs = %{version}-%{release}
> -Requires: %{name}-bash-completion = %{version}-%{release}
> -
> -%description admin
> -The client side utilities to control the libvirt daemon.
> -
> -%package bash-completion
> -Summary: Bash completion script
> -
> -%description bash-completion
> -Bash completion script stub.
> -
>   %if %{with_wireshark}
>   %package wireshark
>   Summary: Wireshark dissector plugin for libvirt RPC transactions
> @@ -1872,10 +1857,12 @@ exit 0
>   
>   %files client
>   %{_mandir}/man1/virsh.1*
> +%{_mandir}/man1/virt-admin.1*
>   %{_mandir}/man1/virt-xml-validate.1*
>   %{_mandir}/man1/virt-pki-validate.1*
>   %{_mandir}/man1/virt-host-validate.1*
>   %{_bindir}/virsh
> +%{_bindir}/virt-admin
>   %{_bindir}/virt-xml-validate
>   %{_bindir}/virt-pki-validate
>   %{_bindir}/virt-host-validate
> @@ -1886,8 +1873,9 @@ exit 0
>   %{_datadir}/systemtap/tapset/libvirt_qemu_probes*.stp
>   %endif
>   
> +%{_datadir}/bash-completion/completions/vsh
>   %{_datadir}/bash-completion/completions/virsh
> -
> +%{_datadir}/bash-completion/completions/virt-admin
>   
>   %{_unitdir}/libvirt-guests.service
>   %config(noreplace) %{_sysconfdir}/sysconfig/libvirt-guests
> @@ -1911,14 +1899,6 @@ exit 0
>   
>   %{_datadir}/libvirt/test-screenshot.png
>   
> -%files admin
> -%{_mandir}/man1/virt-admin.1*
> -%{_bindir}/virt-admin
> -%{_datadir}/bash-completion/completions/virt-admin
> -
> -%files bash-completion
> -%{_datadir}/bash-completion/completions/vsh
> -
>   %if %{with_wireshark}
>   %files wireshark
>   %{wireshark_plugindir}/libvirt.so
> 

The -client package needs then to obsolete those packages which you're 
removing. Loo around at "Obsoletes:" tag we have around.

However, I'm not sure that virt-admin should go to client package. For 
instance, I can install -client to control remote daemon but virt-admin 
works only locally (because we want it to). Therefore, if anything, it 
should go in the same package as libvirtd. But taking split daemons into 
account - any of them should drag in virt-admin.

Michal

On Thu, Apr 15, 2021 at 11:36:19AM +0200, Michal Privoznik wrote:
> On 4/15/21 11:20 AM, Andrea Bolognani wrote:
> > The former is ridiculously tiny and doesn't have any use on
> > its own, so it hardly warrants the overhead of an additional
> > package; the latter is also very small and, just like virsh,
> > is something that you likely want to have available on any
> > virtualization host to help with management and debugging
> > tasks.
> > 
> > Signed-off-by: Andrea Bolognani <abologna@redhat.com>
> > ---
> >   libvirt.spec.in | 28 ++++------------------------
> >   1 file changed, 4 insertions(+), 24 deletions(-)
> > 
> > diff --git a/libvirt.spec.in b/libvirt.spec.in
> > index f9af330186..80a12a307a 100644
> > --- a/libvirt.spec.in
> > +++ b/libvirt.spec.in
> > @@ -901,7 +901,6 @@ Requires: %{name}-libs = %{version}-%{release}
> >   Requires: gettext
> >   # Needed by virt-pki-validate script.
> >   Requires: gnutls-utils
> > -Requires: %{name}-bash-completion = %{version}-%{release}
> >   %description client
> >   The client binaries needed to access the virtualization
> > @@ -918,20 +917,6 @@ Requires: cyrus-sasl-gssapi
> >   %description libs
> >   Shared libraries for accessing the libvirt daemon.
> > -%package admin
> > -Summary: Set of tools to control libvirt daemon
> > -Requires: %{name}-libs = %{version}-%{release}
> > -Requires: %{name}-bash-completion = %{version}-%{release}
> > -
> > -%description admin
> > -The client side utilities to control the libvirt daemon.
> > -
> > -%package bash-completion
> > -Summary: Bash completion script
> > -
> > -%description bash-completion
> > -Bash completion script stub.
> > -
> >   %if %{with_wireshark}
> >   %package wireshark
> >   Summary: Wireshark dissector plugin for libvirt RPC transactions
> > @@ -1872,10 +1857,12 @@ exit 0
> >   %files client
> >   %{_mandir}/man1/virsh.1*
> > +%{_mandir}/man1/virt-admin.1*
> >   %{_mandir}/man1/virt-xml-validate.1*
> >   %{_mandir}/man1/virt-pki-validate.1*
> >   %{_mandir}/man1/virt-host-validate.1*
> >   %{_bindir}/virsh
> > +%{_bindir}/virt-admin
> >   %{_bindir}/virt-xml-validate
> >   %{_bindir}/virt-pki-validate
> >   %{_bindir}/virt-host-validate
> > @@ -1886,8 +1873,9 @@ exit 0
> >   %{_datadir}/systemtap/tapset/libvirt_qemu_probes*.stp
> >   %endif
> > +%{_datadir}/bash-completion/completions/vsh
> >   %{_datadir}/bash-completion/completions/virsh
> > -
> > +%{_datadir}/bash-completion/completions/virt-admin
> >   %{_unitdir}/libvirt-guests.service
> >   %config(noreplace) %{_sysconfdir}/sysconfig/libvirt-guests
> > @@ -1911,14 +1899,6 @@ exit 0
> >   %{_datadir}/libvirt/test-screenshot.png
> > -%files admin
> > -%{_mandir}/man1/virt-admin.1*
> > -%{_bindir}/virt-admin
> > -%{_datadir}/bash-completion/completions/virt-admin
> > -
> > -%files bash-completion
> > -%{_datadir}/bash-completion/completions/vsh
> > -
> >   %if %{with_wireshark}
> >   %files wireshark
> >   %{wireshark_plugindir}/libvirt.so
> > 
> 
> The -client package needs then to obsolete those packages which you're
> removing. Loo around at "Obsoletes:" tag we have around.

Also needs a Provides tag to satisy any deps other packages have on it

> However, I'm not sure that virt-admin should go to client package. For
> instance, I can install -client to control remote daemon but virt-admin
> works only locally (because we want it to). Therefore, if anything, it
> should go in the same package as libvirtd. But taking split daemons into
> account - any of them should drag in virt-admin.

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, 2021-04-15 at 11:10 +0100, Daniel P. Berrangé wrote:
> On Thu, Apr 15, 2021 at 11:36:19AM +0200, Michal Privoznik wrote:
> > The -client package needs then to obsolete those packages which you're
> > removing. Loo around at "Obsoletes:" tag we have around.
> 
> Also needs a Provides tag to satisy any deps other packages have on it

Makes sense, will do.

> > However, I'm not sure that virt-admin should go to client package. For
> > instance, I can install -client to control remote daemon but virt-admin
> > works only locally (because we want it to). Therefore, if anything, it
> > should go in the same package as libvirtd. But taking split daemons into
> > account - any of them should drag in virt-admin.

Some of the things that currently live in the -client package are
also local-only: virt-host-validate, virt-pki-validate and
libvirt-guests. I agree that the distinction is not entirely
clear-cut, but virt-admin fits slightly better in -client than in
-daemon IMHO.

-- 
Andrea Bolognani / Red Hat / Virtualization

On Thu, Apr 15, 2021 at 02:44:09PM +0200, Andrea Bolognani wrote:
> On Thu, 2021-04-15 at 11:10 +0100, Daniel P. Berrangé wrote:
> > On Thu, Apr 15, 2021 at 11:36:19AM +0200, Michal Privoznik wrote:
> > > The -client package needs then to obsolete those packages which you're
> > > removing. Loo around at "Obsoletes:" tag we have around.
> > 
> > Also needs a Provides tag to satisy any deps other packages have on it
> 
> Makes sense, will do.
> 
> > > However, I'm not sure that virt-admin should go to client package. For
> > > instance, I can install -client to control remote daemon but virt-admin
> > > works only locally (because we want it to). Therefore, if anything, it
> > > should go in the same package as libvirtd. But taking split daemons into
> > > account - any of them should drag in virt-admin.
> 
> Some of the things that currently live in the -client package are
> also local-only: virt-host-validate, virt-pki-validate and
> libvirt-guests. I agree that the distinction is not entirely
> clear-cut, but virt-admin fits slightly better in -client than in
> -daemon IMHO.

Those existing things are arguably in the wrong place too, but I guess
no one cared as they're just small shell scripts. 


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, Apr 15, 2021 at 11:20:14AM +0200, Andrea Bolognani wrote:
> The former is ridiculously tiny and doesn't have any use on
> its own, so it hardly warrants the overhead of an additional
> package; the latter is also very small and, just like virsh,
> is something that you likely want to have available on any
> virtualization host to help with management and debugging
> tasks.

Excellent; this could save a lot of time for certain management tools
that use containers to launch libvirtd; and then don't have
'virt-admin'.


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

I see that Dan and Michal had couple of comments.  Regardless, the
idea itself is good; FWIW:

Sounds-very-good-to-me'd-by: Kashyap Chamarthy <kchamart@redhat.com>

> ---
>  libvirt.spec.in | 28 ++++------------------------
>  1 file changed, 4 insertions(+), 24 deletions(-)
> 
> diff --git a/libvirt.spec.in b/libvirt.spec.in
> index f9af330186..80a12a307a 100644
> --- a/libvirt.spec.in
> +++ b/libvirt.spec.in
> @@ -901,7 +901,6 @@ Requires: %{name}-libs = %{version}-%{release}
>  Requires: gettext
>  # Needed by virt-pki-validate script.
>  Requires: gnutls-utils
> -Requires: %{name}-bash-completion = %{version}-%{release}
>  
>  %description client
>  The client binaries needed to access the virtualization
> @@ -918,20 +917,6 @@ Requires: cyrus-sasl-gssapi
>  %description libs
>  Shared libraries for accessing the libvirt daemon.
>  
> -%package admin
> -Summary: Set of tools to control libvirt daemon
> -Requires: %{name}-libs = %{version}-%{release}
> -Requires: %{name}-bash-completion = %{version}-%{release}
> -
> -%description admin
> -The client side utilities to control the libvirt daemon.
> -
> -%package bash-completion
> -Summary: Bash completion script
> -
> -%description bash-completion
> -Bash completion script stub.
> -
>  %if %{with_wireshark}
>  %package wireshark
>  Summary: Wireshark dissector plugin for libvirt RPC transactions
> @@ -1872,10 +1857,12 @@ exit 0
>  
>  %files client
>  %{_mandir}/man1/virsh.1*
> +%{_mandir}/man1/virt-admin.1*
>  %{_mandir}/man1/virt-xml-validate.1*
>  %{_mandir}/man1/virt-pki-validate.1*
>  %{_mandir}/man1/virt-host-validate.1*
>  %{_bindir}/virsh
> +%{_bindir}/virt-admin
>  %{_bindir}/virt-xml-validate
>  %{_bindir}/virt-pki-validate
>  %{_bindir}/virt-host-validate
> @@ -1886,8 +1873,9 @@ exit 0
>  %{_datadir}/systemtap/tapset/libvirt_qemu_probes*.stp
>  %endif
>  
> +%{_datadir}/bash-completion/completions/vsh
>  %{_datadir}/bash-completion/completions/virsh
> -
> +%{_datadir}/bash-completion/completions/virt-admin
>  
>  %{_unitdir}/libvirt-guests.service
>  %config(noreplace) %{_sysconfdir}/sysconfig/libvirt-guests
> @@ -1911,14 +1899,6 @@ exit 0
>  
>  %{_datadir}/libvirt/test-screenshot.png
>  
> -%files admin
> -%{_mandir}/man1/virt-admin.1*
> -%{_bindir}/virt-admin
> -%{_datadir}/bash-completion/completions/virt-admin
> -
> -%files bash-completion
> -%{_datadir}/bash-completion/completions/vsh
> -
>  %if %{with_wireshark}
>  %files wireshark
>  %{wireshark_plugindir}/libvirt.so
> -- 
> 2.26.3
> 

-- 
/kashyap