[libvirt] [PATCH 0/5] tools: Adapt to newer wireshark

While implementing some other feature I wanted to use wireshark to debug a migration issue. Only then I found that our wireshark plugin is no longer being loaded. Here are the fixes. Michal Prívozník (5): tools: Cleanup packet-libvirt.h tools: Keep wireshark plugin registration code in git wireshark: Provide registration code for newer wireshark m4: Put wireshark plugin into epan/ directory tools: Drop support for pre-2.4.0 wireshark .gitignore | 1 - cfg.mk | 2 +- libvirt.spec.in | 10 +- m4/virt-wireshark.m4 | 10 +- tools/Makefile.am | 15 +- tools/wireshark/src/packet-libvirt.c | 106 +++++++----- tools/wireshark/src/packet-libvirt.h | 93 +---------- tools/wireshark/src/plugin.c | 86 ++++++++++ tools/wireshark/util/make-dissector-reg | 205 ------------------------ 9 files changed, 170 insertions(+), 358 deletions(-) create mode 100644 tools/wireshark/src/plugin.c delete mode 100755 tools/wireshark/util/make-dissector-reg -- 2.19.2

Move the majority of the packet-libvirt.h content into packet-libvirt.c and expose only register functions which are the only ones that are not static. The rationale behind is that packet-libvirt.h will be included from packet.c and therefore the header file needs to be as clean as possible. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- cfg.mk | 2 +- tools/wireshark/src/packet-libvirt.c | 69 ++++++++++++++++++++- tools/wireshark/src/packet-libvirt.h | 93 +--------------------------- 3 files changed, 71 insertions(+), 93 deletions(-) diff --git a/cfg.mk b/cfg.mk index 5b6ea2504c..c2524de5fc 100644 --- a/cfg.mk +++ b/cfg.mk @@ -1283,7 +1283,7 @@ exclude_file_name_regexp--sc_correct_id_types = \ exclude_file_name_regexp--sc_m4_quote_check = m4/virt-lib.m4 exclude_file_name_regexp--sc_prohibit_include_public_headers_quote = \ - ^(src/internal\.h$$|tools/wireshark/src/packet-libvirt.h$$) + ^(src/internal\.h$$|tools/wireshark/src/packet-libvirt.c$$) exclude_file_name_regexp--sc_prohibit_include_public_headers_brackets = \ ^(tools/|examples/|include/libvirt/(virterror|libvirt(-(admin|qemu|lxc))?)\.h$$) diff --git a/tools/wireshark/src/packet-libvirt.c b/tools/wireshark/src/packet-libvirt.c index a71ad9f812..e759e81bae 100644 --- a/tools/wireshark/src/packet-libvirt.c +++ b/tools/wireshark/src/packet-libvirt.c @@ -29,6 +29,19 @@ #include "packet-libvirt.h" #include "internal.h" +#ifndef LIBVIRT_PORT +# define LIBVIRT_PORT 16509 +#endif + +#define VIR_HEADER_LEN 28 + +#ifdef DEBUG +# define dbg(fmt, ...) \ + g_print("[LIBVIRT] " fmt " at " __FILE__ " line %d\n", ##__VA_ARGS__, __LINE__) +#else +# define dbg(fmt, ...) +#endif + /* Wireshark 1.12 brings API change */ #define WIRESHARK_VERSION \ ((VERSION_MAJOR * 1000 * 1000) + \ @@ -80,6 +93,58 @@ XDR_PRIMITIVE_DISSECTOR(float, gfloat, float) XDR_PRIMITIVE_DISSECTOR(double, gdouble, double) XDR_PRIMITIVE_DISSECTOR(bool, bool_t, boolean) +typedef gboolean (*vir_xdr_dissector_t)(tvbuff_t *tvb, proto_tree *tree, XDR *xdrs, int hf); + +typedef struct vir_dissector_index vir_dissector_index_t; +struct vir_dissector_index { + guint32 proc; + vir_xdr_dissector_t args; + vir_xdr_dissector_t ret; + vir_xdr_dissector_t msg; +}; + +enum vir_net_message_type { + VIR_NET_CALL = 0, + VIR_NET_REPLY = 1, + VIR_NET_MESSAGE = 2, + VIR_NET_STREAM = 3, + VIR_NET_CALL_WITH_FDS = 4, + VIR_NET_REPLY_WITH_FDS = 5, + VIR_NET_STREAM_HOLE = 6, +}; + +enum vir_net_message_status { + VIR_NET_OK = 0, + VIR_NET_ERROR = 1, + VIR_NET_CONTINUE = 2, +}; + +enum vir_program_data_index { + VIR_PROGRAM_PROCHFVAR, + VIR_PROGRAM_PROCSTRINGS, + VIR_PROGRAM_DISSECTORS, + VIR_PROGRAM_DISSECTORS_LEN, + VIR_PROGRAM_LAST, +}; + +static const value_string type_strings[] = { + { VIR_NET_CALL, "CALL" }, + { VIR_NET_REPLY, "REPLY" }, + { VIR_NET_MESSAGE, "MESSAGE" }, + { VIR_NET_STREAM, "STREAM" }, + { VIR_NET_CALL_WITH_FDS, "CALL_WITH_FDS" }, + { VIR_NET_REPLY_WITH_FDS, "REPLY_WITH_FDS" }, + { VIR_NET_STREAM_HOLE, "STREAM_HOLE" }, + { -1, NULL } +}; + +static const value_string status_strings[] = { + { VIR_NET_OK, "OK" }, + { VIR_NET_ERROR, "ERROR" }, + { VIR_NET_CONTINUE, "CONTINUE" }, + { -1, NULL } +}; + static gboolean dissect_xdr_string(tvbuff_t *tvb, proto_tree *tree, XDR *xdrs, int hf, guint32 maxlen) @@ -357,6 +422,8 @@ dissect_xdr_stream_hole(tvbuff_t *tvb, proto_tree *tree, XDR *xdrs, int hf) return TRUE; } +#include "libvirt/protocol.h" + static void dissect_libvirt_payload(tvbuff_t *tvb, proto_tree *tree, guint32 prog, guint32 proc, guint32 type, guint32 status) @@ -374,7 +441,7 @@ dissect_libvirt_payload(tvbuff_t *tvb, proto_tree *tree, goto unknown; dissect_libvirt_payload_xdr_data(tvb, tree, payload_length, status, xd); } else if (status == VIR_NET_ERROR) { - dissect_libvirt_payload_xdr_data(tvb, tree, payload_length, status, VIR_ERROR_MESSAGE_DISSECTOR); + dissect_libvirt_payload_xdr_data(tvb, tree, payload_length, status, dissect_xdr_remote_error); } else if (type == VIR_NET_STREAM) { /* implicitly, status == VIR_NET_CONTINUE */ dissect_libvirt_stream(tvb, tree, payload_length); } else if (type == VIR_NET_STREAM_HOLE) { diff --git a/tools/wireshark/src/packet-libvirt.h b/tools/wireshark/src/packet-libvirt.h index 4f2a275ba6..3b7a0f054d 100644 --- a/tools/wireshark/src/packet-libvirt.h +++ b/tools/wireshark/src/packet-libvirt.h @@ -20,96 +20,7 @@ #ifndef LIBVIRT_PACKET_LIBVIRT_H # define LIBVIRT_PACKET_LIBVIRT_H -# include "libvirt/libvirt.h" - -# ifndef LIBVIRT_PORT -# define LIBVIRT_PORT 16509 -# endif - -# define VIR_HEADER_LEN 28 - -# ifdef DEBUG -# define dbg(fmt, ...) \ - g_print("[LIBVIRT] " fmt " at " __FILE__ " line %d\n", ##__VA_ARGS__, __LINE__) -# else -# define dbg(fmt, ...) -# endif - -typedef gboolean (*vir_xdr_dissector_t)(tvbuff_t *tvb, proto_tree *tree, XDR *xdrs, int hf); - -typedef struct vir_dissector_index vir_dissector_index_t; -struct vir_dissector_index { - guint32 proc; - vir_xdr_dissector_t args; - vir_xdr_dissector_t ret; - vir_xdr_dissector_t msg; -}; - -enum vir_net_message_type { - VIR_NET_CALL = 0, - VIR_NET_REPLY = 1, - VIR_NET_MESSAGE = 2, - VIR_NET_STREAM = 3, - VIR_NET_CALL_WITH_FDS = 4, - VIR_NET_REPLY_WITH_FDS = 5, - VIR_NET_STREAM_HOLE = 6, -}; - -enum vir_net_message_status { - VIR_NET_OK = 0, - VIR_NET_ERROR = 1, - VIR_NET_CONTINUE = 2, -}; - -enum vir_program_data_index { - VIR_PROGRAM_PROCHFVAR, - VIR_PROGRAM_PROCSTRINGS, - VIR_PROGRAM_DISSECTORS, - VIR_PROGRAM_DISSECTORS_LEN, - VIR_PROGRAM_LAST, -}; - -static const value_string type_strings[] = { - { VIR_NET_CALL, "CALL" }, - { VIR_NET_REPLY, "REPLY" }, - { VIR_NET_MESSAGE, "MESSAGE" }, - { VIR_NET_STREAM, "STREAM" }, - { VIR_NET_CALL_WITH_FDS, "CALL_WITH_FDS" }, - { VIR_NET_REPLY_WITH_FDS, "REPLY_WITH_FDS" }, - { VIR_NET_STREAM_HOLE, "STREAM_HOLE" }, - { -1, NULL } -}; - -static const value_string status_strings[] = { - { VIR_NET_OK, "OK" }, - { VIR_NET_ERROR, "ERROR" }, - { VIR_NET_CONTINUE, "CONTINUE" }, - { -1, NULL } -}; - -# define VIR_ERROR_MESSAGE_DISSECTOR dissect_xdr_remote_error - -static gboolean dissect_xdr_int(tvbuff_t *tvb, proto_tree *tree, XDR *xdrs, int hf); -static gboolean dissect_xdr_u_int(tvbuff_t *tvb, proto_tree *tree, XDR *xdrs, int hf); -static gboolean dissect_xdr_short(tvbuff_t *tvb, proto_tree *tree, XDR *xdrs, int hf); -static gboolean dissect_xdr_u_short(tvbuff_t *tvb, proto_tree *tree, XDR *xdrs, int hf); -static gboolean dissect_xdr_char(tvbuff_t *tvb, proto_tree *tree, XDR *xdrs, int hf); -static gboolean dissect_xdr_u_char(tvbuff_t *tvb, proto_tree *tree, XDR *xdrs, int hf); -static gboolean dissect_xdr_hyper(tvbuff_t *tvb, proto_tree *tree, XDR *xdrs, int hf); -static gboolean dissect_xdr_u_hyper(tvbuff_t *tvb, proto_tree *tree, XDR *xdrs, int hf); -static gboolean dissect_xdr_float(tvbuff_t *tvb, proto_tree *tree, XDR *xdrs, int hf); -static gboolean dissect_xdr_double(tvbuff_t *tvb, proto_tree *tree, XDR *xdrs, int hf); -static gboolean dissect_xdr_bool(tvbuff_t *tvb, proto_tree *tree, XDR *xdrs, int hf); -static gboolean dissect_xdr_string(tvbuff_t *tvb, proto_tree *tree, XDR *xdrs, int hf, guint32 maxlen); -static gboolean dissect_xdr_opaque(tvbuff_t *tvb, proto_tree *tree, XDR *xdrs, int hf, guint32 size); -static gboolean dissect_xdr_bytes(tvbuff_t *tvb, proto_tree *tree, XDR *xdrs, int hf, guint32 maxlen); -static gboolean dissect_xdr_pointer(tvbuff_t *tvb, proto_tree *tree, XDR *xdrs, int hf, - vir_xdr_dissector_t dp); -static gboolean dissect_xdr_vector(tvbuff_t *tvb, proto_tree *tree, XDR *xdrs, int hf, gint ett, - int rhf, const gchar *rtype, guint32 size, vir_xdr_dissector_t dp); -static gboolean dissect_xdr_array(tvbuff_t *tvb, proto_tree *tree, XDR *xdrs, int hf, gint ett, - int rhf, const gchar *rtype, guint32 maxlen, vir_xdr_dissector_t dp); - -# include "libvirt/protocol.h" +void proto_register_libvirt(void); +void proto_reg_handoff_libvirt(void); #endif /* LIBVIRT_PACKET_LIBVIRT_H */ -- 2.19.2

