Re: [Spice-devel] [libvirt] [PATCH v4] qemu: Use heads parameter for QXL driver
On Tue, Jul 21, 2015 at 11:34:08AM -0400, Laine Stump wrote: On 07/21/2015 09:41 AM, Daniel P. Berrange wrote: On Tue, Jul 21, 2015 at 03:35:50PM +0200, Martin Kletzander wrote: On Tue, Jul 21, 2015 at 01:50:22PM +0100, Daniel P. Berrange wrote: On Tue, Jul 21, 2015 at 11:44:27AM +0200, Martin Kletzander wrote: On Tue, Jul 21, 2015 at 09:36:55AM +0200, Christophe Fergeau wrote: On Mon, Jul 20, 2015 at 11:25:52AM +0200, Martin Kletzander wrote: I spend all morning fixing this to be installed properly in the system. Anyway, I finally managed to make this work and found out the guest I used for it is not ready to have multiple monitors. Anyway, looking at everything else this definitely works, so ACK, I'll push it in a while. I'm still a bit worried that all existing guests will have heads='1' in their XML as libvirt is adding that by default, but I don't really see a way around it :-/ We should make sure libvirt stops adding it from now on though ;) Well, how would you then limit the outputs to 1? Having said that, I have no idea why we started adding heads=1 automatically and playing with different changes, we have another bug in the parsing/formatting code. You'll also won't be able to migrate from older libvirt with heads='1' to newer ones. I haven't realized that. I'm thinking about reverting the change in libvirt or even using heads 1. Although that won't migrate either. So the only other thing that we can do is to introduce new parameter, like maxHeads. The sound you just heard is me slapping my face because again there will multiple parameters meaning similar things... Introducing a new parameter is not really viable IMHO, because as you say it'll leave us with two things meaning the same, and will not be compatible with existing apps that know about the current parameter. I think we need to figure out a way to drop the 'heads=1' from any existing configs in /etc/libvirt/qemu when we start up with the new libvirtd for the first time. I'm already working on an RFC that would differentiate between loading XMLs on daemon start-up and defining them. The purpose of that is so that we can make incompatible XML changes without losing any domains, but that's not the point here. If we were to drop heads='1' anywhere, this is the place. The ABI change would be minimal for the guest. It isn't sufficient to just differentiate loading on startup vs defining them IIUC. We need to differentiate loading on startup from XML previously written by a libvirtd X.Y.Z which is why I think we need to have a version number recorded in the XML ultimately. But a version number isn't sufficient to describe exactly what the previous libvirt was capable of. Many times new features (externally visible only in the XML) are backported to earlier versions of libvirt downstream (e.g. libvirt-0.10.2 that is used in RHEL6.x and CentOS 6.x), so this still doesn't buy us perfection. Downstream maintainers could make it better by paying very close attention to any use of this version number when backporting new stuff, but there would still be problems if someone decided to build and install an upstream libvirt on a system that previously had some hybrid downstream build. (Not saying we shouldn't do it, just that it's no perfect :-) Yep, understood. I'm thinking purely in terms of upstream where we do not backport features to stable branches. Distros which get into the feature backport game have to deal with that pain themselves. Regards, 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 :| ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH v2 12/43] Generate some definition for dissector
On Wed, Jul 08, 2015 at 02:53:45PM +0100, Frediano Ziglio wrote: Generate global definitions. Generate function to registers various dissector components. For the moment the field array is empty bu we save some global to be able to register new ones. Add a base test for generated dissector. Signed-off-by: Frediano Ziglio fzig...@redhat.com --- Makefile.am | 2 +- codegen/Makefile.am | 40 + codegen/dissector_test.c| 81 + configure.ac| 2 ++ python_modules/dissector.py | 87 +++-- spice_codegen.py| 2 +- 6 files changed, 209 insertions(+), 5 deletions(-) create mode 100644 codegen/Makefile.am create mode 100644 codegen/dissector_test.c diff --git a/Makefile.am b/Makefile.am index 380bf24..382a0ea 100644 --- a/Makefile.am +++ b/Makefile.am @@ -1,7 +1,7 @@ NULL = ACLOCAL_AMFLAGS = -I m4 -SUBDIRS = python_modules common +SUBDIRS = python_modules common codegen DIST_SUBDIRS = spice-protocol $(SUBDIRS) EXTRA_DIST = \ diff --git a/codegen/Makefile.am b/codegen/Makefile.am new file mode 100644 index 000..129543c --- /dev/null +++ b/codegen/Makefile.am @@ -0,0 +1,40 @@ +NULL = + +AM_CPPFLAGS =\ + -I$(top_srcdir) \ + $(WIRESHARK_CFLAGS) \ + $(SPICE_COMMON_CFLAGS) \ + $(NULL) + +dissector_test_LDADD = \ + $(SPICE_COMMON_LIBS)\ + $(NULL) + +MARSHALLERS_DEPS = \ + $(top_srcdir)/python_modules/__init__.py\ + $(top_srcdir)/python_modules/codegen.py \ + $(top_srcdir)/python_modules/demarshal.py \ + $(top_srcdir)/python_modules/marshal.py \ + $(top_srcdir)/python_modules/ptypes.py \ + $(top_srcdir)/python_modules/spice_parser.py\ + $(top_srcdir)/python_modules/dissector.py \ + $(top_srcdir)/spice_codegen.py \ + $(NULL) + +test.o: test.h This test.o dep (and the similar one in another commit) is odd. Missing BUILT_SOURCES? + +test.c: $(top_srcdir)/spice.proto $(MARSHALLERS_DEPS) + $(AM_V_GEN)$(PYTHON) $(top_srcdir)/spice_codegen.py --generate-dissector --client $ $@ /dev/null + +test.h: $(top_srcdir)/spice.proto $(MARSHALLERS_DEPS) + $(AM_V_GEN)$(PYTHON) $(top_srcdir)/spice_codegen.py --generate-dissector --client $ --header $@ /dev/null + +TESTS = dissector_test +check_PROGRAMS = dissector_test + +dissector_test_SOURCES = dissector_test.c test.c test.h + +EXTRA_DIST = \ + $(NULL) + +-include $(top_srcdir)/git.mk diff --git a/configure.ac b/configure.ac index 4287f92..a156cae 100644 --- a/configure.ac +++ b/configure.ac @@ -38,6 +38,7 @@ SPICE_CHECK_PIXMAN(SPICE_COMMON) SPICE_CHECK_SMARTCARD(SPICE_COMMON) SPICE_CHECK_CELT051(SPICE_COMMON) SPICE_CHECK_GLIB2(SPICE_COMMON) +PKG_CHECK_MODULES(WIRESHARK, wireshark) This should be optional. Christophe pgpEGh1PO2uvd.pgp Description: PGP signature ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH v2 12/43] Generate some definition for dissector
- Original Message - From: Christophe Fergeau cferg...@redhat.com To: Frediano Ziglio fzig...@redhat.com Cc: spice-devel@lists.freedesktop.org Sent: Tuesday, July 21, 2015 4:37:15 PM Subject: Re: [Spice-devel] [PATCH v2 12/43] Generate some definition for dissector On Wed, Jul 08, 2015 at 02:53:45PM +0100, Frediano Ziglio wrote: Generate global definitions. Generate function to registers various dissector components. For the moment the field array is empty bu we save some global to be able to register new ones. Add a base test for generated dissector. Signed-off-by: Frediano Ziglio fzig...@redhat.com --- Makefile.am | 2 +- codegen/Makefile.am | 40 + codegen/dissector_test.c| 81 + configure.ac| 2 ++ python_modules/dissector.py | 87 +++-- spice_codegen.py| 2 +- 6 files changed, 209 insertions(+), 5 deletions(-) create mode 100644 codegen/Makefile.am create mode 100644 codegen/dissector_test.c diff --git a/Makefile.am b/Makefile.am index 380bf24..382a0ea 100644 --- a/Makefile.am +++ b/Makefile.am @@ -1,7 +1,7 @@ NULL = ACLOCAL_AMFLAGS = -I m4 -SUBDIRS = python_modules common +SUBDIRS = python_modules common codegen DIST_SUBDIRS = spice-protocol $(SUBDIRS) EXTRA_DIST = \ diff --git a/codegen/Makefile.am b/codegen/Makefile.am new file mode 100644 index 000..129543c --- /dev/null +++ b/codegen/Makefile.am @@ -0,0 +1,40 @@ +NULL = + +AM_CPPFLAGS = \ + -I$(top_srcdir) \ + $(WIRESHARK_CFLAGS) \ + $(SPICE_COMMON_CFLAGS) \ + $(NULL) + +dissector_test_LDADD = \ + $(SPICE_COMMON_LIBS)\ + $(NULL) + +MARSHALLERS_DEPS = \ + $(top_srcdir)/python_modules/__init__.py\ + $(top_srcdir)/python_modules/codegen.py \ + $(top_srcdir)/python_modules/demarshal.py \ + $(top_srcdir)/python_modules/marshal.py \ + $(top_srcdir)/python_modules/ptypes.py \ + $(top_srcdir)/python_modules/spice_parser.py\ + $(top_srcdir)/python_modules/dissector.py \ + $(top_srcdir)/spice_codegen.py \ + $(NULL) + +test.o: test.h This test.o dep (and the similar one in another commit) is odd. Missing BUILT_SOURCES? No, it's just manual, see http://www.delorie.com/gnu/docs/automake/automake_69.html and http://www.delorie.com/gnu/docs/automake/automake_70.html, basically not all cases are covered by automatic generated dependency. + +test.c: $(top_srcdir)/spice.proto $(MARSHALLERS_DEPS) + $(AM_V_GEN)$(PYTHON) $(top_srcdir)/spice_codegen.py --generate-dissector --client $ $@ /dev/null + +test.h: $(top_srcdir)/spice.proto $(MARSHALLERS_DEPS) + $(AM_V_GEN)$(PYTHON) $(top_srcdir)/spice_codegen.py --generate-dissector --client $ --header $@ /dev/null + +TESTS = dissector_test +check_PROGRAMS = dissector_test + +dissector_test_SOURCES = dissector_test.c test.c test.h + +EXTRA_DIST = \ + $(NULL) + +-include $(top_srcdir)/git.mk diff --git a/configure.ac b/configure.ac index 4287f92..a156cae 100644 --- a/configure.ac +++ b/configure.ac @@ -38,6 +38,7 @@ SPICE_CHECK_PIXMAN(SPICE_COMMON) SPICE_CHECK_SMARTCARD(SPICE_COMMON) SPICE_CHECK_CELT051(SPICE_COMMON) SPICE_CHECK_GLIB2(SPICE_COMMON) +PKG_CHECK_MODULES(WIRESHARK, wireshark) This should be optional. Oh... is it not in this way? I though so. Christophe Frediano ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH v3 49/51] Allow ws_as attribute on switch members
Signed-off-by: Frediano Ziglio fzig...@redhat.com
---
python_modules/dissector.py | 4
1 file changed, 4 insertions(+)
diff --git a/python_modules/dissector.py b/python_modules/dissector.py
index 3e1fd6d..2a25a51 100644
--- a/python_modules/dissector.py
+++ b/python_modules/dissector.py
@@ -430,6 +430,10 @@ def write_switch(writer, container, switch, dest, scope):
m = c.member
with writer.if_block(check, not first, False) as block:
t = m.member_type
+if m.has_attr('ws_as'):
+type_name = m.attributes['ws_as'][0]
+assert(ptypes.type_exists(type_name))
+t = ptypes.lookup_type(type_name)
if switch.has_attr(anon):
dest2 = dest
else:
--
2.1.0
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH v3 42/51] Test decorated array
Try to test possible combinations of different attributes with arrays to make sure output is what is expected. Signed-off-by: Frediano Ziglio fzig...@redhat.com --- codegen/out_array_primitive.txt | 109 + codegen/out_array_raw.txt | 12 +++- codegen/out_array_struct.txt| 129 ++-- codegen/test.proto | 15 - 4 files changed, 245 insertions(+), 20 deletions(-) diff --git a/codegen/out_array_primitive.txt b/codegen/out_array_primitive.txt index 3a77f37..c621bda 100644 --- a/codegen/out_array_primitive.txt +++ b/codegen/out_array_primitive.txt @@ -1,25 +1,110 @@ --- tree --- item -Text: 0 (0) -Name: array -Abbrev: spice2.auto.ArrayPrimitive_array_array +Text: test text +Name: test desc +Abbrev: spice2.name5 +--- tree +--- item +Text: 0 (0) +Name: array1 +Abbrev: spice2.auto.ArrayPrimitive_array_array1 +Type: FT_UINT16 +Base: BASE_DEC +--- item +Text: 1 (0x1) +Name: array1 +Abbrev: spice2.auto.ArrayPrimitive_array_array1 +Type: FT_UINT16 +Base: BASE_DEC +--- item +Text: 2 (0x2) +Name: array1 +Abbrev: spice2.auto.ArrayPrimitive_array_array1 +Type: FT_UINT16 +Base: BASE_DEC +--- item +Text: 3 (0x3) +Name: array1 +Abbrev: spice2.auto.ArrayPrimitive_array_array1 +Type: FT_UINT16 +Base: BASE_DEC +--- item +Text: +Name: test desc +Abbrev: spice2.name6 +--- tree +--- item +Text: 4 (0x4) +Name: array2 +Abbrev: spice2.auto.ArrayPrimitive_array_array2 +Type: FT_UINT16 +Base: BASE_DEC +--- item +Text: 5 (0x5) +Name: array2 +Abbrev: spice2.auto.ArrayPrimitive_array_array2 +Type: FT_UINT16 +Base: BASE_DEC +--- item +Text: 6 (0x6) +Name: array2 +Abbrev: spice2.auto.ArrayPrimitive_array_array2 +Type: FT_UINT16 +Base: BASE_DEC +--- item +Text: 7 (0x7) +Name: array2 +Abbrev: spice2.auto.ArrayPrimitive_array_array2 +Type: FT_UINT16 +Base: BASE_DEC +--- item +Text: test text +--- tree +--- item +Text: 8 (0x8) +Name: array3 +Abbrev: spice2.auto.ArrayPrimitive_array_array3 +Type: FT_UINT16 +Base: BASE_DEC +--- item +Text: 9 (0x9) +Name: array3 +Abbrev: spice2.auto.ArrayPrimitive_array_array3 +Type: FT_UINT16 +Base: BASE_DEC +--- item +Text: 10 (0xa) +Name: array3 +Abbrev: spice2.auto.ArrayPrimitive_array_array3 +Type: FT_UINT16 +Base: BASE_DEC +--- item +Text: 11 (0xb) +Name: array3 +Abbrev: spice2.auto.ArrayPrimitive_array_array3 +Type: FT_UINT16 +Base: BASE_DEC +--- item +Text: 12 (0xc) +Name: array4 +Abbrev: spice2.auto.ArrayPrimitive_array_array4 Type: FT_UINT16 Base: BASE_DEC --- item -Text: 1 (0x1) -Name: array -Abbrev: spice2.auto.ArrayPrimitive_array_array +Text: 13 (0xd) +Name: array4 +Abbrev: spice2.auto.ArrayPrimitive_array_array4 Type: FT_UINT16 Base: BASE_DEC --- item -Text: 2 (0x2) -Name: array -Abbrev: spice2.auto.ArrayPrimitive_array_array +Text: 14 (0xe) +Name: array4 +Abbrev: spice2.auto.ArrayPrimitive_array_array4 Type: FT_UINT16 Base: BASE_DEC --- item -Text: 3 (0x3) -Name: array -Abbrev: spice2.auto.ArrayPrimitive_array_array +Text: 15 (0xf) +Name: array4 +Abbrev: spice2.auto.ArrayPrimitive_array_array4 Type: FT_UINT16 Base: BASE_DEC diff --git a/codegen/out_array_raw.txt b/codegen/out_array_raw.txt index 31b510c..68a16cd 100644 --- a/codegen/out_array_raw.txt +++ b/codegen/out_array_raw.txt @@ -1,3 +1,13 @@ --- tree --- item -Text: array +Text: test text +Name: test desc +Abbrev: spice2.name1 +--- item +Text: +Name: test desc +Abbrev: spice2.name2 +--- item +Text: test text +--- item +Text: array4 diff --git a/codegen/out_array_struct.txt b/codegen/out_array_struct.txt index 53d28ef..7da1cc7 100644 --- a/codegen/out_array_struct.txt +++ b/codegen/out_array_struct.txt @@ -1,9 +1,130 @@ --- tree --- item +Text: test text +Name: test desc +Abbrev: spice2.name5 +--- tree +--- item +Text: Dummy +--- tree +---
[Spice-devel] [PATCH v3 50/51] Allow to use bytes_count and bytes array length in dissector
Was not implemented. These attributes allows to specify length of
other fields in bytes. Previously was possibly only to specify
arrays length in number of elements.
Signed-off-by: Frediano Ziglio fzig...@redhat.com
---
python_modules/dissector.py | 64 +
1 file changed, 41 insertions(+), 23 deletions(-)
diff --git a/python_modules/dissector.py b/python_modules/dissector.py
index 2a25a51..931e83e 100644
--- a/python_modules/dissector.py
+++ b/python_modules/dissector.py
@@ -378,15 +378,15 @@ def read_array_len(writer, prefix, array, dest, scope,
is_ptr):
nelements = %s__array__nelements % prefix
else:
nelements = %s__nelements % prefix
+nbytes = %s__nbytes % prefix
if dest.is_toplevel() and scope.variable_defined(nelements):
-return nelements # Already there for toplevel, need not recalculate
+return (nelements, None) # Already there for toplevel, need not
recalculate
# just reuse variable, there is no problem for cast as we always store in
guint32
if array.is_identifier_length():
nelements = dest.read_ref(array.size)
dest.write_ref(writer, dest.ref_size(array.size), prefix +
'.nelements', nelements)
-return nelements
+return (nelements, None)
element_type = array.element_type
-scope.variable_def(guint32, nelements)
if array.is_constant_length():
writer.assign(nelements, array.size)
elif array.is_remaining_length():
@@ -408,12 +408,17 @@ def read_array_len(writer, prefix, array, dest, scope,
is_ptr):
else:
writer.assign(nelements, ((%sU * (guint32) %s + 7U) / 8U ) * %s
% (bpp, width_v, rows_v))
elif array.is_bytes_length():
-writer.assign(nelements, dest.read_ref(array.size[2]))
+scope.variable_def(guint32, nbytes)
+writer.assign(nbytes, dest.read_ref(array.size[1]))
+# TODO compute a better size
+dest.write_ref(writer, 32, prefix+'.nbytes', nbytes)
+return (None, nbytes)
else:
raise NotImplementedError(TODO array size type not handled yet)
+scope.variable_def(guint32, nelements)
# TODO compute a better size
dest.write_ref(writer, 32, prefix+'.nelements', nelements)
-return nelements
+return (nelements, None)
def write_switch(writer, container, switch, dest, scope):
@@ -449,8 +454,8 @@ def write_switch(writer, container, switch, dest, scope):
elif t.is_primitive():
write_member_primitive(writer, container, m, t,
WSAttributes(t, m.attributes), dest2, scope)
elif t.is_array():
-nelements = read_array_len(writer, m.name, t, dest, block,
False)
-write_array(writer, container, m, nelements, t, dest2, block)
+(nelements, nbytes) = read_array_len(writer, m.name, t, dest,
block, False)
+write_array(writer, container, m, nelements, nbytes, t, dest2,
block)
else:
writer.todo(Can't handle type %s % m.member_type)
@@ -462,10 +467,10 @@ def write_switch(writer, container, switch, dest, scope):
writer.assign(output, save_output + %s %
switch.get_fixed_nw_size())
-def write_array_core(writer, container, member, nelements, array, dest, scope):
+def write_array_core(writer, container, member, nelements, nbytes, array,
dest, scope):
element_type = array.element_type
-with writer.index() as index, writer.for_loop(index, nelements) as
array_scope:
+def write_item(index):
dest.index = index
if element_type.is_primitive():
write_member_primitive(writer, container, member, element_type,
WSAttributes(element_type, array.item_attrs), dest, scope)
@@ -474,7 +479,24 @@ def write_array_core(writer, container, member, nelements,
array, dest, scope):
write_struct(writer, member, element_type, index, dest, scope)
dest.index = None
-def write_array(writer, container, member, nelements, array, dest, scope):
+if nbytes:
+scope.variable_def('guint32', 'start_offset')
+scope.variable_def('guint32', 'save_end')
+writer.assign('start_offset', 'offset')
+writer.assign('save_end', 'glb-message_end')
+writer.assign('glb-message_end', '%s + start_offset' % nbytes)
+with writer.index() as index:
+writer.assign(index, 0)
+with writer.while_loop('offset %s + start_offset' % nbytes) as
array_scope:
+write_item(index)
+writer.increment(index, 1)
+dest.write_ref(writer, 32, member.name + '.nelements', index)
+writer.assign('glb-message_end', 'save_end')
+else:
+with writer.index() as index, writer.for_loop(index, nelements) as
array_scope:
+write_item(index)
+
+def write_array(writer, container, member, nelements, nbytes, array, dest,
scope):
assert(container and member)
ws =
[Spice-devel] [PATCH v3 41/51] Handle text formatting of different elements
Support ws_txt and ws_txt_n attributes.
These attributes are there to allow to set specific text to different
elements.
The can be used to output in a single line the features of a structure.
Or they can be used to format small array items.
They can applied to almost everything from primitives, arrays,
structure or even pointers.
Signed-off-by: Frediano Ziglio fzig...@redhat.com
---
codegen/dissector_test.c | 28 ++
codegen/out_array_raw.txt| 4 +-
codegen/out_array_struct.txt | 52 ++-
codegen/out_struct1.txt | 13 +--
python_modules/dissector.py | 205 +++
5 files changed, 256 insertions(+), 46 deletions(-)
diff --git a/codegen/dissector_test.c b/codegen/dissector_test.c
index 96b3107..84abacf 100644
--- a/codegen/dissector_test.c
+++ b/codegen/dissector_test.c
@@ -276,6 +276,34 @@ WS_DLL_PUBLIC proto_tree*
proto_item_add_subtree(proto_item *ti, const gint idx)
return res;
}
+WS_DLL_PUBLIC void proto_item_set_end(proto_item *ti, tvbuff_t *tvb, gint end)
+{
+ assert(tvb);
+ assert(end = 0);
+ if (!ti)
+ return;
+ check_item(ti);
+ tvb_bytes(tvb, 0, end);
+ assert(ti-finfo-start = end);
+ ti-finfo-length = end - ti-finfo-start;
+ check_item(ti);
+}
+
+WS_DLL_PUBLIC void proto_item_set_text(proto_item *ti, const char *format, ...)
+{
+ va_list ap;
+ assert(format);
+ if (!ti)
+ return;
+
+ check_item(ti);
+ va_start(ap, format);
+ vsnprintf(ti-finfo-rep-representation,
sizeof(ti-finfo-rep-representation),
+ format, ap);
+ va_end(ap);
+ check_item(ti);
+}
+
struct all_ti
{
proto_item ti;
diff --git a/codegen/out_array_raw.txt b/codegen/out_array_raw.txt
index f276871..31b510c 100644
--- a/codegen/out_array_raw.txt
+++ b/codegen/out_array_raw.txt
@@ -1,5 +1,3 @@
--- tree
--- item
-Text:
-Name: array
-Abbrev: spice2.auto.ArrayRaw_array_array
+Text: array
diff --git a/codegen/out_array_struct.txt b/codegen/out_array_struct.txt
index eb03cd8..53d28ef 100644
--- a/codegen/out_array_struct.txt
+++ b/codegen/out_array_struct.txt
@@ -1,25 +1,37 @@
--- tree
--- item
-Text: 0 (0)
-Name: dummy
-Abbrev: spice2.auto.Dummy_dummy
-Type: FT_UINT16
-Base: BASE_DEC
+Text: Dummy
+--- tree
+--- item
+Text: 0 (0)
+Name: dummy
+Abbrev: spice2.auto.Dummy_dummy
+Type: FT_UINT16
+Base: BASE_DEC
--- item
-Text: 1 (0x1)
-Name: dummy
-Abbrev: spice2.auto.Dummy_dummy
-Type: FT_UINT16
-Base: BASE_DEC
+Text: Dummy
+--- tree
+--- item
+Text: 1 (0x1)
+Name: dummy
+Abbrev: spice2.auto.Dummy_dummy
+Type: FT_UINT16
+Base: BASE_DEC
--- item
-Text: 2 (0x2)
-Name: dummy
-Abbrev: spice2.auto.Dummy_dummy
-Type: FT_UINT16
-Base: BASE_DEC
+Text: Dummy
+--- tree
+--- item
+Text: 2 (0x2)
+Name: dummy
+Abbrev: spice2.auto.Dummy_dummy
+Type: FT_UINT16
+Base: BASE_DEC
--- item
-Text: 3 (0x3)
-Name: dummy
-Abbrev: spice2.auto.Dummy_dummy
-Type: FT_UINT16
-Base: BASE_DEC
+Text: Dummy
+--- tree
+--- item
+Text: 3 (0x3)
+Name: dummy
+Abbrev: spice2.auto.Dummy_dummy
+Type: FT_UINT16
+Base: BASE_DEC
diff --git a/codegen/out_struct1.txt b/codegen/out_struct1.txt
index a1d429c..993e77c 100644
--- a/codegen/out_struct1.txt
+++ b/codegen/out_struct1.txt
@@ -1,7 +1,10 @@
--- tree
--- item
-Text: 33154 (0x8182)
-Name: dummy
-Abbrev: spice2.auto.Dummy_dummy
-Type: FT_UINT16
-Base: BASE_DEC
+Text: Dummy
+--- tree
+--- item
+Text: 33154 (0x8182)
+Name: dummy
+Abbrev: spice2.auto.Dummy_dummy
+Type: FT_UINT16
+Base: BASE_DEC
diff --git a/python_modules/dissector.py b/python_modules/dissector.py
index 84fbddb..3f63341 100644
--- a/python_modules/dissector.py
+++ b/python_modules/dissector.py
@@ -3,6 +3,7 @@ from . import ptypes
from . import codegen
import re
+from contextlib import contextmanager
import types
@@ -195,6 +196,7 @@ class Destination:
self.reuse_scope = scope
self.parent_dest = None
self.level = Level()
+self.index = None
def child_sub(self, member, scope):
return SubDestination(self, member, scope)
@@ -292,7 +294,6 @@ def get_primitive_ft_type(t):
# write a field
def write_wireshark_field(writer, container, member, t, ws, tree, dest, size,
encoding='ENC_LITTLE_ENDIAN', prefix=''):
-assert(member and container)
size_name = ''
@@ -343,7 +344,17 @@ def write_wireshark_field(writer, container,
[Spice-devel] [PATCH v3 30/51] Generate code to output parse structure
Write a function and call it to be able to reuse it.
Signed-off-by: Frediano Ziglio fzig...@redhat.com
---
codegen/Makefile.am | 1 +
codegen/check_dissector | 2 ++
codegen/out_struct1.txt | 7 +++
codegen/test.proto | 9 +
python_modules/dissector.py | 20
5 files changed, 39 insertions(+)
create mode 100644 codegen/out_struct1.txt
diff --git a/codegen/Makefile.am b/codegen/Makefile.am
index 1281690..d8da936 100644
--- a/codegen/Makefile.am
+++ b/codegen/Makefile.am
@@ -59,6 +59,7 @@ EXTRA_DIST = \
out_empty.txt \
data_base1 \
out_base1.txt \
+ out_struct1.txt \
$(NULL)
CLEANFILES = test.c test.h enums.h dissector.c dissector.h *.trs
check_dissector.txt
diff --git a/codegen/check_dissector b/codegen/check_dissector
index bff3853..767ee9d 100755
--- a/codegen/check_dissector
+++ b/codegen/check_dissector
@@ -54,4 +54,6 @@ fi
# check just base types
check data_base1 1 1 out_base1.txt
+check data_base1 1 1 out_struct1.txt --client
+
exit 0
diff --git a/codegen/out_struct1.txt b/codegen/out_struct1.txt
new file mode 100644
index 000..a1d429c
--- /dev/null
+++ b/codegen/out_struct1.txt
@@ -0,0 +1,7 @@
+--- tree
+--- item
+Text: 33154 (0x8182)
+Name: dummy
+Abbrev: spice2.auto.Dummy_dummy
+Type: FT_UINT16
+Base: BASE_DEC
diff --git a/codegen/test.proto b/codegen/test.proto
index 28d7769..06fa303 100644
--- a/codegen/test.proto
+++ b/codegen/test.proto
@@ -30,6 +30,10 @@ flags32 F32 {
N, O, P, Q
};
+struct Dummy {
+uint16 dummy;
+};
+
channel BaseChannel {
server:
message {
@@ -49,6 +53,11 @@ channel BaseChannel {
F32 f32;
} Base1 = 1;
Empty empty = 100;
+
+ client:
+message {
+Dummy struct;
+} Struct1 = 1;
};
protocol Spice {
diff --git a/python_modules/dissector.py b/python_modules/dissector.py
index aa71615..edc6dc3 100644
--- a/python_modules/dissector.py
+++ b/python_modules/dissector.py
@@ -307,9 +307,29 @@ def write_array(writer, container, member, nelements,
array, dest, scope):
def write_pointer(writer, container, member, t, dest, scope):
assert(t.is_pointer())
+
+def write_struct_func(writer, t, func_name, index):
+func_name = 'dissect_spice_struct_' + t.name
+
+if writer.is_generated(struct, t.name):
+return
+writer.set_is_generated(struct, t.name)
+
+writer = writer.function_helper()
+scope = writer.function(func_name, guint32, GlobalInfo *glb _U_,
proto_tree *tree _U_, guint32 offset, gint32 index _U_, True)
+dest = RootDestination(scope)
+write_container_parser(writer, t, dest)
+writer.statement('return offset')
+writer.end_block()
+
+
def write_struct(writer, member, t, index, dest, scope):
assert(t.is_struct())
+func_name = 'dissect_spice_struct_' + t.name
+write_struct_func(writer, t, func_name, index)
+writer.assign('offset', '%s(glb, %s, offset, %s)' % (func_name,
dest.level.tree, index))
+
def write_member_primitive(writer, container, member, t, dest, scope):
assert(t.is_primitive())
--
2.1.0
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH v3 40/51] Handle flags
Instead of only show the hexadecimal value show all bits.
Signed-off-by: Frediano Ziglio fzig...@redhat.com
---
codegen/Makefile.am | 1 +
codegen/check_dissector | 3 ++
codegen/dissector_test.c| 22 +
codegen/out_base1.txt | 81 +
codegen/out_flags1.txt | 107
codegen/test.proto | 10 +
python_modules/dissector.py | 92 ++---
7 files changed, 300 insertions(+), 16 deletions(-)
create mode 100644 codegen/out_flags1.txt
diff --git a/codegen/Makefile.am b/codegen/Makefile.am
index 9e02d87..15e277e 100644
--- a/codegen/Makefile.am
+++ b/codegen/Makefile.am
@@ -65,6 +65,7 @@ EXTRA_DIST = \
out_array_raw.txt \
out_array_struct.txt\
out_channel.txt \
+ out_flags1.txt \
$(NULL)
CLEANFILES = test.c test.h enums.h dissector.c dissector.h *.trs
check_dissector.txt
diff --git a/codegen/check_dissector b/codegen/check_dissector
index e2d06b5..94ce0bc 100755
--- a/codegen/check_dissector
+++ b/codegen/check_dissector
@@ -62,4 +62,7 @@ check data_u16s 1 102 out_array_struct.txt --client
check data_base1 1 2 out_channel.txt
+# flags and descriptions
+check data_base1 1 3 out_flags1.txt
+
exit 0
diff --git a/codegen/dissector_test.c b/codegen/dissector_test.c
index dcc0134..96b3107 100644
--- a/codegen/dissector_test.c
+++ b/codegen/dissector_test.c
@@ -254,6 +254,28 @@ expert_add_info_format(packet_info *pinfo, proto_item *pi,
expert_field *eiindex
return;
}
+WS_DLL_PUBLIC proto_tree* proto_item_add_subtree(proto_item *ti, const gint
idx)
+{
+ proto_tree *res;
+
+ assert(idx = first_tree_registered);
+ assert(idx = last_tree_registered);
+ if (!ti)
+ return NULL;
+
+ assert(ti-tree_data == NULL);
+ assert(ti-first_child == NULL);
+ assert(ti-last_child == NULL);
+ res = calloc(1, sizeof(*res));
+ assert(res);
+ res-tree_data = (void *) res;
+ ti-first_child = res;
+ ti-last_child = res;
+ check_tree(res);
+ check_item(ti);
+ return res;
+}
+
struct all_ti
{
proto_item ti;
diff --git a/codegen/out_base1.txt b/codegen/out_base1.txt
index 7921afd..66b42bc 100644
--- a/codegen/out_base1.txt
+++ b/codegen/out_base1.txt
@@ -66,20 +66,83 @@
Type: FT_UINT32
Base: BASE_DEC
--- item
-Text: H (1)
-Name: f8
-Abbrev: spice2.auto.msg_base_Base1_f8
+Text: 1 (0x1)
+Name: F8
+Abbrev: spice2.f8_flags
Type: FT_UINT8
Base: BASE_HEX
+--- tree
+--- item
+Text: Not set
+Name: K
+Abbrev: spice2.F8_k
+Type: FT_BOOLEAN
+Base: 4
+--- item
+Text: Not set
+Name: J
+Abbrev: spice2.F8_j
+Type: FT_BOOLEAN
+Base: 4
+--- item
+Text: Not set
+Name: I
+Abbrev: spice2.F8_i
+Type: FT_BOOLEAN
+Base: 4
+--- item
+Text: Set
+Name: H
+Abbrev: spice2.F8_h
+Type: FT_BOOLEAN
+Base: 4
--- item
-Text: L (1)
-Name: f16
-Abbrev: spice2.auto.msg_base_Base1_f16
+Text: 1 (0x1)
+Name: F16
+Abbrev: spice2.f16_flags
Type: FT_UINT16
Base: BASE_HEX
+--- tree
+--- item
+Text: Not set
+Name: M
+Abbrev: spice2.F16_m
+Type: FT_BOOLEAN
+Base: 2
+--- item
+Text: Set
+Name: L
+Abbrev: spice2.F16_l
+Type: FT_BOOLEAN
+Base: 2
--- item
-Text: N (1)
-Name: f32
-Abbrev: spice2.auto.msg_base_Base1_f32
+Text: 1 (0x1)
+Name: F32
+Abbrev: spice2.f32_flags
Type: FT_UINT32
Base: BASE_HEX
+--- tree
+--- item
+Text: Not set
+Name: Q
+Abbrev: spice2.F32_q
+Type: FT_BOOLEAN
+Base: 4
+--- item
+Text: Not set
+Name: P
+Abbrev: spice2.F32_p
+Type: FT_BOOLEAN
+Base: 4
+--- item
+Text: Not set
+Name: O
+Abbrev: spice2.F32_o
+Type: FT_BOOLEAN
+Base: 4
+--- item
+Text: Set
+Name: N
+Abbrev: spice2.F32_n
+Type: FT_BOOLEAN
+Base: 4
diff --git a/codegen/out_flags1.txt b/codegen/out_flags1.txt
new file mode 100644
index 000..0776300
--- /dev/null
+++ b/codegen/out_flags1.txt
@@ -0,0 +1,107 @@
+--- tree
+--- item
+Text: 130 (0x82)
+Name: Test flags
+Abbrev: spice2.tests_flag
+Type: FT_UINT8
+Base:
[Spice-devel] [PATCH v3 37/51] Allow to specify 'CHANNEL' as type
Type will be mapped to an enumerator containing all channel types.
Signed-off-by: Frediano Ziglio fzig...@redhat.com
---
codegen/Makefile.am | 1 +
codegen/check_dissector | 2 ++
codegen/out_channel.txt | 7 +++
codegen/test.proto | 15 +++
python_modules/dissector.py | 14 +-
spice.proto | 2 +-
6 files changed, 35 insertions(+), 6 deletions(-)
create mode 100644 codegen/out_channel.txt
diff --git a/codegen/Makefile.am b/codegen/Makefile.am
index bf5bbc7..9e02d87 100644
--- a/codegen/Makefile.am
+++ b/codegen/Makefile.am
@@ -64,6 +64,7 @@ EXTRA_DIST = \
out_array_primitive.txt \
out_array_raw.txt \
out_array_struct.txt\
+ out_channel.txt \
$(NULL)
CLEANFILES = test.c test.h enums.h dissector.c dissector.h *.trs
check_dissector.txt
diff --git a/codegen/check_dissector b/codegen/check_dissector
index 20d83ff..e2d06b5 100755
--- a/codegen/check_dissector
+++ b/codegen/check_dissector
@@ -60,4 +60,6 @@ check data_u16s 1 100 out_array_primitive.txt --client
check data_u16s 1 101 out_array_raw.txt --client
check data_u16s 1 102 out_array_struct.txt --client
+check data_base1 1 2 out_channel.txt
+
exit 0
diff --git a/codegen/out_channel.txt b/codegen/out_channel.txt
new file mode 100644
index 000..5dac17c
--- /dev/null
+++ b/codegen/out_channel.txt
@@ -0,0 +1,7 @@
+--- tree
+--- item
+Text: TEST3 (130)
+Name: channel
+Abbrev: spice2.auto.msg_base_Channel_channel
+Type: FT_UINT8
+Base: BASE_DEC
diff --git a/codegen/test.proto b/codegen/test.proto
index b28520c..0f14125 100644
--- a/codegen/test.proto
+++ b/codegen/test.proto
@@ -64,6 +64,9 @@ channel BaseChannel {
F16 f16;
F32 f32;
} Base1 = 1;
+message {
+uint8 channel @ws_type(CHANNEL);
+} Channel;
Empty empty = 100;
client:
@@ -76,6 +79,18 @@ channel BaseChannel {
ArrayStruct array_struct;
};
+channel Test1Channel: BaseChannel {
+};
+
+channel Test2Channel: BaseChannel {
+};
+
+channel Test3Channel: Test2Channel {
+};
+
protocol Spice {
BaseChannel base = 1;
+Test1Channel test1;
+Test2Channel test2;
+Test3Channel test3 = 130;
};
diff --git a/python_modules/dissector.py b/python_modules/dissector.py
index 2df4723..5639baa 100644
--- a/python_modules/dissector.py
+++ b/python_modules/dissector.py
@@ -276,11 +276,15 @@ def write_wireshark_field(writer, container, member, t,
ws, tree, size, encoding
# override type
if ws.type:
-f_type = 'FT_%s' % ws.type
-base = 'BASE_NONE'
-vals = 'NULL'
-if f_type == 'FT_BOOLEAN':
-vals = 'TFS(tfs_set_notset)'
+if ws.type == 'CHANNEL':
+base = 'BASE_DEC'
+vals = 'VALS(channel_types_vs)'
+else:
+f_type = 'FT_%s' % ws.type
+base = 'BASE_NONE'
+vals = 'NULL'
+if f_type == 'FT_BOOLEAN':
+vals = 'TFS(tfs_set_notset)'
# override base
if ws.base:
diff --git a/spice.proto b/spice.proto
index 52d6971..fe0eb34 100644
--- a/spice.proto
+++ b/spice.proto
@@ -199,7 +199,7 @@ channel BaseChannel {
};
struct ChannelId {
-uint8 type @ws(Channel type, channel_type);
+uint8 type @ws(Channel type, channel_type) @ws_type(CHANNEL);
uint8 id @ws(Channel ID, channel_id);
} @ws_txt_n(channels[%u], INDEX);
--
2.1.0
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH v3 18/51] Allows to specify attributes for array items and pointers
Specifying attributes for items allows to specify different attribute for the same member where some are specific to the item while the other to the array. The element attributes are attached to the array as they cannot be attached to the type as the object is unique for each type. Same for pointers but in this case these attributes are attached directly to the pointer. Signed-off-by: Frediano Ziglio fzig...@redhat.com --- python_modules/ptypes.py | 9 +++-- python_modules/spice_parser.py | 18 +++--- 2 files changed, 18 insertions(+), 9 deletions(-) diff --git a/python_modules/ptypes.py b/python_modules/ptypes.py index 3a1acbd..a899b6c 100644 --- a/python_modules/ptypes.py +++ b/python_modules/ptypes.py @@ -480,12 +480,16 @@ class FlagsType(EnumBaseType): writer.newline() class ArrayType(Type): -def __init__(self, element_type, size): +def __init__(self, element_type, size, item_attribute_list): Type.__init__(self) self.name = None self.element_type = element_type self.size = size +self.item_attrs = fix_attributes(item_attribute_list) +wrong = [k for k in self.item_attrs.keys() if k[:2] != 'ws'] +if len(wrong) != 0: +assert False, 'Attributes %s not expected in item list' % wrong def __str__(self): if self.size == None: @@ -560,11 +564,12 @@ class ArrayType(Type): return self.element_type.c_type() class PointerType(Type): -def __init__(self, target_type): +def __init__(self, target_type, attribute_list): Type.__init__(self) self.name = None self.target_type = target_type self.pointer_size = default_pointer_size +self.attributes = fix_attributes(attribute_list) def __str__(self): return %s* % (str(self.target_type)) diff --git a/python_modules/spice_parser.py b/python_modules/spice_parser.py index 06000a4..5326e59 100644 --- a/python_modules/spice_parser.py +++ b/python_modules/spice_parser.py @@ -16,16 +16,20 @@ cvtInt = lambda toks: int(toks[0]) def parseVariableDef(toks): t = toks[0][0] -pointer = toks[0][1] -name = toks[0][2] -array_size = toks[0][3] -attributes = toks[0][4] +item_attrs = toks[0][1] +pointer = toks[0][2] +pointer_attrs = toks[0][3] +name = toks[0][4] +array_size = toks[0][5] +attributes = toks[0][6] if array_size != None: -t = ptypes.ArrayType(t, array_size) +t = ptypes.ArrayType(t, array_size, item_attrs) +else: +assert len(item_attrs) == 0, Cannot specify item attributes without an array if pointer != None: -t = ptypes.PointerType(t) +t = ptypes.PointerType(t, pointer_attrs) return ptypes.Member(name, t, attributes) @@ -105,7 +109,7 @@ def SPICE_BNF(): arraySizeSpecBytes = Group(bytes_ + lparen + identifier + comma + identifier + rparen) arraySizeSpecCString = Group(cstring_ + lparen + rparen) arraySizeSpec = lbrack + Optional(identifier ^ integer ^ arraySizeSpecImage ^ arraySizeSpecBytes ^arraySizeSpecCString, default=) + rbrack -variableDef = Group(typeSpec + Optional(*, default=None) + identifier + Optional(arraySizeSpec, default=None) + attributes - semi) \ +variableDef = Group(typeSpec + attributes + Optional(*, default=None) + attributes + identifier + Optional(arraySizeSpec, default=None) + attributes - semi) \ .setParseAction(parseVariableDef) switchCase = Group(Group(OneOrMore(default_.setParseAction(replaceWith(None)) + colon | Group(case_.suppress() + Optional(!, default=) + identifier) + colon)) + variableDef) \ -- 2.1.0 ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH v3 08/51] codegen: Remove old ptr32 attribute
This attribute is not use in code.
Signed-off-by: Frediano Ziglio fzig...@redhat.com
---
python_modules/ptypes.py | 4
1 file changed, 4 deletions(-)
diff --git a/python_modules/ptypes.py b/python_modules/ptypes.py
index d8b6c90..cc6960f 100644
--- a/python_modules/ptypes.py
+++ b/python_modules/ptypes.py
@@ -108,8 +108,6 @@ valid_attributes={
# for a switch this indicates that on network
# will occupy always same size (maximum size required for all members)
'fixedsize',
-# use 32 bit pointer
-'ptr32',
}
attributes_with_arguments={
@@ -616,8 +614,6 @@ class Member(Containee):
self.container = container
self.member_type = self.member_type.resolve()
self.member_type.register()
-if self.has_attr(ptr32) and self.member_type.is_pointer():
-self.member_type.set_ptr_size(4)
for i in propagated_attributes:
if self.has_attr(i):
self.member_type.attributes[i] = self.attributes[i]
--
2.1.0
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH v3 28/51] show primitive as primitive
--- python_modules/dissector.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python_modules/dissector.py b/python_modules/dissector.py index 413aca4..a7add96 100644 --- a/python_modules/dissector.py +++ b/python_modules/dissector.py @@ -223,7 +223,7 @@ def write_wireshark_field(writer, container, member, t, tree, size, encoding='EN f_type = 'FT_NONE' base = 'BASE_NONE' vals = 'NULL' -if encoding == 'ENC_LITTLE_ENDIAN': +if t.is_primitive(): assert(t.is_primitive()) base = 'BASE_DEC' f_type = get_primitive_ft_type(t) -- 2.1.0 ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH v3 29/51] Read array size
Signed-off-by: Frediano Ziglio fzig...@redhat.com
---
python_modules/dissector.py | 48 +++--
1 file changed, 46 insertions(+), 2 deletions(-)
diff --git a/python_modules/dissector.py b/python_modules/dissector.py
index a7add96..aa71615 100644
--- a/python_modules/dissector.py
+++ b/python_modules/dissector.py
@@ -255,10 +255,53 @@ def write_wireshark_field(writer, container, member, t,
tree, size, encoding='EN
hf_defs.writeln('},')
+# Note: during parsing, byte_size types have been converted to count during
validation
+def read_array_len(writer, prefix, array, dest, scope, is_ptr):
+if is_ptr:
+nelements = %s__array__nelements % prefix
+else:
+nelements = %s__nelements % prefix
+if dest.is_toplevel() and scope.variable_defined(nelements):
+return nelements # Already there for toplevel, need not recalculate
+# just reuse variable, there is no problem for cast as we always store in
guint32
+if array.is_identifier_length():
+nelements = dest.read_ref(array.size)
+dest.write_ref(writer, dest.ref_size(array.size), prefix +
'.nelements', nelements)
+return nelements
+element_type = array.element_type
+scope.variable_def(guint32, nelements)
+if array.is_constant_length():
+writer.assign(nelements, array.size)
+elif array.is_remaining_length():
+if element_type.is_fixed_nw_size():
+if element_type.get_fixed_nw_size() == 1:
+writer.assign(nelements, glb-message_end - offset)
+else:
+writer.assign(nelements, (glb-message_end - offset) / (%s)
%(element_type.get_fixed_nw_size()))
+else:
+raise NotImplementedError(TODO array[] of dynamic element size
not done yet)
+elif array.is_image_size_length():
+(bpp, width, rows) = array.size[1:]
+width_v = dest.read_ref(width)
+rows_v = dest.read_ref(rows)
+if bpp == 8:
+writer.assign(nelements, ((guint32) %s * %s) % (width_v, rows_v))
+elif bpp == 1:
+writer.assign(nelements, (((guint32) %s + 7U) / 8U ) * %s %
(width_v, rows_v))
+else:
+writer.assign(nelements, ((%sU * (guint32) %s + 7U) / 8U ) * %s
% (bpp, width_v, rows_v))
+elif array.is_bytes_length():
+writer.assign(nelements, dest.read_ref(array.size[2]))
+else:
+raise NotImplementedError(TODO array size type not handled yet)
+# TODO compute a better size
+dest.write_ref(writer, 32, prefix+'.nelements', nelements)
+return nelements
+
def write_switch(writer, container, switch, dest, scope):
pass
-def write_array(writer, container, member, array, dest, scope):
+def write_array(writer, container, member, nelements, array, dest, scope):
assert(container and member)
def write_pointer(writer, container, member, t, dest, scope):
@@ -307,7 +350,8 @@ def write_member(writer, container, member, dest, scope):
elif t.is_primitive():
write_member_primitive(writer, container, member, t, dest, scope)
elif t.is_array():
-write_array(writer, container, member, t, dest, scope)
+nelements = read_array_len(writer, member.name, t, dest, scope, False)
+write_array(writer, container, member, nelements, t, dest, scope)
elif t.is_struct():
write_struct(writer, member, t, '-1', dest, scope)
--
2.1.0
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH v3 21/51] Add code to handle destination variable
Add some classes to be able to store retrieved data from structure
and messages.
The idea is to generate code dynamically when variable are readed.
Signed-off-by: Frediano Ziglio fzig...@redhat.com
---
python_modules/dissector.py | 104 +++-
1 file changed, 103 insertions(+), 1 deletion(-)
diff --git a/python_modules/dissector.py b/python_modules/dissector.py
index 40e348a..588becd 100644
--- a/python_modules/dissector.py
+++ b/python_modules/dissector.py
@@ -66,6 +66,106 @@ def write_parser_helpers(writer):
writer.writeln('#endif')
writer.newline()
+# generate code to declare a variable only when needed
+# code is generated only when we read the reference
+class Reference:
+def __init__(self, writer, name):
+self.defined = False
+self.written = False
+self.name = name
+# create a subwriter to write code to read variable only once
+self.writer = writer.get_subwriter()
+
+def write(self, size, value, scope):
+if not size in (8, 16, 32, 64):
+raise Exception('Unknown size %d for %s' % (size, self.name))
+assert(not self.defined or (value, size) == (self.value, self.size))
+if not self.defined:
+self.value = value
+self.size = size
+self.scope = scope
+self.defined = True
+
+def read(self):
+# variable not yet defined
+assert(self.defined)
+if not self.written:
+assert(not self.scope.variable_defined(self.name))
+t = { 8: 'guint32', 16: 'guint32', 32: 'guint32', 64: 'guint64'
}[self.size]
+self.scope.variable_def(t, self.name)
+self.writer.assign(self.name, self.value)
+self.written = True
+return self.name
+
+class Level:
+def __init__(self, n=0):
+self.level = n
+def __enter__(self):
+self.level += 1
+def __exit__(self, exc_type, exc_value, traceback):
+self.level -= 1
+def __getattr__(self, name):
+if not name in {'tree', 'ti'}:
+raise Exception('Not possible to get name %s' % name)
+return name if self.level == 0 else name + str(self.level)
+
+# represent part of a destination to write to
+# for instance if we are parsing a structure dest represent that structure
output
+class Destination:
+def __init__(self, scope):
+self.refs = {}
+self.is_helper = False
+self.reuse_scope = scope
+self.parent_dest = None
+self.level = Level()
+
+def child_sub(self, member, scope):
+return SubDestination(self, member, scope)
+
+def declare(self, writer):
+return writer.optional_block(self.reuse_scope)
+
+def is_toplevel(self):
+return self.parent_dest == None and not self.is_helper
+
+def read_ref(self, member):
+return self.get_ref(member).read()
+
+def write_ref(self, writer, size, member, value):
+ref = self.get_ref(member, writer)
+ref.write(size, value, self.reuse_scope)
+
+def ref_size(self, member):
+return self.get_ref(member).size
+
+class RootDestination(Destination):
+def __init__(self, scope):
+Destination.__init__(self, scope)
+self.base_var = fld
+
+def get_ref(self, member, writer=None):
+name = (self.base_var + . + member).replace('.', '__')
+if name in self.refs:
+return self.refs[name]
+if not writer:
+raise Exception('trying to read a reference to %s' % member)
+self.refs[name] = ref = Reference(writer, name)
+return ref
+
+def declare(self, writer):
+return writer.no_block(self.reuse_scope)
+
+class SubDestination(Destination):
+def __init__(self, parent_dest, member, scope):
+Destination.__init__(self, scope)
+self.parent_dest = parent_dest
+self.member = member
+self.level = parent_dest.level
+
+def get_ref(self, member, writer=None):
+return self.parent_dest.get_ref(self.member + . + member, writer)
+
+
def write_msg_parser(writer, message, server):
msg_name = message.c_name()
function_name = dissect_spice_%s_%s % ('server' if server else 'client',
msg_name)
@@ -80,7 +180,9 @@ def write_msg_parser(writer, message, server):
writer.ifdef(message)
parent_scope = writer.function(function_name,
guint32,
- GlobalInfo *glb _U_, proto_tree *tree0
_U_, guint32 offset, True)
+ GlobalInfo *glb _U_, proto_tree *tree _U_,
guint32 offset, True)
+
+dest = RootDestination(parent_scope)
writer.statement(return offset)
writer.end_block()
--
2.1.0
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH v3 23/51] Write function to write members
Check members are all of a giver type and call stubs for each type.
Signed-off-by: Frediano Ziglio fzig...@redhat.com
---
python_modules/dissector.py | 50 +++--
1 file changed, 48 insertions(+), 2 deletions(-)
diff --git a/python_modules/dissector.py b/python_modules/dissector.py
index ed3b939..cd653b3 100644
--- a/python_modules/dissector.py
+++ b/python_modules/dissector.py
@@ -166,9 +166,55 @@ class SubDestination(Destination):
return self.parent_dest.get_ref(self.member + . + member, writer)
-def write_member_parser(writer, container, member, dest, scope):
+def write_switch(writer, container, switch, dest, scope):
pass
+def write_array(writer, container, member, array, dest, scope):
+assert(container and member)
+
+def write_pointer(writer, container, member, t, dest, scope):
+assert(t.is_pointer())
+
+def write_struct(writer, member, t, index, dest, scope):
+assert(t.is_struct())
+
+def write_member_primitive(writer, container, member, t, dest, scope):
+assert(t.is_primitive())
+
+def write_member(writer, container, member, dest, scope):
+
+if member.has_attr(virtual):
+dest.write_ref(writer, 32, member.name,
member.attributes[virtual][0])
+return
+
+writer.comment(member.name)
+writer.newline()
+
+if member.is_switch():
+write_switch(writer, container, member, dest, scope)
+return
+
+if member.has_attr('ws_as'):
+type_name = member.attributes['ws_as'][0]
+assert(ptypes.type_exists(type_name))
+t = ptypes.lookup_type(type_name)
+else:
+t = member.member_type
+
+if t.is_pointer():
+# TODO case not handled
+if not member.has_attr(nocopy):
+write_pointer(writer, container, member, t, dest, scope)
+elif t.is_primitive():
+write_member_primitive(writer, container, member, t, dest, scope)
+elif t.is_array():
+write_array(writer, container, member, t, dest, scope)
+
+elif t.is_struct():
+write_struct(writer, member, t, '-1', dest, scope)
+else:
+raise NotImplementedError(TODO can't handle parsing of %s % t)
+
def write_container_parser(writer, container, dest):
if container.has_attr('ws_as'):
type_name = container.attributes['ws_as'][0]
@@ -179,7 +225,7 @@ def write_container_parser(writer, container, dest):
for m in container.members:
if m.has_minor_attr():
writer.begin_block(if (minor = %s) % m.get_minor_attr())
-write_member_parser(writer, container, m, dest, scope)
+write_member(writer, container, m, dest, scope)
if m.has_minor_attr():
writer.end_block()
--
2.1.0
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH v3 46/51] Allows to specify ws_as attribute for messages
Signed-off-by: Frediano Ziglio fzig...@redhat.com
---
python_modules/dissector.py| 7 ++-
python_modules/ptypes.py | 3 ++-
python_modules/spice_parser.py | 4 ++--
3 files changed, 10 insertions(+), 4 deletions(-)
diff --git a/python_modules/dissector.py b/python_modules/dissector.py
index 5c6b6fc..3e1fd6d 100644
--- a/python_modules/dissector.py
+++ b/python_modules/dissector.py
@@ -914,7 +914,12 @@ def write_channel_parser(writer, channel, server):
writer.write(static const parse_msg_func_t funcs%d[%d] = % (d, r[1]
- r[0]))
writer.begin_block()
for i in range(r[0], r[1]):
-func = write_msg_parser(helpers, ids[i].message_type, server)
+message_type = ids[i].message_type
+if ids[i].has_attr('ws_as'):
+type_name = ids[i].attributes['ws_as'][0]
+assert(ptypes.type_exists(type_name))
+message_type = ptypes.lookup_type(type_name)
+func = write_msg_parser(helpers, message_type, server)
writer.write(func)
if i != r[1] -1:
writer.write(,)
diff --git a/python_modules/ptypes.py b/python_modules/ptypes.py
index 8c92493..e5e59e1 100644
--- a/python_modules/ptypes.py
+++ b/python_modules/ptypes.py
@@ -997,11 +997,12 @@ class MessageType(ContainerType):
return codegen.prefix_camel(Msg, self.name)
class ChannelMember(Containee):
-def __init__(self, name, message_type, value):
+def __init__(self, name, message_type, attribute_list, value):
Containee.__init__(self)
self.name = name
self.message_type = message_type
self.value = value
+self.attributes = fix_attributes(attribute_list)
def resolve(self, channel):
self.channel = channel
diff --git a/python_modules/spice_parser.py b/python_modules/spice_parser.py
index 3fde843..958bf11 100644
--- a/python_modules/spice_parser.py
+++ b/python_modules/spice_parser.py
@@ -129,8 +129,8 @@ def SPICE_BNF():
messageSpec = Group(message_ + messageBody +
attributes).setParseAction(lambda toks: ptypes.MessageType(None, toks[0][1],
toks[0][2])) | typename
channelParent = Optional(colon + typename, default=None)
-channelMessage = Group(messageSpec + identifier + Optional(equals +
integer, default=None) + semi) \
-.setParseAction(lambda toks: ptypes.ChannelMember(toks[0][1],
toks[0][0], toks[0][2]))
+channelMessage = Group(messageSpec + identifier + attributes +
Optional(equals + integer, default=None) + semi) \
+.setParseAction(lambda toks: ptypes.ChannelMember(toks[0][1],
toks[0][0], toks[0][2], toks[0][3]))
channelBody = channelParent + Group(lbrace + ZeroOrMore( server_ +
colon | client_ + colon | channelMessage) + rbrace)
enum_ = (enum32_ | enum16_ | enum8_)
--
2.1.0
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH v3 48/51] Test we don't override text handling other fields
Is possible the nested fields override text as text could be updated
at the end of container.
Check we don't do it and we correctly nest protocol items.
Signed-off-by: Frediano Ziglio fzig...@redhat.com
---
codegen/check_dissector | 1 +
codegen/data_struct2| Bin 0 - 9 bytes
codegen/out_struct2.txt | 28
codegen/test.proto | 9 +
4 files changed, 38 insertions(+)
create mode 100644 codegen/data_struct2
create mode 100644 codegen/out_struct2.txt
diff --git a/codegen/check_dissector b/codegen/check_dissector
index 86b78eb..b6ee52a 100755
--- a/codegen/check_dissector
+++ b/codegen/check_dissector
@@ -55,6 +55,7 @@ fi
check data_base1 1 1 out_base1.txt
check data_base1 1 1 out_struct1.txt --client
+check data_struct2 1 2 out_struct2.txt --client
check data_u16s 1 100 out_array_primitive.txt --client
check data_u16s 1 101 out_array_raw.txt --client
diff --git a/codegen/data_struct2 b/codegen/data_struct2
new file mode 100644
index
..dabffc9f0eed655407b62a7e00d28bb6137d9846
GIT binary patch
literal 9
QcmZRnW?*1oVc=i@00bofi~s-t
literal 0
HcmV?d1
diff --git a/codegen/out_struct2.txt b/codegen/out_struct2.txt
new file mode 100644
index 000..7cda803
--- /dev/null
+++ b/codegen/out_struct2.txt
@@ -0,0 +1,28 @@
+--- tree
+--- item
+Text: size 4
+--- tree
+--- item
+Text: 4 (0x4)
+Name: size
+Abbrev: spice2.auto.StructTxts_size
+Type: FT_UINT8
+Base: BASE_DEC
+--- item
+Text: data is 123
+Name: data
+Abbrev: spice2.auto.StructTxts_data
+Type: FT_UINT32
+Base: BASE_DEC
+--- item
+Text: array[0] = 4
+Name: array
+Abbrev: spice2.auto.StructTxts_array_array
+Type: FT_UINT16
+Base: BASE_DEC
+--- item
+Text: array[1] = 8
+Name: array
+Abbrev: spice2.auto.StructTxts_array_array
+Type: FT_UINT16
+Base: BASE_DEC
diff --git a/codegen/test.proto b/codegen/test.proto
index 7bba890..3b4e48f 100644
--- a/codegen/test.proto
+++ b/codegen/test.proto
@@ -66,6 +66,12 @@ message Array2 {
uint16 @ws_txt(small_named[%u]: %u, INDEX, small_named) small_named[2]
@ws_desc(Small array);
};
+struct StructTxts {
+uint8 size;
+uint32 data @ws_txt(data is %u, data);
+uint16 @ws_txt_n(array[%u] = %u, INDEX, array) array[2];
+} @ws_txt(size %u, size);
+
channel BaseChannel {
server:
message {
@@ -99,6 +105,9 @@ channel BaseChannel {
message {
Dummy struct;
} Struct1 = 1;
+message {
+StructTxts struct;
+} struct2;
ArrayPrimitive array_primitive = 100;
ArrayRaw array_raw;
--
2.1.0
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH v3 39/51] Allows to have two type with different size to point to same field name
Signed-off-by: Frediano Ziglio fzig...@redhat.com
---
python_modules/dissector.py | 14 +++---
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/python_modules/dissector.py b/python_modules/dissector.py
index c88118f..554f59c 100644
--- a/python_modules/dissector.py
+++ b/python_modules/dissector.py
@@ -45,17 +45,14 @@ class HF:
hf_writer.variable_def(static int, %s = -1 % self.hf_name)
def create(self):
-other = self.fields.get(self.ws_name)
+other = self.fields.get(self.hf_name)
if other:
-for f in 'hf_name desc ws_name base vals mask'.split():
+for f in 'hf_name desc ws_name f_type base vals mask'.split():
if other.__dict__[f] != self.__dict__[f]:
raise Exception('HF Field different from previous
for\n\t%s\n\t%s' % (other, self))
-if other.f_type != self.f_type:
-if other.f_type[:7] != 'FT_UINT' or self.f_type[:7] !=
'FT_UINT':
-raise Exception('HF Field different from previous
for\n\t%s\n\t%s' % (other, self))
return
-self.fields[self.ws_name] = self
+self.fields[self.hf_name] = self
self.add_wireshark_field()
@@ -297,6 +294,8 @@ def write_wireshark_field(writer, container, member, t, ws,
tree, size, encoding
assert(member and container)
+size_name = ''
+
# compute proper type
f_type = 'FT_NONE'
base = 'BASE_NONE'
@@ -305,6 +304,7 @@ def write_wireshark_field(writer, container, member, t, ws,
tree, size, encoding
assert(t.is_primitive())
base = 'BASE_DEC'
f_type = get_primitive_ft_type(t)
+size_name = str(t.get_fixed_nw_size() * 8)
if isinstance(t, ptypes.FlagsType):
# show flag as hexadecimal for now
base = 'BASE_HEX'
@@ -342,7 +342,7 @@ def write_wireshark_field(writer, container, member, t, ws,
tree, size, encoding
hf_name = member_hf_name(container, member)
ws_name = 'auto.' + hf_name[3:]
else:
-hf_name = 'hf_%s' % ws_name.replace('.', '_')
+hf_name = 'hf_%s%s' % (ws_name.replace('.', '_'), size_name)
writer.statement(%sproto_tree_add_item(%s, %s, glb-tvb, offset, %s, %s)
%
(prefix, tree, hf_name, size, encoding))
--
2.1.0
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH v3 51/51] Describe Quic image format from dissector
Signed-off-by: Frediano Ziglio fzig...@redhat.com
---
spice.proto | 25 -
1 file changed, 24 insertions(+), 1 deletion(-)
diff --git a/spice.proto b/spice.proto
index 880a8be..270464f 100644
--- a/spice.proto
+++ b/spice.proto
@@ -687,6 +687,29 @@ struct Surface {
uint32 surface_id;
} @ws_txt(Surface ID: %u, surface_id);
+enum32 quic_image_type {
+INVALID,
+GRAY,
+RGB16,
+RGB24,
+RGB32,
+RGBA
+} @ws(QUIC image type, quic_type) @prefix(WSQUIC_IMAGE_TYPE_);
+
+struct ImageQuic {
+uint32 magic @ws_desc(QUIC magic (QUIC));
+uint16 major @ws(QUIC major version, quic_major_version);
+uint16 minor @ws(QUIC minor version, quic_minor_version);
+quic_image_type type;
+uint32 width @ws(Width, quic_width);
+uint32 height @ws(Height, image_height);
+uint8 data[] @end @ws_txt(QUIC compressed image data (%u bytes),
data.nelements);
+};
+
+struct ImageQuicData {
+uint32 data_size @bytes_count(dummy) @ws_txt(QUIC image size: %u bytes,
data_size);
+ImageQuic image[bytes(data_size, dummy)] @nomarshal @chunk;
+};
struct Image {
struct ImageDescriptor {
@@ -701,7 +724,7 @@ struct Image {
case BITMAP:
BitmapData bitmap;
case QUIC:
-BinaryData quic;
+BinaryData quic @ws_as(ImageQuicData);
case LZ_RGB:
case GLZ_RGB:
BinaryData lz_rgb;
--
2.1.0
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH v3 44/51] test: Add check on output strings
Make sure the generated dissector contains all strings from
the protocol file.
Signed-off-by: Frediano Ziglio fzig...@redhat.com
---
codegen/Makefile.am | 2 +-
codegen/check_strings | 33 +
2 files changed, 34 insertions(+), 1 deletion(-)
create mode 100755 codegen/check_strings
diff --git a/codegen/Makefile.am b/codegen/Makefile.am
index e169d88..000a41b 100644
--- a/codegen/Makefile.am
+++ b/codegen/Makefile.am
@@ -45,7 +45,7 @@ dissector.h: $(top_srcdir)/spice.proto $(MARSHALLERS_DEPS)
packet-spice.h: $(top_srcdir)/spice.proto $(MARSHALLERS_DEPS)
$(AM_V_GEN)$(PYTHON) $(top_srcdir)/spice_codegen.py
--generate-wireshark-dissector $ $@ /dev/null
-TESTS = check_dissector
+TESTS = check_dissector check_strings
check_PROGRAMS = dissector_test compile_check
dissector_test_SOURCES = dissector_test.c test.c test.h
diff --git a/codegen/check_strings b/codegen/check_strings
new file mode 100755
index 000..0dd08ef
--- /dev/null
+++ b/codegen/check_strings
@@ -0,0 +1,33 @@
+#!/usr/bin/perl
+
+# This scripts check that all strings in the protocol
+# file are found in the output dissector
+use strict;
+
+my $proto = '../spice.proto';
+my $out = 'dissector.c';
+
+open(IN, '', $out) or die Error opening output file $out;
+my @all = IN;
+close(IN);
+@all = map { $_ =~ s/ G_GINT64_MODIFIER //g; $_ } @all;
+my $all = join('', @all);
+
+sub check($) {
+ my $what = shift;
+ open(IN, '', $proto) or die Error opening protocol file $proto;
+ while (IN) {
+ if (m/\@$what\(([^]+)/) {
+ if (index($all, $1) 0) {
+ print $1 not found!\n;
+ }
+ }
+ }
+ close(IN);
+}
+
+check('ws');
+check('ws_desc');
+check('ws_txt');
+check('ws_txt_n');
+
--
2.1.0
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH v3 25/51] test: Allows to write items to tree and dump saved tree
Signed-off-by: Frediano Ziglio fzig...@redhat.com
---
codegen/dissector_test.c | 424 ++-
1 file changed, 423 insertions(+), 1 deletion(-)
diff --git a/codegen/dissector_test.c b/codegen/dissector_test.c
index 25a33b5..5a49f40 100644
--- a/codegen/dissector_test.c
+++ b/codegen/dissector_test.c
@@ -21,6 +21,176 @@ static int last_ei_registered = first_ei_registered - 1;
static int last_tree_registered = first_tree_registered - 1;
static bool got_error = false;
+static GPtrArray *hfs;
+
+struct tvbuff {
+ guint8 *data;
+ size_t len;
+};
+
+static int check(const char *chk_str, int line, int chk, const char *fmt, ...)
+{
+ if (!chk) {
+ va_list ap;
+ va_start(ap, fmt);
+ fprintf(stderr, Check failed at line %d\n, line);
+ fprintf(stderr, Check: %s\n, chk_str);
+ vfprintf(stderr, fmt, ap);
+ fprintf(stderr, \n);
+ va_end(ap);
+ abort();
+ }
+ return 1;
+}
+#define check(chk, ...) check(#chk, __LINE__, chk, __VA_ARGS__)
+
+static int check_tree(proto_node *node)
+{
+ assert(node-tree_data == (void*) node);
+ assert(node-next == NULL);
+ assert(node-finfo == NULL);
+ if (!node-first_child) {
+ assert(node-last_child == NULL);
+ } else {
+ assert(node-last_child-next == NULL);
+ }
+ return 1;
+}
+
+static int check_item(proto_node *node)
+{
+ assert(node-tree_data == NULL);
+ assert(node-finfo != NULL);
+ assert(node-finfo-rep != NULL);
+ assert(node-first_child == node-last_child);
+ assert(node-first_child == NULL || check_tree(node-first_child));
+ assert(node-parent);
+ assert(node-finfo-start = 0);
+ assert(node-finfo-length = 0);
+ return 1;
+}
+
+guint8 *tvb_bytes(tvbuff_t *tvb, const gint offset, unsigned len)
+{
+ if (!tvb)
+ return NULL;
+
+ assert(offset = 0);
+ assert(offset + len = tvb-len);
+ return tvb-data + offset;
+}
+
+static guint64 read_ule(const guint8 *p, unsigned len)
+{
+ guint64 low, high;
+
+ switch (len) {
+ case 1:
+ return p[0];
+ case 2:
+ return p[0] + 0x100u * p[1];
+ case 4:
+ return p[0] + 0x100u * p[1] + 0x1u * p[2] + 0x100u *
p[3];
+ case 8:
+ low = p[0] + 0x100u * p[1] + 0x1u * p[2] + 0x100u *
p[3];
+ p += 4;
+ high = p[0] + 0x100u * p[1] + 0x1u * p[2] + 0x100u *
p[3];
+ return high 32 | low;
+ }
+ assert(0);
+ return 0;
+}
+
+static gint64 read_sle(const guint8 *p, unsigned len)
+{
+ guint64 sign_bit = (((guint64) 0x80) ((len-1) * 8));
+ guint64 val = read_ule(p, len);
+
+ if ((val sign_bit) != 0)
+ return (gint64) ((val ^ sign_bit) - sign_bit);
+ return (gint64) val;
+}
+
+static guint64 tvb_get_ule(tvbuff_t *tvb, const gint offset, unsigned len)
+{
+ guint8 *p = tvb_bytes(tvb, offset, len);
+ if (!p)
+ return 0;
+ return read_ule(p, len);
+}
+
+static gint64 tvb_get_sle(tvbuff_t *tvb, const gint offset, unsigned len)
+{
+ guint8 *p = tvb_bytes(tvb, offset, len);
+ if (!p)
+ return 0;
+ return read_sle(p, len);
+}
+
+static const char *describe_fttype(enum ftenum type)
+{
+ switch (type) {
+#define FT(name) case FT_ ## name: return FT_ #name;
+ FT(NONE)/* used for text labels with no value */
+ FT(PROTOCOL)
+ FT(BOOLEAN) /* TRUE and FALSE come from glib.h */
+ FT(UINT8)
+ FT(UINT16)
+ FT(UINT24) /* really a UINT32, but displayed as 3 hex-digits if
FD_HEX*/
+ FT(UINT32)
+ FT(UINT64)
+ FT(INT8)
+ FT(INT16)
+ FT(INT24) /* same as for UINT24 */
+ FT(INT32)
+ FT(INT64)
+ FT(FLOAT)
+ FT(DOUBLE)
+ FT(ABSOLUTE_TIME)
+ FT(RELATIVE_TIME)
+ FT(STRING)
+ FT(STRINGZ) /* for use with proto_tree_add_item() */
+ FT(UINT_STRING) /* for use with proto_tree_add_item() */
+ FT(ETHER)
+ FT(BYTES)
+ FT(UINT_BYTES)
+ FT(IPv4)
+ FT(IPv6)
+ FT(IPXNET)
+ FT(FRAMENUM)/* a UINT32, but if selected lets you go to frame with
that number */
+ FT(PCRE)/* a compiled Perl-Compatible Regular Expression object
*/
+ FT(GUID)/* GUID, UUID */
+ FT(OID) /* OBJECT IDENTIFIER */
+ FT(EUI64)
+ FT(AX25)
+ FT(VINES)
+ FT(REL_OID) /* RELATIVE-OID */
+ FT(SYSTEM_ID)
+ FT(STRINGZPAD) /* for use with proto_tree_add_item() */
+ default:
+ check(false, Unknown fttype %d, type);
+ break;
+ }
+ return NULL;
+}
+
+static const char *describe_base(int base)
+{
+ switch
[Spice-devel] [PATCH spice 01/13] server: Refactor the Spice server video encoding so alternative implementations can be added. (take 4)
This patch simply replaces the mjpeg_encoder_xxx() call points with a more
object oriented design.
Signed-off-by: Francois Gouget fgou...@codeweavers.com
---
Changes since take 3:
- A NULL video_encoder means that the stream creation failed so
red_stop_stream() no longer sends messages to destroy it in
that case. This avoids getting error messages in spice-gtk.
- Removed some forward declarations and made more functions static.
- Tweaked some video_encoder.h comments.
Changes since take 2:
- No change.
Changes since take 1:
- I fixed the width/height comments and they now state that they always
match src.
server/Makefile.am | 2 +-
server/mjpeg_encoder.c | 227 +++--
server/mjpeg_encoder.h | 114 -
server/red_worker.c| 173 -
server/video_encoder.h | 165 +++
5 files changed, 381 insertions(+), 300 deletions(-)
delete mode 100644 server/mjpeg_encoder.h
create mode 100644 server/video_encoder.h
diff --git a/server/Makefile.am b/server/Makefile.am
index 89a375c..36b6d45 100644
--- a/server/Makefile.am
+++ b/server/Makefile.am
@@ -81,7 +81,6 @@ libspice_server_la_SOURCES = \
main_channel.c \
main_channel.h \
mjpeg_encoder.c \
- mjpeg_encoder.h \
red_bitmap_utils.h \
red_channel.c \
red_channel.h \
@@ -115,6 +114,7 @@ libspice_server_la_SOURCES =\
spicevmc.c \
spice_timer_queue.c \
spice_timer_queue.h \
+ video_encoder.h \
zlib_encoder.c \
zlib_encoder.h \
spice_bitmap_utils.h\
diff --git a/server/mjpeg_encoder.c b/server/mjpeg_encoder.c
index 9a41ef3..48042ba 100644
--- a/server/mjpeg_encoder.c
+++ b/server/mjpeg_encoder.c
@@ -20,7 +20,11 @@
#endif
#include red_common.h
-#include mjpeg_encoder.h
+
+typedef struct MJpegEncoder MJpegEncoder;
+#define VIDEO_ENCODER_T MJpegEncoder
+#include video_encoder.h
+
#include jerror.h
#include jpeglib.h
#include inttypes.h
@@ -154,6 +158,7 @@ typedef struct MJpegEncoderRateControl {
} MJpegEncoderRateControl;
struct MJpegEncoder {
+VideoEncoder base;
uint8_t *row;
uint32_t row_size;
int first_frame;
@@ -165,7 +170,7 @@ struct MJpegEncoder {
void (*pixel_converter)(uint8_t *src, uint8_t *dest);
MJpegEncoderRateControl rate_control;
-MJpegEncoderRateControlCbs cbs;
+VideoEncoderRateControlCbs cbs;
void *cbs_opaque;
/* stats */
@@ -174,11 +179,6 @@ struct MJpegEncoder {
uint32_t num_frames;
};
-static inline void mjpeg_encoder_reset_quality(MJpegEncoder *encoder,
- int quality_id,
- uint32_t fps,
- uint64_t frame_enc_size);
-static uint32_t get_max_fps(uint64_t frame_size, uint64_t bytes_per_sec);
static void mjpeg_encoder_process_server_drops(MJpegEncoder *encoder);
static uint32_t get_min_required_playback_delay(uint64_t frame_enc_size,
uint64_t byte_rate,
@@ -189,49 +189,14 @@ static inline int rate_control_is_active(MJpegEncoder*
encoder)
return encoder-cbs.get_roundtrip_ms != NULL;
}
-MJpegEncoder *mjpeg_encoder_new(uint64_t starting_bit_rate,
-MJpegEncoderRateControlCbs *cbs, void *opaque)
-{
-MJpegEncoder *enc;
-
-spice_assert(!cbs || (cbs cbs-get_roundtrip_ms
cbs-get_source_fps));
-
-enc = spice_new0(MJpegEncoder, 1);
-
-enc-first_frame = TRUE;
-enc-rate_control.byte_rate = starting_bit_rate / 8;
-enc-starting_bit_rate = starting_bit_rate;
-
-if (cbs) {
-struct timespec time;
-
-clock_gettime(CLOCK_MONOTONIC, time);
-enc-cbs = *cbs;
-enc-cbs_opaque = opaque;
-mjpeg_encoder_reset_quality(enc, MJPEG_QUALITY_SAMPLE_NUM / 2, 5, 0);
-enc-rate_control.during_quality_eval = TRUE;
-enc-rate_control.quality_eval_data.type = MJPEG_QUALITY_EVAL_TYPE_SET;
-enc-rate_control.quality_eval_data.reason =
MJPEG_QUALITY_EVAL_REASON_RATE_CHANGE;
-enc-rate_control.warmup_start_time = ((uint64_t) time.tv_sec) *
10 + time.tv_nsec;
-} else {
-enc-cbs.get_roundtrip_ms = NULL;
-mjpeg_encoder_reset_quality(enc, MJPEG_LEGACY_STATIC_QUALITY_ID,
MJPEG_MAX_FPS, 0);
-}
-
-enc-cinfo.err = jpeg_std_error(enc-jerr);
-jpeg_create_compress(enc-cinfo);
-
-return enc;
-}
-
-void mjpeg_encoder_destroy(MJpegEncoder *encoder)
[Spice-devel] [PATCH spice 02/13] server: Add a GStreamer 0.10 MJPEG video encoder and use it by default. (take 4)
The GStreamer video encoder supports both regular and sized streams.
It is otherwise quite basic and lacks any rate control: the bitrate is set at
startup and will not react to changes in the network conditions. Still it
should work fine on LANs.
Signed-off-by: Francois Gouget fgou...@codeweavers.com
---
Changes since take 3:
- Failing to create the pipeline is probably a permanent issue so if
that happens encode_frame() now returns
VIDEO_ENCODER_FRAME_UNSUPPORTED so that Spice falls back to the
non-stream way of sending screen updates.
- The video encoder will not get a zero bit rate so I removed handling
of that case. The bit rate floor has been raised to 128kbps. Going
below that does not seem very meaningful. Calculating the bit rate
ceiling has been moved to the get_bit_rate_cap() function.
- Removed some redundant field initializations and added a comment.
- Added a get_mbps() helper to simplify printing bandwidth values.
- Some reformating and reordering to group related functions.
Changes since take 2:
- I resolved the buffer timestamping issues. Appsrc is no longer a live
source (it had no reason to be) and it performs the timestamping. The
only remaining annoyance is that ffenc_mjpeg requires removing the
pipeline's default clock otherwise it gets stuck with the following
message:
ERROR ffmpeg :0:: Error, Invalid timestamp=0, last=0
I consider this to be an ffenc_mjpeg bug since none of the other
encoders (VP8, H264 and even avenc_mpjpeg in GStreamer 1.0) has this
issue. So I'm keeping the clock removal as a workaround for
ffenc_mjpeg.
- I went back to ffmpegcolorspace because it's the standard way to do
color conversion in GStreamer 0.10 (present in gst-plugins-base),
while autovideoconvert is only present in gst-plugins-bad which I
believe we don't depend on.
- I simplified the source frame copy code (push_raw_frame()). It's also
ready for adding zero-copy once GStreamer 1.0 support is added.
- I improved the autoconf GStreamer 0.10 detection: it's now possible
to require GStreamer support, or just let it be included if present.
This also paves the way for GStreamer 1.0.
- Changed set_appsrc_caps() and construct_pipeline() to return a
gboolean since they only return TRUE/FALSE.
- Fixed some formatting issues, mostly braces placement.
Changes since take 1:
- The width/height comments are now correct in the previous patch of
the series.
- Fixed the pipeline reconfiguration (for video size changes) so it
still works if we have to fall back to rebuilding the pipeline from
scratch.
- Added a comment suggesting to avoid copying the compressed buffer as
a future improvement. I think this will require changing the way the
output buffer is allocated though. So I'm leaving that for after the
basic support committed.
configure.ac | 26 +++
server/Makefile.am | 8 +
server/gstreamer_encoder.c | 451 +
server/red_worker.c| 11 +-
server/video_encoder.h | 6 +
5 files changed, 500 insertions(+), 2 deletions(-)
create mode 100644 server/gstreamer_encoder.c
diff --git a/configure.ac b/configure.ac
index 5b4caa4..ca91b5a 100644
--- a/configure.ac
+++ b/configure.ac
@@ -80,6 +80,30 @@ AS_IF([test x$enable_opengl != xno], [
SPICE_CHECK_SMARTCARD([SMARTCARD])
AM_CONDITIONAL(SUPPORT_SMARTCARD, test x$have_smartcard = xyes)
+AC_ARG_ENABLE(gstreamer,
+ AS_HELP_STRING([--enable-gstreamer=@:@auto/yes/no@:@],
+ [Enable GStreamer 0.10 support]),
+ [],
+ [enable_gstreamer=auto])
+
+if test x$enable_gstreamer != xno; then
+PKG_CHECK_MODULES(GSTREAMER_0_10, [gstreamer-0.10, gstreamer-app-0.10],
+ [enable_gstreamer=yes
+ have_gstreamer_0_10=yes],
+ [have_gstreamer_0_10=no])
+if test x$have_gstreamer_0_10 = xyes; then
+AC_SUBST(GSTREAMER_0_10_CFLAGS)
+AC_SUBST(GSTREAMER_0_10_LIBS)
+AS_VAR_APPEND([SPICE_REQUIRES], [ gstreamer-0.10 gstreamer-app-0.10])
+AC_DEFINE([HAVE_GSTREAMER_0_10], [1], [Define if supporting GStreamer
0.10])
+elif test x$enable_gstreamer = xyes; then
+AC_MSG_ERROR([GStreamer 0.10 support requested but not found. You may
set GSTREAMER_0_10_CFLAGS and GSTREAMER_0_10_LIBS to avoid the need to call
pkg-config.])
+fi
+else
+have_gstreamer_0_10=no
+fi
+AM_CONDITIONAL(SUPPORT_GSTREAMER_0_10, test x$have_gstreamer_0_10 = xyes)
+
AC_ARG_ENABLE([automated_tests],
AS_HELP_STRING([--enable-automated-tests], [Enable automated
tests using spicy-screenshot (part of spice--gtk)]),,
[enable_automated_tests=no])
@@ -311,6 +335,8 @@ echo
Smartcard:${have_smartcard}
+GStreamer 0.10: ${have_gstreamer_0_10}
+
SASL
[Spice-devel] [PATCH protocol 04/13] Add support for the VP8 and H264 video codecs and for advertising supported video codecs. (take 4)
Clients that support multiple codecs should advertise the
SPICE_DISPLAY_CAP_MULTI_CODEC capability and one SPICE_DISPLAY_CAP_CODEC_XXX
per supported codec.
---
This should be followed by a spice-common commit that ensures we get the
right version of these headers (possibly that commit could be part of
the commit of 03/13).
Changes since take 3:
- None
Changes since take 2:
- This patch also adds h264.
spice/enums.h| 2 ++
spice/protocol.h | 4
2 files changed, 6 insertions(+)
diff --git a/spice/enums.h b/spice/enums.h
index 6a0ab0b..36b0ea3 100644
--- a/spice/enums.h
+++ b/spice/enums.h
@@ -139,6 +139,8 @@ typedef enum SpicePathFlags {
typedef enum SpiceVideoCodecType {
SPICE_VIDEO_CODEC_TYPE_MJPEG = 1,
+SPICE_VIDEO_CODEC_TYPE_VP8,
+SPICE_VIDEO_CODEC_TYPE_H264,
SPICE_VIDEO_CODEC_TYPE_ENUM_END
} SpiceVideoCodecType;
diff --git a/spice/protocol.h b/spice/protocol.h
index d3c5962..614dcf1 100644
--- a/spice/protocol.h
+++ b/spice/protocol.h
@@ -135,6 +135,10 @@ enum {
SPICE_DISPLAY_CAP_STREAM_REPORT,
SPICE_DISPLAY_CAP_LZ4_COMPRESSION,
SPICE_DISPLAY_CAP_PREF_COMPRESSION,
+SPICE_DISPLAY_CAP_MULTI_CODEC,
+SPICE_DISPLAY_CAP_CODEC_MJPEG,
+SPICE_DISPLAY_CAP_CODEC_VP8,
+SPICE_DISPLAY_CAP_CODEC_H264,
};
enum {
--
2.1.4
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH spice 05/13] server: Add VP8 support, a video codec preference list and compatibility checks with the Spice client. (take 4)
The video encoder preferences can be expressed by building a semi-colon
separated list of encoder:codec pairs. For instance 'gstreamer:vp8;spice:mjpeg'
to pick first the GStreamer VP8 video encoder first and used the original MJPEG
video encoder one as a fallback.
The server has a default preference list which can also be selected by
specifying 'auto' as the preferences list.
The client compatibility check relies on the following new capabilities:
* SPICE_DISPLAY_CAP_MULTI_CODEC which denotes a recent client that supports
multiple codecs. This capability is needed to not have to hardcode that MJPEG
is supported. This makes it possible to write clients that don't support MJPEG.
* SPICE_DISPLAY_CAP_CODEC_XXX, where XXX is a supported codec, for now MJPEG
and VP8.
Signed-off-by: Francois Gouget fgou...@codeweavers.com
---
Note that this patch depends on the 03/13 and 04/13 spice-common
submodule patches.
Changes since take 3:
- Proper fallback in case the server and client have no video codec
in common.
- Check whether g_get_num_processors() is available for compatibility
with glib 2.35 and older.
Changes since take 2:
- We now allow the VP8 encoder to produce P frames since this does not
increase the latency (i.e. it still lets us get the compressed frames
right away), while improving the compression ratio.
- It turns out the above also helps with CPU usage and lets us
further configure the encoder for speed.
- Only construct_pipeline() really needs to know the encoder name so
we no longer keep it in a GstEncoder field.
- Fixed some formatting issues, mostly braces placement.
Changes since take 1:
- Only set the parameters supported by the current encoder.
- reconfigure_pipeline() handling has been fixed in the previous patch.
- The video codecs list now has a more sensible default and it's
possible to explicitly request this default by specifying 'auto' in
the configuration files.
configure.ac | 4 ++
server/gstreamer_encoder.c | 66 ++---
server/mjpeg_encoder.c | 7 +-
server/red_dispatcher.c| 172 -
server/red_dispatcher.h| 8 +++
server/red_worker.c| 82 +
server/red_worker.h| 18 +
server/reds.c | 12
server/spice-server.h | 1 +
server/spice-server.syms | 1 +
server/video_encoder.h | 18 +++--
11 files changed, 343 insertions(+), 46 deletions(-)
diff --git a/configure.ac b/configure.ac
index ca91b5a..e5be699 100644
--- a/configure.ac
+++ b/configure.ac
@@ -136,6 +136,10 @@ AS_IF([test x$have_smartcard = xyes], [
PKG_CHECK_MODULES([GLIB2], [glib-2.0 = 2.22])
AS_VAR_APPEND([SPICE_REQUIRES], [ glib-2.0 = 2.22])
+AC_CHECK_LIB(glib-2.0, g_get_num_processors,
+ AC_DEFINE([HAVE_G_GET_NUMPROCESSORS], 1, [Defined if we have
g_get_num_processors()]),,
+ $GLIB2_LIBS)
+
PKG_CHECK_MODULES(PIXMAN, pixman-1 = 0.17.7)
AC_SUBST(PIXMAN_CFLAGS)
AC_SUBST(PIXMAN_LIBS)
diff --git a/server/gstreamer_encoder.c b/server/gstreamer_encoder.c
index 8a21ad0..140261e 100644
--- a/server/gstreamer_encoder.c
+++ b/server/gstreamer_encoder.c
@@ -190,10 +190,27 @@ static gboolean set_appsrc_caps(GstEncoder *encoder)
/* A helper for gst_encoder_encode_frame(). */
static gboolean construct_pipeline(GstEncoder *encoder, const SpiceBitmap
*bitmap)
{
+gboolean no_clock = FALSE;
+const gchar* gstenc_name;
+switch (encoder-base.codec_type)
+{
+case SPICE_VIDEO_CODEC_TYPE_MJPEG:
+gstenc_name = ffenc_mjpeg;
+no_clock = TRUE;
+break;
+case SPICE_VIDEO_CODEC_TYPE_VP8:
+gstenc_name = vp8enc;
+break;
+default:
+spice_warning(unsupported codec type %d, encoder-base.codec_type);
+return FALSE;
+}
+
GError *err = NULL;
-const gchar* desc = appsrc name=src format=2 do-timestamp=true !
ffmpegcolorspace ! ffenc_mjpeg name=encoder ! appsink name=sink;
+gchar *desc = g_strdup_printf(appsrc name=src format=2 do-timestamp=true
! ffmpegcolorspace ! %s name=encoder ! appsink name=sink, gstenc_name);
spice_debug(GStreamer pipeline: %s, desc);
encoder-pipeline = gst_parse_launch_full(desc, NULL,
GST_PARSE_FLAG_FATAL_ERRORS, err);
+g_free(desc);
if (!encoder-pipeline) {
spice_warning(GStreamer error: %s, err-message);
g_clear_error(err);
@@ -203,19 +220,36 @@ static gboolean construct_pipeline(GstEncoder *encoder,
const SpiceBitmap *bitma
encoder-gstenc = gst_bin_get_by_name(GST_BIN(encoder-pipeline),
encoder);
encoder-appsink =
GST_APP_SINK(gst_bin_get_by_name(GST_BIN(encoder-pipeline), sink));
+/* Configure the encoders for a zero-frame latency, and real-time speed */
+adjust_bit_rate(encoder);
+g_object_set(G_OBJECT(encoder-gstenc), bitrate, encoder-bit_rate,
NULL);
+if (encoder-base.codec_type == SPICE_VIDEO_CODEC_TYPE_VP8)
[Spice-devel] [PATCH 00/13] Add GStreamer support for video streams (take 4)
This follows up on the previous patch series with the addition of full bit rate support. This brings the GStreamer video encoder to feature parity with the builtin Spice MJPEG one so I think it's ready to be committed. To summarize the changes since the last round: - The bit rate is automatically adjusted based on the network conditions, both down and up. - Zero-copy for the compressed output buffer. - Proper fallback in case the server and client have no video codec in common (which would only happen if the client does not support MJPEG streams). As for the previous round I'm only sending the patches needed for the Spice server to limit the size of this series. The GStreamer MJPEG encoder is fully compatible with existing clients so this should not hinder testing. I will post a new spice-gtk patch series soon but in the meantime one can fetch patches from GitHub to test the VP8 and h264 codecs. See the gst branch of the repositories below: spice: https://github.com/fgouget/spice spice-gtk: https://github.com/fgouget/spice-gtk xf86-video-qxl: https://github.com/fgouget/xf86-video-qxl spice-common: https://github.com/fgouget/spice-common spice-protocol: https://github.com/fgouget/spice-protocol (there's also 'extras' branches with more experimental/future patches for the curious) For spice-html5 and QEMU one would have to refer to the patches posted previously on spice-devel. They should still work with this series. Let me know if there are changes that are needed for inclusion. -- Francois Gouget fgou...@codeweavers.com ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH common 03/13] spice.proto: Add support for the VP8 and h264 video codecs. (take 4)
---
Changes since take 3:
- None
Changes since take 2:
- This patch also adds h264.
spice.proto | 2 ++
1 file changed, 2 insertions(+)
diff --git a/spice.proto b/spice.proto
index 4ea1263..fa4d448 100644
--- a/spice.proto
+++ b/spice.proto
@@ -329,6 +329,8 @@ flags8 path_flags { /* TODO: C enum names changes */
enum8 video_codec_type {
MJPEG = 1,
+VP8,
+H264,
};
flags8 stream_flags {
--
2.1.4
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH v3 35/51] Introduce a class to handle wireshark attributes
---
python_modules/dissector.py | 39 +++
1 file changed, 39 insertions(+)
diff --git a/python_modules/dissector.py b/python_modules/dissector.py
index d623576..827f7f0 100644
--- a/python_modules/dissector.py
+++ b/python_modules/dissector.py
@@ -25,6 +25,45 @@ hf_writer = None
hf_defs = None
+# handle wireshark attributes
+# is quite complex as attributes ca come from
+# member or array
+# - pointers, have their attributes
+# - array, get from member
+# - primitive or structure from array, specific attributes or type
+# - primitive or structure not in array, member or type
+class WSAttributes:
+def __init__(self, t, other=None):
+self.add_attrs = other
+self.attrs = t.attributes
+
+def _getattr(self, name, default=[None,None]):
+if self.add_attrs and name in self.add_attrs:
+return self.add_attrs[name]
+return self.attrs.get(name, default)
+
+def __getattr__(self, name):
+if name == 'name':
+val = self._getattr('ws_name')[0]
+if val is None:
+val = self._getattr('ws')[1]
+elif name == 'desc':
+val = self._getattr('ws_desc')[0]
+if val is None:
+val = self._getattr('ws')[0]
+elif name in {'type', 'base'}:
+val = self._getattr('ws_' + name)[0]
+elif name in {'txt', 'txt_n'}:
+val = self._getattr('ws_' + name,None)
+else:
+raise AttributeError('Attribute %s not supported' % name)
+self.__dict__[name] = val
+return val
+
+def has_txts(self):
+return self.txt is not None or self.txt_n is not None
+
+
def write_parser_helpers(writer):
if writer.is_generated(helper, demarshaller):
return
--
2.1.0
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH v3 26/51] test: Add code to read input from a file (or stdin)
This allow to emulate reading from network
Signed-off-by: Frediano Ziglio fzig...@redhat.com
---
codegen/check_dissector | 4 ++--
codegen/dissector_test.c | 32 +---
2 files changed, 31 insertions(+), 5 deletions(-)
diff --git a/codegen/check_dissector b/codegen/check_dissector
index f1444a2..7d6d086 100755
--- a/codegen/check_dissector
+++ b/codegen/check_dissector
@@ -6,8 +6,8 @@ error() {
exit 1
}
-./dissector_test 1 100
-if ./dissector_test 1 99; then
+./dissector_test 1 100 /dev/null
+if ./dissector_test 1 99 /dev/null; then
error This test should fail
fi
exit 0
diff --git a/codegen/dissector_test.c b/codegen/dissector_test.c
index 5a49f40..ba87367 100644
--- a/codegen/dissector_test.c
+++ b/codegen/dissector_test.c
@@ -15,6 +15,7 @@ enum {
first_hf_registered = 0x1,
first_ei_registered = 0x2,
first_tree_registered = 0x3,
+ max_message_size = 0x10,
};
static int last_hf_registered = first_hf_registered - 1;
static int last_ei_registered = first_ei_registered - 1;
@@ -474,8 +475,9 @@ static const struct option long_options[] = {
{ client, 0, NULL, 'c' },
{ output-file, required_argument, NULL, 'o' },
{ xml, 0, NULL, 'x' },
+ { input-file, required_argument, NULL, 'i' },
};
-static const char options[] = hscxo:;
+static const char options[] = hscxo:i:;
static void syntax(FILE *f, int exit_value)
{
@@ -488,6 +490,7 @@ static void syntax(FILE *f, int exit_value)
-c, --client Process client messages\n
-x, --xmlOutput in XML format\n
-o, --output-file=FILE Output to specified file\n
+ -i, --input-file=FILEInput from specified file\n
);
exit(exit_value);
}
@@ -495,8 +498,12 @@ static void syntax(FILE *f, int exit_value)
int main(int argc, char **argv)
{
int channel, message_type;
+ guint8 *buf, *p, *pend;
+ size_t size;
+ FILE *f = stdin;
GlobalInfo glb;
proto_tree tree;
+ struct tvbuff tvb;
spice_dissect_func_t (*msg_func)(guint8 channel);
spice_dissect_func_t channel_func = NULL;
@@ -526,6 +533,10 @@ int main(int argc, char **argv)
output_file = fopen(optarg, w);
check(output_file != NULL, Error opening output file);
break;
+ case 'i':
+ f = fopen(optarg, rb);
+ check(f != NULL, Error opening input file);
+ break;
default:
syntax(stderr, EXIT_FAILURE);
break;
@@ -540,10 +551,25 @@ int main(int argc, char **argv)
hfs = g_ptr_array_new_with_free_func(free);
spice_register_fields(1, NULL);
+ /* read message from standard input */
+ buf = malloc(max_message_size);
+ assert(buf);
+ p = buf;
+ pend = buf + max_message_size;
+ while (p pend (size = fread(p, 1, pend - p, f)) 0)
+ p += size;
+ assert(p pend);
+ assert(!ferror(f));
+ size = p - buf;
+
+ memset(tvb, 0, sizeof(tvb));
+ tvb.data = buf;
+ tvb.len = size;
+
memset(glb, 0, sizeof(glb));
- glb.tvb = NULL;
+ glb.tvb = tvb;
glb.message_offset = 0;
- glb.message_end = 0;
+ glb.message_end = tvb.len;
channel_func = msg_func(channel);
assert(channel_func);
--
2.1.0
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH v3 31/51] Handle array
If just raw data add a single field.
If more complex loop through them.
Signed-off-by: Frediano Ziglio fzig...@redhat.com
---
codegen/Makefile.am | 4
codegen/check_dissector | 4
codegen/data_u16s | Bin 0 - 2000 bytes
codegen/out_array_primitive.txt | 25 +
codegen/out_array_raw.txt | 5 +
codegen/out_array_struct.txt| 25 +
codegen/test.proto | 16
python_modules/dissector.py | 15 ++-
8 files changed, 93 insertions(+), 1 deletion(-)
create mode 100755 codegen/data_u16s
create mode 100644 codegen/out_array_primitive.txt
create mode 100644 codegen/out_array_raw.txt
create mode 100644 codegen/out_array_struct.txt
diff --git a/codegen/Makefile.am b/codegen/Makefile.am
index d8da936..bf5bbc7 100644
--- a/codegen/Makefile.am
+++ b/codegen/Makefile.am
@@ -60,6 +60,10 @@ EXTRA_DIST = \
data_base1 \
out_base1.txt \
out_struct1.txt \
+ data_u16s \
+ out_array_primitive.txt \
+ out_array_raw.txt \
+ out_array_struct.txt\
$(NULL)
CLEANFILES = test.c test.h enums.h dissector.c dissector.h *.trs
check_dissector.txt
diff --git a/codegen/check_dissector b/codegen/check_dissector
index 767ee9d..20d83ff 100755
--- a/codegen/check_dissector
+++ b/codegen/check_dissector
@@ -56,4 +56,8 @@ check data_base1 1 1 out_base1.txt
check data_base1 1 1 out_struct1.txt --client
+check data_u16s 1 100 out_array_primitive.txt --client
+check data_u16s 1 101 out_array_raw.txt --client
+check data_u16s 1 102 out_array_struct.txt --client
+
exit 0
diff --git a/codegen/data_u16s b/codegen/data_u16s
new file mode 100755
index
..a9120dacd440f4139125ddf1cef775451e326edd
GIT binary patch
literal 2000
zcmeIy0~Zzs06@{3Z7kbfHkNIwrtxi+qP{ROUt!o+cy43=lc!s+(!^W1ruBdA%zlJ
z7-5AIUIYE5?K^cMH5{NG5sc%*y4yQp7;_-D3QdHNGh4+Qb;M4)Y3Ro%AxuD3jl1
zmPJCWS2uux#X5dUisu#KtYB4p|B!~DyFy+N-CwaGRi8ayb3C+q_Qfis;0UcYO1BS
zI_j#Yz6Kgq_IE!rHQ7RX|9EqT4}9~w%Td0gN{1stc$L?8^*Kdg-l?zWV8JfPn@X
z~BL1HOz1$j5NwV~jPcoR%C$z)SZHO+K0%rwhvbIdi*d!hJ$YM(@wajuWthCB%
zYpk`-dK+xC$!1$@was=r?6k{nd+fE(eg_M$YK9D;;3VeJKj^opRb4XPtB21sDD2
zf0tZ##Z}i_cf(D$+;+!Z_uTiuLytW6#8b~a_rgoBy!OUh@4WZHN1uH5#aG{a_k%$H
QKvXMfd9b%e~JJ91Ud%iJOBUy
literal 0
HcmV?d1
diff --git a/codegen/out_array_primitive.txt b/codegen/out_array_primitive.txt
new file mode 100644
index 000..3a77f37
--- /dev/null
+++ b/codegen/out_array_primitive.txt
@@ -0,0 +1,25 @@
+--- tree
+--- item
+Text: 0 (0)
+Name: array
+Abbrev: spice2.auto.ArrayPrimitive_array_array
+Type: FT_UINT16
+Base: BASE_DEC
+--- item
+Text: 1 (0x1)
+Name: array
+Abbrev: spice2.auto.ArrayPrimitive_array_array
+Type: FT_UINT16
+Base: BASE_DEC
+--- item
+Text: 2 (0x2)
+Name: array
+Abbrev: spice2.auto.ArrayPrimitive_array_array
+Type: FT_UINT16
+Base: BASE_DEC
+--- item
+Text: 3 (0x3)
+Name: array
+Abbrev: spice2.auto.ArrayPrimitive_array_array
+Type: FT_UINT16
+Base: BASE_DEC
diff --git a/codegen/out_array_raw.txt b/codegen/out_array_raw.txt
new file mode 100644
index 000..f276871
--- /dev/null
+++ b/codegen/out_array_raw.txt
@@ -0,0 +1,5 @@
+--- tree
+--- item
+Text:
+Name: array
+Abbrev: spice2.auto.ArrayRaw_array_array
diff --git a/codegen/out_array_struct.txt b/codegen/out_array_struct.txt
new file mode 100644
index 000..eb03cd8
--- /dev/null
+++ b/codegen/out_array_struct.txt
@@ -0,0 +1,25 @@
+--- tree
+--- item
+Text: 0 (0)
+Name: dummy
+Abbrev: spice2.auto.Dummy_dummy
+Type: FT_UINT16
+Base: BASE_DEC
+--- item
+Text: 1 (0x1)
+Name: dummy
+Abbrev: spice2.auto.Dummy_dummy
+Type: FT_UINT16
+Base: BASE_DEC
+--- item
+Text: 2 (0x2)
+Name: dummy
+Abbrev: spice2.auto.Dummy_dummy
+Type: FT_UINT16
+Base: BASE_DEC
+--- item
+Text: 3 (0x3)
+Name: dummy
+Abbrev: spice2.auto.Dummy_dummy
+Type: FT_UINT16
+Base: BASE_DEC
diff --git a/codegen/test.proto b/codegen/test.proto
index 06fa303..2fd930b 100644
--- a/codegen/test.proto
+++ b/codegen/test.proto
@@ -34,6 +34,18 @@ struct Dummy {
uint16 dummy;
};
+message ArrayRaw {
+uint8 array[4];
+};
+
+message ArrayPrimitive {
+uint16 array[4];
+};
+
+message ArrayStruct {
+Dummy array[4];
+};
+
channel BaseChannel {
server:
message {
@@ -58,6 +70,10 @@ channel BaseChannel {
message {
Dummy struct;
} Struct1 = 1;
+
+ArrayPrimitive array_primitive = 100;
+ArrayRaw array_raw;
+ArrayStruct array_struct;
};
protocol Spice {
diff --git a/python_modules/dissector.py b/python_modules/dissector.py
index edc6dc3..caf817f 100644
---
[Spice-devel] [PATCH v3 34/51] Handle pointers
Read the pointer and if not 0 create a function to parse it and
call it.
Signed-off-by: Frediano Ziglio fzig...@redhat.com
---
python_modules/dissector.py | 49 +
1 file changed, 49 insertions(+)
diff --git a/python_modules/dissector.py b/python_modules/dissector.py
index 3d822cc..d623576 100644
--- a/python_modules/dissector.py
+++ b/python_modules/dissector.py
@@ -357,9 +357,58 @@ def write_array(writer, container, member, nelements,
array, dest, scope):
assert(element_type.is_struct())
write_struct(writer, member, element_type, index, dest, scope)
+
+def write_ptr_function(writer, target_type, container, member, dest, scope):
+nelements = ''
+if target_type.is_array():
+parse_function = parse_array_%s %
target_type.element_type.primitive_type()
+nelements = ', guint32 nelements'
+else:
+parse_function = parse_struct_%s % target_type.c_type()
+if writer.is_generated(parser, parse_function):
+return '%s(glb, %s, offset)' % (parse_function, dest.level.tree)
+
+writer.set_is_generated(parser, parse_function)
+
+writer = writer.function_helper()
+scope = writer.function(parse_function, guint32, GlobalInfo *glb _U_,
proto_tree *tree _U_%s, guint32 offset % nelements, True)
+
+writer.newline()
+
+dest = RootDestination(scope)
+dest.is_helper = True
+if target_type.is_array():
+write_array(writer, None, None, nelements, target_type, dest, scope)
+else:
+write_container_parser(writer, target_type, dest)
+
+writer.statement(return offset)
+
+writer.end_block()
+
+return '%s(glb, %s, offset)' % (parse_function, dest.level.tree)
+
+def read_ptr(writer, t):
+writer.assign('ptr', '%s(glb-tvb, offset)' % primitive_read_func(t))
+writer.increment('offset', str(t.get_fixed_nw_size()))
+
def write_pointer(writer, container, member, t, dest, scope):
assert(t.is_pointer())
+if not scope.variable_defined('ptr'):
+scope.variable_def('guint32', 'ptr')
+read_ptr(writer, t)
+with writer.if_block('ptr'):
+writer.variable_def('guint32', 'save_offset = offset')
+writer.assign('offset', 'ptr + glb-message_offset')
+if t.target_type.is_array():
+nelements = read_array_len(writer, member.name, t.target_type,
dest, scope, True)
+write_array(writer, container, member, nelements, t.target_type,
dest, scope)
+else:
+stmt = write_ptr_function(writer, t.target_type, container,
member, dest, scope)
+writer.statement(stmt)
+writer.assign('offset', 'save_offset')
+
def write_struct_func(writer, t, func_name, index):
func_name = 'dissect_spice_struct_' + t.name
--
2.1.0
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH v3 38/51] Use a class to register wireshark fields
Allow to reuse code and check better if field was already registered
Signed-off-by: Frediano Ziglio fzig...@redhat.com
---
python_modules/dissector.py | 58 +
1 file changed, 48 insertions(+), 10 deletions(-)
diff --git a/python_modules/dissector.py b/python_modules/dissector.py
index 5639baa..c88118f 100644
--- a/python_modules/dissector.py
+++ b/python_modules/dissector.py
@@ -21,8 +21,49 @@ def new_ett(writer):
return name
+# handle registration of wireshark flags
hf_writer = None
hf_defs = None
+class HF:
+fields = {}
+
+def __init__(self, hf_name, desc=''):
+self.hf_name = hf_name
+self.desc = desc
+self.mask = '0'
+
+def __str__(self):
+x = []
+for f in 'hf_name desc ws_name f_type base vals mask'.split():
+x.append(f + ':' + self.__dict__[f])
+return ' '.join(x)
+
+# declare a field identifier
+def add_wireshark_field(self):
+if hf_writer.variable_defined(self.hf_name):
+raise Exception('HF field %s already defined' % self.hf_name)
+hf_writer.variable_def(static int, %s = -1 % self.hf_name)
+
+def create(self):
+other = self.fields.get(self.ws_name)
+if other:
+for f in 'hf_name desc ws_name base vals mask'.split():
+if other.__dict__[f] != self.__dict__[f]:
+raise Exception('HF Field different from previous
for\n\t%s\n\t%s' % (other, self))
+if other.f_type != self.f_type:
+if other.f_type[:7] != 'FT_UINT' or self.f_type[:7] !=
'FT_UINT':
+raise Exception('HF Field different from previous
for\n\t%s\n\t%s' % (other, self))
+return
+
+self.fields[self.ws_name] = self
+
+self.add_wireshark_field()
+
+hf_defs.writeln('{ %s,' % self.hf_name)
+hf_defs.writeln(' { %s, spice2.%s,' % (self.desc, self.ws_name))
+hf_defs.writeln('%s, %s, %s, %s,' % (self.f_type, self.base,
self.vals, self.mask))
+hf_defs.writeln('NULL, HFILL }')
+hf_defs.writeln('},')
# handle wireshark attributes
@@ -306,16 +347,13 @@ def write_wireshark_field(writer, container, member, t,
ws, tree, size, encoding
writer.statement(%sproto_tree_add_item(%s, %s, glb-tvb, offset, %s, %s)
%
(prefix, tree, hf_name, size, encoding))
-# TODO handle better duplications
-if hf_writer.variable_defined(hf_name):
-return
-hf_writer.variable_def(static int, %s = -1 % hf_name)
-
-hf_defs.writeln('{ %s,' % hf_name)
-hf_defs.writeln(' { %s, spice2.%s,' % (desc, ws_name))
-hf_defs.writeln('%s, %s, %s, 0,' % (f_type, base, vals))
-hf_defs.writeln('NULL, HFILL }')
-hf_defs.writeln('},')
+# write definition
+hf = HF(hf_name, desc)
+hf.ws_name = ws_name
+hf.f_type = f_type
+hf.base = base
+hf.vals = vals
+hf.create()
# Note: during parsing, byte_size types have been converted to count during
validation
--
2.1.0
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH v3 11/51] Start adding code to generate wireshark dissector
Added a stub dissector.py code.
Added file to Makefiles.
Add option to call dissector and code to call it.
Signed-off-by: Frediano Ziglio fzig...@redhat.com
---
common/Makefile.am | 1 +
python_modules/Makefile.am | 1 +
python_modules/dissector.py | 14 ++
spice_codegen.py| 17 ++---
4 files changed, 30 insertions(+), 3 deletions(-)
create mode 100644 python_modules/dissector.py
diff --git a/common/Makefile.am b/common/Makefile.am
index b4384e8..5e1bffe 100644
--- a/common/Makefile.am
+++ b/common/Makefile.am
@@ -106,6 +106,7 @@ MARSHALLERS_DEPS = \
$(top_srcdir)/python_modules/marshal.py \
$(top_srcdir)/python_modules/ptypes.py \
$(top_srcdir)/python_modules/spice_parser.py\
+ $(top_srcdir)/python_modules/dissector.py \
$(top_srcdir)/spice_codegen.py \
$(NULL)
diff --git a/python_modules/Makefile.am b/python_modules/Makefile.am
index 50e1a71..5983d5b 100644
--- a/python_modules/Makefile.am
+++ b/python_modules/Makefile.am
@@ -7,6 +7,7 @@ PYTHON_MODULES =\
marshal.py \
ptypes.py \
spice_parser.py \
+ dissector.py\
$(NULL)
EXTRA_DIST = $(PYTHON_MODULES)
diff --git a/python_modules/dissector.py b/python_modules/dissector.py
new file mode 100644
index 000..90ba498
--- /dev/null
+++ b/python_modules/dissector.py
@@ -0,0 +1,14 @@
+
+from . import codegen
+
+def write_protocol_parser(writer, proto):
+pass
+
+def write_includes(writer):
+writer.newline()
+writer.writeln('#include epan/packet.h')
+writer.writeln('#include epan/conversation.h')
+writer.writeln('#include epan/expert.h')
+writer.newline()
+writer.writeln('#include packet-spice.h')
+writer.newline()
diff --git a/spice_codegen.py b/spice_codegen.py
index 84790af..8cfec1a 100755
--- a/spice_codegen.py
+++ b/spice_codegen.py
@@ -9,6 +9,7 @@ from python_modules import ptypes
from python_modules import codegen
from python_modules import demarshal
from python_modules import marshal
+from python_modules import dissector
import six
def write_channel_enums(writer, channel, client, describe):
@@ -110,7 +111,7 @@ parser.add_option(-e, --generate-enums,
action=store_true, dest=generate_enums, default=False,
help=Generate enums)
parser.add_option(-w, --generate-wireshark-dissector,
- action=store_true, dest=generate_dissector,
default=False,
+ action=store_true, dest=generate_enum_dissector,
default=False,
help=Generate Wireshark dissector definitions)
parser.add_option(-d, --generate-demarshallers,
action=store_true, dest=generate_demarshallers,
default=False,
@@ -118,6 +119,9 @@ parser.add_option(-d, --generate-demarshallers,
parser.add_option(-m, --generate-marshallers,
action=store_true, dest=generate_marshallers,
default=False,
help=Generate message marshallers)
+parser.add_option(--generate-dissector,
+ action=store_true, dest=generate_dissector,
default=False,
+ help=Generate dissector)
parser.add_option(-P, --private-marshallers,
action=store_true, dest=private_marshallers,
default=False,
help=Generate private message marshallers)
@@ -213,8 +217,15 @@ if options.includes:
writer.header.writeln('#include %s' % i)
writer.writeln('#include %s' % i)
-if options.generate_enums or options.generate_dissector:
-write_enums(writer, options.generate_dissector)
+if options.generate_enums or options.generate_enum_dissector:
+write_enums(writer, options.generate_enum_dissector)
+
+if options.generate_dissector:
+if options.generate_demarshallers:
+print sys.stderr, You can't specify --generate-demarshallers with
--generate-dissector
+sys.exit(1)
+dissector.write_includes(writer)
+dissector.write_protocol_parser(writer, proto)
if options.generate_demarshallers:
if not options.server and not options.client:
--
2.1.0
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH v3 03/51] codegen: Fix typo in variable name
Signed-off-by: Frediano Ziglio fzig...@redhat.com --- python_modules/codegen.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python_modules/codegen.py b/python_modules/codegen.py index f324498..55500b7 100644 --- a/python_modules/codegen.py +++ b/python_modules/codegen.py @@ -193,7 +193,7 @@ class CodeWriter: def unindent(self): self.indentation -= 4 if self.indentation 0: -self.indenttation = 0 +self.indentation = 0 def begin_block(self, prefix= , comment = ): if len(prefix) 0: -- 2.1.0 ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH v3 47/51] Improve agent dissectors
Before was dump as raw data
Signed-off-by: Frediano Ziglio fzig...@redhat.com
---
spice.proto | 128
1 file changed, 120 insertions(+), 8 deletions(-)
diff --git a/spice.proto b/spice.proto
index fe0eb34..880a8be 100644
--- a/spice.proto
+++ b/spice.proto
@@ -212,6 +212,124 @@ struct DstInfo {
uint8 *cert_subject_data[cert_subject_size] @zero_terminated @marshall
@ws(data, data) @ws_type(BYTES);
} @ctype(SpiceMigrationDstInfo);
+enum32 agent_type {
+MOUSE_STATE = 1,
+MONITORS_CONFIG,
+REPLY,
+CLIPBOARD,
+DISPLAY_CONFIG,
+ANNOUNCE_CAPABILITIES,
+CLIPBOARD_GRAB,
+CLIPBOARD_REQUEST,
+CLIPBOARD_RELEASE,
+FILE_XFER_START,
+FILE_XFER_STATUS,
+FILE_XFER_DATA,
+CLIENT_DISCONNECTED,
+END_MESSAGE
+} @ws(Agent message type, agent_message_type) @prefix(VS_AGENT_);
+
+flags16 mouse_button_mask {
+LEFT,
+MIDDLE,
+RIGHT
+} @ws(Mouse button state, button_state) @ws_base(DEC);
+
+struct AgentMouseState {
+Point point;
+mouse_button_mask buttons_state;
+uint8 display_id @ws(Mouse display ID, mouse_display_id);
+};
+
+struct AgentMonitorConfig {
+uint32 height @ws(Height, agent_monitor_height);
+uint32 width @ws(Width, agent_monitor_width);
+uint32 depth @ws(Depth, agent_monitor_depth);
+uint32 x @ws(x, agent_monitor_x);
+uint32 y @ws(y, agent_monitor_y);
+} @ws_txt_n(Monitor Config #%u, INDEX);
+
+enum32 agent_reply_error {
+SUCCESS = 0,
+ERROR
+} @prefix(WSVD_AGENT_) @ws(Error, vd_agent_reply_error);
+
+flags32 agent_caps {
+MOUSE_STATE @ws(Mouse State, vd_agent_cap_mouse_state),
+MONITORS_CONFIG @ws(Monitors config, vd_agent_cap_monitors_config),
+REPLY @ws(Reply, vd_agent_cap_reply),
+CLIPBOARD @ws(Clipboard, vd_agent_cap_clipboard),
+DISPLAY_CONFIG @ws(Display config, vd_agent_cap_display_config),
+CLIPBOARD_BY_DEMAND @ws(Clipboard by demand,
vd_agent_cap_clipboard_by_demand),
+CLIPBOARD_SELECTION @ws(Clipboard selection,
vd_agent_cap_clipboard_selection),
+SPARSE_MONITORS_CONFIG @ws(Sparse monitors config,
vd_agent_cap_sparse_monitors_config),
+GUEST_LINEEND_LF @ws(Guest line-end LF, vd_agent_cap_guest_lineend_lf),
+GUEST_LINEEND_CRL @ws(Guest line-end CRLF,
vd_agent_cap_guest_lineend_crlf)
+} @prefix(WSVD_AGENT_CAP_);
+
+struct AgentMonitorsConfig {
+uint32 num_monitors @ws(Number of monitors, agent_num_monitors);
+uint32 use_position @ws(Use position,
vd_agent_monitors_config_flag_use_pos) @ws_type(BOOLEAN);
+AgentMonitorConfig configs[num_monitors];
+};
+
+struct AgentReply {
+uint32 type @ws(Type, vd_agent_reply_type);
+agent_reply_error error;
+};
+
+struct AgentCapabilities {
+uint32 request @ws(Request, vd_agent_caps_request);
+agent_caps reply;
+};
+
+struct AgentClipboardGrab {
+uint8 selection @ws(Agent clipboard selection,
main_agent_clipboard_selection);
+uint8 reserved[3];
+};
+
+enum32 agent_clipboard_type {
+NONE = 0,
+UTF8_TEXT,
+IMAGE_PNG,
+IMAGE_BMP,
+IMAGE_TIFF,
+IMAGE_JPG
+} @ws(Agent clipboard type, main_agent_clipboard_type)
@prefix(WSVD_AGENT_CLIPBOARD_);
+
+struct AgentClipboardRequest {
+uint8 selection @ws(Agent clipboard selection,
main_agent_clipboard_selection);
+uint8 reserved[3];
+agent_clipboard_type type;
+};
+
+message AgentData {
+uint32 protocol @ws(Agent protocol version, main_agent_protocol);
+agent_type type;
+uint64 opaque @ws(Agent opaque, main_agent_opaque);
+uint32 size @ws(Agent message size, main_agent_size);
+switch (type) {
+case MOUSE_STATE:
+ AgentMouseState mouse_state;
+case MONITORS_CONFIG:
+ AgentMonitorsConfig monitors_config;
+case REPLY:
+ AgentReply reply;
+case CLIPBOARD:
+ Data text;
+case DISPLAY_CONFIG:
+ uint32 config;
+case ANNOUNCE_CAPABILITIES:
+ AgentCapabilities capabilities;
+case CLIPBOARD_GRAB:
+ AgentClipboardGrab grab;
+case CLIPBOARD_REQUEST:
+ AgentClipboardRequest request;
+case CLIPBOARD_RELEASE:
+ Empty release;
+} u @anon;
+};
+
channel MainChannel : BaseChannel {
server:
message {
@@ -251,7 +369,7 @@ channel MainChannel : BaseChannel {
link_err error_code @ws(spice ERROR, error_code);
} @ctype(SpiceMsgMainAgentDisconnect) agent_disconnected;
-Data agent_data;
+Data agent_data @ws_as(AgentData);
message {
uint32 num_tokens @ws(Agent token, main_agent_token);
@@ -308,7 +426,7 @@ channel MainChannel : BaseChannel {
uint32 num_tokens @ws(Agent tokens, main_agent_tokens);
} agent_start;
-Data agent_data;
+Data agent_data @ws_as(AgentData);
message {
uint32 num_tokens @ws(Agent token, main_agent_token);
@@ -970,12 +1088,6 @@ enum8 mouse_button {
DOWN,
};
-flags16 mouse_button_mask {
-LEFT,
-MIDDLE,
-RIGHT
-} @ws(Mouse button state,
[Spice-devel] [PATCH v3 43/51] Add some format checks with arrays
Check different format or small arrays formatting.
Is possible to omit to have a tree and dump just the elements.
This is useful for small arrays or if is possible to format all
the item (for instance having a small structure) in a single line.
Signed-off-by: Frediano Ziglio fzig...@redhat.com
---
codegen/Makefile.am | 1 +
codegen/check_dissector | 2 ++
codegen/out_array2.txt | 28
codegen/test.proto | 8
4 files changed, 39 insertions(+)
create mode 100644 codegen/out_array2.txt
diff --git a/codegen/Makefile.am b/codegen/Makefile.am
index 15e277e..e169d88 100644
--- a/codegen/Makefile.am
+++ b/codegen/Makefile.am
@@ -58,6 +58,7 @@ EXTRA_DIST = \
data_empty \
out_empty.txt \
data_base1 \
+ out_array2.txt \
out_base1.txt \
out_struct1.txt \
data_u16s \
diff --git a/codegen/check_dissector b/codegen/check_dissector
index 94ce0bc..86b78eb 100755
--- a/codegen/check_dissector
+++ b/codegen/check_dissector
@@ -65,4 +65,6 @@ check data_base1 1 2 out_channel.txt
# flags and descriptions
check data_base1 1 3 out_flags1.txt
+check data_u16s 1 103 out_array2.txt --client
+
exit 0
diff --git a/codegen/out_array2.txt b/codegen/out_array2.txt
new file mode 100644
index 000..f695ca1
--- /dev/null
+++ b/codegen/out_array2.txt
@@ -0,0 +1,28 @@
+--- tree
+--- item
+Text: small[0]: 0
+Name: small
+Abbrev: spice2.auto.Array2_array_small
+Type: FT_UINT16
+Base: BASE_DEC
+--- item
+Text: small[1]: 1
+Name: small
+Abbrev: spice2.auto.Array2_array_small
+Type: FT_UINT16
+Base: BASE_DEC
+--- item
+Text: Small array
+--- tree
+--- item
+Text: small_named[0]: 2
+Name: small_named
+Abbrev: spice2.auto.Array2_array_small_named
+Type: FT_UINT16
+Base: BASE_DEC
+--- item
+Text: small_named[1]: 3
+Name: small_named
+Abbrev: spice2.auto.Array2_array_small_named
+Type: FT_UINT16
+Base: BASE_DEC
diff --git a/codegen/test.proto b/codegen/test.proto
index 4eaa858..7bba890 100644
--- a/codegen/test.proto
+++ b/codegen/test.proto
@@ -59,6 +59,13 @@ message ArrayStruct {
Dummy array4[4];
};
+message Array2 {
+/* small arrays, format with text and numbers, no tree generated */
+uint16 @ws_txt(small[%u]: %u, INDEX, small) small[2];
+/* small arrays, format with text and numbers, tree generated due to
description */
+uint16 @ws_txt(small_named[%u]: %u, INDEX, small_named) small_named[2]
@ws_desc(Small array);
+};
+
channel BaseChannel {
server:
message {
@@ -96,6 +103,7 @@ channel BaseChannel {
ArrayPrimitive array_primitive = 100;
ArrayRaw array_raw;
ArrayStruct array_struct;
+Array2 array2;
};
channel Test1Channel: BaseChannel {
--
2.1.0
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH v3 45/51] Allows to specify wireshark name for enumerators
Signed-off-by: Frediano Ziglio fzig...@redhat.com
---
python_modules/dissector.py| 2 +-
python_modules/ptypes.py | 91 +++---
python_modules/spice_parser.py | 2 +-
3 files changed, 35 insertions(+), 60 deletions(-)
diff --git a/python_modules/dissector.py b/python_modules/dissector.py
index 3f63341..5c6b6fc 100644
--- a/python_modules/dissector.py
+++ b/python_modules/dissector.py
@@ -737,7 +737,7 @@ def write_flags_func(writer, t, ws, tree, ti):
desc = t.descs[v] if t.descs[v] else t.names[v]
hf = HF(name, desc)
-hf.ws_name = '%s_%s' % (t.name, t.names[v].lower())
+hf.ws_name = '%s_%s' % (t.name, t.names[v].lower()) if not
t.ws_names[v] else t.ws_names[v]
hf.f_type = 'FT_BOOLEAN'
hf.base = str(bits)
hf.vals = 'TFS(tfs_set_notset)'
diff --git a/python_modules/ptypes.py b/python_modules/ptypes.py
index a899b6c..8c92493 100644
--- a/python_modules/ptypes.py
+++ b/python_modules/ptypes.py
@@ -321,6 +321,39 @@ class TypeAlias(Type):
return self.name
class EnumBaseType(Type):
+def __init__(self, bits, name, enums, attribute_list):
+Type.__init__(self)
+self.bits = bits
+self.name = name
+
+last = -1
+names = {}
+values = {}
+descs = {}
+ws_names = {}
+for v in enums:
+name = v[0]
+desc = v[1][1]
+ws_name = None if len(v[1]) 3 else v[1][2]
+if len(v) 2:
+value = v[2]
+else:
+value = last + 1
+last = value
+
+assert value not in names
+names[value] = name
+descs[value] = desc
+ws_names[value] = ws_name
+values[name] = value
+
+self.names = names
+self.values = values
+self.descs = descs
+self.ws_names = ws_names
+
+self.attributes = fix_attributes(attribute_list)
+
def is_enum(self):
return isinstance(self, EnumType)
@@ -365,35 +398,6 @@ class EnumBaseType(Type):
class EnumType(EnumBaseType):
-def __init__(self, bits, name, enums, attribute_list):
-Type.__init__(self)
-self.bits = bits
-self.name = name
-
-last = -1
-names = {}
-values = {}
-descs = {}
-for v in enums:
-name = v[0]
-desc = v[1][1]
-if len(v) 2:
-value = v[2]
-else:
-value = last + 1
-last = value
-
-assert value not in names
-names[value] = name
-descs[value] = desc
-values[name] = value
-
-self.names = names
-self.values = values
-self.descs = descs
-
-self.attributes = fix_attributes(attribute_list)
-
def __str__(self):
return enum %s % self.name
@@ -422,35 +426,6 @@ class EnumType(EnumBaseType):
writer.newline()
class FlagsType(EnumBaseType):
-def __init__(self, bits, name, flags, attribute_list):
-Type.__init__(self)
-self.bits = bits
-self.name = name
-
-last = -1
-names = {}
-values = {}
-descs = {}
-for v in flags:
-name = v[0]
-desc = v[1][1]
-if len(v) 2:
-value = v[2]
-else:
-value = last + 1
-last = value
-
-assert value not in names
-names[value] = name
-descs[value] = desc
-values[name] = value
-
-self.names = names
-self.values = values
-self.descs = descs
-
-self.attributes = fix_attributes(attribute_list)
-
def __str__(self):
return flags %s % self.name
diff --git a/python_modules/spice_parser.py b/python_modules/spice_parser.py
index 5326e59..3fde843 100644
--- a/python_modules/spice_parser.py
+++ b/python_modules/spice_parser.py
@@ -124,7 +124,7 @@ def SPICE_BNF():
int32_ ^ uint32_ ^ int64_ ^ uint64_ ^
typename).setName(type)
-flagsBody = enumBody = Group(lbrace + delimitedList(Group (enumname +
Optional(Group(ws_desc), default=[None,None]) + Optional(equals + integer))) +
Optional(comma) + rbrace)
+flagsBody = enumBody = Group(lbrace + delimitedList(Group (enumname +
Optional(Group(ws | ws_desc), default=[None,None]) + Optional(equals +
integer))) + Optional(comma) + rbrace)
messageSpec = Group(message_ + messageBody +
attributes).setParseAction(lambda toks: ptypes.MessageType(None, toks[0][1],
toks[0][2])) | typename
--
2.1.0
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH v3 07/51] codegen: Do some check on attributes
Verify that the attribute is known. This could help for instance to
avoid some future typo mistake.
Also we have a list of attributes we can comment.
Signed-off-by: Frediano Ziglio fzig...@redhat.com
---
python_modules/ptypes.py | 72
1 file changed, 72 insertions(+)
diff --git a/python_modules/ptypes.py b/python_modules/ptypes.py
index 845fa73..d8b6c90 100644
--- a/python_modules/ptypes.py
+++ b/python_modules/ptypes.py
@@ -62,11 +62,79 @@ class FixedSize:
# other members
propagated_attributes=[ptr_array, nonnull, chunk]
+valid_attributes={
+# embedded/appended at the end of the structure
+'end',
+# the C structure contains a pointer to data
+# for instance we want to write an array to an allocated array
+'to_ptr',
+# write output to this C structure
+'ctype',
+# prefix for flags/values enumerations
+'prefix',
+# use in demarshaller to use directly data from message without copy
+'nocopy',
+# store member array in a pointer
+# similar to to_ptr but has an additional argument as C field to
+# store length
+'as_ptr',
+# do not generate marshal code
+# used for last members to be able to marshall them manually
+'nomarshal',
+# ??? not used by python code
+'zero_terminated',
+'marshall',
+# this pointer member cannot be null
+'nonnull',
+# this flag member contains only a single flag
+'unique_flag',
+'ptr_array',
+'outvar',
+# C structure has an anonymous member (used in switch)
+'anon',
+'chunk',
+# this channel is contained in an #ifdef section
+# the argument specify the preprocessor define to check
+'ifdef',
+# write this member as zero on network
+'zero',
+# specify minor version required for these members
+'minor',
+# this member contains the byte count for an array.
+# the argument is the member name for item count (not bytes)
+'bytes_count',
+# this attribute does not exists on the network, fill just structure with
the value
+'virtual',
+# for a switch this indicates that on network
+# will occupy always same size (maximum size required for all members)
+'fixedsize',
+# use 32 bit pointer
+'ptr32',
+}
+
+attributes_with_arguments={
+'ctype',
+'prefix',
+'as_ptr',
+'outvar',
+'ifdef',
+'minor',
+'bytes_count',
+'virtual',
+}
+
def fix_attributes(attribute_list):
attrs = {}
for attr in attribute_list:
name = attr[0][1:]
lst = attr[1:]
+if not name in valid_attributes:
+raise Exception(Attribute %s not recognized % name)
+if not name in attributes_with_arguments:
+if len(lst) 0:
+raise Exception(Attribute %s specified with options % name)
+elif len(lst) 1:
+raise Exception(Attribute %s has more than 1 argument % name)
attrs[name] = lst
return attrs
@@ -139,6 +207,8 @@ class Type:
_types_by_name[self.name] = self
def has_attr(self, name):
+if not name in valid_attributes:
+raise Exception('attribute %s not expected' % name)
return name in self.attributes
class TypeRef(Type):
@@ -522,6 +592,8 @@ class Containee:
return not self.is_switch() and self.member_type.is_primitive()
def has_attr(self, name):
+if not name in valid_attributes:
+raise Exception('attribute %s not expected' % name)
return name in self.attributes
def has_minor_attr(self):
--
2.1.0
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH v3 01/51] codegen: Import six module before first use
The module was used in the initial try/except so make sure it is already imported. Signed-off-by: Frediano Ziglio fzig...@redhat.com --- python_modules/spice_parser.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/python_modules/spice_parser.py b/python_modules/spice_parser.py index d60bb10..80b559b 100644 --- a/python_modules/spice_parser.py +++ b/python_modules/spice_parser.py @@ -1,3 +1,5 @@ +import six + try: from pyparsing import Literal, CaselessLiteral, Word, OneOrMore, ZeroOrMore, \ Forward, delimitedList, Group, Optional, Combine, alphas, nums, restOfLine, cStyleComment, \ @@ -8,7 +10,6 @@ except ImportError: from . import ptypes -import six import sys cvtInt = lambda toks: int(toks[0]) -- 2.1.0 ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH v3 02/51] codegen: Simplify if/else blocks
Blocks was mainly the same, reduce code. Signed-off-by: Frediano Ziglio fzig...@redhat.com --- python_modules/marshal.py | 29 +++-- 1 file changed, 11 insertions(+), 18 deletions(-) diff --git a/python_modules/marshal.py b/python_modules/marshal.py index b77b910..1d38d3d 100644 --- a/python_modules/marshal.py +++ b/python_modules/marshal.py @@ -380,25 +380,18 @@ def write_protocol_marshaller(writer, proto, is_server, private_marshallers): writer.ifdef(channel.attributes[ifdef][0]) writer.header.ifdef(channel.attributes[ifdef][0]) if is_server: -for m in channel.client_messages: -message = m.message_type -f = write_message_marshaller(writer, message, is_server, private_marshallers) -if channel.has_attr(ifdef) and f not in functions: -functions[f] = channel.attributes[ifdef][0] -elif message.has_attr(ifdef) and f not in functions: -functions[f] = message.attributes[ifdef][0] -else: -functions[f] = True +messages = channel.client_messages else: -for m in channel.server_messages: -message = m.message_type -f = write_message_marshaller(writer, message, is_server, private_marshallers) -if channel.has_attr(ifdef) and f not in functions: -functions[f] = channel.attributes[ifdef][0] -elif message.has_attr(ifdef) and f not in functions: -functions[f] = message.attributes[ifdef][0] -else: -functions[f] = True +messages = channel.server_messages +for m in messages: +message = m.message_type +f = write_message_marshaller(writer, message, is_server, private_marshallers) +if channel.has_attr(ifdef) and f not in functions: +functions[f] = channel.attributes[ifdef][0] +elif message.has_attr(ifdef) and f not in functions: +functions[f] = message.attributes[ifdef][0] +else: +functions[f] = True if channel.has_attr(ifdef): writer.endif(channel.attributes[ifdef][0]) writer.header.endif(channel.attributes[ifdef][0]) -- 2.1.0 ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH v3 04/51] codegen: Optimize code indentation avoiding loop
Signed-off-by: Frediano Ziglio fzig...@redhat.com --- python_modules/codegen.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/python_modules/codegen.py b/python_modules/codegen.py index 55500b7..02ffdb9 100644 --- a/python_modules/codegen.py +++ b/python_modules/codegen.py @@ -130,8 +130,7 @@ class CodeWriter: return if self.at_line_start: -for i in range(self.indentation): -self.out.write(u ) +self.out.write(u * self.indentation) self.at_line_start = False self.out.write(s) return self -- 2.1.0 ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH v3 09/51] codegen: Check we don't pop too much indexes
--- python_modules/codegen.py | 1 + 1 file changed, 1 insertion(+) diff --git a/python_modules/codegen.py b/python_modules/codegen.py index 02ffdb9..c470988 100644 --- a/python_modules/codegen.py +++ b/python_modules/codegen.py @@ -357,6 +357,7 @@ class CodeWriter: return index def push_index(self): +assert self.current_index 0 self.current_index = self.current_index - 1 class Index: -- 2.1.0 ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH v3 16/51] Allows to specify some new attributes for wireshark
To make output more useful fields from the protocol should have
additional information like description, name, type and so on.
List of attributes added:
- ws_desc, just a simple description
- ws_name name of the field. See below
- ws allow to specify a description and a name. Useful as easy to
type. If you have a name there is no sense to have no description
- ws_type allows to override type detected, for instance to
specify is a boolean instead of an int
- ws_base like ws_type but for base (hexadecimal instead of decimal)
- ws_txt and ws_txt_n allows to specify formatting. The difference
between formatting and description is that description is static
while these new attributes contain a formatting string as first
argument followed by arguments representing fields or INDEX for
the index if we are dumping a structure contained in an array.
The distinction between ws_txt and ws_txt_n is the item contained
in an array or not
- ws_inline, this is tricky attribute. It allow to embed structure
dissecting in the same function. This allow format string and other
field (for instance switch or array sizes) to be seen by current
generated function
- ws_as, dissect this structure as another type. Useful to avoid
changing the protocol but show a slightly modified version.
This is used for instance to show two fields like x and y as a
single point. Could also be used to dump a binary data with
more detail but avoid to change marshalling/demarshalling
These attributes required some changes in the parser as previously
arguments could be only integers and identifiers while they require
string and multiple identifiers (like this.that).
In wireshark names are important as they can be used to do
queries about packet with specific features.
Signed-off-by: Frediano Ziglio fzig...@redhat.com
---
python_modules/ptypes.py | 29 +++--
python_modules/spice_parser.py | 13 +++--
2 files changed, 38 insertions(+), 4 deletions(-)
diff --git a/python_modules/ptypes.py b/python_modules/ptypes.py
index cc6960f..bbf158e 100644
--- a/python_modules/ptypes.py
+++ b/python_modules/ptypes.py
@@ -108,6 +108,26 @@ valid_attributes={
# for a switch this indicates that on network
# will occupy always same size (maximum size required for all members)
'fixedsize',
+
+# description
+'ws_desc',
+# field name
+'ws_name',
+# combine description + name
+# name will be used for filtering
+'ws',
+# type (BOOLEAN, STRINGZ)
+'ws_type',
+# force wireshark base
+'ws_base',
+# text to use for format, you can specify parameters
+'ws_txt',
+'ws_txt_n',
+# put structure not in a separate function
+# used to be able to retrieve fields from parent
+'ws_inline',
+# handle this container as another type
+'ws_as',
}
attributes_with_arguments={
@@ -119,6 +139,13 @@ attributes_with_arguments={
'minor',
'bytes_count',
'virtual',
+'ws',
+'ws_desc',
+'ws_type',
+'ws_base',
+'ws_txt',
+'ws_txt_n',
+'ws_as',
}
def fix_attributes(attribute_list):
@@ -131,8 +158,6 @@ def fix_attributes(attribute_list):
if not name in attributes_with_arguments:
if len(lst) 0:
raise Exception(Attribute %s specified with options % name)
-elif len(lst) 1:
-raise Exception(Attribute %s has more than 1 argument % name)
attrs[name] = lst
return attrs
diff --git a/python_modules/spice_parser.py b/python_modules/spice_parser.py
index 97af8b2..a0f969a 100644
--- a/python_modules/spice_parser.py
+++ b/python_modules/spice_parser.py
@@ -3,7 +3,7 @@ import six
try:
from pyparsing import Literal, CaselessLiteral, Word, OneOrMore,
ZeroOrMore, \
Forward, delimitedList, Group, Optional, Combine, alphas, nums,
restOfLine, cStyleComment, \
-alphanums, ParseException, ParseResults, Keyword, StringEnd,
replaceWith
+alphanums, ParseException, ParseResults, Keyword, StringEnd,
replaceWith, QuotedString
except ImportError:
six.print_(Module pyparsing not found.)
exit(1)
@@ -77,20 +77,29 @@ def SPICE_BNF():
switch_= Keyword(switch)
default_ = Keyword(default)
case_ = Keyword(case)
+ws_= Keyword(@ws)
+ws_txt_= Keyword(@ws_txt) | Keyword(@ws_txt_n)
+ws_desc_ = Keyword(@ws_desc)
identifier = Word( alphas, alphanums + _ )
+multi_identifier = Word( alphas, alphanums + _. )
enumname = Word( alphanums + _ )
integer = ( Combine( CaselessLiteral(0x) + Word( nums+abcdefABCDEF
) ) |
Word( nums++-, nums )
).setName(int).setParseAction(cvtInt)
+string = QuotedString(quoteChar='', escChar='\\')
+
typename = identifier.copy().setParseAction(lambda toks :
ptypes.TypeRef(str(toks[0])))
# This is just normal types, i.e. not
[Spice-devel] [PATCH v3 33/51] Implement ws_inline attribute
This attribute allow structure to be aligned instead of be contained
in a separate function.
This is helpful as variable are declared in the function so allows
other member to reference to a nested structure.
Signed-off-by: Frediano Ziglio fzig...@redhat.com
---
python_modules/dissector.py | 11 ---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/python_modules/dissector.py b/python_modules/dissector.py
index b204c61..3d822cc 100644
--- a/python_modules/dissector.py
+++ b/python_modules/dissector.py
@@ -378,9 +378,14 @@ def write_struct_func(writer, t, func_name, index):
def write_struct(writer, member, t, index, dest, scope):
assert(t.is_struct())
-func_name = 'dissect_spice_struct_' + t.name
-write_struct_func(writer, t, func_name, index)
-writer.assign('offset', '%s(glb, %s, offset, %s)' % (func_name,
dest.level.tree, index))
+if member.has_attr('ws_inline'):
+dest = dest.child_sub(member.name, scope)
+with writer.block() as scope:
+write_container_parser(writer, t, dest)
+else:
+func_name = 'dissect_spice_struct_' + t.name
+write_struct_func(writer, t, func_name, index)
+writer.assign('offset', '%s(glb, %s, offset, %s)' % (func_name,
dest.level.tree, index))
def write_member_primitive(writer, container, member, t, dest, scope):
assert(t.is_primitive())
--
2.1.0
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH v3 32/51] Handle switch
Signed-off-by: Frediano Ziglio fzig...@redhat.com --- python_modules/dissector.py | 41 - 1 file changed, 40 insertions(+), 1 deletion(-) diff --git a/python_modules/dissector.py b/python_modules/dissector.py index caf817f..b204c61 100644 --- a/python_modules/dissector.py +++ b/python_modules/dissector.py @@ -298,8 +298,47 @@ def read_array_len(writer, prefix, array, dest, scope, is_ptr): dest.write_ref(writer, 32, prefix+'.nelements', nelements) return nelements + def write_switch(writer, container, switch, dest, scope): -pass +var = container.lookup_member(switch.variable) +var_type = var.member_type + +if switch.has_attr(fixedsize): +scope.variable_def(guint32, save_output) +writer.assign(save_output, output) + +first = True +for c in switch.cases: +check = c.get_check(dest.read_ref(switch.variable), var_type) +m = c.member +with writer.if_block(check, not first, False) as block: +t = m.member_type +if switch.has_attr(anon): +dest2 = dest +else: +if t.is_struct(): +dest2 = dest.child_sub(switch.name + . + m.name, block) +else: +dest2 = dest.child_sub(switch.name, block) + +if t.is_pointer(): +write_pointer(writer, container, m, t, dest2, block) +elif t.is_struct(): +write_struct(writer, m, t, '-1', dest2, scope) +elif t.is_primitive(): +write_member_primitive(writer, container, m, t, dest2, scope) +elif t.is_array(): +nelements = read_array_len(writer, m.name, t, dest, block, False) +write_array(writer, container, m, nelements, t, dest2, block) +else: +writer.todo(Can't handle type %s % m.member_type) + +first = False + +writer.newline() + +if switch.has_attr(fixedsize): +writer.assign(output, save_output + %s % switch.get_fixed_nw_size()) def write_array(writer, container, member, nelements, array, dest, scope): assert(container and member) -- 2.1.0 ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH v3 00/51] Implements partial wireshark dissector from protocol specifications
As we have a file to specify the protocol and as is hard to align wireshark dissector for each change we made I'm trying to do part of this job to a code generator. The idea is to have the dissector split in two part, one hand written and the other automatic. I tested that changing the code and protocol file the generated code for marshalling/unmarshalling is still the same. I have a small network capture to check that output is similar to current dissector. I tried to copy wireshark field names, formatting and descriptions. A known issue is that the protocol did not contains some details for images and VDAgent so these packet could look a bit poor. First patches marked as codegen: are not really related to this set but just minor changes. The protocol file is mainly decorated with additional attributes (all starting with ws), see patch Allows to specify some new attributes for wireshark. I would like to have some comment on implementation, the attributes used or anothing you can think of. Changes since v2: Patches changes: - generate packet-spice.h automatically from spice.proto; - additional checks for flags and arrays; - use proper index using ws_txt_n attribute; - do not override text to wrong tree if there are nested level; - support ws_txt and ws_txt_n attribute on flags; - updated some comment on attributes (Christophe); - replace some tabs with spaces as all code is indented with spaces (Christophe); - updated some description (similar to old dissector). Patches added: - check that we used all strings (ws_desc and so on) in spice.proto; - allow to specify a field name for enumerators; - allow to specify ws_as attribute for messages; - additional tests; - agent dissecting; - initial image (now Quic) dissecting. Changes from v1: - better generation of header file; - full support for ws_txt and ws_txt_n attributes; - generate tree, not only flat list of endless items; - automated tests during patches. Frediano Ziglio (51): codegen: Import six module before first use codegen: Simplify if/else blocks codegen: Fix typo in variable name codegen: Optimize code indentation avoiding loop codegen: Remove duplicate variable initialization codegen: Reuse code to fix attribute from prototype file codegen: Do some check on attributes codegen: Remove old ptr32 attribute codegen: Check we don't pop too much indexes codegen: Allows to specify C type for index variable Start adding code to generate wireshark dissector Generate some definition for dissector Add new_ett function to be able to create new trees Decorate writer class to make easier ifdef/endif handling Generate scheleton for messages and channels Allows to specify some new attributes for wireshark Allows to specify descriptions for enumerations Allows to specify attributes for array items and pointers Decorate protocol file with attributes for wireshark Change code generated index type Add code to handle destination variable Parse containers Write function to write members Read values from primitive fields test: Allows to write items to tree and dump saved tree test: Add code to read input from a file (or stdin) Handle base fields show primitive as primitive Read array size Generate code to output parse structure Handle array Handle switch Implement ws_inline attribute Handle pointers Introduce a class to handle wireshark attributes Allow to override default values generated for the fields Allow to specify 'CHANNEL' as type Use a class to register wireshark fields Allows to have two type with different size to point to same field name Handle flags Handle text formatting of different elements Test decorated array Add some format checks with arrays test: Add check on output strings Allows to specify wireshark name for enumerators Allows to specify ws_as attribute for messages Improve agent dissectors Test we don't override text handling other fields Allow ws_as attribute on switch members Allow to use bytes_count and bytes array length in dissector Describe Quic image format from dissector Makefile.am |2 +- codegen/Makefile.am | 74 +++ codegen/check_dissector | 71 +++ codegen/check_strings | 33 ++ codegen/data_base1 | Bin 0 - 44 bytes codegen/data_empty |0 codegen/data_struct2| Bin 0 - 9 bytes codegen/data_u16s | Bin 0 - 2000 bytes codegen/dissector_test.c| 660 +++ codegen/out_array2.txt | 28 + codegen/out_array_primitive.txt | 110 codegen/out_array_raw.txt | 13 + codegen/out_array_struct.txt| 158 ++ codegen/out_base1.txt | 148 ++ codegen/out_channel.txt |7 + codegen/out_empty.txt |1 + codegen/out_flags1.txt | 107 codegen/out_struct1.txt | 10 + codegen/out_struct2.txt |
[Spice-devel] [PATCH v3 10/51] codegen: Allows to specify C type for index variable
This to prepare to generate wireshark dissector which use
glib types instead of new C ones (for compatibility with some
compiler).
Signed-off-by: Frediano Ziglio fzig...@redhat.com
---
python_modules/codegen.py | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/python_modules/codegen.py b/python_modules/codegen.py
index c470988..f7a2048 100644
--- a/python_modules/codegen.py
+++ b/python_modules/codegen.py
@@ -81,6 +81,7 @@ class CodeWriter:
self.has_error_check = False
self.options = {}
self.function_helper_writer = None
+self.index_type = 'uint32_t'
def set_option(self, opt, value = True):
self.options[opt] = value
@@ -113,6 +114,7 @@ class CodeWriter:
self.contents.append(self.out)
writer.indentation = self.indentation
writer.at_line_start = self.at_line_start
+writer.index_type = self.index_type
writer.generated = self.generated
writer.options = self.options
writer.public_prefix = self.public_prefix
@@ -353,7 +355,7 @@ class CodeWriter:
def pop_index(self):
index = self.indexes[self.current_index]
self.current_index = self.current_index + 1
-self.add_function_variable(uint32_t, index)
+self.add_function_variable(self.index_type, index)
return index
def push_index(self):
--
2.1.0
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH v3 24/51] Read values from primitive fields
Store into references for future usage.
Signed-off-by: Frediano Ziglio fzig...@redhat.com
---
python_modules/dissector.py | 32
1 file changed, 32 insertions(+)
diff --git a/python_modules/dissector.py b/python_modules/dissector.py
index cd653b3..da89def 100644
--- a/python_modules/dissector.py
+++ b/python_modules/dissector.py
@@ -166,6 +166,29 @@ class SubDestination(Destination):
return self.parent_dest.get_ref(self.member + . + member, writer)
+def primitive_read_func(t):
+assert(t.is_primitive())
+
+signed = False
+if isinstance(t, ptypes.IntegerType) and t.signed:
+signed = True
+
+size = t.get_fixed_nw_size()
+if size == 1:
+if signed:
+return '(gint8) tvb_get_guint8'
+return 'tvb_get_guint8'
+elif size == 2:
+if signed:
+return '(gint32) (gint16) tvb_get_letohs'
+return 'tvb_get_letohs'
+elif size == 4:
+return 'tvb_get_letohl'
+elif size == 8:
+return 'tvb_get_letoh64'
+raise NotImplementedError('primitive size not supported %s %s' % (size, t))
+
+
def write_switch(writer, container, switch, dest, scope):
pass
@@ -181,6 +204,15 @@ def write_struct(writer, member, t, index, dest, scope):
def write_member_primitive(writer, container, member, t, dest, scope):
assert(t.is_primitive())
+if member.has_attr(bytes_count):
+raise NotImplementedError(bytes_count not implemented)
+if member.has_attr(bytes_count):
+dest_var = member.attributes[bytes_count][0]
+else:
+dest_var = member.name
+dest.write_ref(writer, t.get_fixed_nw_size() * 8, dest_var, '%s(glb-tvb,
offset)' % primitive_read_func(t))
+writer.increment(offset, t.get_fixed_nw_size())
+
def write_member(writer, container, member, dest, scope):
if member.has_attr(virtual):
--
2.1.0
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH v3 20/51] Change code generated index type
Signed-off-by: Frediano Ziglio fzig...@redhat.com --- python_modules/dissector.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/python_modules/dissector.py b/python_modules/dissector.py index af0494c..40e348a 100644 --- a/python_modules/dissector.py +++ b/python_modules/dissector.py @@ -236,6 +236,8 @@ def write_protocol_parser(writer, proto): decorate_writer(writer) +writer.index_type = 'guint32' + write_parser_helpers(writer) # put fields declaration first -- 2.1.0 ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH v3 36/51] Allow to override default values generated for the fields
ws_type override the type (BOOLEAN - FT_BOOLEAN).
ws_base override the base (DEC - BASE_HEX).
ws_desc override the description.
ws allow to specify description and name for wireshark. Name is
important as allows filters. Having a single attribute with 2 values
allows to quickly specify the main attributes.
Signed-off-by: Frediano Ziglio fzig...@redhat.com
---
codegen/dissector_test.c| 2 ++
codegen/out_base1.txt | 14 +++---
codegen/test.proto | 8
python_modules/dissector.py | 45 ++---
4 files changed, 47 insertions(+), 22 deletions(-)
diff --git a/codegen/dissector_test.c b/codegen/dissector_test.c
index 79f8592..dcc0134 100644
--- a/codegen/dissector_test.c
+++ b/codegen/dissector_test.c
@@ -206,6 +206,8 @@ lookup_values(char *label, value_string *vals, guint64
value)
return false;
}
+const true_false_string tfs_set_notset = { Set, Not set };
+
WS_DLL_PUBLIC void
proto_register_field_array(const int parent, hf_register_info *hf, const int
num_records)
{
diff --git a/codegen/out_base1.txt b/codegen/out_base1.txt
index 49a7426..7921afd 100644
--- a/codegen/out_base1.txt
+++ b/codegen/out_base1.txt
@@ -1,10 +1,10 @@
--- tree
--- item
-Text: 130 (0x82)
+Text: Not set
Name: u8
Abbrev: spice2.auto.msg_base_Base1_u8
-Type: FT_UINT8
-Base: BASE_DEC
+Type: FT_BOOLEAN
+Base: 0
--- item
Text: -127 (0xff81)
Name: i8
@@ -16,17 +16,17 @@
Name: u16
Abbrev: spice2.auto.msg_base_Base1_u16
Type: FT_UINT16
-Base: BASE_DEC
+Base: BASE_DEC_HEX
--- item
Text: -31743 (0x8401)
-Name: i16
+Name: Signed 16
Abbrev: spice2.auto.msg_base_Base1_i16
Type: FT_INT16
Base: BASE_DEC
--- item
Text: 2231566849 (0x85030201)
-Name: u32
-Abbrev: spice2.auto.msg_base_Base1_u32
+Name: Unsigned 32
+Abbrev: spice2.uint.test
Type: FT_UINT32
Base: BASE_DEC
--- item
diff --git a/codegen/test.proto b/codegen/test.proto
index 2fd930b..b28520c 100644
--- a/codegen/test.proto
+++ b/codegen/test.proto
@@ -49,11 +49,11 @@ message ArrayStruct {
channel BaseChannel {
server:
message {
-uint8 u8;
+uint8 u8 @ws_type(BOOLEAN);
int8 i8;
-uint16 u16;
-int16 i16;
-uint32 u32;
+uint16 u16 @ws_base(DEC_HEX);
+int16 i16 @ws_desc(Signed 16);
+uint32 u32 @ws(Unsigned 32, uint.test);
int32 i32;
uint64 u64;
int64 i64;
diff --git a/python_modules/dissector.py b/python_modules/dissector.py
index 827f7f0..2df4723 100644
--- a/python_modules/dissector.py
+++ b/python_modules/dissector.py
@@ -252,12 +252,10 @@ def get_primitive_ft_type(t):
return FT_%sINT%d % (unsigned, size * 8)
# write a field
-def write_wireshark_field(writer, container, member, t, tree, size,
encoding='ENC_LITTLE_ENDIAN', prefix=''):
+def write_wireshark_field(writer, container, member, t, ws, tree, size,
encoding='ENC_LITTLE_ENDIAN', prefix=''):
assert(member and container)
-hf_name = member_hf_name(container, member)
-
# compute proper type
f_type = 'FT_NONE'
base = 'BASE_NONE'
@@ -276,8 +274,30 @@ def write_wireshark_field(writer, container, member, t,
tree, size, encoding='EN
assert(t.has_name())
vals = 'VALS(%s_vs)' % codegen.prefix_underscore_lower(t.name)
-desc = member.name
-ws_name = 'auto.' + hf_name[3:]
+# override type
+if ws.type:
+f_type = 'FT_%s' % ws.type
+base = 'BASE_NONE'
+vals = 'NULL'
+if f_type == 'FT_BOOLEAN':
+vals = 'TFS(tfs_set_notset)'
+
+# override base
+if ws.base:
+base = 'BASE_%s' % ws.base
+
+# read description
+desc = ws.desc
+if desc is None:
+desc = member.name
+
+# read name
+ws_name = ws.name
+if not ws_name:
+hf_name = member_hf_name(container, member)
+ws_name = 'auto.' + hf_name[3:]
+else:
+hf_name = 'hf_%s' % ws_name.replace('.', '_')
writer.statement(%sproto_tree_add_item(%s, %s, glb-tvb, offset, %s, %s)
%
(prefix, tree, hf_name, size, encoding))
@@ -365,7 +385,7 @@ def write_switch(writer, container, switch, dest, scope):
elif t.is_struct():
write_struct(writer, m, t, '-1', dest2, scope)
elif t.is_primitive():
-write_member_primitive(writer, container, m, t, dest2, scope)
+write_member_primitive(writer, container, m, t,
WSAttributes(t, m.attributes), dest2, scope)
elif t.is_array():
nelements = read_array_len(writer, m.name, t, dest, block,
False)
write_array(writer, container, m, nelements, t, dest2, block)
@@ -382,16 +402,18 @@ def write_switch(writer, container, switch, dest, scope):
def write_array(writer, container, member,
[Spice-devel] [PATCH v3 15/51] Generate scheleton for messages and channels
Messages are not generated as stub.
Code partially from demarshaller.
Signed-off-by: Frediano Ziglio fzig...@redhat.com
---
codegen/Makefile.am | 3 +-
codegen/check_dissector | 13 +
codegen/dissector_test.c| 54 -
python_modules/dissector.py | 138 +++-
4 files changed, 203 insertions(+), 5 deletions(-)
create mode 100755 codegen/check_dissector
diff --git a/codegen/Makefile.am b/codegen/Makefile.am
index 129543c..b147b85 100644
--- a/codegen/Makefile.am
+++ b/codegen/Makefile.am
@@ -29,12 +29,13 @@ test.c: $(top_srcdir)/spice.proto $(MARSHALLERS_DEPS)
test.h: $(top_srcdir)/spice.proto $(MARSHALLERS_DEPS)
$(AM_V_GEN)$(PYTHON) $(top_srcdir)/spice_codegen.py
--generate-dissector --client $ --header $@ /dev/null
-TESTS = dissector_test
+TESTS = check_dissector
check_PROGRAMS = dissector_test
dissector_test_SOURCES = dissector_test.c test.c test.h
EXTRA_DIST = \
+ check_dissector \
$(NULL)
-include $(top_srcdir)/git.mk
diff --git a/codegen/check_dissector b/codegen/check_dissector
new file mode 100755
index 000..f1444a2
--- /dev/null
+++ b/codegen/check_dissector
@@ -0,0 +1,13 @@
+#!/bin/bash
+set -e
+
+error() {
+ echo $* 2
+ exit 1
+}
+
+./dissector_test 1 100
+if ./dissector_test 1 99; then
+ error This test should fail
+fi
+exit 0
diff --git a/codegen/dissector_test.c b/codegen/dissector_test.c
index 3212655..25a33b5 100644
--- a/codegen/dissector_test.c
+++ b/codegen/dissector_test.c
@@ -4,6 +4,7 @@
#include stdlib.h
#include unistd.h
#include getopt.h
+#include stdbool.h
#include assert.h
#include epan/expert.h
@@ -18,6 +19,7 @@ enum {
static int last_hf_registered = first_hf_registered - 1;
static int last_ei_registered = first_ei_registered - 1;
static int last_tree_registered = first_tree_registered - 1;
+static bool got_error = false;
WS_DLL_PUBLIC void
proto_register_field_array(const int parent, hf_register_info *hf, const int
num_records)
@@ -54,24 +56,46 @@ expert_register_field_array(expert_module_t* module,
ei_register_info *ei, const
}
}
+WS_DLL_PUBLIC void
+expert_add_info_format(packet_info *pinfo, proto_item *pi, expert_field
*eiindex,
+ const char *format, ...)
+{
+ assert(format);
+ got_error = true;
+ if (!pi)
+ return;
+}
+
static const struct option long_options[] = {
{ help, 0, NULL, 'h' },
+ { server, 0, NULL, 's' },
+ { client, 0, NULL, 'c' },
};
-static const char options[] = h;
+static const char options[] = hsc;
static void syntax(FILE *f, int exit_value)
{
fprintf(f,
- Usage: dissector_test [OPTIONS]\n
+ Usage: dissector_test [OPTIONS] CHANNEL MESSAGE_TYPE\n
\n
Options:\n
-h, --help Show this help\n
+ -s, --server Process server messages (default)\n
+ -c, --client Process client messages\n
);
exit(exit_value);
}
int main(int argc, char **argv)
{
+ int channel, message_type;
+ GlobalInfo glb;
+ proto_tree tree;
+ spice_dissect_func_t (*msg_func)(guint8 channel);
+ spice_dissect_func_t channel_func = NULL;
+
+ msg_func = spice_server_channel_get_dissect;
+
while (1) {
int option_index;
int opt = getopt_long(argc, argv, options, long_options,
option_index);
@@ -82,13 +106,37 @@ int main(int argc, char **argv)
case 'h':
syntax(stdout, EXIT_SUCCESS);
break;
+ case 's':
+ msg_func = spice_server_channel_get_dissect;
+ break;
+ case 'c':
+ msg_func = spice_client_channel_get_dissect;
+ break;
default:
syntax(stderr, EXIT_FAILURE);
break;
}
}
+ if (optind + 2 argc)
+ syntax(stderr, EXIT_FAILURE);
+
+ channel = strtol(argv[optind++], NULL, 0);
+ message_type = strtol(argv[optind++], NULL, 0);
spice_register_fields(1, NULL);
- return EXIT_SUCCESS;
+ memset(glb, 0, sizeof(glb));
+ glb.tvb = NULL;
+ glb.message_offset = 0;
+ glb.message_end = 0;
+
+ channel_func = msg_func(channel);
+ assert(channel_func);
+
+ memset(tree, 0, sizeof(tree));
+
+ /* TODO check offset ?? */
+ channel_func(message_type, glb, tree, 0);
+
+ return got_error ? EXIT_FAILURE : EXIT_SUCCESS;
}
diff --git a/python_modules/dissector.py b/python_modules/dissector.py
index f9ad08a..af0494c 100644
--- a/python_modules/dissector.py
+++ b/python_modules/dissector.py
@@ -1,4 +1,5 @@
+from . import ptypes
from . import
[Spice-devel] [PATCH v3 22/51] Parse containers
Parse all members of the containers
Signed-off-by: Frediano Ziglio fzig...@redhat.com
---
python_modules/dissector.py | 19 +++
1 file changed, 19 insertions(+)
diff --git a/python_modules/dissector.py b/python_modules/dissector.py
index 588becd..ed3b939 100644
--- a/python_modules/dissector.py
+++ b/python_modules/dissector.py
@@ -166,6 +166,24 @@ class SubDestination(Destination):
return self.parent_dest.get_ref(self.member + . + member, writer)
+def write_member_parser(writer, container, member, dest, scope):
+pass
+
+def write_container_parser(writer, container, dest):
+if container.has_attr('ws_as'):
+type_name = container.attributes['ws_as'][0]
+assert(ptypes.type_exists(type_name))
+container = ptypes.lookup_type(type_name)
+
+with dest.declare(writer) as scope:
+for m in container.members:
+if m.has_minor_attr():
+writer.begin_block(if (minor = %s) % m.get_minor_attr())
+write_member_parser(writer, container, m, dest, scope)
+if m.has_minor_attr():
+writer.end_block()
+
+
def write_msg_parser(writer, message, server):
msg_name = message.c_name()
function_name = dissect_spice_%s_%s % ('server' if server else 'client',
msg_name)
@@ -183,6 +201,7 @@ def write_msg_parser(writer, message, server):
GlobalInfo *glb _U_, proto_tree *tree _U_,
guint32 offset, True)
dest = RootDestination(parent_scope)
+write_container_parser(writer, message, dest)
writer.statement(return offset)
writer.end_block()
--
2.1.0
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH v3 13/51] Add new_ett function to be able to create new trees
Use an array to declare tree items instead of allocating it statically.
This save a bit of memory and it also generate smaller code to read.
A tree in wireshark represent an item which could be expanded.
Possibly wireshark is using these registrations to save expansion
state in the user interface.
Signed-off-by: Frediano Ziglio fzig...@redhat.com
---
codegen/dissector_test.c| 13 +
python_modules/dissector.py | 28
2 files changed, 41 insertions(+)
diff --git a/codegen/dissector_test.c b/codegen/dissector_test.c
index 6189da0..3212655 100644
--- a/codegen/dissector_test.c
+++ b/codegen/dissector_test.c
@@ -13,9 +13,11 @@
enum {
first_hf_registered = 0x1,
first_ei_registered = 0x2,
+ first_tree_registered = 0x3,
};
static int last_hf_registered = first_hf_registered - 1;
static int last_ei_registered = first_ei_registered - 1;
+static int last_tree_registered = first_tree_registered - 1;
WS_DLL_PUBLIC void
proto_register_field_array(const int parent, hf_register_info *hf, const int
num_records)
@@ -31,6 +33,17 @@ proto_register_field_array(const int parent,
hf_register_info *hf, const int num
}
WS_DLL_PUBLIC void
+proto_register_subtree_array(gint *const *indices, const int num_indices)
+{
+ int i;
+ assert(num_indices = 0);
+ for (i = 0; i num_indices; ++i) {
+ assert(indices *indices[i] == -1);
+ *indices[i] = ++last_tree_registered;
+ }
+}
+
+WS_DLL_PUBLIC void
expert_register_field_array(expert_module_t* module, ei_register_info *ei,
const int num_records)
{
int i;
diff --git a/python_modules/dissector.py b/python_modules/dissector.py
index 45f8737..52234fc 100644
--- a/python_modules/dissector.py
+++ b/python_modules/dissector.py
@@ -2,6 +2,22 @@
from . import codegen
import re
+
+# generate a new tree identifier
+ett_writer = None
+ett_num = 0
+def new_ett(writer):
+global ett_writer
+global ett_num
+
+if ett_num and ett_num % 16 == 0:
+ett_writer.newline()
+name = 'etts[%u]' % ett_num
+ett_num = ett_num + 1
+ett_writer.write('-1, ')
+return name
+
+
hf_writer = None
hf_defs = None
@@ -54,6 +70,10 @@ def write_protocol_definitions(writer):
writer.newline()
writer.function('spice_register_fields', 'void', 'int proto,
expert_module_t* expert_proto')
+writer.variable_def('guint', 'n');
+writer.variable_def('gint *', 'ett[array_length(etts)]')
+writer.newline()
+
writer.write(static hf_register_info hf[] = )
writer.begin_block()
hf_defs = writer.get_subwriter()
@@ -66,17 +86,25 @@ def write_protocol_definitions(writer):
writer.end_block(semicolon = True)
writer.newline()
+with writer.for_loop('n', 'array_length(etts)'):
+writer.assign('ett[n]', 'etts[n]')
+
writer.statement('proto_register_field_array(proto, hf, array_length(hf))')
+writer.statement('proto_register_subtree_array(ett, array_length(etts))')
writer.statement('expert_register_field_array(expert_proto, ei,
array_length(ei))')
writer.end_block()
def write_protocol_parser(writer, proto):
global hf_writer
+global ett_writer
write_parser_helpers(writer)
# put fields declaration first
+with writer.block('static gint etts[] =', semicolon=True) as scope:
+ett_writer = scope
+writer.newline()
hf_writer = writer.get_subwriter()
# put fields definition at last
--
2.1.0
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH v3 17/51] Allows to specify descriptions for enumerations
These descriptions will be used to show in wireshark dissector.
Signed-off-by: Frediano Ziglio fzig...@redhat.com
---
python_modules/ptypes.py | 16
python_modules/spice_parser.py | 2 +-
2 files changed, 13 insertions(+), 5 deletions(-)
diff --git a/python_modules/ptypes.py b/python_modules/ptypes.py
index bbf158e..3a1acbd 100644
--- a/python_modules/ptypes.py
+++ b/python_modules/ptypes.py
@@ -373,20 +373,24 @@ class EnumType(EnumBaseType):
last = -1
names = {}
values = {}
+descs = {}
for v in enums:
name = v[0]
-if len(v) 1:
-value = v[1]
+desc = v[1][1]
+if len(v) 2:
+value = v[2]
else:
value = last + 1
last = value
assert value not in names
names[value] = name
+descs[value] = desc
values[name] = value
self.names = names
self.values = values
+self.descs = descs
self.attributes = fix_attributes(attribute_list)
@@ -426,20 +430,24 @@ class FlagsType(EnumBaseType):
last = -1
names = {}
values = {}
+descs = {}
for v in flags:
name = v[0]
-if len(v) 1:
-value = v[1]
+desc = v[1][1]
+if len(v) 2:
+value = v[2]
else:
value = last + 1
last = value
assert value not in names
names[value] = name
+descs[value] = desc
values[name] = value
self.names = names
self.values = values
+self.descs = descs
self.attributes = fix_attributes(attribute_list)
diff --git a/python_modules/spice_parser.py b/python_modules/spice_parser.py
index a0f969a..06000a4 100644
--- a/python_modules/spice_parser.py
+++ b/python_modules/spice_parser.py
@@ -120,7 +120,7 @@ def SPICE_BNF():
int32_ ^ uint32_ ^ int64_ ^ uint64_ ^
typename).setName(type)
-flagsBody = enumBody = Group(lbrace + delimitedList(Group (enumname +
Optional(equals + integer))) + Optional(comma) + rbrace)
+flagsBody = enumBody = Group(lbrace + delimitedList(Group (enumname +
Optional(Group(ws_desc), default=[None,None]) + Optional(equals + integer))) +
Optional(comma) + rbrace)
messageSpec = Group(message_ + messageBody +
attributes).setParseAction(lambda toks: ptypes.MessageType(None, toks[0][1],
toks[0][2])) | typename
--
2.1.0
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH v3 27/51] Handle base fields
Add fields to base tree (so basically there is no tree).
Names is now generated from container + member name.
The check for duplicate is not that strong, should check if same field
is defined with same attributes.
Signed-off-by: Frediano Ziglio fzig...@redhat.com
---
codegen/Makefile.am | 39
codegen/check_dissector | 50 --
codegen/data_base1 | Bin 0 - 44 bytes
codegen/data_empty | 0
codegen/dissector_test.c| 22 ++--
codegen/out_base1.txt | 85
codegen/out_empty.txt | 1 +
codegen/test.proto | 56 +
python_modules/dissector.py | 67 ++
9 files changed, 308 insertions(+), 12 deletions(-)
create mode 100644 codegen/data_base1
create mode 100644 codegen/data_empty
create mode 100644 codegen/out_base1.txt
create mode 100644 codegen/out_empty.txt
create mode 100644 codegen/test.proto
diff --git a/codegen/Makefile.am b/codegen/Makefile.am
index b147b85..1281690 100644
--- a/codegen/Makefile.am
+++ b/codegen/Makefile.am
@@ -6,7 +6,7 @@ AM_CPPFLAGS = \
$(SPICE_COMMON_CFLAGS) \
$(NULL)
-dissector_test_LDADD = \
+AM_LDFLAGS = \
$(SPICE_COMMON_LIBS)\
$(NULL)
@@ -21,21 +21,46 @@ MARSHALLERS_DEPS = \
$(top_srcdir)/spice_codegen.py \
$(NULL)
-test.o: test.h
+test.o: test.h enums.h
-test.c: $(top_srcdir)/spice.proto $(MARSHALLERS_DEPS)
- $(AM_V_GEN)$(PYTHON) $(top_srcdir)/spice_codegen.py
--generate-dissector --client $ $@ /dev/null
+test.c: test.proto $(MARSHALLERS_DEPS)
+ $(AM_V_GEN)$(PYTHON) $(top_srcdir)/spice_codegen.py
--generate-dissector $ --include epan/packet.h --include enums.h $@ /dev/null
-test.h: $(top_srcdir)/spice.proto $(MARSHALLERS_DEPS)
- $(AM_V_GEN)$(PYTHON) $(top_srcdir)/spice_codegen.py
--generate-dissector --client $ --header $@ /dev/null
+test.h: test.proto $(MARSHALLERS_DEPS)
+ $(AM_V_GEN)$(PYTHON) $(top_srcdir)/spice_codegen.py
--generate-dissector $ --header $@ /dev/null
+
+enums.h: test.proto $(MARSHALLERS_DEPS)
+ $(AM_V_GEN)$(PYTHON) $(top_srcdir)/spice_codegen.py
--generate-wireshark-dissector $ $@ /dev/null
+
+dissector_test.o: test.h
+
+dissector.o: dissector.h packet-spice.h
+
+dissector.c: $(top_srcdir)/spice.proto $(MARSHALLERS_DEPS)
+ $(AM_V_GEN)$(PYTHON) $(top_srcdir)/spice_codegen.py
--generate-dissector $ $@ /dev/null
+
+dissector.h: $(top_srcdir)/spice.proto $(MARSHALLERS_DEPS)
+ $(AM_V_GEN)$(PYTHON) $(top_srcdir)/spice_codegen.py
--generate-dissector $ --header $@ /dev/null
+
+packet-spice.h: $(top_srcdir)/spice.proto $(MARSHALLERS_DEPS)
+ $(AM_V_GEN)$(PYTHON) $(top_srcdir)/spice_codegen.py
--generate-wireshark-dissector $ $@ /dev/null
TESTS = check_dissector
-check_PROGRAMS = dissector_test
+check_PROGRAMS = dissector_test compile_check
dissector_test_SOURCES = dissector_test.c test.c test.h
+# this just to make sure the more complicate dissector continue to compile
+compile_check_SOURCES = dissector_test.c dissector.c dissector.h
EXTRA_DIST = \
check_dissector \
+ test.proto \
+ data_empty \
+ out_empty.txt \
+ data_base1 \
+ out_base1.txt \
$(NULL)
+CLEANFILES = test.c test.h enums.h dissector.c dissector.h *.trs
check_dissector.txt
+
-include $(top_srcdir)/git.mk
diff --git a/codegen/check_dissector b/codegen/check_dissector
index 7d6d086..bff3853 100755
--- a/codegen/check_dissector
+++ b/codegen/check_dissector
@@ -1,13 +1,57 @@
#!/bin/bash
-set -e
+
+# temporary file
+tmp=check_dissector.txt
error() {
echo $* 2
exit 1
}
-./dissector_test 1 100 /dev/null
-if ./dissector_test 1 99 /dev/null; then
+set -ex
+
+test -x dissector_test
+
+# check some output
+# argument:
+# - input file
+# - channel
+# - message type
+# - output file to check
+# - rest copied
+check() {
+ local in channel msg_type out
+ set +x
+ in=$1
+ shift
+ channel=$1
+ shift
+ msg_type=$1
+ shift
+ out=$1
+ set +e
+ shift
+ ./dissector_test -i $in $@ $channel $msg_type $tmp
+ res=$?
+ if [ $res -eq 0 ]; then
+ if [ $out != ]; then
+ diff -u $tmp $out
+ res=$?
+ fi
+ fi
+ set -ex
+ return $res
+}
+
+# check empty message
+check data_empty 1 100 out_empty.txt
+
+# check not existing message fails
+if check data_empty 1 99; then
error This test should fail
fi
+
+# check just base types
+check data_base1 1 1
[Spice-devel] [PATCH v3 05/51] codegen: Remove duplicate variable initialization
Signed-off-by: Frediano Ziglio fzig...@redhat.com --- python_modules/spice_parser.py | 1 - 1 file changed, 1 deletion(-) diff --git a/python_modules/spice_parser.py b/python_modules/spice_parser.py index 80b559b..97af8b2 100644 --- a/python_modules/spice_parser.py +++ b/python_modules/spice_parser.py @@ -58,7 +58,6 @@ def SPICE_BNF(): uint64_= Keyword(uint64).setParseAction(replaceWith(ptypes.uint64)) # keywords -channel_ = Keyword(channel) enum32_= Keyword(enum32).setParseAction(replaceWith(32)) enum16_= Keyword(enum16).setParseAction(replaceWith(16)) enum8_ = Keyword(enum8).setParseAction(replaceWith(8)) -- 2.1.0 ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH v3 06/51] codegen: Reuse code to fix attribute from prototype file
Signed-off-by: Frediano Ziglio fzig...@redhat.com
---
python_modules/ptypes.py | 32
1 file changed, 16 insertions(+), 16 deletions(-)
diff --git a/python_modules/ptypes.py b/python_modules/ptypes.py
index d031d09..845fa73 100644
--- a/python_modules/ptypes.py
+++ b/python_modules/ptypes.py
@@ -62,6 +62,14 @@ class FixedSize:
# other members
propagated_attributes=[ptr_array, nonnull, chunk]
+def fix_attributes(attribute_list):
+attrs = {}
+for attr in attribute_list:
+name = attr[0][1:]
+lst = attr[1:]
+attrs[name] = lst
+return attrs
+
class Type:
def __init__(self):
self.attributes = {}
@@ -178,8 +186,7 @@ class TypeAlias(Type):
Type.__init__(self)
self.name = name
self.the_type = the_type
-for attr in attribute_list:
-self.attributes[attr[0][1:]] = attr[1:]
+self.attributes = fix_attributes(attribute_list)
def get_type(self, recursive=False):
if recursive:
@@ -288,8 +295,7 @@ class EnumType(EnumBaseType):
self.names = names
self.values = values
-for attr in attribute_list:
-self.attributes[attr[0][1:]] = attr[1:]
+self.attributes = fix_attributes(attribute_list)
def __str__(self):
return enum %s % self.name
@@ -342,8 +348,7 @@ class FlagsType(EnumBaseType):
self.names = names
self.values = values
-for attr in attribute_list:
-self.attributes[attr[0][1:]] = attr[1:]
+self.attributes = fix_attributes(attribute_list)
def __str__(self):
return flags %s % self.name
@@ -533,8 +538,7 @@ class Member(Containee):
Containee.__init__(self)
self.name = name
self.member_type = member_type
-for attr in attribute_list:
-self.attributes[attr[0][1:]] = attr[1:]
+self.attributes = fix_attributes(attribute_list)
def resolve(self, container):
self.container = container
@@ -636,8 +640,7 @@ class Switch(Containee):
self.variable = variable
self.name = name
self.cases = cases
-for attr in attribute_list:
-self.attributes[attr[0][1:]] = attr[1:]
+self.attributes = fix_attributes(attribute_list)
def is_switch(self):
return True
@@ -846,8 +849,7 @@ class StructType(ContainerType):
self.members_by_name = {}
for m in members:
self.members_by_name[m.name] = m
-for attr in attribute_list:
-self.attributes[attr[0][1:]] = attr[1:]
+self.attributes = fix_attributes(attribute_list)
def __str__(self):
if self.name == None:
@@ -869,8 +871,7 @@ class MessageType(ContainerType):
for m in members:
self.members_by_name[m.name] = m
self.reverse_members = {} # ChannelMembers referencing this message
-for attr in attribute_list:
-self.attributes[attr[0][1:]] = attr[1:]
+self.attributes = fix_attributes(attribute_list)
def __str__(self):
if self.name == None:
@@ -938,8 +939,7 @@ class ChannelType(Type):
self.base = base
self.member_name = None
self.members = members
-for attr in attribute_list:
-self.attributes[attr[0][1:]] = attr[1:]
+self.attributes = fix_attributes(attribute_list)
def __str__(self):
if self.name == None:
--
2.1.0
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH v3 12/51] Generate some definition for dissector
Generate global definitions.
Generate function to registers various dissector components.
For the moment the field array is empty bu we save some global to
be able to register new ones.
Add a base test for generated dissector.
Signed-off-by: Frediano Ziglio fzig...@redhat.com
---
Makefile.am | 2 +-
codegen/Makefile.am | 40 +
codegen/dissector_test.c| 81 +
configure.ac| 2 ++
python_modules/dissector.py | 87 +++--
spice_codegen.py| 2 +-
6 files changed, 209 insertions(+), 5 deletions(-)
create mode 100644 codegen/Makefile.am
create mode 100644 codegen/dissector_test.c
diff --git a/Makefile.am b/Makefile.am
index 380bf24..382a0ea 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -1,7 +1,7 @@
NULL =
ACLOCAL_AMFLAGS = -I m4
-SUBDIRS = python_modules common
+SUBDIRS = python_modules common codegen
DIST_SUBDIRS = spice-protocol $(SUBDIRS)
EXTRA_DIST = \
diff --git a/codegen/Makefile.am b/codegen/Makefile.am
new file mode 100644
index 000..129543c
--- /dev/null
+++ b/codegen/Makefile.am
@@ -0,0 +1,40 @@
+NULL =
+
+AM_CPPFLAGS = \
+ -I$(top_srcdir) \
+ $(WIRESHARK_CFLAGS) \
+ $(SPICE_COMMON_CFLAGS) \
+ $(NULL)
+
+dissector_test_LDADD = \
+ $(SPICE_COMMON_LIBS)\
+ $(NULL)
+
+MARSHALLERS_DEPS = \
+ $(top_srcdir)/python_modules/__init__.py\
+ $(top_srcdir)/python_modules/codegen.py \
+ $(top_srcdir)/python_modules/demarshal.py \
+ $(top_srcdir)/python_modules/marshal.py \
+ $(top_srcdir)/python_modules/ptypes.py \
+ $(top_srcdir)/python_modules/spice_parser.py\
+ $(top_srcdir)/python_modules/dissector.py \
+ $(top_srcdir)/spice_codegen.py \
+ $(NULL)
+
+test.o: test.h
+
+test.c: $(top_srcdir)/spice.proto $(MARSHALLERS_DEPS)
+ $(AM_V_GEN)$(PYTHON) $(top_srcdir)/spice_codegen.py
--generate-dissector --client $ $@ /dev/null
+
+test.h: $(top_srcdir)/spice.proto $(MARSHALLERS_DEPS)
+ $(AM_V_GEN)$(PYTHON) $(top_srcdir)/spice_codegen.py
--generate-dissector --client $ --header $@ /dev/null
+
+TESTS = dissector_test
+check_PROGRAMS = dissector_test
+
+dissector_test_SOURCES = dissector_test.c test.c test.h
+
+EXTRA_DIST = \
+ $(NULL)
+
+-include $(top_srcdir)/git.mk
diff --git a/codegen/dissector_test.c b/codegen/dissector_test.c
new file mode 100644
index 000..6189da0
--- /dev/null
+++ b/codegen/dissector_test.c
@@ -0,0 +1,81 @@
+#undef NDEBUG
+#include stdarg.h
+#include stdio.h
+#include stdlib.h
+#include unistd.h
+#include getopt.h
+#include assert.h
+
+#include epan/expert.h
+
+#include test.h
+
+enum {
+ first_hf_registered = 0x1,
+ first_ei_registered = 0x2,
+};
+static int last_hf_registered = first_hf_registered - 1;
+static int last_ei_registered = first_ei_registered - 1;
+
+WS_DLL_PUBLIC void
+proto_register_field_array(const int parent, hf_register_info *hf, const int
num_records)
+{
+ int i;
+ assert(num_records = 0);
+ for (i = 0; i num_records; ++i) {
+ assert(hf);
+ assert(hf[i].p_id);
+ assert(*hf[i].p_id == -1);
+ *hf[i].p_id = ++last_hf_registered;
+ }
+}
+
+WS_DLL_PUBLIC void
+expert_register_field_array(expert_module_t* module, ei_register_info *ei,
const int num_records)
+{
+ int i;
+ assert(num_records = 0);
+ for (i = 0; i num_records; ++i) {
+ assert(ei ei[i].ids-ei == -1);
+ ei[i].ids-ei = ++last_ei_registered;
+ }
+}
+
+static const struct option long_options[] = {
+ { help, 0, NULL, 'h' },
+};
+static const char options[] = h;
+
+static void syntax(FILE *f, int exit_value)
+{
+ fprintf(f,
+ Usage: dissector_test [OPTIONS]\n
+ \n
+ Options:\n
+ -h, --help Show this help\n
+ );
+ exit(exit_value);
+}
+
+int main(int argc, char **argv)
+{
+ while (1) {
+ int option_index;
+ int opt = getopt_long(argc, argv, options, long_options,
option_index);
+ if (opt == -1)
+ break;
+
+ switch (opt) {
+ case 'h':
+ syntax(stdout, EXIT_SUCCESS);
+ break;
+ default:
+ syntax(stderr, EXIT_FAILURE);
+ break;
+ }
+ }
+
+ spice_register_fields(1, NULL);
+
+ return EXIT_SUCCESS;
+}
diff --git a/configure.ac b/configure.ac
index 4287f92..a156cae 100644
--- a/configure.ac
+++
[Spice-devel] [PATCH v3 19/51] Decorate protocol file with attributes for wireshark
Signed-off-by: Frediano Ziglio fzig...@redhat.com
---
spice.proto | 416
1 file changed, 219 insertions(+), 197 deletions(-)
diff --git a/spice.proto b/spice.proto
index 4ea1263..52d6971 100644
--- a/spice.proto
+++ b/spice.proto
@@ -5,14 +5,14 @@
typedef fixed28_4 int32 @ctype(SPICE_FIXED28_4);
struct Point {
-int32 x;
-int32 y;
-};
+int32 x @ws(x, point32.x);
+int32 y @ws(y, point32.y);
+} @ws_txt(POINT (%d, %d), x, y);
struct Point16 {
-int16 x;
-int16 y;
-};
+int16 x @ws(x, point16.x);
+int16 y @ws(y, point16.y);
+} @ws_txt(POINT16 (%d, %d), x, y);
struct PointFix {
fixed28_4 x;
@@ -20,11 +20,14 @@ struct PointFix {
};
struct Rect {
-int32 top;
-int32 left;
-int32 bottom;
-int32 right;
-};
+int32 top @ws(top, rect.top);
+int32 left @ws(left, rect.left);
+int32 bottom @ws(bottom, rect.bottom);
+int32 right @ws(right, rect.right);
+}
+@ws_txt(RECT: (%d-%d, %d-%d), left, top, right, bottom)
+@ws_txt_n(RECT %u: (%d-%d, %d-%d), INDEX, left, top, right, bottom)
+;
struct Transform {
uint32 t00;
@@ -96,10 +99,15 @@ enum32 notify_visibility {
};
flags16 mouse_mode {
-SERVER,
-CLIENT,
+SERVER @ws_desc(Server mode),
+CLIENT @ws_desc(Client mode),
};
+flags32 mouse_mode32 {
+SERVER @ws_desc(Server mode),
+CLIENT @ws_desc(Client mode),
+} @ws(Supported mouse modes, supported_mouse_modes);
+
enum16 pubkey_type {
INVALID,
RSA,
@@ -117,12 +125,12 @@ message Empty {
};
message Data {
-uint8 data[] @end @ctype(uint8_t);
+uint8 data[] @end @ctype(uint8_t) @ws(data, data) @ws_type(BYTES);
} @nocopy;
struct ChannelWait {
uint8 channel_type;
-uint8 channel_id;
+uint8 channel_id @ws_desc(Channel ID);
uint64 message_serial;
} @ctype(SpiceWaitForChannel);
@@ -135,14 +143,14 @@ channel BaseChannel {
Data migrate_data;
message {
- uint32 generation;
- uint32 window;
+ uint32 generation @ws(Set ACK generation, red_set_ack_generation);
+ uint32 window @ws(Set ACK window (messages), red_set_ack_window);
} set_ack;
message {
- uint32 id;
- uint64 timestamp;
- uint8 data[] @ctype(uint8_t) @as_ptr(data_len);
+ uint32 id @ws(Ping ID, ping_id);
+ uint64 timestamp @ws(timestamp, timestamp);
+ uint8 data[] @ctype(uint8_t) @as_ptr(data_len) @ws_txt(PING DATA (%d
bytes), data.nelements);
} ping;
message {
@@ -156,12 +164,12 @@ channel BaseChannel {
} @ctype(SpiceMsgDisconnect) disconnecting;
message {
- uint64 time_stamp;
- notify_severity severity;
- notify_visibility visibilty;
- uint32 what; /* error_code/warn_code/info_code */
- uint32 message_len;
- uint8 message[message_len] @end @nomarshal;
+ uint64 time_stamp @ws(timestamp, timestamp);
+ notify_severity severity @ws(Severity, notify_severity);
+ notify_visibility visibilty @ws(Visibility, notify_visibility);
+ uint32 what @ws(error/warn/info code, notify_code); /*
error_code/warn_code/info_code */
+ uint32 message_len @ws(Message length, notify_message_length);
+ uint8 message[message_len] @end @nomarshal @ws(Message,
notify_message) @ws_type(STRING);
} notify;
Data list; /* the msg body is SpiceSubMessageList */
@@ -170,14 +178,14 @@ channel BaseChannel {
client:
message {
- uint32 generation;
+ uint32 generation @ws(Set ACK generation, red_set_ack_generation);
} ack_sync;
Empty ack;
message {
- uint32 id;
- uint64 timestamp;
+ uint32 id @ws(Ping ID, ping_id);
+ uint64 timestamp @ws(timestamp, timestamp);
} @ctype(SpiceMsgPing) pong;
Empty migrate_flush_mark;
@@ -191,17 +199,17 @@ channel BaseChannel {
};
struct ChannelId {
-uint8 type;
-uint8 id;
-};
+uint8 type @ws(Channel type, channel_type);
+uint8 id @ws(Channel ID, channel_id);
+} @ws_txt_n(channels[%u], INDEX);
struct DstInfo {
- uint16 port;
- uint16 sport;
+ uint16 port @ws(Migrate Dest Port, migrate_dest_port);
+ uint16 sport @ws(Migrate Dest Secure Port, migrate_dest_sport);
uint32 host_size;
- uint8 *host_data[host_size] @zero_terminated @marshall @nonnull;
+ uint8 *host_data[host_size] @zero_terminated @marshall @nonnull
@ws(data, data) @ws_type(BYTES);
uint32 cert_subject_size;
- uint8 *cert_subject_data[cert_subject_size] @zero_terminated @marshall;
+ uint8 *cert_subject_data[cert_subject_size] @zero_terminated @marshall
@ws(data, data) @ws_type(BYTES);
} @ctype(SpiceMigrationDstInfo);
channel MainChannel : BaseChannel {
@@ -213,69 +221,69 @@ channel MainChannel : BaseChannel {
Empty migrate_cancel;
message {
- uint32 session_id;
- uint32 display_channels_hint;
- uint32 supported_mouse_modes;
-
[Spice-devel] [PATCH v3 14/51] Decorate writer class to make easier ifdef/endif handling
I'm generating code for dissector from demarshaller. Make simple to hangle ifdef/endif not having to check manually attribute. Signed-off-by: Frediano Ziglio fzig...@redhat.com --- python_modules/dissector.py | 18 ++ 1 file changed, 18 insertions(+) diff --git a/python_modules/dissector.py b/python_modules/dissector.py index 52234fc..f9ad08a 100644 --- a/python_modules/dissector.py +++ b/python_modules/dissector.py @@ -2,6 +2,8 @@ from . import codegen import re +import types + # generate a new tree identifier ett_writer = None @@ -95,10 +97,26 @@ def write_protocol_definitions(writer): writer.end_block() +def decorate_writer(writer): +cls = writer.__class__ + +def create(old): +def ifdef(self, member): +if member.has_attr(ifdef): +old(self, member.attributes[ifdef][0]) +return types.MethodType(ifdef, None, cls) + +cls.ifdef = create(cls.ifdef) +cls.ifdef_else = create(cls.ifdef_else) +cls.endif = create(cls.endif) + + def write_protocol_parser(writer, proto): global hf_writer global ett_writer +decorate_writer(writer) + write_parser_helpers(writer) # put fields declaration first -- 2.1.0 ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH spice 11/13] server: Add h264 support to the GStreamer video encoder. (take 4)
Signed-off-by: Francois Gouget fgou...@codeweavers.com
---
Having support for h264 is interesting in its own right but this shows
adding extra codecs is quite easy.
Changes since take 3:
- None.
server/gstreamer_encoder.c | 17 -
server/red_dispatcher.c| 2 ++
2 files changed, 18 insertions(+), 1 deletion(-)
diff --git a/server/gstreamer_encoder.c b/server/gstreamer_encoder.c
index 2ad1890..52b3e03 100644
--- a/server/gstreamer_encoder.c
+++ b/server/gstreamer_encoder.c
@@ -266,6 +266,9 @@ static gboolean construct_pipeline(GstEncoder *encoder,
const SpiceBitmap *bitma
case SPICE_VIDEO_CODEC_TYPE_VP8:
gstenc_name = vp8enc;
break;
+case SPICE_VIDEO_CODEC_TYPE_H264:
+gstenc_name = x264enc;
+break;
default:
spice_warning(unsupported codec type %d, encoder-base.codec_type);
return FALSE;
@@ -322,6 +325,17 @@ static gboolean construct_pipeline(GstEncoder *encoder,
const SpiceBitmap *bitma
NULL);
break;
}
+case SPICE_VIDEO_CODEC_TYPE_H264:
+g_object_set(G_OBJECT(encoder-gstenc),
+ bitrate, encoder-bit_rate / 1024,
+ byte-stream, TRUE,
+ aud, FALSE,
+ tune, 4, /* Zero latency */
+ intra-refresh, TRUE,
+ sliced-threads, TRUE,
+ speed-preset, 1, /* ultrafast */
+ NULL);
+break;
default:
spice_warning(unknown encoder type %d, encoder-base.codec_type);
reset_pipeline(encoder);
@@ -674,7 +688,8 @@ GstEncoder *create_gstreamer_encoder(SpiceVideoCodecType
codec_type, uint64_t st
spice_assert(!cbs || (cbs cbs-get_roundtrip_ms
cbs-get_source_fps));
if (codec_type != SPICE_VIDEO_CODEC_TYPE_MJPEG
-codec_type != SPICE_VIDEO_CODEC_TYPE_VP8) {
+codec_type != SPICE_VIDEO_CODEC_TYPE_VP8
+codec_type != SPICE_VIDEO_CODEC_TYPE_H264) {
spice_warning(unsupported codec type %d, codec_type);
return NULL;
}
diff --git a/server/red_dispatcher.c b/server/red_dispatcher.c
index d896d00..14d3f86 100644
--- a/server/red_dispatcher.c
+++ b/server/red_dispatcher.c
@@ -268,12 +268,14 @@ static create_video_encoder_proc video_encoder_procs[] = {
static const EnumNames video_codec_names[] = {
{SPICE_VIDEO_CODEC_TYPE_MJPEG, mjpeg},
{SPICE_VIDEO_CODEC_TYPE_VP8, vp8},
+{SPICE_VIDEO_CODEC_TYPE_H264, h264},
{0, NULL},
};
static const EnumNames video_cap_names[] = {
{SPICE_DISPLAY_CAP_CODEC_MJPEG, mjpeg},
{SPICE_DISPLAY_CAP_CODEC_VP8, vp8},
+{SPICE_DISPLAY_CAP_CODEC_H264, h264},
{0, NULL},
};
--
2.1.4
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH spice 09/13] server: Add GStreamer 1.0 support to the GStreamer video encoder. (take 4)
Signed-off-by: Francois Gouget fgou...@codeweavers.com
---
By default GStreamer 1.0 is used if available, otherwise GStreamer 0.10
is used and Spice is compiled without GStreamer support as a last
resort. It's possible to explicitly require a specific Gstreamer version
with --enable-gstreamer=1.0 and --enable-gstreamer=0.10, or for any
version with --enable-gstreamer=yes.
If you get an error while building the pipeline for MJPEG, then it
probably means you need the fix for this bug:
https://bugzilla.gnome.org/show_bug.cgi?id=750398
Changes since take 3:
- Check whether g_get_num_processors() is available for compatibility
with glib 2.35 and older.
- avenc_mjpeg needs the clock to be removed just like ffenc_mjpeg, its
0.10 counterpart.
- Support for the new output buffer handling.
configure.ac | 35 +++
server/Makefile.am | 12 +--
server/gstreamer_encoder.c | 84 +-
server/red_dispatcher.c| 2 +-
4 files changed, 116 insertions(+), 17 deletions(-)
diff --git a/configure.ac b/configure.ac
index e5be699..702cd49 100644
--- a/configure.ac
+++ b/configure.ac
@@ -81,14 +81,32 @@ SPICE_CHECK_SMARTCARD([SMARTCARD])
AM_CONDITIONAL(SUPPORT_SMARTCARD, test x$have_smartcard = xyes)
AC_ARG_ENABLE(gstreamer,
- AS_HELP_STRING([--enable-gstreamer=@:@auto/yes/no@:@],
- [Enable GStreamer 0.10 support]),
+ AS_HELP_STRING([--enable-gstreamer=@:@auto/0.10/1.0/yes/no@:@],
+ [Enable GStreamer support]),
[],
[enable_gstreamer=auto])
-if test x$enable_gstreamer != xno; then
+if test x$enable_gstreamer != xno test x$enable_gstreamer != x0.10;
then
+PKG_CHECK_MODULES(GSTREAMER_1_0, [gstreamer-1.0, gstreamer-app-1.0],
+ [enable_gstreamer=1.0
+ have_gstreamer_1_0=yes],
+ [have_gstreamer_1_0=no])
+if test x$have_gstreamer_1_0 = xyes; then
+AC_SUBST(GSTREAMER_1_0_CFLAGS)
+AC_SUBST(GSTREAMER_1_0_LIBS)
+AS_VAR_APPEND([SPICE_REQUIRES], [ gstreamer-1.0 gstreamer-app-1.0])
+AC_DEFINE([HAVE_GSTREAMER_1_0], [1], [Define if supporting GStreamer
1.0])
+elif test x$enable_gstreamer = x1.0; then
+AC_MSG_ERROR([GStreamer 1.0 support requested but not found. You may
set GSTREAMER_1_0_CFLAGS and GSTREAMER_1_0_LIBS to avoid the need to call
pkg-config.])
+fi
+else
+have_gstreamer_1_0=no
+fi
+AM_CONDITIONAL(SUPPORT_GSTREAMER_1_0, test x$have_gstreamer_1_0 = xyes)
+
+if test x$enable_gstreamer != xno test x$enable_gstreamer != x1.0;
then
PKG_CHECK_MODULES(GSTREAMER_0_10, [gstreamer-0.10, gstreamer-app-0.10],
- [enable_gstreamer=yes
+ [enable_gstreamer=0.10
have_gstreamer_0_10=yes],
[have_gstreamer_0_10=no])
if test x$have_gstreamer_0_10 = xyes; then
@@ -96,7 +114,7 @@ if test x$enable_gstreamer != xno; then
AC_SUBST(GSTREAMER_0_10_LIBS)
AS_VAR_APPEND([SPICE_REQUIRES], [ gstreamer-0.10 gstreamer-app-0.10])
AC_DEFINE([HAVE_GSTREAMER_0_10], [1], [Define if supporting GStreamer
0.10])
-elif test x$enable_gstreamer = xyes; then
+elif test x$enable_gstreamer = x0.10; then
AC_MSG_ERROR([GStreamer 0.10 support requested but not found. You may
set GSTREAMER_0_10_CFLAGS and GSTREAMER_0_10_LIBS to avoid the need to call
pkg-config.])
fi
else
@@ -104,6 +122,11 @@ else
fi
AM_CONDITIONAL(SUPPORT_GSTREAMER_0_10, test x$have_gstreamer_0_10 = xyes)
+if test x$enable_gstreamer = xyes; then
+AC_MSG_ERROR(GStreamer support requested but not found)
+fi
+AS_IF([test x$enable_gstreamer = xauto], [enable_gstreamer=no])
+
AC_ARG_ENABLE([automated_tests],
AS_HELP_STRING([--enable-automated-tests], [Enable automated
tests using spicy-screenshot (part of spice--gtk)]),,
[enable_automated_tests=no])
@@ -339,7 +362,7 @@ echo
Smartcard:${have_smartcard}
-GStreamer 0.10: ${have_gstreamer_0_10}
+GStreamer:${enable_gstreamer}
SASL support: ${enable_sasl}
diff --git a/server/Makefile.am b/server/Makefile.am
index 4921bc3..9fb0c8e 100644
--- a/server/Makefile.am
+++ b/server/Makefile.am
@@ -12,6 +12,7 @@ AM_CPPFLAGS = \
$(SLIRP_CFLAGS) \
$(SMARTCARD_CFLAGS) \
$(GSTREAMER_0_10_CFLAGS)\
+ $(GSTREAMER_1_0_CFLAGS) \
$(SSL_CFLAGS) \
$(VISIBILITY_HIDDEN_CFLAGS) \
$(WARN_CFLAGS) \
@@ -42,7 +43,8 @@ libspice_server_la_LIBADD =
\
$(PIXMAN_LIBS)
[Spice-devel] [PATCH spice 10/13] server: Avoid copying the input frame in the GStreamer encoder. (take 4)
To do so we reference the source bitmap chunks in the GStreamer buffer and rely
on the buffer's lifetime being short enough.
Note that we can only avoid copies for the first 1 Mpixels or so. That's
because Spice splits larger frames into more chunks and we can fit memory
fragments inside a GStreamer buffer. So for those we will avoid copies for the
first 3840 KB and copy the rest.
Signed-off-by: Francois Gouget fgou...@codeweavers.com
---
This makes it possible to avoid copying the source frame when using
GStreamer 1.0. Paradoxically we are still forced to do some copying for
the larger frames.
Here is how it works: Spice's frame buffer is composed of multiple
memory chunks. So we use GStreamer's GstMemory objects to wrap each
chunk and put them all to form the GstBuffer we pass to the pipeline.
However there's a limit to the number of memory objects that we can put
in a GstBuffer. Usually that limit is 16. Furthermore Spice splits the
frame into 256KB chunks. so this approach only works up to 4MB or
between 1 and 1.3Mpixels. For larger frames we use the zero-copy
approach for the first 15 frame chunks, and copy all the rest into the
16th memory chunk.
So on my machine, for a 720x304 video the frame copy time goes from
around 265us (line-by-line) to 5us. For a 1440x900 frame it goes from
about 1610us with the old line-by-line copy code, down to 1490us for the
new chunk-by-chunk one, and down to about 440us when minimizing copies.
The latter matches well with the ~0.3Mpixels we have to copy after the
first 1Mpixels.
To avoid these copies it will be necessary to either increase the size
of the Spice chunks or to increase the number of memory objects that
GstBuffers can hold.
Changes since take 3:
- None.
server/gstreamer_encoder.c | 107 ++---
1 file changed, 101 insertions(+), 6 deletions(-)
diff --git a/server/gstreamer_encoder.c b/server/gstreamer_encoder.c
index ac8f5c0..2ad1890 100644
--- a/server/gstreamer_encoder.c
+++ b/server/gstreamer_encoder.c
@@ -35,6 +35,10 @@ typedef struct GstEncoder GstEncoder;
#define GSTE_DEFAULT_FPS 30
+#ifndef HAVE_GSTREAMER_0_10
+# define DO_ZERO_COPY
+#endif
+
typedef struct {
SpiceBitmapFmt spice_format;
@@ -86,6 +90,11 @@ struct GstEncoder {
/* The default video buffer */
GstVideoBuffer *default_buffer;
+#ifdef DO_ZERO_COPY
+/* Set to TRUE until GStreamer no longer needs the raw bitmap buffer. */
+gboolean needs_bitmap;
+#endif
+
/* The frame counter for GStreamer buffers */
uint32_t frame;
@@ -355,6 +364,15 @@ static void reconfigure_pipeline(GstEncoder *encoder)
}
}
+#ifdef DO_ZERO_COPY
+/* A helper for push_raw_frame() */
+static void unref_bitmap(gpointer mem)
+{
+GstEncoder *encoder = (GstEncoder*)mem;
+encoder-needs_bitmap = FALSE;
+}
+#endif
+
/* A helper for gst_encoder_encode_frame(). */
static int push_raw_frame(GstEncoder *encoder, const SpiceBitmap *bitmap,
const SpiceRect *src, int top_down)
@@ -362,15 +380,14 @@ static int push_raw_frame(GstEncoder *encoder, const
SpiceBitmap *bitmap,
const uint32_t stream_height = src-bottom - src-top;
const uint32_t stream_stride = (src-right - src-left) *
encoder-format-bpp / 8;
uint32_t len = stream_stride * stream_height;
-GstBuffer *buffer = gst_buffer_new_and_alloc(len);
#ifdef HAVE_GSTREAMER_0_10
+GstBuffer *buffer = gst_buffer_new_and_alloc(len);
uint8_t *b = GST_BUFFER_DATA(buffer);
+uint8_t *dst = b;
#else
-GstMapInfo map;
-gst_buffer_map(buffer, map, GST_MAP_WRITE);
-uint8_t *b = map.data;
+GstBuffer *buffer = gst_buffer_new();
+GstMapInfo map = { .memory = NULL };
#endif
-uint8_t *dst = b;
/* Note that we should not reorder the lines, even if top_down is false.
* It just changes the number of lines to skip at the start of the bitmap.
@@ -382,6 +399,70 @@ static int push_raw_frame(GstEncoder *encoder, const
SpiceBitmap *bitmap,
if (stream_stride == bitmap-stride) {
/* We can copy the bitmap chunk by chunk */
+#ifdef DO_ZERO_COPY
+/* We cannot control the lifetime of the bitmap but we can wrap it in
+ * the buffer anyway because:
+ * - Before returning from gst_encoder_encode_frame() we wait for the
+ * pipeline to have converted this frame into a compressed buffer.
+ * So it has to have gone through the frame at least once.
+ * - For all encoders but MJPEG, the first element of the pipeline will
+ * convert the bitmap to another image format which entails copying
+ * it. So by the time the encoder starts its work, this buffer will
+ * not be needed anymore.
+ * - The MJPEG encoder does not perform inter-frame compression and
thus
+ * does not need to keep hold of this buffer once it has processed
it.
+ */
+while (chunk_offset =
[Spice-devel] [PATCH spice 12/13] server: Shape the bit rate of the GStreamer video encoders output.
The GStreamer encoders don't follow the specified bit rate very closely: they
can decide to exceed it for ten seconds or more if they consider the scene
deserves it. Such long bursts are enough to cause network congestion, resulting
in long bouts of dropped frames.
So the GStreamer video encoder now uses a virtual buffer to shape the
compressed video output and ensure we don't exceed the target bit rate for any
significant length of time, which makes it possible to use higher bit rates
overall.
It also keeps track of the encoded frame size so it can gather statistics and
call update_client_playback_delay() with accurate information and also annotate
the client report debug traces with the corresponding bit rate information.
Signed-off-by: Francois Gouget fgou...@codeweavers.com
---
The bit rate control code can be split into two parts:
1. Code to turn the raw video into a compressed stream of the given bit rate.
2. Feedback code that adjusts the bit rate based on the network conditions.
In theory the GStreamer encoders implement part 1 for us but, as stated
in the commit message, in practice they are not strict enough (and most
cannot be tweaked in this respect).
It's interesting to note that if the feedback mechanism gets good,
timely information we can pretty much do without this patch: when
GStreamer exceeds the set bit rate the feedback mechanism will notice a
degradation of the network conditions and lower the target bit rate
which will lower the GStreamer's output bit rate (even if it still
exceeds the target bit rate a bit).
However this is suboptimal as it will force the feedback mechanism to
either change the bit rate more frequently (causing a GStreamer pipeline
reinitialization most of the time and degrading compression levels), or
select a lower bit rate so the network capacity is not exceeded even if
GStreamer goes 10 or 20% above the set target.
Also note that despite wanting to avoid exceeding the target bit rate,
part 1 cannot consider frames individually, if only to correctly handle
VP8's large I vs. P frame size discrepancy. Exceeding the target bit
rate for sub-second periods generally does not cause network congestion
hence the selection of a 300ms virtual buffer.
server/gstreamer_encoder.c | 287 +++--
1 file changed, 279 insertions(+), 8 deletions(-)
diff --git a/server/gstreamer_encoder.c b/server/gstreamer_encoder.c
index 52b3e03..4089ead 100644
--- a/server/gstreamer_encoder.c
+++ b/server/gstreamer_encoder.c
@@ -39,6 +39,9 @@ typedef struct GstEncoder GstEncoder;
# define DO_ZERO_COPY
#endif
+#define NANO_SECOND (10LL)
+#define MILLI_SECOND (1000LL)
+#define NANO_MS (NANO_SECOND / MILLI_SECOND)
typedef struct {
SpiceBitmapFmt spice_format;
@@ -59,6 +62,11 @@ struct GstVideoBuffer {
gboolean persistent;
};
+typedef struct {
+uint32_t mm_time;
+uint32_t size;
+} GstFrameInformation;
+
struct GstEncoder {
VideoEncoder base;
@@ -98,11 +106,71 @@ struct GstEncoder {
/* The frame counter for GStreamer buffers */
uint32_t frame;
+
+/* -- Encoded frame statistics -- */
+
+/* Should be = than FRAME_STATISTICS_COUNT. This is also used to annotate
+ * the client report debug traces with bit rate information.
+ */
+# define GSTE_HISTORY_SIZE 60
+
+/* A circular buffer containing the past encoded frames information. */
+GstFrameInformation history[GSTE_HISTORY_SIZE];
+
+/* The indices of the oldest and newest frames in the history buffer. */
+uint32_t history_first;
+uint32_t history_last;
+
+/* How many frames to take into account when computing the effective
+ * bit rate, average frame size, etc. This should be large enough so the
+ * I and P frames average out, and short enough for it to reflect the
+ * current situation.
+ */
+# define GSTE_FRAME_STATISTICS_COUNT 21
+
+/* The index of the oldest frame taken into account for the statistics. */
+uint32_t stat_first;
+
+/* Used to compute the average frame size. */
+uint64_t stat_sum;
+
+/* Tracks the maximum frame size. */
+uint32_t stat_maximum;
+
+
+/* -- Encoder bit rate control --
+ *
+ * GStreamer encoders don't follow the specified bit rate very
+ * closely. These fields are used to ensure we don't exceed the desired
+ * stream bit rate, regardless of the GStreamer encoder's output.
+ */
+
/* The bit rate target for the outgoing network stream. (bits per second)
*/
uint64_t bit_rate;
/* The minimum bit rate */
# define GSTE_MIN_BITRATE (128 * 1024)
+
+/* The bit rate control is performed using a virtual buffer to allow short
+ * term variations: bursts are allowed until the virtual buffer is full.
+ * Then frames are dropped to limit the bit rate. VBUFFER_SIZE defines the
+ * size of the virtual buffer in milliseconds worth of
[Spice-devel] [PATCH qxl 07/13] Xspice: Add a --video-codecs option to specify which encoder:codec to use. (take 4)
---
Changes since take 3:
- None.
scripts/Xspice | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/scripts/Xspice b/scripts/Xspice
index 281535d..02875f5 100755
--- a/scripts/Xspice
+++ b/scripts/Xspice
@@ -86,6 +86,7 @@ parser.add_argument('--zlib-glz-wan-compression',
# TODO - sound support
parser.add_argument('--streaming-video', choices=['off', 'all', 'filter'],
default='filter', help='filter by default')
+parser.add_argument('--video-codecs', help=Sets a semicolon-separated list of
preferred video codecs to use. Each takes the form encoder:codec, with
spice:mjpeg being the default and other options being provided by gstreamer for
the mjpeg, vp8 and h264 codecs.)
add_boolean('--ipv4-only')
add_boolean('--ipv6-only')
parser.add_argument('--vdagent', action='store_true', dest='vdagent_enabled',
default=False, help='launch vdagent vdagentd. They provide clipboard
resolution automation')
@@ -251,7 +252,7 @@ var_args = ['port', 'tls_port', 'disable_ticketing',
'x509_key_file', 'x509_key_password',
'tls_ciphers', 'dh_file', 'password', 'image_compression',
'jpeg_wan_compression', 'zlib_glz_wan_compression',
-'streaming_video', 'deferred_fps', 'exit_on_disconnect',
+'streaming_video', 'video_codecs', 'deferred_fps', 'exit_on_disconnect',
'vdagent_enabled', 'vdagent_virtio_path', 'vdagent_uinput_path',
'vdagent_uid', 'vdagent_gid']
--
2.1.4
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH qxl 06/13] spiceqxl: Add SpiceVideoCodecs and $XSPICE_VIDEO_CODECS for specifying video codec preferences. (take 4)
---
Changes since take 3:
- None.
Changes since take 1:
- Fixed a brace placement.
examples/spiceqxl.xorg.conf.example | 7 +++
src/qxl.h | 1 +
src/qxl_driver.c| 2 ++
src/spiceqxl_spice_server.c | 15 +++
4 files changed, 25 insertions(+)
diff --git a/examples/spiceqxl.xorg.conf.example
b/examples/spiceqxl.xorg.conf.example
index d15f7f2..a82c2be 100644
--- a/examples/spiceqxl.xorg.conf.example
+++ b/examples/spiceqxl.xorg.conf.example
@@ -51,6 +51,13 @@ Section Device
# defaults to filter.
#Option SpiceStreamingVideo
+# Set video codecs to use. Provide a semicolon list of
+# codecs, in preference order. Each codec requires an encoder
+# which can be one of spice or gstreamer, and then a codec type,
+# for instance mjpeg or vp8. The default is spice:mjpeg,
+# which uses the builtin mjpeg encoder.
+#Option SpiceVideoCodecs
+
# Set zlib glz wan compression. Options are auto, never, always.
# defaults to auto.
#Option SpiceZlibGlzWanCompression
diff --git a/src/qxl.h b/src/qxl.h
index ff55604..5cc8d05 100644
--- a/src/qxl.h
+++ b/src/qxl.h
@@ -158,6 +158,7 @@ enum {
OPTION_SURFACE_BUFFER_SIZE,
OPTION_COMMAND_BUFFER_SIZE,
OPTION_SPICE_SMARTCARD_FILE,
+OPTION_SPICE_VIDEO_CODECS,
#endif
OPTION_COUNT,
};
diff --git a/src/qxl_driver.c b/src/qxl_driver.c
index ce0a88e..d036dac 100644
--- a/src/qxl_driver.c
+++ b/src/qxl_driver.c
@@ -155,6 +155,8 @@ const OptionInfoRec DefaultOptions[] =
CommandBufferSize,OPTV_INTEGER,
{DEFAULT_COMMAND_BUFFER_SIZE}, FALSE},
{ OPTION_SPICE_SMARTCARD_FILE,
SpiceSmartcardFile, OPTV_STRING,{0}, FALSE},
+{ OPTION_SPICE_VIDEO_CODECS,
+ SpiceVideoCodecs, OPTV_STRING,{0}, FALSE},
#endif
{ -1, NULL, OPTV_NONE, {0}, FALSE }
diff --git a/src/spiceqxl_spice_server.c b/src/spiceqxl_spice_server.c
index 14ee752..2f39561 100644
--- a/src/spiceqxl_spice_server.c
+++ b/src/spiceqxl_spice_server.c
@@ -173,6 +173,9 @@ void xspice_set_spice_server_options(OptionInfoPtr options)
const char *streaming_video =
get_str_option(options, OPTION_SPICE_STREAMING_VIDEO,
XSPICE_STREAMING_VIDEO);
+const char *video_codecs =
+get_str_option(options, OPTION_SPICE_VIDEO_CODECS,
+ XSPICE_VIDEO_CODECS);
int agent_mouse =
get_bool_option(options, OPTION_SPICE_AGENT_MOUSE,
XSPICE_AGENT_MOUSE);
@@ -295,6 +298,18 @@ void xspice_set_spice_server_options(OptionInfoPtr options)
spice_server_set_streaming_video(spice_server, streaming_video_opt);
}
+if (video_codecs) {
+#if SPICE_SERVER_VERSION = 0x000c06 /* 0.12.6 */
+if (spice_server_set_video_codecs(spice_server, video_codecs)) {
+fprintf(stderr, spice: invalid video encoder %s\n, video_codecs);
+exit(1);
+}
+#else
+fprintf(stderr, spice: video_codecs are not available (spice =
0.12.6 required)\n);
+exit(1);
+#endif
+}
+
spice_server_set_agent_mouse
(spice_server, agent_mouse);
spice_server_set_playback_compression
--
2.1.4
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH spice 08/13] server: Let the video encoder manage the compressed buffer and avoid copying it.
This way the video encoder is not forced to use malloc()/free().
This also allows more flexibility in how the video encoder manages the buffer
which allows for a zero-copy implementation in both video encoders.
The current implementations also ensure that there is no reallocation of the
VideoBuffer structure.
Signed-off-by: Francois Gouget fgou...@codeweavers.com
---
This is a new patch which makes it possible to avoid copying the output
buffer in the GStreamer video encoder.
Before:
- Spice was handling the initial output buffer allocation, and was also
freeing it when destroying the stream.
- When calling encode_frame() Spice knew that the buffer was no
longer needed for the previous frame. So it just reused it, saving on
reallocations.
- The video encoder was responsible for reallocating the buffer when
needed. In the mjpeg_encoder case this was actually done by
the jpeg library, and thus out of the hands of the encoder itself.
- The GStreamer pipeline allocates its own output buffer which forced
the video encoder to copy it to the Spice-provided output buffer.
Now:
- The video encoders do all the output buffer allocations /
deallocations and give a pointer to the buffer to the rest of the
Spice code.
- The video encoders could assume that the buffer is no longer used by
the time a new call to encode_frame() is made. But I prefer to
decouple this a bit more. So the Spice code is reponsible for
indicating when it no longer needs a given output buffer. This means
it could hold on to more than one output buffer at a time (for
buffering or whatever).
- The video encoders optimize the case where the previous frame's
output buffer has been freed before the next encode_frame() call.
- The GStreamer video encoder can now pass the (wrapped) GStreamer
output buffer to the Spice code and thus avoid a copy. When Spice
releases the output buffer the corresponding GStreamer buffer it
released.
server/gstreamer_encoder.c | 81 ++
server/mjpeg_encoder.c | 106 +++--
server/red_worker.c| 31 ++---
server/video_encoder.h | 45 ---
4 files changed, 187 insertions(+), 76 deletions(-)
diff --git a/server/gstreamer_encoder.c b/server/gstreamer_encoder.c
index 140261e..f7938ae 100644
--- a/server/gstreamer_encoder.c
+++ b/server/gstreamer_encoder.c
@@ -26,6 +26,8 @@
#include red_common.h
+typedef struct GstVideoBuffer GstVideoBuffer;
+#define VIDEO_BUFFER_T GstVideoBuffer
typedef struct GstEncoder GstEncoder;
#define VIDEO_ENCODER_T GstEncoder
#include video_encoder.h
@@ -44,6 +46,12 @@ typedef struct {
int red_mask;
} SpiceFormatForGStreamer;
+struct GstVideoBuffer {
+VideoBuffer base;
+GstBuffer *gst_buffer;
+gboolean persistent;
+};
+
struct GstEncoder {
VideoEncoder base;
@@ -72,6 +80,9 @@ struct GstEncoder {
GstElement *gstenc;
GstAppSink *appsink;
+/* The default video buffer */
+GstVideoBuffer *default_buffer;
+
/* The frame counter for GStreamer buffers */
uint32_t frame;
@@ -83,6 +94,35 @@ struct GstEncoder {
};
+/* -- The GstVideoBuffer implementation -- */
+
+static inline GstVideoBuffer* gst_video_buffer_ref(GstVideoBuffer *buffer)
+{
+buffer-base.ref_count++;
+return buffer;
+}
+
+static void gst_video_buffer_unref(GstVideoBuffer *buffer)
+{
+if (--buffer-base.ref_count == 0) {
+gst_buffer_unref(buffer-gst_buffer);
+if (!buffer-persistent) {
+free(buffer);
+}
+}
+}
+
+static GstVideoBuffer* create_gst_video_buffer(gboolean persistent)
+{
+GstVideoBuffer *buffer = spice_new0(GstVideoBuffer, 1);
+buffer-base.ref = gst_video_buffer_ref;
+buffer-base.unref = gst_video_buffer_unref;
+buffer-persistent = persistent;
+buffer-base.ref_count = persistent ? 0 : 1;
+return buffer;
+}
+
+
/* -- Miscellaneous GstEncoder helpers -- */
static inline double get_mbps(uint64_t bit_rate)
@@ -358,24 +398,24 @@ static int push_raw_frame(GstEncoder *encoder, const
SpiceBitmap *bitmap,
}
/* A helper for gst_encoder_encode_frame(). */
-static int pull_compressed_buffer(GstEncoder *encoder,
- uint8_t **outbuf, size_t *outbuf_size,
- int *data_size)
+static int pull_compressed_buffer(GstEncoder *encoder, GstVideoBuffer **buffer)
{
-GstBuffer *buffer = gst_app_sink_pull_buffer(encoder-appsink);
-if (buffer) {
-int len = GST_BUFFER_SIZE(buffer);
-spice_assert(outbuf outbuf_size);
-if (!*outbuf || *outbuf_size len) {
-*outbuf = spice_realloc(*outbuf, len);
-*outbuf_size = len;
-}
-/* TODO Try to avoid this copy by changing the GstBuffer handling */
-memcpy(*outbuf, GST_BUFFER_DATA(buffer),
[Spice-devel] [PATCH spice 13/13] server: Automatically adapt the GStreamer video encoder bit rate to the network conditions.
The video encoder uses the client reports and/or notifications of server frame
drops as its feedback mechanisms.
It uses these to figure out the lowest bit rate that causes negative feedback,
and the highest bit rate that allows a return to positive feedbacks. It then
works to narrow this range and settles on the lower end once the spread has
gone below a given threshold.
All the while it monitors the effective bit rate to ensure the target bit rate
does not grow significantly beyond what the GStreamer encoder will produce.
As soon as the network feedback indicates a significant degradation the bit
rate is lowered to minimize the risk of long freezes.
It also relies on the existing shaping of the GStreamer output bit rate to
minimize the pipeline reconfigurations.
Signed-off-by: Francois Gouget fgou...@codeweavers.com
---
A word about the effective bit rate monitoring: when the video switches
from high motion scenes to a mostly static talking head the GStreamer
encoder may decide to save bits for later resulting in an effective bit
rate that is half or less of its target. As a result all the network
indicators will turn green, causing the bit rate control code to think
the target can safely be increased. But increasing the target has no
impact on the bit rate of the GStreamer output. So in such a situation
it's possible to increase the target from 4Mbps to 20Mbps while the
effective bit rate remains stuck at 2Mbps. But of course everything
crumbles when the next action scene starts: then the GStreamer encoder
will make full use of the 20Mbps, causing entwork congestion and the bit
rate control algorithm will lose time halving the target bit rate
multiple times before reaching values more in line with what the actual
network capacity. Checking that the effective bit rate corresponds to
the target before any increase avoids this issue.
The feedback mechanism mostly uses the video margin in the client stream
reports. When all is good these are consistently close to the maximum
value. So this forms a sort of buffer: when it's half empty or more the
bit rate really must be reduced.
Regarding reacting to network congestion, there is always a lag between
the congestion occuring and the video encoder being made aware of it,
and then from there to the video encoder's actions having an effect. So
it's important to react as quickly as possible so the network congestion
does not escalate to dropped frames.
- This means algorithms that operate on their own schedule, like every
20 frames, are proscribed.
- Also server-side frame drops are systematically preceded by
client-side frame drops. So server-side frame drops must be acted on
immediately.
server/gstreamer_encoder.c | 396 ++---
1 file changed, 374 insertions(+), 22 deletions(-)
diff --git a/server/gstreamer_encoder.c b/server/gstreamer_encoder.c
index 4089ead..a46af7b 100644
--- a/server/gstreamer_encoder.c
+++ b/server/gstreamer_encoder.c
@@ -67,6 +67,12 @@ typedef struct {
uint32_t size;
} GstFrameInformation;
+enum GstBitRateStatus {
+GSTE_BITRATE_DECREASING,
+GSTE_BITRATE_INCREASING,
+GSTE_BITRATE_STABLE,
+};
+
struct GstEncoder {
VideoEncoder base;
@@ -106,6 +112,12 @@ struct GstEncoder {
/* The frame counter for GStreamer buffers */
uint32_t frame;
+/* The GStreamer bit rate. */
+uint64_t video_bit_rate;
+
+/* Don't bother changing the GStreamer bit rate if close enough. */
+# define GSTE_VIDEO_BITRATE_MARGIN 0.05
+
/* -- Encoded frame statistics -- */
@@ -140,7 +152,7 @@ struct GstEncoder {
/* -- Encoder bit rate control --
*
- * GStreamer encoders don't follow the specified bit rate very
+ * GStreamer encoders don't follow the specified video_bit_rate very
* closely. These fields are used to ensure we don't exceed the desired
* stream bit rate, regardless of the GStreamer encoder's output.
*/
@@ -148,7 +160,7 @@ struct GstEncoder {
/* The bit rate target for the outgoing network stream. (bits per second)
*/
uint64_t bit_rate;
-/* The minimum bit rate */
+/* The minimum bit rate / bit rate increment. */
# define GSTE_MIN_BITRATE (128 * 1024)
/* The bit rate control is performed using a virtual buffer to allow short
@@ -171,6 +183,89 @@ struct GstEncoder {
/* How big of a margin to take to cover for latency jitter. */
# define GSTE_LATENCY_MARGIN 0.1
+
+
+/* -- Network bit rate control --
+ *
+ * State information for figuring out the optimal bit rate for the current
+ * network conditions.
+ */
+
+/* The mm_time of the last bit rate change. */
+uint32_t last_change;
+
+/* How much to reduce the bit rate in case of network congestion. */
+# define GSTE_BITRATE_CUT 2
+# define GSTE_BITRATE_REDUCE (4.0 / 3.0)
+
+/* Never increase the bit rate
Re: [Spice-devel] [spice-gtk PATCH] Handle single headed monitors that have a non-zero x, y config
Hi,
On Mon, Jul 20, 2015 at 06:35:28PM -0400, Sandy Stutsman wrote:
Hi Again.
diff --git a/src/spice-widget.c b/src/spice-widget.c
index 59f9792..3ec2e65 100644
--- a/src/spice-widget.c
+++ b/src/spice-widget.c
@@ -293,7 +293,12 @@ static void update_monitor_area(SpiceDisplay
*display)
goto whole;
}
-update_area(display, c-x, c-y, c-width, c-height);
+/* If only one head on this monitor, update the whole area */
+if(monitors-len == 1) {
+update_area(display, 0, 0, c-width, c-height);
+} else {
+update_area(display, c-x, c-y, c-width, c-height);
+}
I did a little more testing today. I did see the Waiting for display ...2
message whenever
I started the remote-viewer with 1 monitor and tried to add a second. It
happened both
with and without the patch. If I added the second monitor, closed the viewer
and
re-opened it, the second monitor will display just fine.
Great, so it isn't really related to this patch.
The good news is that when the this patch is paired with the monitor config
qxl patch,
https://bugzilla.redhat.com/show_bug.cgi?id=1202419, I don't see the problem
at all.
Right, this is the patch I was missing then:
http://cgit.freedesktop.org/spice/win32/qxl/commit/?id=ed37b635188893719c59d71c031feddd01408f36
Do you have an qxl installer with the above patch for me to test it locally?
I'm asking because Pavel verified a problem in the following test:
1-) Connected with remove-viewer and both displays enabled
2-) Disable display 1
3-) Disconnect
4-) Connect again
R: Only Display 2 is enabled and it isn't possible to enable Display 1
If the above is also fixed with newer qxl them I don't see problems with
this patch as the main concern was related to Linux guest and I wasn't
able to find any problem...
Best,
Victor Toso
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [spice-common] proto: Remove space before tab
On Mon, Jul 20, 2015 at 09:56:10PM +0200, Pavel Grunt wrote: Ack, Thanks, pushed. Christophe pgpAKEx_nmMZU.pgp Description: PGP signature ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [libvirt] [PATCH v4] qemu: Use heads parameter for QXL driver
Hi, On Mon, Jul 20, 2015 at 11:25:52AM +0200, Martin Kletzander wrote: In the meantime, is the only thing this does limiting the maximum? Is it there just to save some memory or why? Because otherwise I can't see the use-case in that. I'm not saying there isn't one, just that I can't find it. And I even looked under the fridge :) There are 2 main uses for this feature: - first, it's indeed to help in setting up guest memory size, you need a bit more than 16MB of VRAM per full-HD display that you want your guest to support, and the failure modes when the guest tries to go over that limit are not explicit at all. Thus it can be helpful to enforce the limitation to just 1 monitor if you don't want to assign a lot of VRAM to your guest - the second one is more cosmetic, management innterfaces (oVirt) let you select the number of screens you want in the VM, but at the moment it's not used at all on the SPICE client side, you always get a list of 4 displays which you can enable in one of the client menus. After the work Frediano has done, you only see 1 entry in that client menu if you said in the VM XML that you only want 1 head. Hope that helps, Christophe pgpX0m2ee7YpP.pgp Description: PGP signature ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH 08/33] codegen: Remove old ptr32 attribute
On Wed, Jul 01, 2015 at 06:10:00PM +0100, Frediano Ziglio wrote:
This attribute is not use in code.
'used'
This was removed in commit 2e8aecc2a56e5b7690d77516e41436b697921b1b I
think. I'd move this commit before the preceding one as you are removing
code that you just added. Or did you use these new checks to make sure
that this is really not used?
Christophe
Well, I could easily add this attribute to the list instead.
I think that 64 pointers in the network was just a mistake of protocol 1.
They means messages are more than 4gb but they are contained (even on protocol
1) in 4gb as message size is 4 bytes. And supporting more bytes would mean that
we should allocate lot of memory as we store entire message in memory. Probably
somebody could use these sizes (even 32 bit) to force allocation of huge memory
(on both client and servers) causing possibly some DoS.
Frediano
Signed-off-by: Frediano Ziglio fzig...@redhat.com
---
python_modules/ptypes.py | 4
1 file changed, 4 deletions(-)
diff --git a/python_modules/ptypes.py b/python_modules/ptypes.py
index 3a307ed..f94fd24 100644
--- a/python_modules/ptypes.py
+++ b/python_modules/ptypes.py
@@ -105,8 +105,6 @@ valid_attributes={
# for a switch this indicate that on network
# will occupy always same size (maximum size required for all members)
'fixedsize',
-# use 32 bit pointer
-'ptr32',
}
attributes_with_arguments={
@@ -613,8 +611,6 @@ class Member(Containee):
self.container = container
self.member_type = self.member_type.resolve()
self.member_type.register()
-if self.has_attr(ptr32) and self.member_type.is_pointer():
-self.member_type.set_ptr_size(4)
for i in propagated_attributes:
if self.has_attr(i):
self.member_type.attributes[i] = self.attributes[i]
--
2.1.0
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [libvirt] [PATCH v4] qemu: Use heads parameter for QXL driver
On Tue, Jul 21, 2015 at 09:36:55AM +0200, Christophe Fergeau wrote: On Mon, Jul 20, 2015 at 11:25:52AM +0200, Martin Kletzander wrote: I spend all morning fixing this to be installed properly in the system. Anyway, I finally managed to make this work and found out the guest I used for it is not ready to have multiple monitors. Anyway, looking at everything else this definitely works, so ACK, I'll push it in a while. I'm still a bit worried that all existing guests will have heads='1' in their XML as libvirt is adding that by default, but I don't really see a way around it :-/ We should make sure libvirt stops adding it from now on though ;) Well, how would you then limit the outputs to 1? Having said that, I have no idea why we started adding heads=1 automatically and playing with different changes, we have another bug in the parsing/formatting code. You'll also won't be able to migrate from older libvirt with heads='1' to newer ones. I haven't realized that. I'm thinking about reverting the change in libvirt or even using heads 1. Although that won't migrate either. So the only other thing that we can do is to introduce new parameter, like maxHeads. The sound you just heard is me slapping my face because again there will multiple parameters meaning similar things... Christophe signature.asc Description: PGP signature ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [libvirt] [PATCH v4] qemu: Use heads parameter for QXL driver
On Mon, Jul 20, 2015 at 11:25:52AM +0200, Martin Kletzander wrote: I spend all morning fixing this to be installed properly in the system. Anyway, I finally managed to make this work and found out the guest I used for it is not ready to have multiple monitors. Anyway, looking at everything else this definitely works, so ACK, I'll push it in a while. I'm still a bit worried that all existing guests will have heads='1' in their XML as libvirt is adding that by default, but I don't really see a way around it :-/ We should make sure libvirt stops adding it from now on though ;) Christophe pgpR2HYmDeHrw.pgp Description: PGP signature ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [libvirt] [PATCH v4] qemu: Use heads parameter for QXL driver
On Tue, Jul 21, 2015 at 03:35:50PM +0200, Martin Kletzander wrote: On Tue, Jul 21, 2015 at 01:50:22PM +0100, Daniel P. Berrange wrote: On Tue, Jul 21, 2015 at 11:44:27AM +0200, Martin Kletzander wrote: On Tue, Jul 21, 2015 at 09:36:55AM +0200, Christophe Fergeau wrote: On Mon, Jul 20, 2015 at 11:25:52AM +0200, Martin Kletzander wrote: I spend all morning fixing this to be installed properly in the system. Anyway, I finally managed to make this work and found out the guest I used for it is not ready to have multiple monitors. Anyway, looking at everything else this definitely works, so ACK, I'll push it in a while. I'm still a bit worried that all existing guests will have heads='1' in their XML as libvirt is adding that by default, but I don't really see a way around it :-/ We should make sure libvirt stops adding it from now on though ;) Well, how would you then limit the outputs to 1? Having said that, I have no idea why we started adding heads=1 automatically and playing with different changes, we have another bug in the parsing/formatting code. You'll also won't be able to migrate from older libvirt with heads='1' to newer ones. I haven't realized that. I'm thinking about reverting the change in libvirt or even using heads 1. Although that won't migrate either. So the only other thing that we can do is to introduce new parameter, like maxHeads. The sound you just heard is me slapping my face because again there will multiple parameters meaning similar things... Introducing a new parameter is not really viable IMHO, because as you say it'll leave us with two things meaning the same, and will not be compatible with existing apps that know about the current parameter. I think we need to figure out a way to drop the 'heads=1' from any existing configs in /etc/libvirt/qemu when we start up with the new libvirtd for the first time. I'm already working on an RFC that would differentiate between loading XMLs on daemon start-up and defining them. The purpose of that is so that we can make incompatible XML changes without losing any domains, but that's not the point here. If we were to drop heads='1' anywhere, this is the place. The ABI change would be minimal for the guest. It isn't sufficient to just differentiate loading on startup vs defining them IIUC. We need to differentiate loading on startup from XML previously written by a libvirtd X.Y.Z which is why I think we need to have a version number recorded in the XML ultimately. To make this work we probably need to write a magic attribute or comment into the XML configs to record which version of libvirt saved it to disk. That way we know whether this is the first time loading the config with the new libvirt. It will help us with similar situations in the future. In this case, we'd just look to see if the libvirt version number was missing from the XML, and if so, then drop heads=1. If a version number is present, then we're new enough, so we can keep it. We'd also ned to pass this version number in the XML when migrating, or possibly via the migration cookie. And you're suggesting to differentiate this by a comment? That's really, *really* hacky. Anyway, do you agree on temporarily reverting the commit so we don't release another version with it? The changes will also be clearer when they will not modify what this commit changed. Actually we'd need to have a formal attribute to deal with the migration case, so slightly less hacky. Yeah, if we can't immediately fix it, we should revert it until we have a decent fix without regression. Likewise probably fix it on stable branch too. Regards, 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 :| ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [libvirt] [PATCH v4] qemu: Use heads parameter for QXL driver
On Tue, Jul 21, 2015 at 01:50:22PM +0100, Daniel P. Berrange wrote: On Tue, Jul 21, 2015 at 11:44:27AM +0200, Martin Kletzander wrote: On Tue, Jul 21, 2015 at 09:36:55AM +0200, Christophe Fergeau wrote: On Mon, Jul 20, 2015 at 11:25:52AM +0200, Martin Kletzander wrote: I spend all morning fixing this to be installed properly in the system. Anyway, I finally managed to make this work and found out the guest I used for it is not ready to have multiple monitors. Anyway, looking at everything else this definitely works, so ACK, I'll push it in a while. I'm still a bit worried that all existing guests will have heads='1' in their XML as libvirt is adding that by default, but I don't really see a way around it :-/ We should make sure libvirt stops adding it from now on though ;) Well, how would you then limit the outputs to 1? Having said that, I have no idea why we started adding heads=1 automatically and playing with different changes, we have another bug in the parsing/formatting code. You'll also won't be able to migrate from older libvirt with heads='1' to newer ones. I haven't realized that. I'm thinking about reverting the change in libvirt or even using heads 1. Although that won't migrate either. So the only other thing that we can do is to introduce new parameter, like maxHeads. The sound you just heard is me slapping my face because again there will multiple parameters meaning similar things... Introducing a new parameter is not really viable IMHO, because as you say it'll leave us with two things meaning the same, and will not be compatible with existing apps that know about the current parameter. I think we need to figure out a way to drop the 'heads=1' from any existing configs in /etc/libvirt/qemu when we start up with the new libvirtd for the first time. I'm already working on an RFC that would differentiate between loading XMLs on daemon start-up and defining them. The purpose of that is so that we can make incompatible XML changes without losing any domains, but that's not the point here. If we were to drop heads='1' anywhere, this is the place. The ABI change would be minimal for the guest. To make this work we probably need to write a magic attribute or comment into the XML configs to record which version of libvirt saved it to disk. That way we know whether this is the first time loading the config with the new libvirt. It will help us with similar situations in the future. In this case, we'd just look to see if the libvirt version number was missing from the XML, and if so, then drop heads=1. If a version number is present, then we're new enough, so we can keep it. We'd also ned to pass this version number in the XML when migrating, or possibly via the migration cookie. And you're suggesting to differentiate this by a comment? That's really, *really* hacky. Anyway, do you agree on temporarily reverting the commit so we don't release another version with it? The changes will also be clearer when they will not modify what this commit changed. Regards, 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 :| signature.asc Description: PGP signature ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [libvirt] [PATCH v4] qemu: Use heads parameter for QXL driver
On Tue, Jul 21, 2015 at 11:44:27AM +0200, Martin Kletzander wrote: On Tue, Jul 21, 2015 at 09:36:55AM +0200, Christophe Fergeau wrote: On Mon, Jul 20, 2015 at 11:25:52AM +0200, Martin Kletzander wrote: I spend all morning fixing this to be installed properly in the system. Anyway, I finally managed to make this work and found out the guest I used for it is not ready to have multiple monitors. Anyway, looking at everything else this definitely works, so ACK, I'll push it in a while. I'm still a bit worried that all existing guests will have heads='1' in their XML as libvirt is adding that by default, but I don't really see a way around it :-/ We should make sure libvirt stops adding it from now on though ;) Well, how would you then limit the outputs to 1? Having said that, I have no idea why we started adding heads=1 automatically and playing with different changes, we have another bug in the parsing/formatting code. You'll also won't be able to migrate from older libvirt with heads='1' to newer ones. I haven't realized that. I'm thinking about reverting the change in libvirt or even using heads 1. Although that won't migrate either. So the only other thing that we can do is to introduce new parameter, like maxHeads. The sound you just heard is me slapping my face because again there will multiple parameters meaning similar things... Introducing a new parameter is not really viable IMHO, because as you say it'll leave us with two things meaning the same, and will not be compatible with existing apps that know about the current parameter. I think we need to figure out a way to drop the 'heads=1' from any existing configs in /etc/libvirt/qemu when we start up with the new libvirtd for the first time. To make this work we probably need to write a magic attribute or comment into the XML configs to record which version of libvirt saved it to disk. That way we know whether this is the first time loading the config with the new libvirt. It will help us with similar situations in the future. In this case, we'd just look to see if the libvirt version number was missing from the XML, and if so, then drop heads=1. If a version number is present, then we're new enough, so we can keep it. We'd also ned to pass this version number in the XML when migrating, or possibly via the migration cookie. Regards, 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 :| ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [libvirt] [PATCH v4] qemu: Use heads parameter for QXL driver
On Tue, Jul 21, 2015 at 09:36:55AM +0200, Christophe Fergeau wrote: On Mon, Jul 20, 2015 at 11:25:52AM +0200, Martin Kletzander wrote: I spend all morning fixing this to be installed properly in the system. Anyway, I finally managed to make this work and found out the guest I used for it is not ready to have multiple monitors. Anyway, looking at everything else this definitely works, so ACK, I'll push it in a while. I'm still a bit worried that all existing guests will have heads='1' in their XML as libvirt is adding that by default, but I don't really see a way around it :-/ We should make sure libvirt stops adding it from now on though ;) Well, how would you then limit the outputs to 1? Having said that, I have no idea why we started adding heads=1 automatically and playing with different changes, we have another bug in the parsing/formatting code. You'll also won't be able to migrate from older libvirt with heads='1' to newer ones. I haven't realized that. I'm thinking about reverting the change in libvirt or even using heads 1. Although that won't migrate either. So the only other thing that we can do is to introduce new parameter, like maxHeads. The sound you just heard is me slapping my face because again there will multiple parameters meaning similar things... heads specify the number of heads the emulated virtual card has. I think the problem was specify the number for QXL which never honored this setting. I don't think you can't migrate, just after migration the machine won't allow more than 1 head. Unfortunately the XML file does not have any field to specify the libvirt version was meant for. It would be useful to add so if you see the version it means that heads==1 means 1 if not it means is not defined. I'm against a new parameter which specify the same meaning of another parameter. I would instead something like validHeads or headsSet with a false as default. Christophe Frediano ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [libvirt] [PATCH v4] qemu: Use heads parameter for QXL driver
On Tue, Jul 21, 2015 at 08:41:33AM -0400, Frediano Ziglio wrote: On Tue, Jul 21, 2015 at 09:36:55AM +0200, Christophe Fergeau wrote: On Mon, Jul 20, 2015 at 11:25:52AM +0200, Martin Kletzander wrote: I spend all morning fixing this to be installed properly in the system. Anyway, I finally managed to make this work and found out the guest I used for it is not ready to have multiple monitors. Anyway, looking at everything else this definitely works, so ACK, I'll push it in a while. I'm still a bit worried that all existing guests will have heads='1' in their XML as libvirt is adding that by default, but I don't really see a way around it :-/ We should make sure libvirt stops adding it from now on though ;) Well, how would you then limit the outputs to 1? Having said that, I have no idea why we started adding heads=1 automatically and playing with different changes, we have another bug in the parsing/formatting code. You'll also won't be able to migrate from older libvirt with heads='1' to newer ones. I haven't realized that. I'm thinking about reverting the change in libvirt or even using heads 1. Although that won't migrate either. So the only other thing that we can do is to introduce new parameter, like maxHeads. The sound you just heard is me slapping my face because again there will multiple parameters meaning similar things... heads specify the number of heads the emulated virtual card has. I think the problem was specify the number for QXL which never honored this setting. I don't think you can't migrate, just after migration the machine won't allow more than 1 head. Unfortunately the XML file does not have any field to specify the libvirt version was meant for. It would be useful to add so if you see the version it means that heads==1 means 1 if not it means is not defined. I'm against a new parameter which specify the same meaning of another parameter. I would instead something like validHeads or headsSet with a false as default. [Would you mind convincing your mail client to wrap long lines?] Well, libvirt is, unfortunately, by definition, backward compatible. So we guarantee, that you don't have to change this stuff and you can migrate to newer ones. The problem here with the migration is that the guests will be binary incompatible, the amount of memory you need to transfer from source will not fit the destination. Until we reach a decision, I would revert the patch. We need to decide how to handle this. About the new name. I take it as 'heads' defines how many monitors there are and 'maxHeads' would define the maximum of them to have. The difference would be there of course only for qxl. I don't think cirrus, for example, can dynamically change number of monitors connected, can it? We had similar problem with the ram/vram/vgaram attributes and we ended up with all of them and bunch of lines dealing with just the update and migration compatibility. Having headsSet is pointless. How would you decide whether heads is actually 1 or not if you were to parse an XML with heads='1'? Dropping heads='1' is not an option as well since that would make other video models (that support heads=) change their settings. And nobody could set heads='1' any more without also setting another parameter. The problem is we screwed up when we defaulted heads to a number even though there are devices that do not handle that properly. That's the past, we can't do much about that due to back-compat (Aargh!), but that's how it is, unfortunately). That are my thoughts on why maxHeads should be a new parameter that would set max_outputs for the qxl device. signature.asc Description: PGP signature ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
