5:26 a.m.
Well, I've just updated wireshark on my system and encountered couple of
compile errors while building libvirt. Here are the fixes. Fortunately, none of
them requires us to increase the version number of wireshark that's required.
However, I'd like to discuss how are we going to handle this. I mean, at
wireshark they don't seem so committed to API stability as we are. Therefore
this patch set. In the long term I don't see us adapting to every single API
change, or do I? Although I am not sure what are our options here.
Michal Privoznik (5):
wireshark: s/proto_tree_add_text/proto_tree_add_item/
wireshark: s/ep_alloc/wmem_alloc/
wireshark: s/tvb_length/tvb_captured_length/
wireshark: Replace WIRESHARK_COMPAT with actual version comparison
wireshark: Fix header of get_message_len()
tools/wireshark/src/packet-libvirt.c | 27 ++++++++++++++-------------
tools/wireshark/util/genxdrstub.pl | 7 +++++--
2 files changed, 19 insertions(+), 15 deletions(-)
--
2.4.10
5:26 a.m.
New subject: [libvirt] [PATCH 1/5] wireshark: s/proto_tree_add_text/proto_tree_add_item/
In the wireshark commit e2735ecfdd7a96c they dropped
proto_tree_add_text in favor of proto_tree_add_item. Adapt to
this change.
Moreover, the proto_tree_add_item API is around for ages and we
are already using it anyway. Therefore we don't need to change
required version of wireshark.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
tools/wireshark/src/packet-libvirt.c | 2 +-
tools/wireshark/util/genxdrstub.pl | 7 +++++--
2 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/tools/wireshark/src/packet-libvirt.c b/tools/wireshark/src/packet-libvirt.c
index f7aa7ed..3103562 100644
--- a/tools/wireshark/src/packet-libvirt.c
+++ b/tools/wireshark/src/packet-libvirt.c
@@ -56,7 +56,7 @@ static int hf_libvirt_serial = -1;
static int hf_libvirt_status = -1;
static int hf_libvirt_stream = -1;
static int hf_libvirt_num_of_fds = -1;
-static int hf_libvirt_unknown = -1;
+int hf_libvirt_unknown = -1;
static gint ett_libvirt = -1;
#define XDR_PRIMITIVE_DISSECTOR(xtype, ctype, ftype) \
diff --git a/tools/wireshark/util/genxdrstub.pl b/tools/wireshark/util/genxdrstub.pl
index 76ccda9..07f0ff7 100755
--- a/tools/wireshark/util/genxdrstub.pl
+++ b/tools/wireshark/util/genxdrstub.pl
@@ -57,6 +57,9 @@ for my $proto (@ARGV) {
$c->add_header_file($name, sub {
dbg "*** Start parsing $proto\n";
+
+ $c->print("extern int hf_libvirt_unknown;\n");
+
my @lexs = Lexicalizer->parse($source);
for my $lex (@lexs) {
next if $lex->ident eq "enum $name\_procedure";
@@ -903,7 +906,7 @@ static gboolean dissect_xdr_<%= $ident %>(tvbuff_t *tvb,
proto_tree *tree, XDR *
<% } %>
}
} else {
- proto_tree_add_text(tree, tvb, start, -1, "(unknown)");
+ proto_tree_add_item(tree, hf_libvirt_unknown, tvb, start, -1, ENC_NA);
}
return FALSE;
}
@@ -933,7 +936,7 @@ static gboolean dissect_xdr_<%= $ident %>(tvbuff_t *tvb,
proto_tree *tree, XDR *
<% } %>
}
if (!rc) {
- proto_tree_add_text(tree, tvb, start, -1, "(unknown)");
+ proto_tree_add_item(tree, hf_libvirt_unknown, tvb, start, -1, ENC_NA);
}
return rc;
}
--
2.4.10
5:26 a.m.
New subject: [libvirt] [PATCH 2/5] wireshark: s/ep_alloc/wmem_alloc/
In wireshark, they have introduced their own memory allocator
wmem. This means that we need to adapt our code to that change
too. Notably 0ad15f88ccf434e8210ca is the wireshark commit you
want to look at. It's the one where they dropped the old API. The
new allocator has been introduced in 84cc3daa (v1.10.0), however,
was not exposed until 5c05c9e0 (v1.10.0). Since we already are
requiring 1.11.3 or higher no other change is needed.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
tools/wireshark/src/packet-libvirt.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/wireshark/src/packet-libvirt.c b/tools/wireshark/src/packet-libvirt.c
index 3103562..f7c8e0c 100644
--- a/tools/wireshark/src/packet-libvirt.c
+++ b/tools/wireshark/src/packet-libvirt.c
@@ -113,7 +113,7 @@ format_xdr_bytes(guint8 *bytes, guint32 length)
if (length == 0)
return "";
- buf = ep_alloc(length*2 + 1);
+ buf = wmem_alloc(wmem_packet_scope(), length*2 + 1);
for (i = 0; i < length; i++) {
/* We know that buf has enough size to contain
2 * length + '\0' characters. */
--
2.4.10
5:26 a.m.
New subject: [libvirt] [PATCH 3/5] wireshark: s/tvb_length/tvb_captured_length/
In wireshak commit 22149c55 (v.1.11.3) the API was renamed.
Follow the change in our code too. Since the wireshark change was
made in the very same version that we require at least we are
good to go.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
tools/wireshark/src/packet-libvirt.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/wireshark/src/packet-libvirt.c b/tools/wireshark/src/packet-libvirt.c
index f7c8e0c..d0b0852 100644
--- a/tools/wireshark/src/packet-libvirt.c
+++ b/tools/wireshark/src/packet-libvirt.c
@@ -338,7 +338,7 @@ dissect_libvirt_payload(tvbuff_t *tvb, proto_tree *tree,
{
gssize payload_length;
- payload_length = tvb_length(tvb) - VIR_HEADER_LEN;
+ payload_length = tvb_captured_length(tvb) - VIR_HEADER_LEN;
if (payload_length <= 0)
return; /* No payload */
@@ -405,7 +405,7 @@ dissect_libvirt_message(tvbuff_t *tvb, packet_info *pinfo, proto_tree
*tree,
proto_item *ti;
proto_tree *libvirt_tree;
- ti = proto_tree_add_item(tree, proto_libvirt, tvb, 0, tvb_length(tvb), ENC_NA);
+ ti = proto_tree_add_item(tree, proto_libvirt, tvb, 0, tvb_captured_length(tvb),
ENC_NA);
libvirt_tree = proto_item_add_subtree(ti, ett_libvirt);
offset = 0;
--
2.4.10
5:26 a.m.
New subject: [libvirt] [PATCH 4/5] wireshark: Replace WIRESHARK_COMPAT with actual version comparison
In the upcoming patch we will need yet another #ifdef code block
depending on wireshark version. Instead of defining
WIRESHARK_COMPAT2 or something lets just compare the version
right at the place so that we can clearly see what version broke
API.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
tools/wireshark/src/packet-libvirt.c | 14 +++++---------
1 file changed, 5 insertions(+), 9 deletions(-)
diff --git a/tools/wireshark/src/packet-libvirt.c b/tools/wireshark/src/packet-libvirt.c
index d0b0852..ac120b5 100644
--- a/tools/wireshark/src/packet-libvirt.c
+++ b/tools/wireshark/src/packet-libvirt.c
@@ -36,16 +36,12 @@
#include "packet-libvirt.h"
#include "internal.h"
+/* Wireshark 1.12 brings API change */
#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;
@@ -316,7 +312,7 @@ 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
+#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);
@@ -362,7 +358,7 @@ 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
+#if WIRESHARK_VERSION < 1012000
static void
dissect_libvirt_message(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree)
#else
@@ -430,7 +426,7 @@ dissect_libvirt_message(tvbuff_t *tvb, packet_info *pinfo, proto_tree
*tree,
dissect_libvirt_payload(tvb, libvirt_tree, prog, proc, type, status);
}
-#ifndef WIRESHARK_COMPAT
+#if WIRESHARK_VERSION >= 1012000
return 0;
#endif
}
@@ -446,7 +442,7 @@ 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. */
-#ifdef WIRESHARK_COMPAT
+#if WIRESHARK_VERSION < 1012000
tcp_dissect_pdus(tvb, pinfo, tree, TRUE, 4,
get_message_len, dissect_libvirt_message);
#else
--
2.4.10
5:26 a.m.
New subject: [libvirt] [PATCH 5/5] wireshark: Fix header of get_message_len()
In wireshark commit ceb8d954 (v1.99.2) they have changed the
signature of a function that determines how long a libvirt packet
is. Now it accepts a void pointer for passing data into the
function. Well, this is nice, but we don't need it right now.
Anyway, we have to change our code.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
tools/wireshark/src/packet-libvirt.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/tools/wireshark/src/packet-libvirt.c b/tools/wireshark/src/packet-libvirt.c
index ac120b5..aa1c323 100644
--- a/tools/wireshark/src/packet-libvirt.c
+++ b/tools/wireshark/src/packet-libvirt.c
@@ -431,8 +431,13 @@ dissect_libvirt_message(tvbuff_t *tvb, packet_info *pinfo, proto_tree
*tree,
#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);
}
--
2.4.10
10:12 a.m.
On Mon, Jan 04, 2016 at 12:26:22PM +0100, Michal Privoznik wrote:
Well, I've just updated wireshark on my system and encountered
couple of
compile errors while building libvirt. Here are the fixes. Fortunately, none of
them requires us to increase the version number of wireshark that's required.
However, I'd like to discuss how are we going to handle this. I mean, at
wireshark they don't seem so committed to API stability as we are. Therefore
this patch set. In the long term I don't see us adapting to every single API
change, or do I? Although I am not sure what are our options here.
I thought about this long and hard. And we discussed it together
off-list as well. And the only conclusion I could come up with is that
if wireshark people don't want our dissector in, with us updating it
every once in a while, then I would just say that we support few last
versions (intentionally arbitrary number, so that we can drop support
for older stuff whenever we please). And for those few versions we
would just #ifdef the heck out of it. It's just that I don't feel like
keeping all the old versions around in separate files or anything like
it. I'm more than open to other ideas, though.
Anyway, ACK for this series at least.
3238
days inactive
3245
days old
6 comments
2 participants
participants (2)
-
Martin Kletzander -
Michal Privoznik