In order to be able to dissect libvirt protocol the wireshark plugin needs to be registered. So far this plugin registration code was generated on every build using a script that was copied over from wireshark's tools/ directory. This is suboptimal, because the way that plugins register changes across wireshark releases. Therefore, let's keep the generated file in the git, put the command line used to generate the file into a comment and remove the script. This solution allows us to put different registration mechanism into one file (under #ifdef-s) and thus compile with wider range of wireshark releases. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- .gitignore | 1 - tools/Makefile.am | 15 +- tools/wireshark/src/plugin.c | 45 ++++++ tools/wireshark/util/make-dissector-reg | 205 ------------------------ 4 files changed, 48 insertions(+), 218 deletions(-) create mode 100644 tools/wireshark/src/plugin.c delete mode 100755 tools/wireshark/util/make-dissector-reg diff --git a/.gitignore b/.gitignore index df0ac8e3d4..3303eed411 100644 --- a/.gitignore +++ b/.gitignore @@ -190,7 +190,6 @@ /tools/virt-admin /tools/virt-*-validate /tools/virt-sanlock-cleanup -/tools/wireshark/src/plugin.c /tools/wireshark/src/libvirt /update.log GPATH diff --git a/tools/Makefile.am b/tools/Makefile.am index 613c9a77f0..f2f84f7852 100644 --- a/tools/Makefile.am +++ b/tools/Makefile.am @@ -444,9 +444,7 @@ uninstall-bash-completion: endif ! WITH_BASH_COMPLETION -EXTRA_DIST += \ - wireshark/util/genxdrstub.pl \ - wireshark/util/make-dissector-reg +EXTRA_DIST += wireshark/util/genxdrstub.pl if WITH_WIRESHARK_DISSECTOR @@ -454,19 +452,14 @@ ws_plugin_LTLIBRARIES = wireshark/src/libvirt.la wireshark_src_libvirt_la_CFLAGS = \ -I wireshark/src $(WIRESHARK_DISSECTOR_CFLAGS) $(XDR_CFLAGS) wireshark_src_libvirt_la_LDFLAGS = -avoid-version -module -nodist_wireshark_src_libvirt_la_SOURCES = wireshark/src/plugin.c wireshark_src_libvirt_la_SOURCES = \ wireshark/src/packet-libvirt.h \ - wireshark/src/packet-libvirt.c + wireshark/src/packet-libvirt.c \ + wireshark/src/plugin.c wireshark/src/packet-libvirt.c: wireshark/src/packet-libvirt.h \ wireshark/src/libvirt/protocol.h -wireshark/src/plugin.c: wireshark/src/packet-libvirt.c - $(AM_V_GEN)cd wireshark/src && \ - $(abs_top_srcdir)/tools/wireshark/util/make-dissector-reg \ - . plugin packet-libvirt.c - WS_DISSECTOR_PROTO_FILES = \ $(abs_top_srcdir)/src/remote/remote_protocol.x \ $(abs_top_srcdir)/src/remote/qemu_protocol.x \ @@ -481,8 +474,6 @@ wireshark/src/libvirt/protocol.h: wireshark/util/genxdrstub.pl \ $(PERL) $(abs_top_srcdir)/tools/wireshark/util/genxdrstub.pl \ $(WS_DISSECTOR_PROTO_FILES) -CLEANFILES += wireshark/src/plugin.c - endif WITH_WIRESHARK_DISSECTOR if WITH_BSD_NSS diff --git a/tools/wireshark/src/plugin.c b/tools/wireshark/src/plugin.c new file mode 100644 index 0000000000..2a85688f43 --- /dev/null +++ b/tools/wireshark/src/plugin.c @@ -0,0 +1,45 @@ +/* + * plugin.c: Wireshark's plugin registration + * + * The registration routines were generated using wireshark's + * make-dissector-reg script (found under wirshark.git/tools/): + * + * libvirt.git/tools/wireshark/src $ \ + * /path/to/wireshark.git/tools/make-dissector-reg \ + * . plugin packet-libvirt.c + * + */ + +#include <config.h> + +#include <gmodule.h> + +/* plugins are DLLs */ +#define WS_BUILD_DLL +#include <wireshark/ws_symbol_export.h> + +#include "packet-libvirt.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; + +/* Start the functions we need for the plugin stuff */ + +WS_DLL_PUBLIC_NOEXTERN void +plugin_register(void) +{ + proto_register_libvirt(); +} +WS_DLL_PUBLIC_NOEXTERN void +plugin_reg_handoff(void) +{ + proto_reg_handoff_libvirt(); +} +#endif diff --git a/tools/wireshark/util/make-dissector-reg b/tools/wireshark/util/make-dissector-reg deleted file mode 100755 index 6fa25c1eed..0000000000 --- a/tools/wireshark/util/make-dissector-reg +++ /dev/null @@ -1,205 +0,0 @@ -#! /bin/sh -# This program is free software; you can redistribute it and/or -# modify it under the terms of the GNU General Public License -# as published by the Free Software Foundation; either version 2 -# of the License, or (at your option) any later version. -# -# This program is distributed in the hope that it will be useful, -# but WITHOUT ANY WARRANTY; without even the implied warranty of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -# GNU General Public License for more details. -# -# You should have received a copy of the GNU General Public License -# along with this program; if not, see <http://www.gnu.org/licenses/>. -# -# Copied from Wireshark(http://www.wireshark.org/) - -# -# The first argument is the directory in which the source files live. -# -srcdir="$1" -shift - -# -# The second argument is either "plugin" or "dissectors"; if it's -# "plugin", we build a plugin.c for a plugin, and if it's -# "dissectors", we build a register.c for libwireshark. -# -registertype="$1" -shift -if [ "$registertype" = plugin ] -then - outfile="plugin.c" -elif [ "$registertype" = dissectors ] -then - outfile="register.c" -else - echo "Unknown output type '$registertype'" 1>&2 - exit 1 -fi - -# -# All subsequent arguments are the files to scan. -# -rm -f ${outfile}-tmp -echo '/* Do not modify this file. */' >${outfile}-tmp -echo '/* It is created automatically by the Makefile. */'>>${outfile}-tmp -if [ "$registertype" = plugin ] -then - cat <<"EOF" >>${outfile}-tmp -#include "config.h" - -#include <gmodule.h> - -/* plugins are DLLs */ -#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; - -/* Start the functions we need for the plugin stuff */ - -WS_DLL_PUBLIC_NOEXTERN void -plugin_register (void) -{ -EOF -# -# Build code to call all the protocol registration routines. -# -for f in "$@" -do - if [ -f $f ] - then - srcfile=$f - else - srcfile=$srcdir/$f - fi - grep '^proto_register_[a-z_0-9A-Z]* *(' $srcfile 2>/dev/null | grep -v ';' -done | sed -e 's/^.*://' -e 's/^\([a-z_0-9A-Z]*\).*/ {extern void \1 (void); \1 ();}/' >>${outfile}-tmp -for f in "$@" -do - if [ -f $f ] - then - srcfile=$f - else - srcfile=$srcdir/$f - fi - grep '^void proto_register_[a-z_0-9A-Z]* *(' $srcfile 2>/dev/null | grep -v ';' -done | sed -e 's/^.*://' -e 's/^void \([a-z_0-9A-Z]*\).*/ {extern void \1 (void); \1 ();}/' >>${outfile}-tmp -else - cat <<"EOF" >>${outfile}-tmp -#include "register.h" -void -register_all_protocols(register_cb cb, gpointer client_data) -{ -EOF -# -# Build code to call all the protocol registration routines. -# -for f in "$@" -do - if [ -f $f ] - then - srcfile=$f - else - srcfile=$srcdir/$f - fi - grep '^proto_register_[a-z_0-9A-Z]* *(' $srcfile 2>/dev/null | grep -v ';' -done | sed -e 's/^.*://' -e 's/^\([a-z_0-9A-Z]*\).*/ {extern void \1 (void); if(cb) (*cb)(RA_REGISTER, \"\1\", client_data); \1 ();}/' >>${outfile}-tmp -for f in "$@" -do - if [ -f $f ] - then - srcfile=$f - else - srcfile=$srcdir/$f - fi - grep '^void proto_register_[a-z_0-9A-Z]* *(' $srcfile 2>/dev/null | grep -v ';' -done | sed -e 's/^.*://' -e 's/^void \([a-z_0-9A-Z]*\).*/ {extern void \1 (void); if(cb) (*cb)(RA_REGISTER, \"\1\", client_data); \1 ();}/' >>${outfile}-tmp - -fi -echo '}' >>${outfile}-tmp - - -# -# Build code to call all the protocol handoff registration routines. -# -if [ "$registertype" = plugin ] -then - cat <<"EOF" >>${outfile}-tmp -WS_DLL_PUBLIC_NOEXTERN void -plugin_reg_handoff(void) -{ -EOF -for f in "$@" -do - if [ -f $f ] - then - srcfile=$f - else - srcfile=$srcdir/$f - fi - grep '^proto_reg_handoff_[a-z_0-9A-Z]* *(' $srcfile 2>/dev/null | grep -v ';' -done | sed -e 's/^.*://' -e 's/^\([a-z_0-9A-Z]*\).*/ {extern void \1 (void); \1 ();}/' >>${outfile}-tmp -for f in "$@" -do - if [ -f $f ] - then - srcfile=$f - else - srcfile=$srcdir/$f - fi - grep '^void proto_reg_handoff_[a-z_0-9A-Z]* *(' $srcfile 2>/dev/null | grep -v ';' -done | sed -e 's/^.*://' -e 's/^void \([a-z_0-9A-Z]*\).*/ {extern void \1 (void); \1 ();}/' >>${outfile}-tmp -else - cat <<"EOF" >>${outfile}-tmp -void -register_all_protocol_handoffs(register_cb cb, gpointer client_data) -{ -EOF -for f in "$@" -do - if [ -f $f ] - then - srcfile=$f - else - srcfile=$srcdir/$f - fi - grep '^proto_reg_handoff_[a-z_0-9A-Z]* *(' $srcfile 2>/dev/null | grep -v ';' -done | sed -e 's/^.*://' -e 's/^\([a-z_0-9A-Z]*\).*/ {extern void \1 (void); if(cb) (*cb)(RA_HANDOFF, \"\1\", client_data); \1 ();}/' >>${outfile}-tmp -for f in "$@" -do - if [ -f $f ] - then - srcfile=$f - else - srcfile=$srcdir/$f - fi - grep '^void proto_reg_handoff_[a-z_0-9A-Z]* *(' $srcfile 2>/dev/null | grep -v ';' -done | sed -e 's/^.*://' -e 's/^void \([a-z_0-9A-Z]*\).*/ {extern void \1 (void); if(cb) (*cb)(RA_HANDOFF, \"\1\", client_data); \1 ();}/' >>${outfile}-tmp -fi -echo '}' >>${outfile}-tmp -if [ "$registertype" = plugin ] -then - echo '#endif' >>${outfile}-tmp -else - cat <<"EOF" >>${outfile}-tmp -gulong register_count(void) -{ -EOF - proto_regs=`grep RA_REGISTER ${outfile}-tmp | wc -l` - handoff_regs=`grep RA_HANDOFF ${outfile}-tmp | wc -l` - echo " return $proto_regs + $handoff_regs;" >>${outfile}-tmp - echo '}' >>${outfile}-tmp -fi - -# Only overwrite outfile if it differs from newly generated file -mv ${outfile}-tmp ${outfile} -- 2.19.2

As advertised in previous commits, wireshark has changed the way that plugins register. In fact, it has done so two times since the last time we've touched our code (wireshark v2.5.0 and v2.9.0). Use the wireshark script from respective releases to generate newer registration callbacks and put them into our code. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tools/wireshark/src/plugin.c | 61 ++++++++++++++++++++++++++++++++---- 1 file changed, 55 insertions(+), 6 deletions(-) diff --git a/tools/wireshark/src/plugin.c b/tools/wireshark/src/plugin.c index 2a85688f43..3741334e85 100644 --- a/tools/wireshark/src/plugin.c +++ b/tools/wireshark/src/plugin.c @@ -2,10 +2,10 @@ * plugin.c: Wireshark's plugin registration * * The registration routines were generated using wireshark's - * make-dissector-reg script (found under wirshark.git/tools/): + * make-plugin-reg.py script (found under wirshark.git/tools/): * * libvirt.git/tools/wireshark/src $ \ - * /path/to/wireshark.git/tools/make-dissector-reg \ + * /path/to/wireshark.git/tools/make-plugin-reg.py \ * . plugin packet-libvirt.c * */ @@ -14,20 +14,31 @@ #include <gmodule.h> +#include <wireshark/config.h> +#include <wireshark/epan/proto.h> /* plugins are DLLs */ #define WS_BUILD_DLL #include <wireshark/ws_symbol_export.h> #include "packet-libvirt.h" +/* Let the plugin version be the version of libvirt */ +#define PLUGIN_VERSION VERSION + +#define WIRESHARK_VERSION \ + ((VERSION_MAJOR * 1000 * 1000) + \ + (VERSION_MINOR * 1000) + \ + (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 WS_DLL_PUBLIC_NOEXTERN +# define WS_DLL_PUBLIC_NOEXTERN WS_DLL_PUBLIC_DEF +# endif -#ifndef ENABLE_STATIC +# ifndef ENABLE_STATIC WS_DLL_PUBLIC_NOEXTERN const gchar version[] = VERSION; /* Start the functions we need for the plugin stuff */ @@ -42,4 +53,42 @@ plugin_reg_handoff(void) { proto_reg_handoff_libvirt(); } +# endif + +#elif WIRESHARK_VERSION < 2009000 + +WS_DLL_PUBLIC_DEF const gchar plugin_version[] = PLUGIN_VERSION; +WS_DLL_PUBLIC_DEF const gchar plugin_release[] = VERSION_RELEASE; + +WS_DLL_PUBLIC void plugin_register(void); + +void plugin_register(void) +{ + static proto_plugin plug_libvirt; + + plug_libvirt.register_protoinfo = proto_register_libvirt; + plug_libvirt.register_handoff = proto_reg_handoff_libvirt; + proto_register_plugin(&plug_libvirt); +} + +#else /* WIRESHARK_VERSION >= 2009000 */ + +void proto_register_libvirt(void); +void proto_reg_handoff_libvirt(void); + +WS_DLL_PUBLIC_DEF const gchar plugin_version[] = PLUGIN_VERSION; +WS_DLL_PUBLIC_DEF const int plugin_want_major = VERSION_MAJOR; +WS_DLL_PUBLIC_DEF const int plugin_want_minor = VERSION_MINOR; + +WS_DLL_PUBLIC void plugin_register(void); + +void plugin_register(void) +{ + static proto_plugin plug_libvirt; + + plug_libvirt.register_protoinfo = proto_register_libvirt; + plug_libvirt.register_handoff = proto_reg_handoff_libvirt; + proto_register_plugin(&plug_libvirt); +} + #endif -- 2.19.2

Since wirshark-2.5.0 toplevel plugins are no longer loaded. Only plugins from epan/, wiretap/ or codecs/ subdirs are. Update the plugin dir we generate. This is safe to do even for older wiresharks, since they load plugins from there too. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- libvirt.spec.in | 4 ++-- m4/virt-wireshark.m4 | 6 ++++++ 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/libvirt.spec.in b/libvirt.spec.in index c0e538d92d..13f41bb4a5 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -168,9 +168,9 @@ %define with_wireshark 0%{!?_without_wireshark:1} %endif %if 0%{?fedora} || 0%{?rhel} > 7 - %define wireshark_plugindir %(pkg-config --variable plugindir wireshark) + %define wireshark_plugindir %(pkg-config --variable plugindir wireshark)/epan %else - %define wireshark_plugindir %{_libdir}/wireshark/plugins + %define wireshark_plugindir %{_libdir}/wireshark/plugins/epan %endif # Enable libssh transport for new enough distros diff --git a/m4/virt-wireshark.m4 b/m4/virt-wireshark.m4 index 1283a0f403..a8f8083f5e 100644 --- a/m4/virt-wireshark.m4 +++ b/m4/virt-wireshark.m4 @@ -50,6 +50,12 @@ AC_DEFUN([LIBVIRT_CHECK_WIRESHARK],[ dnl time ws_plugindir='${exec_prefix}'"${ws_plugindir#$ws_exec_prefix}" fi + + dnl Since wireshark 2.5.0 plugins can't leave in top level + dnl plugindir but have to be under one of ["epan", + dnl "wiretap", "codecs"] subdir. The first one looks okay. + ws_plugindir="$ws_plugindir/epan" + elif test "x$with_ws_plugindir" = "xno" || test "x$with_ws_plugindir" = "xyes"; then AC_MSG_ERROR([ws-plugindir must be used only with valid path]) else -- 2.19.2

On Fri, Feb 08, 2019 at 12:23:27PM +0100, Michal Privoznik wrote:
Since wirshark-2.5.0 toplevel plugins are no longer loaded. Only plugins from epan/, wiretap/ or codecs/ subdirs are. Update the plugin dir we generate. This is safe to do even for older wiresharks, since they load plugins from there too.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- libvirt.spec.in | 4 ++-- m4/virt-wireshark.m4 | 6 ++++++ 2 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/libvirt.spec.in b/libvirt.spec.in index c0e538d92d..13f41bb4a5 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -168,9 +168,9 @@ %define with_wireshark 0%{!?_without_wireshark:1} %endif %if 0%{?fedora} || 0%{?rhel} > 7 - %define wireshark_plugindir %(pkg-config --variable plugindir wireshark) + %define wireshark_plugindir %(pkg-config --variable plugindir wireshark)/epan %else - %define wireshark_plugindir %{_libdir}/wireshark/plugins + %define wireshark_plugindir %{_libdir}/wireshark/plugins/epan %endif
# Enable libssh transport for new enough distros diff --git a/m4/virt-wireshark.m4 b/m4/virt-wireshark.m4 index 1283a0f403..a8f8083f5e 100644 --- a/m4/virt-wireshark.m4 +++ b/m4/virt-wireshark.m4 @@ -50,6 +50,12 @@ AC_DEFUN([LIBVIRT_CHECK_WIRESHARK],[ dnl time ws_plugindir='${exec_prefix}'"${ws_plugindir#$ws_exec_prefix}" fi + + dnl Since wireshark 2.5.0 plugins can't leave in top level
s/leave/live/ Jano
+ dnl plugindir but have to be under one of ["epan", + dnl "wiretap", "codecs"] subdir. The first one looks okay. + ws_plugindir="$ws_plugindir/epan" + elif test "x$with_ws_plugindir" = "xno" || test "x$with_ws_plugindir" = "xyes"; then AC_MSG_ERROR([ws-plugindir must be used only with valid path]) else -- 2.19.2
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

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

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

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

On Fri, Feb 08, 2019 at 12:23:23PM +0100, Michal Privoznik wrote:
While implementing some other feature I wanted to use wireshark to debug a migration issue. Only then I found that our wireshark plugin is no longer being loaded. Here are the fixes.
Michal Prívozník (5): tools: Cleanup packet-libvirt.h tools: Keep wireshark plugin registration code in git wireshark: Provide registration code for newer wireshark m4: Put wireshark plugin into epan/ directory tools: Drop support for pre-2.4.0 wireshark
.gitignore | 1 - cfg.mk | 2 +- libvirt.spec.in | 10 +- m4/virt-wireshark.m4 | 10 +- tools/Makefile.am | 15 +- tools/wireshark/src/packet-libvirt.c | 106 +++++++----- tools/wireshark/src/packet-libvirt.h | 93 +---------- tools/wireshark/src/plugin.c | 86 ++++++++++ tools/wireshark/util/make-dissector-reg | 205 ------------------------ 9 files changed, 170 insertions(+), 358 deletions(-) create mode 100644 tools/wireshark/src/plugin.c delete mode 100755 tools/wireshark/util/make-dissector-reg
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
participants (2)
-
Ján Tomko
-
Michal Privoznik