The wireshark-2.4.0 is almost 2 years old now. Assuming anybody
interested in running latest libvirt doesn't run old wireshark,
it is safe to do this. It also simplifies the code.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 libvirt.spec.in                      |  8 ++----
 m4/virt-wireshark.m4                 |  4 +--
 tools/wireshark/src/packet-libvirt.c | 39 ----------------------------
 tools/wireshark/src/plugin.c         | 14 +++-------
 4 files changed, 7 insertions(+), 58 deletions(-)

diff --git a/libvirt.spec.in b/libvirt.spec.in
index 13f41bb4a5..9beffba203 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -166,11 +166,7 @@
 # Enable wireshark plugins for all distros shipping libvirt 1.2.2 or newer
 %if 0%{?fedora}
     %define with_wireshark 0%{!?_without_wireshark:1}
-%endif
-%if 0%{?fedora} || 0%{?rhel} > 7
     %define wireshark_plugindir %(pkg-config --variable plugindir wireshark)/epan
-%else
-    %define wireshark_plugindir %{_libdir}/wireshark/plugins/epan
 %endif
 
 # Enable libssh transport for new enough distros
@@ -389,7 +385,7 @@ BuildRequires: numad
 %endif
 
 %if %{with_wireshark}
-BuildRequires: wireshark-devel >= 2.1.0
+BuildRequires: wireshark-devel >= 2.4.0
 %endif
 
 %if %{with_libssh}
@@ -935,7 +931,7 @@ Bash completion script stub.
 %if %{with_wireshark}
 %package wireshark
 Summary: Wireshark dissector plugin for libvirt RPC transactions
-Requires: wireshark >= 1.12.6-4
+Requires: wireshark >= 2.4.0
 Requires: %{name}-libs = %{version}-%{release}
 
 %description wireshark
