[libvirt] [PATCH 0/2] Adapt to wireshark-1.12

With the new wireshark installed I've noticed couple of build breaks. This fixes the problems I've seen. Michal Privoznik (2): wireshark: Include more of libvirt internals wireshark: Honor API change coming with 1.12 release tools/wireshark/src/Makefile.am | 2 +- tools/wireshark/src/packet-libvirt.c | 35 +++++++++++++++++++++++++++++++-- tools/wireshark/src/packet-libvirt.h | 18 ++--------------- tools/wireshark/util/make-dissector-reg | 7 +++++++ 4 files changed, 43 insertions(+), 19 deletions(-) -- 1.8.5.5

The rationale is to not duplicate code which is done in packet-libvirt.h for instance. Moreover, this way we can drop __attribute_((unused)) used int packet-libvirt.c in favor of ATTRIBUTE_UNUSED. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tools/wireshark/src/Makefile.am | 2 +- tools/wireshark/src/packet-libvirt.c | 3 ++- tools/wireshark/src/packet-libvirt.h | 18 ++---------------- 3 files changed, 5 insertions(+), 18 deletions(-) diff --git a/tools/wireshark/src/Makefile.am b/tools/wireshark/src/Makefile.am index 44f22be..40fe368 100644 --- a/tools/wireshark/src/Makefile.am +++ b/tools/wireshark/src/Makefile.am @@ -18,7 +18,7 @@ # # Author: Yuto KAWAMURA(kawamuray) -INCLUDES = -I$(top_srcdir) +INCLUDES = -I$(top_srcdir) -I$(top_srcdir)/src -I$(top_srcdir)/gnulib/lib ws_plugin_LTLIBRARIES = libvirt.la libvirt_la_SOURCES = packet-libvirt.h packet-libvirt.c plugin.c diff --git a/tools/wireshark/src/packet-libvirt.c b/tools/wireshark/src/packet-libvirt.c index 07098bf..5138453 100644 --- a/tools/wireshark/src/packet-libvirt.c +++ b/tools/wireshark/src/packet-libvirt.c @@ -34,6 +34,7 @@ #endif #include <rpc/xdr.h> #include "packet-libvirt.h" +#include "internal.h" static int proto_libvirt = -1; static int hf_libvirt_length = -1; @@ -413,7 +414,7 @@ dissect_libvirt_message(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree) } static guint32 -get_message_len(packet_info *pinfo __attribute__((unused)), tvbuff_t *tvb, int offset) +get_message_len(packet_info *pinfo ATTRIBUTE_UNUSED, tvbuff_t *tvb, int offset) { return tvb_get_ntohl(tvb, offset); } diff --git a/tools/wireshark/src/packet-libvirt.h b/tools/wireshark/src/packet-libvirt.h index 0cab637..af54407 100644 --- a/tools/wireshark/src/packet-libvirt.h +++ b/tools/wireshark/src/packet-libvirt.h @@ -21,6 +21,8 @@ #ifndef _PACKET_LIBVIRT_H_ # define _PACKET_LIBVIRT_H_ +# include "libvirt/libvirt.h" + # ifndef LIBVIRT_PORT # define LIBVIRT_PORT 16509 # endif @@ -84,22 +86,6 @@ static const value_string status_strings[] = { { -1, NULL } }; -/* TODO: These symbols will automatically included in generated headers in the feature */ -# define VIR_SECURITY_MODEL_BUFLEN (256 + 1) -# define VIR_SECURITY_LABEL_BUFLEN (4096 + 1) -# define VIR_SECURITY_DOI_BUFLEN (256 + 1) -# define VIR_UUID_BUFLEN (16) -enum { - VIR_TYPED_PARAM_INT = 1, /* integer case */ - VIR_TYPED_PARAM_UINT = 2, /* unsigned integer case */ - VIR_TYPED_PARAM_LLONG = 3, /* long long case */ - VIR_TYPED_PARAM_ULLONG = 4, /* unsigned long long case */ - VIR_TYPED_PARAM_DOUBLE = 5, /* double case */ - VIR_TYPED_PARAM_BOOLEAN = 6, /* boolean(character) case */ - VIR_TYPED_PARAM_STRING = 7, /* string case */ -}; -/* / */ - # define VIR_ERROR_MESSAGE_DISSECTOR dissect_xdr_remote_error static gboolean dissect_xdr_int(tvbuff_t *tvb, proto_tree *tree, XDR *xdrs, int hf); -- 1.8.5.5

