[libvirt] [PATCH] spec: require libvirt-wireshark from libvirt metapackage
In general, the 'libvirt' metapackage should pull in all subpackages. Fix this for the wireshark subpackage created in commit f9ada9f. * libvirt.spec.in (Requires): Add dependency. Signed-off-by: Eric Blake <eblake@redhat.com> --- libvirt.spec.in | 3 +++ 1 file changed, 3 insertions(+) diff --git a/libvirt.spec.in b/libvirt.spec.in index d3e6048..2d57c71 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -428,6 +428,9 @@ Requires: libvirt-daemon-driver-storage = %{version}-%{release} Requires: libvirt-daemon-driver-network = %{version}-%{release} Requires: libvirt-daemon-driver-nodedev = %{version}-%{release} %endif + %if %{with_wireshark} +Requires: libvirt-wireshark = %{version}-%{release} + %endif %endif Requires: libvirt-client = %{version}-%{release} -- 1.8.5.3
On Wed, Feb 12, 2014 at 01:28:43PM -0700, Eric Blake wrote:
In general, the 'libvirt' metapackage should pull in all subpackages. Fix this for the wireshark subpackage created in commit f9ada9f.
* libvirt.spec.in (Requires): Add dependency.
Signed-off-by: Eric Blake <eblake@redhat.com> --- libvirt.spec.in | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/libvirt.spec.in b/libvirt.spec.in index d3e6048..2d57c71 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -428,6 +428,9 @@ Requires: libvirt-daemon-driver-storage = %{version}-%{release} Requires: libvirt-daemon-driver-network = %{version}-%{release} Requires: libvirt-daemon-driver-nodedev = %{version}-%{release} %endif + %if %{with_wireshark} +Requires: libvirt-wireshark = %{version}-%{release} + %endif %endif Requires: libvirt-client = %{version}-%{release}
-- 1.8.5.3
ACK, Martin
On 02/12/2014 02:20 PM, Martin Kletzander wrote:
On Wed, Feb 12, 2014 at 01:28:43PM -0700, Eric Blake wrote:
In general, the 'libvirt' metapackage should pull in all subpackages. Fix this for the wireshark subpackage created in commit f9ada9f.
* libvirt.spec.in (Requires): Add dependency.
ACK,
Pushed. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 12.02.2014 21:28, Eric Blake wrote:
In general, the 'libvirt' metapackage should pull in all subpackages. Fix this for the wireshark subpackage created in commit f9ada9f.
* libvirt.spec.in (Requires): Add dependency.
Signed-off-by: Eric Blake <eblake@redhat.com> --- libvirt.spec.in | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/libvirt.spec.in b/libvirt.spec.in index d3e6048..2d57c71 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -428,6 +428,9 @@ Requires: libvirt-daemon-driver-storage = %{version}-%{release} Requires: libvirt-daemon-driver-network = %{version}-%{release} Requires: libvirt-daemon-driver-nodedev = %{version}-%{release} %endif + %if %{with_wireshark} +Requires: libvirt-wireshark = %{version}-%{release} + %endif %endif Requires: libvirt-client = %{version}-%{release}
Aah, I see you've already pushed this one. However I have doubts about it. The wireshark plugin is meant for developers, not ordinary users. With this patch: yum install libvirt drags wireshark into the dependencies. Me, as a libvirt developer, am comfortable with it. The ordinary libvirt users who just creates dozen virtual machines may be not. But I'm open to persuasion :) Michal
On Thu, Feb 13, 2014 at 10:17:26AM +0100, Michal Privoznik wrote:
On 12.02.2014 21:28, Eric Blake wrote:
In general, the 'libvirt' metapackage should pull in all subpackages. Fix this for the wireshark subpackage created in commit f9ada9f.
* libvirt.spec.in (Requires): Add dependency.
Signed-off-by: Eric Blake <eblake@redhat.com> --- libvirt.spec.in | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/libvirt.spec.in b/libvirt.spec.in index d3e6048..2d57c71 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -428,6 +428,9 @@ Requires: libvirt-daemon-driver-storage = %{version}-%{release} Requires: libvirt-daemon-driver-network = %{version}-%{release} Requires: libvirt-daemon-driver-nodedev = %{version}-%{release} %endif + %if %{with_wireshark} +Requires: libvirt-wireshark = %{version}-%{release} + %endif %endif Requires: libvirt-client = %{version}-%{release}
Aah, I see you've already pushed this one. However I have doubts about it. The wireshark plugin is meant for developers, not ordinary users. With this patch:
yum install libvirt
drags wireshark into the dependencies. Me, as a libvirt developer, am comfortable with it. The ordinary libvirt users who just creates dozen virtual machines may be not.
But I'm open to persuasion :)
I tend to agree - I don't think we should be pulling in the wireshark RPM, so I'd suggest we revert this patch. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On Thu, Feb 13, 2014 at 10:55:11AM +0000, Daniel P. Berrange wrote:
On Thu, Feb 13, 2014 at 10:17:26AM +0100, Michal Privoznik wrote:
On 12.02.2014 21:28, Eric Blake wrote:
In general, the 'libvirt' metapackage should pull in all subpackages. Fix this for the wireshark subpackage created in commit f9ada9f.
* libvirt.spec.in (Requires): Add dependency.
Signed-off-by: Eric Blake <eblake@redhat.com> --- libvirt.spec.in | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/libvirt.spec.in b/libvirt.spec.in index d3e6048..2d57c71 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -428,6 +428,9 @@ Requires: libvirt-daemon-driver-storage = %{version}-%{release} Requires: libvirt-daemon-driver-network = %{version}-%{release} Requires: libvirt-daemon-driver-nodedev = %{version}-%{release} %endif + %if %{with_wireshark} +Requires: libvirt-wireshark = %{version}-%{release} + %endif %endif Requires: libvirt-client = %{version}-%{release}
Aah, I see you've already pushed this one. However I have doubts about it. The wireshark plugin is meant for developers, not ordinary users. With this patch:
yum install libvirt
drags wireshark into the dependencies. Me, as a libvirt developer, am comfortable with it. The ordinary libvirt users who just creates dozen virtual machines may be not.
But I'm open to persuasion :)
I tend to agree - I don't think we should be pulling in the wireshark RPM, so I'd suggest we revert this patch.
I haven't realized that libvirt-wireshark requires wireshark; I was under the impression that it just installs the compiled dissector files. Similarly how for example logrotate rules should be installed but should not require logrotate to be installed (and if you want it, you can install it and it "Just Works"). That said, I don't know whether wireshark is needed for the installation of libvirt-wireshark, but if it is not, I'd rather see this in: diff --git a/libvirt.spec.in b/libvirt.spec.in index 2d57c71..a3f1170 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -1137,7 +1137,6 @@ virtualization capabilities of recent versions of Linux (and other OSes). %package wireshark Summary: Wireshark dissector plugin for libvirt RPC transactions Group: Development/Libraries -Requires: wireshark Requires: %{name}-client = %{version}-%{release} %description wireshark -- Martin
On Thu, Feb 13, 2014 at 02:11:38PM +0100, Martin Kletzander wrote:
On Thu, Feb 13, 2014 at 10:55:11AM +0000, Daniel P. Berrange wrote:
On Thu, Feb 13, 2014 at 10:17:26AM +0100, Michal Privoznik wrote:
On 12.02.2014 21:28, Eric Blake wrote:
In general, the 'libvirt' metapackage should pull in all subpackages. Fix this for the wireshark subpackage created in commit f9ada9f.
* libvirt.spec.in (Requires): Add dependency.
Signed-off-by: Eric Blake <eblake@redhat.com> --- libvirt.spec.in | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/libvirt.spec.in b/libvirt.spec.in index d3e6048..2d57c71 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -428,6 +428,9 @@ Requires: libvirt-daemon-driver-storage = %{version}-%{release} Requires: libvirt-daemon-driver-network = %{version}-%{release} Requires: libvirt-daemon-driver-nodedev = %{version}-%{release} %endif + %if %{with_wireshark} +Requires: libvirt-wireshark = %{version}-%{release} + %endif %endif Requires: libvirt-client = %{version}-%{release}
Aah, I see you've already pushed this one. However I have doubts about it. The wireshark plugin is meant for developers, not ordinary users. With this patch:
yum install libvirt
drags wireshark into the dependencies. Me, as a libvirt developer, am comfortable with it. The ordinary libvirt users who just creates dozen virtual machines may be not.
But I'm open to persuasion :)
I tend to agree - I don't think we should be pulling in the wireshark RPM, so I'd suggest we revert this patch.
I haven't realized that libvirt-wireshark requires wireshark; I was under the impression that it just installs the compiled dissector files. Similarly how for example logrotate rules should be installed but should not require logrotate to be installed (and if you want it, you can install it and it "Just Works").
That said, I don't know whether wireshark is needed for the installation of libvirt-wireshark, but if it is not, I'd rather see this in:
diff --git a/libvirt.spec.in b/libvirt.spec.in index 2d57c71..a3f1170 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -1137,7 +1137,6 @@ virtualization capabilities of recent versions of Linux (and other OSes). %package wireshark Summary: Wireshark dissector plugin for libvirt RPC transactions Group: Development/Libraries -Requires: wireshark Requires: %{name}-client = %{version}-%{release}
%description wireshark
NB 'wireshark' is just the dissectors and command line utils. The GTK gui is a separate RPM. So I think this dep is reasonable as it is, and we shoulud just revert the previous patch. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On 02/13/2014 06:20 AM, Daniel P. Berrange wrote:
NB 'wireshark' is just the dissectors and command line utils. The GTK gui is a separate RPM. So I think this dep is reasonable as it is, and we shoulud just revert the previous patch.
Revert complete. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
participants (4)
-
Daniel P. Berrange -
Eric Blake -
Martin Kletzander -
Michal Privoznik