diff --git a/m4/virt-wireshark.m4 b/m4/virt-wireshark.m4
index a8f8083f5e..2fbf691590 100644
--- a/m4/virt-wireshark.m4
+++ b/m4/virt-wireshark.m4
@@ -18,14 +18,14 @@ dnl <http://www.gnu.org/licenses/>.
 dnl
 
 AC_DEFUN([LIBVIRT_ARG_WIRESHARK],[
-  LIBVIRT_ARG_WITH_FEATURE([WIRESHARK_DISSECTOR], [wireshark], [check], [1.11.3])
+  LIBVIRT_ARG_WITH_FEATURE([WIRESHARK_DISSECTOR], [wireshark], [check], [2.4.0])
   LIBVIRT_ARG_WITH([WS_PLUGINDIR],
                    [wireshark plugins directory for use when installing
                    wireshark plugin], [check])
 ])
 
 AC_DEFUN([LIBVIRT_CHECK_WIRESHARK],[
-  LIBVIRT_CHECK_PKG([WIRESHARK_DISSECTOR], [wireshark], [1.11.3])
+  LIBVIRT_CHECK_PKG([WIRESHARK_DISSECTOR], [wireshark], [2.4.0])
 
   dnl Check for system location of wireshark plugins
   if test "x$with_wireshark_dissector" != "xno" ; then
diff --git a/tools/wireshark/src/packet-libvirt.c b/tools/wireshark/src/packet-libvirt.c
index e759e81bae..dc3aa410e5 100644
--- a/tools/wireshark/src/packet-libvirt.c
+++ b/tools/wireshark/src/packet-libvirt.c
@@ -42,12 +42,6 @@
 # define dbg(fmt, ...)
 #endif
 
-/* Wireshark 1.12 brings API change */
-#define WIRESHARK_VERSION \
-    ((VERSION_MAJOR * 1000 * 1000) + \
-     (VERSION_MINOR * 1000) + \
-     (VERSION_MICRO))
-
 static int proto_libvirt = -1;
 static int hf_libvirt_length = -1;
 static int hf_libvirt_program = -1;
@@ -373,17 +367,8 @@ dissect_libvirt_payload_xdr_data(tvbuff_t *tvb, proto_tree *tree, gint payload_l
         payload_length -= 4;
     }
 
-#if WIRESHARK_VERSION < 200400
-    payload_tvb = tvb_new_subset(tvb, start, -1, payload_length);
-#else
     payload_tvb = tvb_new_subset_remaining(tvb, start);
-#endif
-
-#if WIRESHARK_VERSION < 1012000
-    payload_data = (caddr_t)tvb_memdup(payload_tvb, 0, payload_length);
-#else
     payload_data = (caddr_t)tvb_memdup(NULL, payload_tvb, 0, payload_length);
-#endif
     xdrmem_create(&xdrs, payload_data, payload_length, XDR_DECODE);
 
     dissect(payload_tvb, tree, &xdrs, -1);
@@ -456,14 +441,9 @@ dissect_libvirt_payload(tvbuff_t *tvb, proto_tree *tree,
     proto_tree_add_item(tree, hf_libvirt_unknown, tvb, VIR_HEADER_LEN, -1, ENC_NA);
 }
 
-#if WIRESHARK_VERSION < 1012000
-static void
-dissect_libvirt_message(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree)
-#else
 static int
 dissect_libvirt_message(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree,
                         void *opaque ATTRIBUTE_UNUSED)
-#endif
 {
     goffset offset;
     guint32 prog, proc, type, serial, status;
@@ -524,44 +504,25 @@ dissect_libvirt_message(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree,
         dissect_libvirt_payload(tvb, libvirt_tree, prog, proc, type, status);
     }
 
-#if WIRESHARK_VERSION >= 1012000
     return 0;
-#endif
 }
 
-#if WIRESHARK_VERSION >= 1099002
 static guint
 get_message_len(packet_info *pinfo ATTRIBUTE_UNUSED, tvbuff_t *tvb, int offset, void *data ATTRIBUTE_UNUSED)
-#else
-static guint32
-get_message_len(packet_info *pinfo ATTRIBUTE_UNUSED, tvbuff_t *tvb, int offset)
-#endif
 {
     return tvb_get_ntohl(tvb, offset);
 }
 
-#if WIRESHARK_VERSION >= 2000001
 static int
 dissect_libvirt(tvbuff_t *tvb, packet_info *pinfo,
                 proto_tree *tree, void *data ATTRIBUTE_UNUSED)
-#else
-static void
-dissect_libvirt(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree)
-#endif
 {
     /* Another magic const - 4; simply, how much bytes
      * is needed to tell the length of libvirt packet. */
-#if WIRESHARK_VERSION < 1012000
-    tcp_dissect_pdus(tvb, pinfo, tree, TRUE, 4,
-                     get_message_len, dissect_libvirt_message);
-#else
     tcp_dissect_pdus(tvb, pinfo, tree, TRUE, 4,
                      get_message_len, dissect_libvirt_message, NULL);
-#endif
 
-#if WIRESHARK_VERSION >= 2000001
     return tvb_captured_length(tvb);
-#endif
 }
 
 void
diff --git a/tools/wireshark/src/plugin.c b/tools/wireshark/src/plugin.c
index 3741334e85..504e4383a7 100644
--- a/tools/wireshark/src/plugin.c
+++ b/tools/wireshark/src/plugin.c
@@ -31,29 +31,21 @@
      (VERSION_MICRO))
 
 #if WIRESHARK_VERSION < 2005000
-/* In 1.12 wireshark WS_DLL_PUBLIC_NOEXTERN was substitued with
- * WS_DLL_PUBLIC_DEF. See wireshark's commit
- * 5d87a8c46171f572568db5a47c093423482e342f for more info. */
-# ifndef WS_DLL_PUBLIC_NOEXTERN
-#  define WS_DLL_PUBLIC_NOEXTERN WS_DLL_PUBLIC_DEF
-# endif
 
-# ifndef ENABLE_STATIC
-WS_DLL_PUBLIC_NOEXTERN const gchar version[] = VERSION;
+WS_DLL_PUBLIC_DEF const gchar version[] = VERSION;
 
 /* Start the functions we need for the plugin stuff */
 
-WS_DLL_PUBLIC_NOEXTERN void
+WS_DLL_PUBLIC_DEF void
 plugin_register(void)
 {
   proto_register_libvirt();
 }
-WS_DLL_PUBLIC_NOEXTERN void
+WS_DLL_PUBLIC_DEF void
 plugin_reg_handoff(void)
 {
   proto_reg_handoff_libvirt();
 }
-# endif
 
 #elif WIRESHARK_VERSION < 2009000
 
-- 
2.19.2

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On Fri, Feb 08, 2019 at 12:23:28PM +0100, Michal Privoznik wrote:
>The wireshark-2.4.0 is almost 2 years old now. Assuming anybody

While 2 years is not that much given our support page:
https://libvirt.org/platforms.html
the wireshark plugin was always an exception to those

>interested in running latest libvirt doesn't run old wireshark,
>it is safe to do this. It also simplifies the code.
>
>Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
>---
> libvirt.spec.in                      |  8 ++----
> m4/virt-wireshark.m4                 |  4 +--
> tools/wireshark/src/packet-libvirt.c | 39 ----------------------------
> tools/wireshark/src/plugin.c         | 14 +++-------
> 4 files changed, 7 insertions(+), 58 deletions(-)
>
>diff --git a/libvirt.spec.in b/libvirt.spec.in
>index 13f41bb4a5..9beffba203 100644
>--- a/libvirt.spec.in
>+++ b/libvirt.spec.in
>@@ -166,11 +166,7 @@
> # Enable wireshark plugins for all distros shipping libvirt 1.2.2 or newer
> %if 0%{?fedora}
>     %define with_wireshark 0%{!?_without_wireshark:1}
>-%endif
>-%if 0%{?fedora} || 0%{?rhel} > 7

This gets rid of the rhel > 7 condition.
If it works on latest Fedora, shouldn't it work on RHEL 8 too?

>     %define wireshark_plugindir %(pkg-config --variable plugindir wireshark)/epan
>-%else
>-    %define wireshark_plugindir %{_libdir}/wireshark/plugins/epan
> %endif
>
> # Enable libssh transport for new enough distros
>@@ -389,7 +385,7 @@ BuildRequires: numad
> %endif
>
> %if %{with_wireshark}
>-BuildRequires: wireshark-devel >= 2.1.0
>+BuildRequires: wireshark-devel >= 2.4.0


> %endif
>
> %if %{with_libssh}
>@@ -935,7 +931,7 @@ Bash completion script stub.
> %if %{with_wireshark}
> %package wireshark
> Summary: Wireshark dissector plugin for libvirt RPC transactions
>-Requires: wireshark >= 1.12.6-4
>+Requires: wireshark >= 2.4.0
> Requires: %{name}-libs = %{version}-%{release}
>
> %description wireshark
>diff --git a/m4/virt-wireshark.m4 b/m4/virt-wireshark.m4
>index a8f8083f5e..2fbf691590 100644
>--- a/m4/virt-wireshark.m4
>+++ b/m4/virt-wireshark.m4
>@@ -18,14 +18,14 @@ dnl <http://www.gnu.org/licenses/>.
> dnl
>
> AC_DEFUN([LIBVIRT_ARG_WIRESHARK],[
>-  LIBVIRT_ARG_WITH_FEATURE([WIRESHARK_DISSECTOR], [wireshark], [check], [1.11.3])
>+  LIBVIRT_ARG_WITH_FEATURE([WIRESHARK_DISSECTOR], [wireshark], [check], [2.4.0])

Even 1.11.3 seems too new for CentOS 7:
https://repology.org/metapackage/wireshark/versions
so we're not really breaking it here by bumping the version

Jano
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On 2/11/19 5:55 PM, Ján Tomko wrote:
> On Fri, Feb 08, 2019 at 12:23:28PM +0100, Michal Privoznik wrote:
>> The wireshark-2.4.0 is almost 2 years old now. Assuming anybody
> 
> While 2 years is not that much given our support page:
> https://libvirt.org/platforms.html
> the wireshark plugin was always an exception to those
> 
>> interested in running latest libvirt doesn't run old wireshark,
>> it is safe to do this. It also simplifies the code.
>>
>> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
>> ---
>> libvirt.spec.in                      |  8 ++----
>> m4/virt-wireshark.m4                 |  4 +--
>> tools/wireshark/src/packet-libvirt.c | 39 ----------------------------
>> tools/wireshark/src/plugin.c         | 14 +++-------
>> 4 files changed, 7 insertions(+), 58 deletions(-)
>>
>> diff --git a/libvirt.spec.in b/libvirt.spec.in
>> index 13f41bb4a5..9beffba203 100644
>> --- a/libvirt.spec.in
>> +++ b/libvirt.spec.in
>> @@ -166,11 +166,7 @@
>> # Enable wireshark plugins for all distros shipping libvirt 1.2.2 or 
>> newer
>> %if 0%{?fedora}
>>     %define with_wireshark 0%{!?_without_wireshark:1}
>> -%endif
>> -%if 0%{?fedora} || 0%{?rhel} > 7
> 
> This gets rid of the rhel > 7 condition.
> If it works on latest Fedora, shouldn't it work on RHEL 8 too?

We're not building wireshark plugin on any RHEL. This is merely dropping 
a specfile variable that was passed to configure only on Fedora not RHEL.

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list