On Fri, Jul 04, 2014 at 10:52:19AM +0200, Michal Privoznik wrote:
The rationale is to not duplicate code which is done in packet-libvirt.h for instance. Moreover, this way we can drop __attribute_((unused)) used int packet-libvirt.c in favor of ATTRIBUTE_UNUSED.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tools/wireshark/src/Makefile.am | 2 +- tools/wireshark/src/packet-libvirt.c | 3 ++- tools/wireshark/src/packet-libvirt.h | 18 ++---------------- 3 files changed, 5 insertions(+), 18 deletions(-)
diff --git a/tools/wireshark/src/Makefile.am b/tools/wireshark/src/Makefile.am index 44f22be..40fe368 100644 --- a/tools/wireshark/src/Makefile.am +++ b/tools/wireshark/src/Makefile.am @@ -18,7 +18,7 @@ # # Author: Yuto KAWAMURA(kawamuray)
-INCLUDES = -I$(top_srcdir) +INCLUDES = -I$(top_srcdir) -I$(top_srcdir)/src -I$(top_srcdir)/gnulib/lib
you need to add includes/ as well if you want to do ...
ws_plugin_LTLIBRARIES = libvirt.la libvirt_la_SOURCES = packet-libvirt.h packet-libvirt.c plugin.c diff --git a/tools/wireshark/src/packet-libvirt.c b/tools/wireshark/src/packet-libvirt.c index 07098bf..5138453 100644 --- a/tools/wireshark/src/packet-libvirt.c +++ b/tools/wireshark/src/packet-libvirt.c @@ -34,6 +34,7 @@ #endif #include <rpc/xdr.h> #include "packet-libvirt.h" +#include "internal.h"
static int proto_libvirt = -1; static int hf_libvirt_length = -1; @@ -413,7 +414,7 @@ dissect_libvirt_message(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree) }
static guint32 -get_message_len(packet_info *pinfo __attribute__((unused)), tvbuff_t *tvb, int offset) +get_message_len(packet_info *pinfo ATTRIBUTE_UNUSED, tvbuff_t *tvb, int offset) { return tvb_get_ntohl(tvb, offset); } diff --git a/tools/wireshark/src/packet-libvirt.h b/tools/wireshark/src/packet-libvirt.h index 0cab637..af54407 100644 --- a/tools/wireshark/src/packet-libvirt.h +++ b/tools/wireshark/src/packet-libvirt.h @@ -21,6 +21,8 @@ #ifndef _PACKET_LIBVIRT_H_ # define _PACKET_LIBVIRT_H_
+# include "libvirt/libvirt.h" +
... this :)
# ifndef LIBVIRT_PORT # define LIBVIRT_PORT 16509 # endif @@ -84,22 +86,6 @@ static const value_string status_strings[] = { { -1, NULL } };
-/* TODO: These symbols will automatically included in generated headers in the feature */ -# define VIR_SECURITY_MODEL_BUFLEN (256 + 1) -# define VIR_SECURITY_LABEL_BUFLEN (4096 + 1) -# define VIR_SECURITY_DOI_BUFLEN (256 + 1) -# define VIR_UUID_BUFLEN (16) -enum { - VIR_TYPED_PARAM_INT = 1, /* integer case */ - VIR_TYPED_PARAM_UINT = 2, /* unsigned integer case */ - VIR_TYPED_PARAM_LLONG = 3, /* long long case */ - VIR_TYPED_PARAM_ULLONG = 4, /* unsigned long long case */ - VIR_TYPED_PARAM_DOUBLE = 5, /* double case */ - VIR_TYPED_PARAM_BOOLEAN = 6, /* boolean(character) case */ - VIR_TYPED_PARAM_STRING = 7, /* string case */ -}; -/* / */ - # define VIR_ERROR_MESSAGE_DISSECTOR dissect_xdr_remote_error
static gboolean dissect_xdr_int(tvbuff_t *tvb, proto_tree *tree, XDR *xdrs, int hf); -- 1.8.5.5
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

At wireshark, they have this promise to change public dissector APIs only with minor version number change. Which they did when releasing the version of 1.12. Firstly, they've changed tvb_memdup() in a0c53ffaa1bb46d8c9db2ec739401aa411c9790e so now it takes four arguments instead of three. The new argument is placed at the very beginning of the list of arguments and basically says the scope where we'd like to allocate the memory. According to the documentation NULL should be the default value. Then, the tcp_dissect_pdus() signature changed too. Well, the function that actually dissects reassembled packets as tcp_dissect_pdus() reorder TCP packets into one big chunk and then calls a user function to dissect the PDU at once. The change is dated back to 8081cf1d90397cbbb4404f9720595e1537ed5e14. Then, WS_DLL_PUBLIC_NOEXTERN was replaced with WS_DLL_PUBLIC_DEF in 5d87a8c46171f572568db5a47c093423482e342f. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tools/wireshark/src/packet-libvirt.c | 32 +++++++++++++++++++++++++++++++- tools/wireshark/util/make-dissector-reg | 7 +++++++ 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/tools/wireshark/src/packet-libvirt.c b/tools/wireshark/src/packet-libvirt.c index 5138453..0fe5f03 100644 --- a/tools/wireshark/src/packet-libvirt.c +++ b/tools/wireshark/src/packet-libvirt.c @@ -36,6 +36,16 @@ #include "packet-libvirt.h" #include "internal.h" +#define WIRESHARK_VERSION \ + ((VERSION_MAJOR * 1000 * 1000) + \ + (VERSION_MINOR * 1000) + \ + (VERSION_MICRO)) + +/* Wireshark 1.12 brings API change */ +#if WIRESHARK_VERSION < 1012000 +# define WIRESHARK_COMPAT +#endif + static int proto_libvirt = -1; static int hf_libvirt_length = -1; static int hf_libvirt_program = -1; @@ -307,7 +317,11 @@ dissect_libvirt_payload_xdr_data(tvbuff_t *tvb, proto_tree *tree, gint payload_l } payload_tvb = tvb_new_subset(tvb, start, -1, payload_length); +#ifdef WIRESHARK_COMPAT 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); @@ -350,8 +364,14 @@ dissect_libvirt_payload(tvbuff_t *tvb, proto_tree *tree, proto_tree_add_item(tree, hf_libvirt_unknown, tvb, VIR_HEADER_LEN, -1, ENC_NA); } +#ifdef WIRESHARK_COMPAT 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; @@ -411,6 +431,10 @@ dissect_libvirt_message(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree) /* Dissect payload remaining */ dissect_libvirt_payload(tvb, libvirt_tree, prog, proc, type, status); } + +#ifndef WIRESHARK_COMPAT + return 0; +#endif } static guint32 @@ -424,7 +448,13 @@ dissect_libvirt(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree) { /* Another magic const - 4; simply, how much bytes * is needed to tell the length of libvirt packet. */ - tcp_dissect_pdus(tvb, pinfo, tree, TRUE, 4, get_message_len, dissect_libvirt_message); +#ifdef WIRESHARK_COMPAT + 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 } void diff --git a/tools/wireshark/util/make-dissector-reg b/tools/wireshark/util/make-dissector-reg index 4f8b4f2..6fa25c1 100755 --- a/tools/wireshark/util/make-dissector-reg +++ b/tools/wireshark/util/make-dissector-reg @@ -55,6 +55,13 @@ then #define WS_BUILD_DLL #include "ws_symbol_export.h" +/* 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; -- 1.8.5.5

On 07/04/2014 10:52 AM, Michal Privoznik wrote:
At wireshark, they have this promise to change public dissector APIs only with minor version number change. Which they did when releasing the version of 1.12.
Firstly, they've changed tvb_memdup() in a0c53ffaa1bb46d8c9db2ec739401aa411c9790e so now it takes four arguments instead of three. The new argument is placed at the very beginning of the list of arguments and basically says the scope where we'd like to allocate the memory. According to the documentation NULL should be the default value.
Then, the tcp_dissect_pdus() signature changed too. Well, the function that actually dissects reassembled packets as tcp_dissect_pdus() reorder TCP packets into one big chunk and then calls a user function to dissect the PDU at once. The change is dated back to 8081cf1d90397cbbb4404f9720595e1537ed5e14.
Then, WS_DLL_PUBLIC_NOEXTERN was replaced with WS_DLL_PUBLIC_DEF in 5d87a8c46171f572568db5a47c093423482e342f.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tools/wireshark/src/packet-libvirt.c | 32 +++++++++++++++++++++++++++++++- tools/wireshark/util/make-dissector-reg | 7 +++++++ 2 files changed, 38 insertions(+), 1 deletion(-)
You can include this bug link in the commit message: https://bugs.gentoo.org/show_bug.cgi?id=508336 Jan

On Fri, Jul 04, 2014 at 10:52:18AM +0200, Michal Privoznik wrote:
With the new wireshark installed I've noticed couple of build breaks. This fixes the problems I've seen.
Michal Privoznik (2): wireshark: Include more of libvirt internals wireshark: Honor API change coming with 1.12 release
tools/wireshark/src/Makefile.am | 2 +- tools/wireshark/src/packet-libvirt.c | 35 +++++++++++++++++++++++++++++++-- tools/wireshark/src/packet-libvirt.h | 18 ++--------------- tools/wireshark/util/make-dissector-reg | 7 +++++++ 4 files changed, 43 insertions(+), 19 deletions(-)
-- 1.8.5.5
We could probably create a new file where we would deal with the differences just to keep the code clean. We also proabbly don't build with any other wireshark versions (I only ever built with 1.10 and 1.12), so that is something someone might try inspecting. However this is enough and working, so it's better than nothing right now, so ACK series if you fix the include in 1/2. Let's hope this won't end up looking like the vbox driver ;) Martin
participants (3)
-
Ján Tomko
-
Martin Kletzander
-
Michal Privoznik