qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [RfC PATCH 00/10] vnc buffer handling
@ 2015-09-24  8:41 Gerd Hoffmann
  2015-09-24  8:41 ` [Qemu-devel] [RfC PATCH 01/10] io/ makefile fluff Gerd Hoffmann
                   ` (11 more replies)
  0 siblings, 12 replies; 22+ messages in thread
From: Gerd Hoffmann @ 2015-09-24  8:41 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Lieven, Gerd Hoffmann

  Hi,

Here is a patch series to improve the vnc buffer handling.  It picks up
the qio_buffer patches from Daniel, adds move calls (move data from one
buffer to another) and tracing, makes vnc use the new features.  Net
effect should be that (a) vnc copies less data around and (b) buffers
don't grow forever.

It's RfC because it depends on wip patches.  My plan is to wait for
Daniels patch series to be merged (which should obsolete patches #1+#2),
then rebase and repost the series.

Patches are also available from git:
  git://git.kraxel.org/qemu rebase/ui-vnc-next

please test & review,
  Gerd

Daniel P. Berrange (1):
  io: pull Buffer code out of VNC module

Gerd Hoffmann (8):
  io/ makefile fluff
  io: add qio_buffer_init
  io: add qio_buffer_move_empty
  io: add qio_buffer_move
  io: add qio_buffer tracing
  name vnc buffers
  vnc: kill jobs queue buffer
  vnc-jobs: move buffer reset, use new buffer move

Peter Lieven (1):
  vnc: make the Buffer capacity increase in powers of two

 Makefile            |   2 +
 Makefile.objs       |   5 ++
 Makefile.target     |   2 +
 include/io/buffer.h | 150 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 io/Makefile.objs    |   1 +
 io/buffer.c         | 119 +++++++++++++++++++++++++++++++++++++++++
 trace-events        |   6 +++
 ui/vnc-auth-sasl.c  |   4 +-
 ui/vnc-enc-tight.c  |  38 ++++++-------
 ui/vnc-enc-zlib.c   |   6 +--
 ui/vnc-enc-zrle.c   |  18 +++----
 ui/vnc-jobs.c       |  16 ++----
 ui/vnc-ws.c         |  36 ++++++-------
 ui/vnc-ws.h         |   6 +--
 ui/vnc.c            |  83 ++++++++++-------------------
 ui/vnc.h            |  50 +++++++-----------
 16 files changed, 389 insertions(+), 153 deletions(-)
 create mode 100644 include/io/buffer.h
 create mode 100644 io/Makefile.objs
 create mode 100644 io/buffer.c

-- 
1.8.3.1

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [Qemu-devel] [RfC PATCH 01/10] io/ makefile fluff
  2015-09-24  8:41 [Qemu-devel] [RfC PATCH 00/10] vnc buffer handling Gerd Hoffmann
@ 2015-09-24  8:41 ` Gerd Hoffmann
  2015-09-24  8:42 ` [Qemu-devel] [RfC PATCH 02/10] io: pull Buffer code out of VNC module Gerd Hoffmann
                   ` (10 subsequent siblings)
  11 siblings, 0 replies; 22+ messages in thread
From: Gerd Hoffmann @ 2015-09-24  8:41 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Lieven, Gerd Hoffmann

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 Makefile        | 2 ++
 Makefile.objs   | 5 +++++
 Makefile.target | 2 ++
 3 files changed, 9 insertions(+)

diff --git a/Makefile b/Makefile
index 8ec9b69..ca18209 100644
--- a/Makefile
+++ b/Makefile
@@ -157,6 +157,7 @@ dummy := $(call unnest-vars,, \
                 crypto-obj-y \
                 crypto-aes-obj-y \
                 qom-obj-y \
+                io-obj-y \
                 common-obj-y \
                 common-obj-m)
 
@@ -180,6 +181,7 @@ SOFTMMU_SUBDIR_RULES=$(filter %-softmmu,$(SUBDIR_RULES))
 $(SOFTMMU_SUBDIR_RULES): $(block-obj-y)
 $(SOFTMMU_SUBDIR_RULES): $(crypto-obj-y)
 $(SOFTMMU_SUBDIR_RULES): $(qom-obj-y)
+$(SOFTMMU_SUBDIR_RULES): $(io-obj-y)
 $(SOFTMMU_SUBDIR_RULES): config-all-devices.mak
 
 subdir-%:
diff --git a/Makefile.objs b/Makefile.objs
index ce87778..8c9461e 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -32,6 +32,11 @@ crypto-aes-obj-y = crypto/
 
 qom-obj-y = qom/
 
+#######################################################################
+# io-obj-y is code used by both qemu system emulation and qemu-img
+
+io-obj-y = io/
+
 ######################################################################
 # smartcard
 
diff --git a/Makefile.target b/Makefile.target
index 962d004..34ddb7e 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -176,6 +176,7 @@ dummy := $(call unnest-vars,.., \
                crypto-obj-y \
                crypto-aes-obj-y \
                qom-obj-y \
+               io-obj-y \
                common-obj-y \
                common-obj-m)
 target-obj-y := $(target-obj-y-save)
@@ -185,6 +186,7 @@ all-obj-y += $(qom-obj-y)
 all-obj-$(CONFIG_SOFTMMU) += $(block-obj-y)
 all-obj-$(CONFIG_USER_ONLY) += $(crypto-aes-obj-y)
 all-obj-$(CONFIG_SOFTMMU) += $(crypto-obj-y)
+all-obj-$(CONFIG_SOFTMMU) += $(io-obj-y)
 
 $(QEMU_PROG_BUILD): config-devices.mak
 
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [Qemu-devel] [RfC PATCH 02/10] io: pull Buffer code out of VNC module
  2015-09-24  8:41 [Qemu-devel] [RfC PATCH 00/10] vnc buffer handling Gerd Hoffmann
  2015-09-24  8:41 ` [Qemu-devel] [RfC PATCH 01/10] io/ makefile fluff Gerd Hoffmann
@ 2015-09-24  8:42 ` Gerd Hoffmann
  2015-09-25  9:57   ` Peter Lieven
  2015-09-24  8:42 ` [Qemu-devel] [RfC PATCH 03/10] vnc: make the Buffer capacity increase in powers of two Gerd Hoffmann
                   ` (9 subsequent siblings)
  11 siblings, 1 reply; 22+ messages in thread
From: Gerd Hoffmann @ 2015-09-24  8:42 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Lieven, Gerd Hoffmann

From: "Daniel P. Berrange" <berrange@redhat.com>

The Buffer code in the VNC server is useful for the IO channel
code, so pull it out into a shared module, QIOBuffer.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 include/io/buffer.h | 118 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 io/Makefile.objs    |   1 +
 io/buffer.c         |  65 +++++++++++++++++++++++++++++
 ui/vnc-auth-sasl.c  |   4 +-
 ui/vnc-enc-tight.c  |  38 ++++++++---------
 ui/vnc-enc-zlib.c   |   6 +--
 ui/vnc-enc-zrle.c   |  18 ++++----
 ui/vnc-jobs.c       |  15 +++----
 ui/vnc-ws.c         |  36 ++++++++--------
 ui/vnc-ws.h         |   6 +--
 ui/vnc.c            |  67 ++++++-----------------------
 ui/vnc.h            |  50 ++++++++--------------
 12 files changed, 276 insertions(+), 148 deletions(-)
 create mode 100644 include/io/buffer.h
 create mode 100644 io/Makefile.objs
 create mode 100644 io/buffer.c

diff --git a/include/io/buffer.h b/include/io/buffer.h
new file mode 100644
index 0000000..2b1b261
--- /dev/null
+++ b/include/io/buffer.h
@@ -0,0 +1,118 @@
+/*
+ * QEMU I/O buffers
+ *
+ * Copyright (c) 2015 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+#ifndef QIO_BUFFER_H__
+#define QIO_BUFFER_H__
+
+#include "qemu-common.h"
+
+typedef struct QIOBuffer QIOBuffer;
+
+/**
+ * QIOBuffer:
+ *
+ * The QIOBuffer object provides a simple dynamically resizing
+ * array, with separate tracking of capacity and usage. This
+ * is typically useful when buffering I/O data.
+ */
+
+struct QIOBuffer {
+    size_t capacity;
+    size_t offset;
+    uint8_t *buffer;
+};
+
+/**
+ * qio_buffer_reserve:
+ * @buffer: the buffer object
+ * @len: the minimum required free space
+ *
+ * Ensure that the buffer has space allocated for at least
+ * @len bytes. If the current buffer is too small, it will
+ * be reallocated, possibly to a larger size than requested.
+ */
+void qio_buffer_reserve(QIOBuffer *buffer, size_t len);
+
+/**
+ * qio_buffer_reset:
+ * @buffer: the buffer object
+ *
+ * Reset the length of the stored data to zero, but do
+ * not free / reallocate the memory buffer
+ */
+void qio_buffer_reset(QIOBuffer *buffer);
+
+/**
+ * qio_buffer_free:
+ * @buffer: the buffer object
+ *
+ * Reset the length of the stored data to zero and also
+ * free the internal memory buffer
+ */
+void qio_buffer_free(QIOBuffer *buffer);
+
+/**
+ * qio_buffer_append:
+ * @buffer: the buffer object
+ * @data: the data block to append
+ * @len: the length of @data in bytes
+ *
+ * Append the contents of @data to the end of the buffer.
+ * The caller must ensure that the buffer has sufficient
+ * free space for @len bytes, typically by calling the
+ * qio_buffer_reserve() method prior to appending.
+ */
+void qio_buffer_append(QIOBuffer *buffer, const void *data, size_t len);
+
+/**
+ * qio_buffer_advance:
+ * @buffer: the buffer object
+ * @len: the number of bytes to skip
+ *
+ * Remove @len bytes of data from the head of the buffer.
+ * The internal buffer will not be reallocated, so will
+ * have at least @len bytes of free space after this
+ * call completes
+ */
+void qio_buffer_advance(QIOBuffer *buffer, size_t len);
+
+/**
+ * qio_buffer_end:
+ * @buffer: the buffer object
+ *
+ * Get a pointer to the tail end of the internal buffer
+ * The returned pointer is only valid until the next
+ * call to qio_buffer_reserve().
+ *
+ * Returns: the tail of the buffer
+ */
+uint8_t *qio_buffer_end(QIOBuffer *buffer);
+
+/**
+ * qio_buffer_empty:
+ * @buffer: the buffer object
+ *
+ * Determine if the buffer contains any current data
+ *
+ * Returns: true if the buffer holds data, false otherwise
+ */
+gboolean qio_buffer_empty(QIOBuffer *buffer);
+
+#endif /* QIO_BUFFER_H__ */
diff --git a/io/Makefile.objs b/io/Makefile.objs
new file mode 100644
index 0000000..3de4e47
--- /dev/null
+++ b/io/Makefile.objs
@@ -0,0 +1 @@
+io-obj-y = buffer.o
diff --git a/io/buffer.c b/io/buffer.c
new file mode 100644
index 0000000..68ae68d
--- /dev/null
+++ b/io/buffer.c
@@ -0,0 +1,65 @@
+/*
+ * QEMU I/O buffers
+ *
+ * Copyright (c) 2015 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+#include "io/buffer.h"
+
+void qio_buffer_reserve(QIOBuffer *buffer, size_t len)
+{
+    if ((buffer->capacity - buffer->offset) < len) {
+        buffer->capacity += (len + 1024);
+        buffer->buffer = g_realloc(buffer->buffer, buffer->capacity);
+    }
+}
+
+gboolean qio_buffer_empty(QIOBuffer *buffer)
+{
+    return buffer->offset == 0;
+}
+
+uint8_t *qio_buffer_end(QIOBuffer *buffer)
+{
+    return buffer->buffer + buffer->offset;
+}
+
+void qio_buffer_reset(QIOBuffer *buffer)
+{
+    buffer->offset = 0;
+}
+
+void qio_buffer_free(QIOBuffer *buffer)
+{
+    g_free(buffer->buffer);
+    buffer->offset = 0;
+    buffer->capacity = 0;
+    buffer->buffer = NULL;
+}
+
+void qio_buffer_append(QIOBuffer *buffer, const void *data, size_t len)
+{
+    memcpy(buffer->buffer + buffer->offset, data, len);
+    buffer->offset += len;
+}
+
+void qio_buffer_advance(QIOBuffer *buffer, size_t len)
+{
+    memmove(buffer->buffer, buffer->buffer + len,
+            (buffer->offset - len));
+    buffer->offset -= len;
+}
diff --git a/ui/vnc-auth-sasl.c b/ui/vnc-auth-sasl.c
index fc732bd..d118266 100644
--- a/ui/vnc-auth-sasl.c
+++ b/ui/vnc-auth-sasl.c
@@ -113,8 +113,8 @@ long vnc_client_read_sasl(VncState *vs)
         return vnc_client_io_error(vs, -1, -EIO);
     VNC_DEBUG("Read SASL Encoded %p size %ld Decoded %p size %d\n",
               encoded, ret, decoded, decodedLen);
-    buffer_reserve(&vs->input, decodedLen);
-    buffer_append(&vs->input, decoded, decodedLen);
+    qio_buffer_reserve(&vs->input, decodedLen);
+    qio_buffer_append(&vs->input, decoded, decodedLen);
     return decodedLen;
 }
 
diff --git a/ui/vnc-enc-tight.c b/ui/vnc-enc-tight.c
index 9a9ddf2..772ec79 100644
--- a/ui/vnc-enc-tight.c
+++ b/ui/vnc-enc-tight.c
@@ -856,7 +856,7 @@ static int tight_compress_data(VncState *vs, int stream_id, size_t bytes,
     }
 
     /* reserve memory in output buffer */
-    buffer_reserve(&vs->tight.zlib, bytes + 64);
+    qio_buffer_reserve(&vs->tight.zlib, bytes + 64);
 
     /* set pointers */
     zstream->next_in = vs->tight.tight.buffer;
@@ -879,7 +879,7 @@ static int tight_compress_data(VncState *vs, int stream_id, size_t bytes,
     tight_send_compact_size(vs, bytes);
     vnc_write(vs, vs->tight.zlib.buffer, bytes);
 
-    buffer_reset(&vs->tight.zlib);
+    qio_buffer_reset(&vs->tight.zlib);
 
     return bytes;
 }
@@ -1053,7 +1053,7 @@ static bool send_gradient_rect(VncState *vs, int x, int y, int w, int h)
     vnc_write_u8(vs, (stream | VNC_TIGHT_EXPLICIT_FILTER) << 4);
     vnc_write_u8(vs, VNC_TIGHT_FILTER_GRADIENT);
 
-    buffer_reserve(&vs->tight.gradient, w * 3 * sizeof (int));
+    qio_buffer_reserve(&vs->tight.gradient, w * 3 * sizeof(int));
 
     if (vs->tight.pixel24) {
         tight_filter_gradient24(vs, vs->tight.tight.buffer, w, h);
@@ -1066,7 +1066,7 @@ static bool send_gradient_rect(VncState *vs, int x, int y, int w, int h)
         bytes = 2;
     }
 
-    buffer_reset(&vs->tight.gradient);
+    qio_buffer_reset(&vs->tight.gradient);
 
     bytes = w * h * bytes;
     vs->tight.tight.offset = bytes;
@@ -1149,7 +1149,7 @@ static int send_palette_rect(VncState *vs, int x, int y,
 static void jpeg_init_destination(j_compress_ptr cinfo)
 {
     VncState *vs = cinfo->client_data;
-    Buffer *buffer = &vs->tight.jpeg;
+    QIOBuffer *buffer = &vs->tight.jpeg;
 
     cinfo->dest->next_output_byte = (JOCTET *)buffer->buffer + buffer->offset;
     cinfo->dest->free_in_buffer = (size_t)(buffer->capacity - buffer->offset);
@@ -1159,10 +1159,10 @@ static void jpeg_init_destination(j_compress_ptr cinfo)
 static boolean jpeg_empty_output_buffer(j_compress_ptr cinfo)
 {
     VncState *vs = cinfo->client_data;
-    Buffer *buffer = &vs->tight.jpeg;
+    QIOBuffer *buffer = &vs->tight.jpeg;
 
     buffer->offset = buffer->capacity;
-    buffer_reserve(buffer, 2048);
+    qio_buffer_reserve(buffer, 2048);
     jpeg_init_destination(cinfo);
     return TRUE;
 }
@@ -1171,7 +1171,7 @@ static boolean jpeg_empty_output_buffer(j_compress_ptr cinfo)
 static void jpeg_term_destination(j_compress_ptr cinfo)
 {
     VncState *vs = cinfo->client_data;
-    Buffer *buffer = &vs->tight.jpeg;
+    QIOBuffer *buffer = &vs->tight.jpeg;
 
     buffer->offset = buffer->capacity - cinfo->dest->free_in_buffer;
 }
@@ -1190,7 +1190,7 @@ static int send_jpeg_rect(VncState *vs, int x, int y, int w, int h, int quality)
         return send_full_color_rect(vs, x, y, w, h);
     }
 
-    buffer_reserve(&vs->tight.jpeg, 2048);
+    qio_buffer_reserve(&vs->tight.jpeg, 2048);
 
     cinfo.err = jpeg_std_error(&jerr);
     jpeg_create_compress(&cinfo);
@@ -1227,7 +1227,7 @@ static int send_jpeg_rect(VncState *vs, int x, int y, int w, int h, int quality)
 
     tight_send_compact_size(vs, vs->tight.jpeg.offset);
     vnc_write(vs, vs->tight.jpeg.buffer, vs->tight.jpeg.offset);
-    buffer_reset(&vs->tight.jpeg);
+    qio_buffer_reset(&vs->tight.jpeg);
 
     return 1;
 }
@@ -1270,7 +1270,7 @@ static void png_write_data(png_structp png_ptr, png_bytep data,
 {
     VncState *vs = png_get_io_ptr(png_ptr);
 
-    buffer_reserve(&vs->tight.png, vs->tight.png.offset + length);
+    qio_buffer_reserve(&vs->tight.png, vs->tight.png.offset + length);
     memcpy(vs->tight.png.buffer + vs->tight.png.offset, data, length);
 
     vs->tight.png.offset += length;
@@ -1351,7 +1351,7 @@ static int send_png_rect(VncState *vs, int x, int y, int w, int h,
 
     png_write_info(png_ptr, info_ptr);
 
-    buffer_reserve(&vs->tight.png, 2048);
+    qio_buffer_reserve(&vs->tight.png, 2048);
     linebuf = qemu_pixman_linebuf_create(PIXMAN_BE_r8g8b8, w);
     buf = (uint8_t *)pixman_image_get_data(linebuf);
     for (dy = 0; dy < h; dy++)
@@ -1377,14 +1377,14 @@ static int send_png_rect(VncState *vs, int x, int y, int w, int h,
 
     tight_send_compact_size(vs, vs->tight.png.offset);
     vnc_write(vs, vs->tight.png.buffer, vs->tight.png.offset);
-    buffer_reset(&vs->tight.png);
+    qio_buffer_reset(&vs->tight.png);
     return 1;
 }
 #endif /* CONFIG_VNC_PNG */
 
 static void vnc_tight_start(VncState *vs)
 {
-    buffer_reset(&vs->tight.tight);
+    qio_buffer_reset(&vs->tight.tight);
 
     // make the output buffer be the zlib buffer, so we can compress it later
     vs->tight.tmp = vs->output;
@@ -1686,13 +1686,13 @@ void vnc_tight_clear(VncState *vs)
         }
     }
 
-    buffer_free(&vs->tight.tight);
-    buffer_free(&vs->tight.zlib);
-    buffer_free(&vs->tight.gradient);
+    qio_buffer_free(&vs->tight.tight);
+    qio_buffer_free(&vs->tight.zlib);
+    qio_buffer_free(&vs->tight.gradient);
 #ifdef CONFIG_VNC_JPEG
-    buffer_free(&vs->tight.jpeg);
+    qio_buffer_free(&vs->tight.jpeg);
 #endif
 #ifdef CONFIG_VNC_PNG
-    buffer_free(&vs->tight.png);
+    qio_buffer_free(&vs->tight.png);
 #endif
 }
diff --git a/ui/vnc-enc-zlib.c b/ui/vnc-enc-zlib.c
index d1b97f2..47ba146 100644
--- a/ui/vnc-enc-zlib.c
+++ b/ui/vnc-enc-zlib.c
@@ -47,7 +47,7 @@ void vnc_zlib_zfree(void *x, void *addr)
 
 static void vnc_zlib_start(VncState *vs)
 {
-    buffer_reset(&vs->zlib.zlib);
+    qio_buffer_reset(&vs->zlib.zlib);
 
     // make the output buffer be the zlib buffer, so we can compress it later
     vs->zlib.tmp = vs->output;
@@ -96,7 +96,7 @@ static int vnc_zlib_stop(VncState *vs)
     }
 
     // reserve memory in output buffer
-    buffer_reserve(&vs->output, vs->zlib.zlib.offset + 64);
+    qio_buffer_reserve(&vs->output, vs->zlib.zlib.offset + 64);
 
     // set pointers
     zstream->next_in = vs->zlib.zlib.buffer;
@@ -148,5 +148,5 @@ void vnc_zlib_clear(VncState *vs)
     if (vs->zlib.stream.opaque) {
         deflateEnd(&vs->zlib.stream);
     }
-    buffer_free(&vs->zlib.zlib);
+    qio_buffer_free(&vs->zlib.zlib);
 }
diff --git a/ui/vnc-enc-zrle.c b/ui/vnc-enc-zrle.c
index ed3b484..bd1e320 100644
--- a/ui/vnc-enc-zrle.c
+++ b/ui/vnc-enc-zrle.c
@@ -36,7 +36,7 @@ static const int bits_per_packed_pixel[] = {
 
 static void vnc_zrle_start(VncState *vs)
 {
-    buffer_reset(&vs->zrle.zrle);
+    qio_buffer_reset(&vs->zrle.zrle);
 
     /* make the output buffer be the zlib buffer, so we can compress it later */
     vs->zrle.tmp = vs->output;
@@ -53,10 +53,10 @@ static void vnc_zrle_stop(VncState *vs)
 static void *zrle_convert_fb(VncState *vs, int x, int y, int w, int h,
                              int bpp)
 {
-    Buffer tmp;
+    QIOBuffer tmp;
 
-    buffer_reset(&vs->zrle.fb);
-    buffer_reserve(&vs->zrle.fb, w * h * bpp + bpp);
+    qio_buffer_reset(&vs->zrle.fb);
+    qio_buffer_reserve(&vs->zrle.fb, w * h * bpp + bpp);
 
     tmp = vs->output;
     vs->output = vs->zrle.fb;
@@ -72,7 +72,7 @@ static int zrle_compress_data(VncState *vs, int level)
 {
     z_streamp zstream = &vs->zrle.stream;
 
-    buffer_reset(&vs->zrle.zlib);
+    qio_buffer_reset(&vs->zrle.zlib);
 
     if (zstream->opaque != vs) {
         int err;
@@ -92,7 +92,7 @@ static int zrle_compress_data(VncState *vs, int level)
     }
 
     /* reserve memory in output buffer */
-    buffer_reserve(&vs->zrle.zlib, vs->zrle.zrle.offset + 64);
+    qio_buffer_reserve(&vs->zrle.zlib, vs->zrle.zrle.offset + 64);
 
     /* set pointers */
     zstream->next_in = vs->zrle.zrle.buffer;
@@ -360,7 +360,7 @@ void vnc_zrle_clear(VncState *vs)
     if (vs->zrle.stream.opaque) {
         deflateEnd(&vs->zrle.stream);
     }
-    buffer_free(&vs->zrle.zrle);
-    buffer_free(&vs->zrle.fb);
-    buffer_free(&vs->zrle.zlib);
+    qio_buffer_free(&vs->zrle.zrle);
+    qio_buffer_free(&vs->zrle.fb);
+    qio_buffer_free(&vs->zrle.zlib);
 }
diff --git a/ui/vnc-jobs.c b/ui/vnc-jobs.c
index 22c9abc..9824c34 100644
--- a/ui/vnc-jobs.c
+++ b/ui/vnc-jobs.c
@@ -54,7 +54,7 @@ struct VncJobQueue {
     QemuCond cond;
     QemuMutex mutex;
     QemuThread thread;
-    Buffer buffer;
+    QIOBuffer buffer;
     bool exit;
     QTAILQ_HEAD(, VncJob) jobs;
 };
@@ -167,7 +167,7 @@ void vnc_jobs_consume_buffer(VncState *vs)
     vnc_lock_output(vs);
     if (vs->jobs_buffer.offset) {
         vnc_write(vs, vs->jobs_buffer.buffer, vs->jobs_buffer.offset);
-        buffer_reset(&vs->jobs_buffer);
+        qio_buffer_reset(&vs->jobs_buffer);
     }
     flush = vs->csock != -1 && vs->abort != true;
     vnc_unlock_output(vs);
@@ -196,7 +196,7 @@ static void vnc_async_encoding_start(VncState *orig, VncState *local)
     local->output =  queue->buffer;
     local->csock = -1; /* Don't do any network work on this thread */
 
-    buffer_reset(&local->output);
+    qio_buffer_reset(&local->output);
 }
 
 static void vnc_async_encoding_end(VncState *orig, VncState *local)
@@ -273,10 +273,11 @@ static int vnc_worker_thread_loop(VncJobQueue *queue)
     vs.output.buffer[saved_offset + 1] = n_rectangles & 0xFF;
 
     vnc_lock_output(job->vs);
+
     if (job->vs->csock != -1) {
-        buffer_reserve(&job->vs->jobs_buffer, vs.output.offset);
-        buffer_append(&job->vs->jobs_buffer, vs.output.buffer,
-                      vs.output.offset);
+        qio_buffer_reserve(&job->vs->jobs_buffer, vs.output.offset);
+        qio_buffer_append(&job->vs->jobs_buffer, vs.output.buffer,
+                          vs.output.offset);
         /* Copy persistent encoding data */
         vnc_async_encoding_end(job->vs, &vs);
 
@@ -310,7 +311,7 @@ static void vnc_queue_clear(VncJobQueue *q)
 {
     qemu_cond_destroy(&queue->cond);
     qemu_mutex_destroy(&queue->mutex);
-    buffer_free(&queue->buffer);
+    qio_buffer_free(&queue->buffer);
     g_free(q);
     queue = NULL; /* Unset global queue */
 }
diff --git a/ui/vnc-ws.c b/ui/vnc-ws.c
index 175ea50..2fe4476 100644
--- a/ui/vnc-ws.c
+++ b/ui/vnc-ws.c
@@ -95,8 +95,8 @@ void vncws_handshake_read(void *opaque)
     /* Typical HTTP headers from novnc are 512 bytes, so limiting
      * total header size to 4096 is easily enough. */
     size_t want = 4096 - vs->ws_input.offset;
-    buffer_reserve(&vs->ws_input, want);
-    ret = vnc_client_read_buf(vs, buffer_end(&vs->ws_input), want);
+    qio_buffer_reserve(&vs->ws_input, want);
+    ret = vnc_client_read_buf(vs, qio_buffer_end(&vs->ws_input), want);
 
     if (!ret) {
         if (vs->csock == -1) {
@@ -111,7 +111,7 @@ void vncws_handshake_read(void *opaque)
     if (handshake_end) {
         qemu_set_fd_handler(vs->csock, vnc_client_read, NULL, vs);
         vncws_process_handshake(vs, vs->ws_input.buffer, vs->ws_input.offset);
-        buffer_advance(&vs->ws_input, handshake_end - vs->ws_input.buffer +
+        qio_buffer_advance(&vs->ws_input, handshake_end - vs->ws_input.buffer +
                 strlen(WS_HANDSHAKE_END));
     } else if (vs->ws_input.offset >= 4096) {
         VNC_DEBUG("End of headers not found in first 4096 bytes\n");
@@ -127,8 +127,8 @@ long vnc_client_read_ws(VncState *vs)
     size_t payload_size, header_size;
     VNC_DEBUG("Read websocket %p size %zd offset %zd\n", vs->ws_input.buffer,
             vs->ws_input.capacity, vs->ws_input.offset);
-    buffer_reserve(&vs->ws_input, 4096);
-    ret = vnc_client_read_buf(vs, buffer_end(&vs->ws_input), 4096);
+    qio_buffer_reserve(&vs->ws_input, 4096);
+    ret = vnc_client_read_buf(vs, qio_buffer_end(&vs->ws_input), 4096);
     if (!ret) {
         return 0;
     }
@@ -146,7 +146,7 @@ long vnc_client_read_ws(VncState *vs)
                 return err;
             }
 
-            buffer_advance(&vs->ws_input, header_size);
+            qio_buffer_advance(&vs->ws_input, header_size);
         }
         if (vs->ws_payload_remain != 0) {
             err = vncws_decode_frame_payload(&vs->ws_input,
@@ -162,10 +162,10 @@ long vnc_client_read_ws(VncState *vs)
             }
             ret += err;
 
-            buffer_reserve(&vs->input, payload_size);
-            buffer_append(&vs->input, payload, payload_size);
+            qio_buffer_reserve(&vs->input, payload_size);
+            qio_buffer_append(&vs->input, payload, payload_size);
 
-            buffer_advance(&vs->ws_input, payload_size);
+            qio_buffer_advance(&vs->ws_input, payload_size);
         }
     } while (vs->ws_input.offset > 0);
 
@@ -178,13 +178,13 @@ long vnc_client_write_ws(VncState *vs)
     VNC_DEBUG("Write WS: Pending output %p size %zd offset %zd\n",
               vs->output.buffer, vs->output.capacity, vs->output.offset);
     vncws_encode_frame(&vs->ws_output, vs->output.buffer, vs->output.offset);
-    buffer_reset(&vs->output);
+    qio_buffer_reset(&vs->output);
     ret = vnc_client_write_buf(vs, vs->ws_output.buffer, vs->ws_output.offset);
     if (!ret) {
         return 0;
     }
 
-    buffer_advance(&vs->ws_output, ret);
+    qio_buffer_advance(&vs->ws_output, ret);
 
     if (vs->ws_output.offset == 0) {
         qemu_set_fd_handler(vs->csock, vnc_client_read, NULL, vs);
@@ -267,8 +267,8 @@ void vncws_process_handshake(VncState *vs, uint8_t *line, size_t size)
     g_free(key);
 }
 
-void vncws_encode_frame(Buffer *output, const void *payload,
-        const size_t payload_size)
+void vncws_encode_frame(QIOBuffer *output, const void *payload,
+                        const size_t payload_size)
 {
     size_t header_size = 0;
     unsigned char opcode = WS_OPCODE_BINARY_FRAME;
@@ -295,12 +295,12 @@ void vncws_encode_frame(Buffer *output, const void *payload,
         header_size = 10;
     }
 
-    buffer_reserve(output, header_size + payload_size);
-    buffer_append(output, header.buf, header_size);
-    buffer_append(output, payload, payload_size);
+    qio_buffer_reserve(output, header_size + payload_size);
+    qio_buffer_append(output, header.buf, header_size);
+    qio_buffer_append(output, payload, payload_size);
 }
 
-int vncws_decode_frame_header(Buffer *input,
+int vncws_decode_frame_header(QIOBuffer *input,
                               size_t *header_size,
                               size_t *payload_remain,
                               WsMask *payload_mask)
@@ -354,7 +354,7 @@ int vncws_decode_frame_header(Buffer *input,
     return 1;
 }
 
-int vncws_decode_frame_payload(Buffer *input,
+int vncws_decode_frame_payload(QIOBuffer *input,
                                size_t *payload_remain, WsMask *payload_mask,
                                uint8_t **payload, size_t *payload_size)
 {
diff --git a/ui/vnc-ws.h b/ui/vnc-ws.h
index 4ab0a8c..2a222a8 100644
--- a/ui/vnc-ws.h
+++ b/ui/vnc-ws.h
@@ -77,13 +77,13 @@ void vncws_handshake_read(void *opaque);
 long vnc_client_write_ws(VncState *vs);
 long vnc_client_read_ws(VncState *vs);
 void vncws_process_handshake(VncState *vs, uint8_t *line, size_t size);
-void vncws_encode_frame(Buffer *output, const void *payload,
+void vncws_encode_frame(QIOBuffer *output, const void *payload,
             const size_t payload_size);
-int vncws_decode_frame_header(Buffer *input,
+int vncws_decode_frame_header(QIOBuffer *input,
                               size_t *header_size,
                               size_t *payload_remain,
                               WsMask *payload_mask);
-int vncws_decode_frame_payload(Buffer *input,
+int vncws_decode_frame_payload(QIOBuffer *input,
                                size_t *payload_remain, WsMask *payload_mask,
                                uint8_t **payload, size_t *payload_size);
 
diff --git a/ui/vnc.c b/ui/vnc.c
index bbe1e91..e4b0e3a 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -647,49 +647,6 @@ void vnc_framebuffer_update(VncState *vs, int x, int y, int w, int h,
     vnc_write_s32(vs, encoding);
 }
 
-void buffer_reserve(Buffer *buffer, size_t len)
-{
-    if ((buffer->capacity - buffer->offset) < len) {
-        buffer->capacity += (len + 1024);
-        buffer->buffer = g_realloc(buffer->buffer, buffer->capacity);
-    }
-}
-
-static int buffer_empty(Buffer *buffer)
-{
-    return buffer->offset == 0;
-}
-
-uint8_t *buffer_end(Buffer *buffer)
-{
-    return buffer->buffer + buffer->offset;
-}
-
-void buffer_reset(Buffer *buffer)
-{
-        buffer->offset = 0;
-}
-
-void buffer_free(Buffer *buffer)
-{
-    g_free(buffer->buffer);
-    buffer->offset = 0;
-    buffer->capacity = 0;
-    buffer->buffer = NULL;
-}
-
-void buffer_append(Buffer *buffer, const void *data, size_t len)
-{
-    memcpy(buffer->buffer + buffer->offset, data, len);
-    buffer->offset += len;
-}
-
-void buffer_advance(Buffer *buf, size_t len)
-{
-    memmove(buf->buffer, buf->buffer + len,
-            (buf->offset - len));
-    buf->offset -= len;
-}
 
 static void vnc_desktop_resize(VncState *vs)
 {
@@ -1236,10 +1193,10 @@ void vnc_disconnect_finish(VncState *vs)
     vnc_lock_output(vs);
     vnc_qmp_event(vs, QAPI_EVENT_VNC_DISCONNECTED);
 
-    buffer_free(&vs->input);
-    buffer_free(&vs->output);
-    buffer_free(&vs->ws_input);
-    buffer_free(&vs->ws_output);
+    qio_buffer_free(&vs->input);
+    qio_buffer_free(&vs->output);
+    qio_buffer_free(&vs->ws_input);
+    qio_buffer_free(&vs->ws_output);
 
     qapi_free_VncClientInfo(vs->info);
 
@@ -1267,7 +1224,7 @@ void vnc_disconnect_finish(VncState *vs)
     if (vs->bh != NULL) {
         qemu_bh_delete(vs->bh);
     }
-    buffer_free(&vs->jobs_buffer);
+    qio_buffer_free(&vs->jobs_buffer);
 
     for (i = 0; i < VNC_STAT_ROWS; ++i) {
         g_free(vs->lossy_rect[i]);
@@ -1409,7 +1366,7 @@ static ssize_t vnc_client_write_plain(VncState *vs)
     if (!ret)
         return 0;
 
-    buffer_advance(&vs->output, ret);
+    qio_buffer_advance(&vs->output, ret);
 
     if (vs->output.offset == 0) {
         qemu_set_fd_handler(vs->csock, vnc_client_read, NULL, vs);
@@ -1512,8 +1469,8 @@ static ssize_t vnc_client_read_plain(VncState *vs)
     ssize_t ret;
     VNC_DEBUG("Read plain %p size %zd offset %zd\n",
               vs->input.buffer, vs->input.capacity, vs->input.offset);
-    buffer_reserve(&vs->input, 4096);
-    ret = vnc_client_read_buf(vs, buffer_end(&vs->input), 4096);
+    qio_buffer_reserve(&vs->input, 4096);
+    ret = vnc_client_read_buf(vs, qio_buffer_end(&vs->input), 4096);
     if (!ret)
         return 0;
     vs->input.offset += ret;
@@ -1571,7 +1528,7 @@ void vnc_client_read(void *opaque)
         }
 
         if (!ret) {
-            buffer_advance(&vs->input, len);
+            qio_buffer_advance(&vs->input, len);
         } else {
             vs->read_handler_expect = ret;
         }
@@ -1580,13 +1537,13 @@ void vnc_client_read(void *opaque)
 
 void vnc_write(VncState *vs, const void *data, size_t len)
 {
-    buffer_reserve(&vs->output, len);
+    qio_buffer_reserve(&vs->output, len);
 
-    if (vs->csock != -1 && buffer_empty(&vs->output)) {
+    if (vs->csock != -1 && qio_buffer_empty(&vs->output)) {
         qemu_set_fd_handler(vs->csock, vnc_client_read, vnc_client_write, vs);
     }
 
-    buffer_append(&vs->output, data, len);
+    qio_buffer_append(&vs->output, data, len);
 }
 
 void vnc_write_s32(VncState *vs, int32_t value)
diff --git a/ui/vnc.h b/ui/vnc.h
index 4dd769c..339a1bf 100644
--- a/ui/vnc.h
+++ b/ui/vnc.h
@@ -34,6 +34,7 @@
 #include "audio/audio.h"
 #include "qemu/bitmap.h"
 #include "crypto/tlssession.h"
+#include "io/buffer.h"
 #include <zlib.h>
 #include <stdbool.h>
 
@@ -56,13 +57,6 @@
  *
  *****************************************************************************/
 
-typedef struct Buffer
-{
-    size_t capacity;
-    size_t offset;
-    uint8_t *buffer;
-} Buffer;
-
 typedef struct VncState VncState;
 typedef struct VncJob VncJob;
 typedef struct VncRect VncRect;
@@ -191,15 +185,15 @@ typedef struct VncTight {
     uint8_t quality;
     uint8_t compression;
     uint8_t pixel24;
-    Buffer tight;
-    Buffer tmp;
-    Buffer zlib;
-    Buffer gradient;
+    QIOBuffer tight;
+    QIOBuffer tmp;
+    QIOBuffer zlib;
+    QIOBuffer gradient;
 #ifdef CONFIG_VNC_JPEG
-    Buffer jpeg;
+    QIOBuffer jpeg;
 #endif
 #ifdef CONFIG_VNC_PNG
-    Buffer png;
+    QIOBuffer png;
 #endif
     int levels[4];
     z_stream stream[4];
@@ -210,18 +204,18 @@ typedef struct VncHextile {
 } VncHextile;
 
 typedef struct VncZlib {
-    Buffer zlib;
-    Buffer tmp;
+    QIOBuffer zlib;
+    QIOBuffer tmp;
     z_stream stream;
     int level;
 } VncZlib;
 
 typedef struct VncZrle {
     int type;
-    Buffer fb;
-    Buffer zrle;
-    Buffer tmp;
-    Buffer zlib;
+    QIOBuffer fb;
+    QIOBuffer zrle;
+    QIOBuffer tmp;
+    QIOBuffer zlib;
     z_stream stream;
     VncPalette palette;
 } VncZrle;
@@ -290,10 +284,10 @@ struct VncState
 
     VncClientInfo *info;
 
-    Buffer output;
-    Buffer input;
-    Buffer ws_input;
-    Buffer ws_output;
+    QIOBuffer output;
+    QIOBuffer input;
+    QIOBuffer ws_input;
+    QIOBuffer ws_output;
     size_t ws_payload_remain;
     WsMask ws_payload_mask;
     /* current output mode information */
@@ -315,7 +309,7 @@ struct VncState
     bool initialized;
     QemuMutex output_mutex;
     QEMUBH *bh;
-    Buffer jobs_buffer;
+    QIOBuffer jobs_buffer;
 
     /* Encoding specific, if you add something here, don't forget to
      *  update vnc_async_encoding_start()
@@ -535,14 +529,6 @@ ssize_t vnc_client_io_error(VncState *vs, ssize_t ret, int last_errno);
 void start_client_init(VncState *vs);
 void start_auth_vnc(VncState *vs);
 
-/* Buffer management */
-void buffer_reserve(Buffer *buffer, size_t len);
-void buffer_reset(Buffer *buffer);
-void buffer_free(Buffer *buffer);
-void buffer_append(Buffer *buffer, const void *data, size_t len);
-void buffer_advance(Buffer *buf, size_t len);
-uint8_t *buffer_end(Buffer *buffer);
-
 
 /* Misc helpers */
 
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [Qemu-devel] [RfC PATCH 03/10] vnc: make the Buffer capacity increase in powers of two
  2015-09-24  8:41 [Qemu-devel] [RfC PATCH 00/10] vnc buffer handling Gerd Hoffmann
  2015-09-24  8:41 ` [Qemu-devel] [RfC PATCH 01/10] io/ makefile fluff Gerd Hoffmann
  2015-09-24  8:42 ` [Qemu-devel] [RfC PATCH 02/10] io: pull Buffer code out of VNC module Gerd Hoffmann
@ 2015-09-24  8:42 ` Gerd Hoffmann
  2015-09-24  8:42 ` [Qemu-devel] [RfC PATCH 04/10] io: add qio_buffer_init Gerd Hoffmann
                   ` (8 subsequent siblings)
  11 siblings, 0 replies; 22+ messages in thread
From: Gerd Hoffmann @ 2015-09-24  8:42 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Lieven, Gerd Hoffmann

From: Peter Lieven <pl@kamp.de>

This makes sure the number of reallocs is in O(log N).

Signed-off-by: Peter Lieven <pl@kamp.de>

[ rebased to io/buffer.c ]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 io/buffer.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/io/buffer.c b/io/buffer.c
index 68ae68d..6aa06e7 100644
--- a/io/buffer.c
+++ b/io/buffer.c
@@ -20,10 +20,13 @@
 
 #include "io/buffer.h"
 
+#define QIO_BUFFER_MIN_INIT_SIZE 4096
+
 void qio_buffer_reserve(QIOBuffer *buffer, size_t len)
 {
     if ((buffer->capacity - buffer->offset) < len) {
-        buffer->capacity += (len + 1024);
+        buffer->capacity = pow2ceil(buffer->offset + len);
+        buffer->capacity = MAX(buffer->capacity, QIO_BUFFER_MIN_INIT_SIZE);
         buffer->buffer = g_realloc(buffer->buffer, buffer->capacity);
     }
 }
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [Qemu-devel] [RfC PATCH 04/10] io: add qio_buffer_init
  2015-09-24  8:41 [Qemu-devel] [RfC PATCH 00/10] vnc buffer handling Gerd Hoffmann
                   ` (2 preceding siblings ...)
  2015-09-24  8:42 ` [Qemu-devel] [RfC PATCH 03/10] vnc: make the Buffer capacity increase in powers of two Gerd Hoffmann
@ 2015-09-24  8:42 ` Gerd Hoffmann
  2015-09-25  9:56   ` Peter Lieven
  2015-09-24  8:42 ` [Qemu-devel] [RfC PATCH 05/10] io: add qio_buffer_move_empty Gerd Hoffmann
                   ` (7 subsequent siblings)
  11 siblings, 1 reply; 22+ messages in thread
From: Gerd Hoffmann @ 2015-09-24  8:42 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Lieven, Gerd Hoffmann

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 include/io/buffer.h | 12 ++++++++++++
 io/buffer.c         | 11 +++++++++++
 2 files changed, 23 insertions(+)

diff --git a/include/io/buffer.h b/include/io/buffer.h
index 2b1b261..cffad19 100644
--- a/include/io/buffer.h
+++ b/include/io/buffer.h
@@ -34,12 +34,24 @@ typedef struct QIOBuffer QIOBuffer;
  */
 
 struct QIOBuffer {
+    char *name;
     size_t capacity;
     size_t offset;
     uint8_t *buffer;
 };
 
 /**
+ * qio_buffer_init:
+ * @buffer: the buffer object
+ * @name: buffer name
+ *
+ * Optionally attach a name to the buffer, to make it easier
+ * to identify in debug traces.
+ */
+void qio_buffer_init(QIOBuffer *buffer, const char *name, ...)
+        GCC_FMT_ATTR(2, 3);
+
+/**
  * qio_buffer_reserve:
  * @buffer: the buffer object
  * @len: the minimum required free space
diff --git a/io/buffer.c b/io/buffer.c
index 6aa06e7..daa3ebf 100644
--- a/io/buffer.c
+++ b/io/buffer.c
@@ -22,6 +22,15 @@
 
 #define QIO_BUFFER_MIN_INIT_SIZE 4096
 
+void qio_buffer_init(QIOBuffer *buffer, const char *name, ...)
+{
+    va_list ap;
+
+    va_start(ap, name);
+    buffer->name = g_strdup_vprintf(name, ap);
+    va_end(ap);
+}
+
 void qio_buffer_reserve(QIOBuffer *buffer, size_t len)
 {
     if ((buffer->capacity - buffer->offset) < len) {
@@ -49,9 +58,11 @@ void qio_buffer_reset(QIOBuffer *buffer)
 void qio_buffer_free(QIOBuffer *buffer)
 {
     g_free(buffer->buffer);
+    g_free(buffer->name);
     buffer->offset = 0;
     buffer->capacity = 0;
     buffer->buffer = NULL;
+    buffer->name = NULL;
 }
 
 void qio_buffer_append(QIOBuffer *buffer, const void *data, size_t len)
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [Qemu-devel] [RfC PATCH 05/10] io: add qio_buffer_move_empty
  2015-09-24  8:41 [Qemu-devel] [RfC PATCH 00/10] vnc buffer handling Gerd Hoffmann
                   ` (3 preceding siblings ...)
  2015-09-24  8:42 ` [Qemu-devel] [RfC PATCH 04/10] io: add qio_buffer_init Gerd Hoffmann
@ 2015-09-24  8:42 ` Gerd Hoffmann
  2015-09-25  9:56   ` Peter Lieven
  2015-09-24  8:42 ` [Qemu-devel] [RfC PATCH 06/10] io: add qio_buffer_move Gerd Hoffmann
                   ` (6 subsequent siblings)
  11 siblings, 1 reply; 22+ messages in thread
From: Gerd Hoffmann @ 2015-09-24  8:42 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Lieven, Gerd Hoffmann

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 include/io/buffer.h | 10 ++++++++++
 io/buffer.c         | 14 ++++++++++++++
 2 files changed, 24 insertions(+)

diff --git a/include/io/buffer.h b/include/io/buffer.h
index cffad19..1dddc73 100644
--- a/include/io/buffer.h
+++ b/include/io/buffer.h
@@ -127,4 +127,14 @@ uint8_t *qio_buffer_end(QIOBuffer *buffer);
  */
 gboolean qio_buffer_empty(QIOBuffer *buffer);
 
+/**
+ * qio_buffer_move_empty:
+ * @to: destination buffer object
+ * @from: source buffer object
+ *
+ * Moves buffer, without copying data.  'to' buffer must be empty.
+ * 'from' buffer is empty and zero-sized on return.
+ */
+void qio_buffer_move_empty(QIOBuffer *to, QIOBuffer *from);
+
 #endif /* QIO_BUFFER_H__ */
diff --git a/io/buffer.c b/io/buffer.c
index daa3ebf..09ca321 100644
--- a/io/buffer.c
+++ b/io/buffer.c
@@ -77,3 +77,17 @@ void qio_buffer_advance(QIOBuffer *buffer, size_t len)
             (buffer->offset - len));
     buffer->offset -= len;
 }
+
+void qio_buffer_move_empty(QIOBuffer *to, QIOBuffer *from)
+{
+    assert(to->offset == 0);
+
+    g_free(to->buffer);
+    to->offset = from->offset;
+    to->capacity = from->capacity;
+    to->buffer = from->buffer;
+
+    from->offset = 0;
+    from->capacity = 0;
+    from->buffer = NULL;
+}
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [Qemu-devel] [RfC PATCH 06/10] io: add qio_buffer_move
  2015-09-24  8:41 [Qemu-devel] [RfC PATCH 00/10] vnc buffer handling Gerd Hoffmann
                   ` (4 preceding siblings ...)
  2015-09-24  8:42 ` [Qemu-devel] [RfC PATCH 05/10] io: add qio_buffer_move_empty Gerd Hoffmann
@ 2015-09-24  8:42 ` Gerd Hoffmann
  2015-09-25  9:57   ` Peter Lieven
  2015-09-24  8:42 ` [Qemu-devel] [RfC PATCH 07/10] io: add qio_buffer tracing Gerd Hoffmann
                   ` (5 subsequent siblings)
  11 siblings, 1 reply; 22+ messages in thread
From: Gerd Hoffmann @ 2015-09-24  8:42 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Lieven, Gerd Hoffmann

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 include/io/buffer.h | 10 ++++++++++
 io/buffer.c         | 16 ++++++++++++++++
 2 files changed, 26 insertions(+)

diff --git a/include/io/buffer.h b/include/io/buffer.h
index 1dddc73..5676aff 100644
--- a/include/io/buffer.h
+++ b/include/io/buffer.h
@@ -137,4 +137,14 @@ gboolean qio_buffer_empty(QIOBuffer *buffer);
  */
 void qio_buffer_move_empty(QIOBuffer *to, QIOBuffer *from);
 
+/**
+ * qio_buffer_move:
+ * @to: destination buffer object
+ * @from: source buffer object
+ *
+ * Moves buffer, copying data (unless 'to' buffer happens to be empty).
+ * 'from' buffer is empty and zero-sized on return.
+ */
+void qio_buffer_move(QIOBuffer *to, QIOBuffer *from);
+
 #endif /* QIO_BUFFER_H__ */
diff --git a/io/buffer.c b/io/buffer.c
index 09ca321..96077d3 100644
--- a/io/buffer.c
+++ b/io/buffer.c
@@ -91,3 +91,19 @@ void qio_buffer_move_empty(QIOBuffer *to, QIOBuffer *from)
     from->capacity = 0;
     from->buffer = NULL;
 }
+
+void qio_buffer_move(QIOBuffer *to, QIOBuffer *from)
+{
+    if (to->offset == 0) {
+        qio_buffer_move_empty(to, from);
+        return;
+    }
+
+    qio_buffer_reserve(to, from->offset);
+    qio_buffer_append(to, from->buffer, from->offset);
+
+    g_free(from->buffer);
+    from->offset = 0;
+    from->capacity = 0;
+    from->buffer = NULL;
+}
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [Qemu-devel] [RfC PATCH 07/10] io: add qio_buffer tracing
  2015-09-24  8:41 [Qemu-devel] [RfC PATCH 00/10] vnc buffer handling Gerd Hoffmann
                   ` (5 preceding siblings ...)
  2015-09-24  8:42 ` [Qemu-devel] [RfC PATCH 06/10] io: add qio_buffer_move Gerd Hoffmann
@ 2015-09-24  8:42 ` Gerd Hoffmann
  2015-09-25  8:10   ` Peter Lieven
  2015-09-24  8:42 ` [Qemu-devel] [RfC PATCH 08/10] name vnc buffers Gerd Hoffmann
                   ` (4 subsequent siblings)
  11 siblings, 1 reply; 22+ messages in thread
From: Gerd Hoffmann @ 2015-09-24  8:42 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Lieven, Gerd Hoffmann

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 io/buffer.c  | 10 ++++++++++
 trace-events |  6 ++++++
 2 files changed, 16 insertions(+)

diff --git a/io/buffer.c b/io/buffer.c
index 96077d3..05425c2 100644
--- a/io/buffer.c
+++ b/io/buffer.c
@@ -19,6 +19,7 @@
  */
 
 #include "io/buffer.h"
+#include "trace.h"
 
 #define QIO_BUFFER_MIN_INIT_SIZE 4096
 
@@ -37,6 +38,7 @@ void qio_buffer_reserve(QIOBuffer *buffer, size_t len)
         buffer->capacity = pow2ceil(buffer->offset + len);
         buffer->capacity = MAX(buffer->capacity, QIO_BUFFER_MIN_INIT_SIZE);
         buffer->buffer = g_realloc(buffer->buffer, buffer->capacity);
+        trace_qio_buffer_resize(buffer->name ?: "unnamed", buffer->capacity);
     }
 }
 
@@ -57,6 +59,7 @@ void qio_buffer_reset(QIOBuffer *buffer)
 
 void qio_buffer_free(QIOBuffer *buffer)
 {
+    trace_qio_buffer_free(buffer->name ?: "unnamed");
     g_free(buffer->buffer);
     g_free(buffer->name);
     buffer->offset = 0;
@@ -80,6 +83,9 @@ void qio_buffer_advance(QIOBuffer *buffer, size_t len)
 
 void qio_buffer_move_empty(QIOBuffer *to, QIOBuffer *from)
 {
+    trace_qio_buffer_move_empty(to->name ?: "unnamed",
+                                from->offset,
+                                from->name ?: "unnamed");
     assert(to->offset == 0);
 
     g_free(to->buffer);
@@ -99,6 +105,10 @@ void qio_buffer_move(QIOBuffer *to, QIOBuffer *from)
         return;
     }
 
+    trace_qio_buffer_move(to->name ?: "unnamed",
+                          from->offset,
+                          from->name ?: "unnamed");
+
     qio_buffer_reserve(to, from->offset);
     qio_buffer_append(to, from->buffer, from->offset);
 
diff --git a/trace-events b/trace-events
index 88a2f14..0f06b64 100644
--- a/trace-events
+++ b/trace-events
@@ -1376,6 +1376,12 @@ spapr_iommu_new_table(uint64_t liobn, void *tcet, void *table, int fd) "liobn=%"
 # hw/ppc/ppc.c
 ppc_tb_adjust(uint64_t offs1, uint64_t offs2, int64_t diff, int64_t seconds) "adjusted from 0x%"PRIx64" to 0x%"PRIx64", diff %"PRId64" (%"PRId64"s)"
 
+# io/buffer.c
+qio_buffer_resize(const char *buf, size_t len) "%s: len %zd"
+qio_buffer_move_empty(const char *buf, size_t len, const char *from) "%s: %zd bytes from %s"
+qio_buffer_move(const char *buf, size_t len, const char *from) "%s: %zd bytes from %s"
+qio_buffer_free(const char *buf) "%s"
+
 # util/hbitmap.c
 hbitmap_iter_skip_words(const void *hb, void *hbi, uint64_t pos, unsigned long cur) "hb %p hbi %p pos %"PRId64" cur 0x%lx"
 hbitmap_reset(void *hb, uint64_t start, uint64_t count, uint64_t sbit, uint64_t ebit) "hb %p items %"PRIu64",%"PRIu64" bits %"PRIu64"..%"PRIu64
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [Qemu-devel] [RfC PATCH 08/10] name vnc buffers
  2015-09-24  8:41 [Qemu-devel] [RfC PATCH 00/10] vnc buffer handling Gerd Hoffmann
                   ` (6 preceding siblings ...)
  2015-09-24  8:42 ` [Qemu-devel] [RfC PATCH 07/10] io: add qio_buffer tracing Gerd Hoffmann
@ 2015-09-24  8:42 ` Gerd Hoffmann
  2015-09-25  7:28   ` Peter Lieven
  2015-09-25  7:58   ` Peter Lieven
  2015-09-24  8:42 ` [Qemu-devel] [RfC PATCH 09/10] vnc: kill jobs queue buffer Gerd Hoffmann
                   ` (3 subsequent siblings)
  11 siblings, 2 replies; 22+ messages in thread
From: Gerd Hoffmann @ 2015-09-24  8:42 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Lieven, Gerd Hoffmann

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 ui/vnc-jobs.c |  1 +
 ui/vnc.c      | 16 ++++++++++++++++
 2 files changed, 17 insertions(+)

diff --git a/ui/vnc-jobs.c b/ui/vnc-jobs.c
index 9824c34..7a234da 100644
--- a/ui/vnc-jobs.c
+++ b/ui/vnc-jobs.c
@@ -303,6 +303,7 @@ static VncJobQueue *vnc_queue_init(void)
 
     qemu_cond_init(&queue->cond);
     qemu_mutex_init(&queue->mutex);
+    qio_buffer_init(&queue->buffer, "vnc-job-queue");
     QTAILQ_INIT(&queue->jobs);
     return queue;
 }
diff --git a/ui/vnc.c b/ui/vnc.c
index e4b0e3a..fc0311f 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -2978,6 +2978,22 @@ static void vnc_connect(VncDisplay *vd, int csock,
     vs->csock = csock;
     vs->vd = vd;
 
+    qio_buffer_init(&vs->input,          "vnc-input/%d", csock);
+    qio_buffer_init(&vs->output,         "vnc-output/%d", csock);
+    qio_buffer_init(&vs->ws_input,       "vnc-ws_input/%d", csock);
+    qio_buffer_init(&vs->ws_output,      "vnc-ws_output/%d", csock);
+    qio_buffer_init(&vs->jobs_buffer,    "vnc-jobs_buffer/%d", csock);
+
+    qio_buffer_init(&vs->tight.tight,    "vnc-tight/%d", csock);
+    qio_buffer_init(&vs->tight.zlib,     "vnc-tight-zlib/%d", csock);
+    qio_buffer_init(&vs->tight.gradient, "vnc-tight-gradient/%d", csock);
+    qio_buffer_init(&vs->tight.jpeg,     "vnc-tight-jpeg/%d", csock);
+    qio_buffer_init(&vs->tight.png,      "vnc-tight-png/%d", csock);
+    qio_buffer_init(&vs->zlib.zlib,      "vnc-zlib/%d", csock);
+    qio_buffer_init(&vs->zrle.zrle,      "vnc-zrle/%d", csock);
+    qio_buffer_init(&vs->zrle.fb,        "vnc-zrle-fb/%d", csock);
+    qio_buffer_init(&vs->zrle.zlib,      "vnc-zrle-zlib/%d", csock);
+
     if (skipauth) {
 	vs->auth = VNC_AUTH_NONE;
 	vs->subauth = VNC_AUTH_INVALID;
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [Qemu-devel] [RfC PATCH 09/10] vnc: kill jobs queue buffer
  2015-09-24  8:41 [Qemu-devel] [RfC PATCH 00/10] vnc buffer handling Gerd Hoffmann
                   ` (7 preceding siblings ...)
  2015-09-24  8:42 ` [Qemu-devel] [RfC PATCH 08/10] name vnc buffers Gerd Hoffmann
@ 2015-09-24  8:42 ` Gerd Hoffmann
  2015-09-25  9:57   ` Peter Lieven
  2015-09-24  8:42 ` [Qemu-devel] [RfC PATCH 10/10] vnc-jobs: move buffer reset, use new buffer move Gerd Hoffmann
                   ` (2 subsequent siblings)
  11 siblings, 1 reply; 22+ messages in thread
From: Gerd Hoffmann @ 2015-09-24  8:42 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Lieven, Gerd Hoffmann

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 ui/vnc-jobs.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/ui/vnc-jobs.c b/ui/vnc-jobs.c
index 7a234da..50e6b37 100644
--- a/ui/vnc-jobs.c
+++ b/ui/vnc-jobs.c
@@ -54,7 +54,6 @@ struct VncJobQueue {
     QemuCond cond;
     QemuMutex mutex;
     QemuThread thread;
-    QIOBuffer buffer;
     bool exit;
     QTAILQ_HEAD(, VncJob) jobs;
 };
@@ -193,7 +192,6 @@ static void vnc_async_encoding_start(VncState *orig, VncState *local)
     local->zlib = orig->zlib;
     local->hextile = orig->hextile;
     local->zrle = orig->zrle;
-    local->output =  queue->buffer;
     local->csock = -1; /* Don't do any network work on this thread */
 
     qio_buffer_reset(&local->output);
@@ -206,8 +204,6 @@ static void vnc_async_encoding_end(VncState *orig, VncState *local)
     orig->hextile = local->hextile;
     orig->zrle = local->zrle;
     orig->lossy_rect = local->lossy_rect;
-
-    queue->buffer = local->output;
 }
 
 static int vnc_worker_thread_loop(VncJobQueue *queue)
@@ -303,7 +299,6 @@ static VncJobQueue *vnc_queue_init(void)
 
     qemu_cond_init(&queue->cond);
     qemu_mutex_init(&queue->mutex);
-    qio_buffer_init(&queue->buffer, "vnc-job-queue");
     QTAILQ_INIT(&queue->jobs);
     return queue;
 }
@@ -312,7 +307,6 @@ static void vnc_queue_clear(VncJobQueue *q)
 {
     qemu_cond_destroy(&queue->cond);
     qemu_mutex_destroy(&queue->mutex);
-    qio_buffer_free(&queue->buffer);
     g_free(q);
     queue = NULL; /* Unset global queue */
 }
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [Qemu-devel] [RfC PATCH 10/10] vnc-jobs: move buffer reset, use new buffer move
  2015-09-24  8:41 [Qemu-devel] [RfC PATCH 00/10] vnc buffer handling Gerd Hoffmann
                   ` (8 preceding siblings ...)
  2015-09-24  8:42 ` [Qemu-devel] [RfC PATCH 09/10] vnc: kill jobs queue buffer Gerd Hoffmann
@ 2015-09-24  8:42 ` Gerd Hoffmann
  2015-09-25  9:39   ` Peter Lieven
  2015-09-24 16:25 ` [Qemu-devel] [RfC PATCH 00/10] vnc buffer handling Daniel P. Berrange
  2015-09-25  9:56 ` Peter Lieven
  11 siblings, 1 reply; 22+ messages in thread
From: Gerd Hoffmann @ 2015-09-24  8:42 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Lieven, Gerd Hoffmann

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 ui/vnc-jobs.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/ui/vnc-jobs.c b/ui/vnc-jobs.c
index 50e6b37..dfc5139 100644
--- a/ui/vnc-jobs.c
+++ b/ui/vnc-jobs.c
@@ -165,8 +165,7 @@ void vnc_jobs_consume_buffer(VncState *vs)
 
     vnc_lock_output(vs);
     if (vs->jobs_buffer.offset) {
-        vnc_write(vs, vs->jobs_buffer.buffer, vs->jobs_buffer.offset);
-        qio_buffer_reset(&vs->jobs_buffer);
+        qio_buffer_move(&vs->output, &vs->jobs_buffer);
     }
     flush = vs->csock != -1 && vs->abort != true;
     vnc_unlock_output(vs);
@@ -193,8 +192,6 @@ static void vnc_async_encoding_start(VncState *orig, VncState *local)
     local->hextile = orig->hextile;
     local->zrle = orig->zrle;
     local->csock = -1; /* Don't do any network work on this thread */
-
-    qio_buffer_reset(&local->output);
 }
 
 static void vnc_async_encoding_end(VncState *orig, VncState *local)
@@ -271,14 +268,13 @@ static int vnc_worker_thread_loop(VncJobQueue *queue)
     vnc_lock_output(job->vs);
 
     if (job->vs->csock != -1) {
-        qio_buffer_reserve(&job->vs->jobs_buffer, vs.output.offset);
-        qio_buffer_append(&job->vs->jobs_buffer, vs.output.buffer,
-                          vs.output.offset);
+        qio_buffer_move(&job->vs->jobs_buffer, &vs.output);
         /* Copy persistent encoding data */
         vnc_async_encoding_end(job->vs, &vs);
 
 	qemu_bh_schedule(job->vs->bh);
     }  else {
+        qio_buffer_reset(&vs.output);
         /* Copy persistent encoding data */
         vnc_async_encoding_end(job->vs, &vs);
     }
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [Qemu-devel] [RfC PATCH 00/10] vnc buffer handling
  2015-09-24  8:41 [Qemu-devel] [RfC PATCH 00/10] vnc buffer handling Gerd Hoffmann
                   ` (9 preceding siblings ...)
  2015-09-24  8:42 ` [Qemu-devel] [RfC PATCH 10/10] vnc-jobs: move buffer reset, use new buffer move Gerd Hoffmann
@ 2015-09-24 16:25 ` Daniel P. Berrange
  2015-09-25  9:56 ` Peter Lieven
  11 siblings, 0 replies; 22+ messages in thread
From: Daniel P. Berrange @ 2015-09-24 16:25 UTC (permalink / raw)
  To: Gerd Hoffmann; +Cc: Peter Lieven, qemu-devel

On Thu, Sep 24, 2015 at 10:41:58AM +0200, Gerd Hoffmann wrote:
>   Hi,
> 
> Here is a patch series to improve the vnc buffer handling.  It picks up
> the qio_buffer patches from Daniel, adds move calls (move data from one
> buffer to another) and tracing, makes vnc use the new features.  Net
> effect should be that (a) vnc copies less data around and (b) buffers
> don't grow forever.
> 
> It's RfC because it depends on wip patches.  My plan is to wait for
> Daniels patch series to be merged (which should obsolete patches #1+#2),
> then rebase and repost the series.

I have a quick read through the patch series and it all looks pretty
good to me.

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 :|

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [Qemu-devel] [RfC PATCH 08/10] name vnc buffers
  2015-09-24  8:42 ` [Qemu-devel] [RfC PATCH 08/10] name vnc buffers Gerd Hoffmann
@ 2015-09-25  7:28   ` Peter Lieven
  2015-09-25  7:58   ` Peter Lieven
  1 sibling, 0 replies; 22+ messages in thread
From: Peter Lieven @ 2015-09-25  7:28 UTC (permalink / raw)
  To: Gerd Hoffmann, qemu-devel

Am 24.09.2015 um 10:42 schrieb Gerd Hoffmann:
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  ui/vnc-jobs.c |  1 +
>  ui/vnc.c      | 16 ++++++++++++++++
>  2 files changed, 17 insertions(+)
>
> diff --git a/ui/vnc-jobs.c b/ui/vnc-jobs.c
> index 9824c34..7a234da 100644
> --- a/ui/vnc-jobs.c
> +++ b/ui/vnc-jobs.c
> @@ -303,6 +303,7 @@ static VncJobQueue *vnc_queue_init(void)
>  
>      qemu_cond_init(&queue->cond);
>      qemu_mutex_init(&queue->mutex);
> +    qio_buffer_init(&queue->buffer, "vnc-job-queue");
>      QTAILQ_INIT(&queue->jobs);
>      return queue;
>  }
> diff --git a/ui/vnc.c b/ui/vnc.c
> index e4b0e3a..fc0311f 100644
> --- a/ui/vnc.c
> +++ b/ui/vnc.c
> @@ -2978,6 +2978,22 @@ static void vnc_connect(VncDisplay *vd, int csock,
>      vs->csock = csock;
>      vs->vd = vd;
>  
> +    qio_buffer_init(&vs->input,          "vnc-input/%d", csock);
> +    qio_buffer_init(&vs->output,         "vnc-output/%d", csock);
> +    qio_buffer_init(&vs->ws_input,       "vnc-ws_input/%d", csock);
> +    qio_buffer_init(&vs->ws_output,      "vnc-ws_output/%d", csock);
> +    qio_buffer_init(&vs->jobs_buffer,    "vnc-jobs_buffer/%d", csock);
> +
> +    qio_buffer_init(&vs->tight.tight,    "vnc-tight/%d", csock);
> +    qio_buffer_init(&vs->tight.zlib,     "vnc-tight-zlib/%d", csock);
> +    qio_buffer_init(&vs->tight.gradient, "vnc-tight-gradient/%d", csock);
> +    qio_buffer_init(&vs->tight.jpeg,     "vnc-tight-jpeg/%d", csock);
> +    qio_buffer_init(&vs->tight.png,      "vnc-tight-png/%d", csock);

png and jpeg support is not generally available:

diff --git a/ui/vnc.c b/ui/vnc.c
index fc0311f..454087f 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -2987,8 +2987,12 @@ static void vnc_connect(VncDisplay *vd, int csock,
     qio_buffer_init(&vs->tight.tight,    "vnc-tight/%d", csock);
     qio_buffer_init(&vs->tight.zlib,     "vnc-tight-zlib/%d", csock);
     qio_buffer_init(&vs->tight.gradient, "vnc-tight-gradient/%d", csock);
+#ifdef CONFIG_VNC_JPEG
     qio_buffer_init(&vs->tight.jpeg,     "vnc-tight-jpeg/%d", csock);
+#endif
+#ifdef CONFIG_VNC_PNG
     qio_buffer_init(&vs->tight.png,      "vnc-tight-png/%d", csock);
+#endif
     qio_buffer_init(&vs->zlib.zlib,      "vnc-zlib/%d", csock);
     qio_buffer_init(&vs->zrle.zrle,      "vnc-zrle/%d", csock);
     qio_buffer_init(&vs->zrle.fb,        "vnc-zrle-fb/%d", csock);

Peter

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [Qemu-devel] [RfC PATCH 08/10] name vnc buffers
  2015-09-24  8:42 ` [Qemu-devel] [RfC PATCH 08/10] name vnc buffers Gerd Hoffmann
  2015-09-25  7:28   ` Peter Lieven
@ 2015-09-25  7:58   ` Peter Lieven
  1 sibling, 0 replies; 22+ messages in thread
From: Peter Lieven @ 2015-09-25  7:58 UTC (permalink / raw)
  To: Gerd Hoffmann, qemu-devel

Am 24.09.2015 um 10:42 schrieb Gerd Hoffmann:
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  ui/vnc-jobs.c |  1 +
>  ui/vnc.c      | 16 ++++++++++++++++
>  2 files changed, 17 insertions(+)
>
> diff --git a/ui/vnc-jobs.c b/ui/vnc-jobs.c
> index 9824c34..7a234da 100644
> --- a/ui/vnc-jobs.c
> +++ b/ui/vnc-jobs.c
> @@ -303,6 +303,7 @@ static VncJobQueue *vnc_queue_init(void)
>  
>      qemu_cond_init(&queue->cond);
>      qemu_mutex_init(&queue->mutex);
> +    qio_buffer_init(&queue->buffer, "vnc-job-queue");
>      QTAILQ_INIT(&queue->jobs);
>      return queue;
>  }
> diff --git a/ui/vnc.c b/ui/vnc.c
> index e4b0e3a..fc0311f 100644
> --- a/ui/vnc.c
> +++ b/ui/vnc.c
> @@ -2978,6 +2978,22 @@ static void vnc_connect(VncDisplay *vd, int csock,
>      vs->csock = csock;
>      vs->vd = vd;
>  
> +    qio_buffer_init(&vs->input,          "vnc-input/%d", csock);
> +    qio_buffer_init(&vs->output,         "vnc-output/%d", csock);
> +    qio_buffer_init(&vs->ws_input,       "vnc-ws_input/%d", csock);
> +    qio_buffer_init(&vs->ws_output,      "vnc-ws_output/%d", csock);
> +    qio_buffer_init(&vs->jobs_buffer,    "vnc-jobs_buffer/%d", csock);
> +
> +    qio_buffer_init(&vs->tight.tight,    "vnc-tight/%d", csock);
> +    qio_buffer_init(&vs->tight.zlib,     "vnc-tight-zlib/%d", csock);
> +    qio_buffer_init(&vs->tight.gradient, "vnc-tight-gradient/%d", csock);
> +    qio_buffer_init(&vs->tight.jpeg,     "vnc-tight-jpeg/%d", csock);
> +    qio_buffer_init(&vs->tight.png,      "vnc-tight-png/%d", csock);
> +    qio_buffer_init(&vs->zlib.zlib,      "vnc-zlib/%d", csock);
> +    qio_buffer_init(&vs->zrle.zrle,      "vnc-zrle/%d", csock);
> +    qio_buffer_init(&vs->zrle.fb,        "vnc-zrle-fb/%d", csock);
> +    qio_buffer_init(&vs->zrle.zlib,      "vnc-zrle-zlib/%d", csock);
> +
>      if (skipauth) {
>  	vs->auth = VNC_AUTH_NONE;
>  	vs->subauth = VNC_AUTH_INVALID;

You also might want to name this one:

diff --git a/ui/vnc-jobs.c b/ui/vnc-jobs.c
index dfc5139..65bae26 100644
--- a/ui/vnc-jobs.c
+++ b/ui/vnc-jobs.c
@@ -211,6 +211,8 @@ static int vnc_worker_thread_loop(VncJobQueue *queue)
     int n_rectangles;
     int saved_offset;
 
+    qio_buffer_init(&vs.output, "vnc-worker_output");
+
     vnc_lock_queue(queue);
     while (QTAILQ_EMPTY(&queue->jobs) && !queue->exit) {
         qemu_cond_wait(&queue->cond, &queue->mutex);


Peter

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [Qemu-devel] [RfC PATCH 07/10] io: add qio_buffer tracing
  2015-09-24  8:42 ` [Qemu-devel] [RfC PATCH 07/10] io: add qio_buffer tracing Gerd Hoffmann
@ 2015-09-25  8:10   ` Peter Lieven
  0 siblings, 0 replies; 22+ messages in thread
From: Peter Lieven @ 2015-09-25  8:10 UTC (permalink / raw)
  To: Gerd Hoffmann, qemu-devel

Am 24.09.2015 um 10:42 schrieb Gerd Hoffmann:
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  io/buffer.c  | 10 ++++++++++
>  trace-events |  6 ++++++
>  2 files changed, 16 insertions(+)
>
> diff --git a/io/buffer.c b/io/buffer.c
> index 96077d3..05425c2 100644
> --- a/io/buffer.c
> +++ b/io/buffer.c
> @@ -19,6 +19,7 @@
>   */
>  
>  #include "io/buffer.h"
> +#include "trace.h"
>  
>  #define QIO_BUFFER_MIN_INIT_SIZE 4096
>  
> @@ -37,6 +38,7 @@ void qio_buffer_reserve(QIOBuffer *buffer, size_t len)
>          buffer->capacity = pow2ceil(buffer->offset + len);
>          buffer->capacity = MAX(buffer->capacity, QIO_BUFFER_MIN_INIT_SIZE);
>          buffer->buffer = g_realloc(buffer->buffer, buffer->capacity);
> +        trace_qio_buffer_resize(buffer->name ?: "unnamed", buffer->capacity);
>      }
>  }
>  
> @@ -57,6 +59,7 @@ void qio_buffer_reset(QIOBuffer *buffer)
>  
>  void qio_buffer_free(QIOBuffer *buffer)
>  {
> +    trace_qio_buffer_free(buffer->name ?: "unnamed");
>      g_free(buffer->buffer);
>      g_free(buffer->name);
>      buffer->offset = 0;
> @@ -80,6 +83,9 @@ void qio_buffer_advance(QIOBuffer *buffer, size_t len)
>  
>  void qio_buffer_move_empty(QIOBuffer *to, QIOBuffer *from)
>  {
> +    trace_qio_buffer_move_empty(to->name ?: "unnamed",
> +                                from->offset,
> +                                from->name ?: "unnamed");
>      assert(to->offset == 0);
>  
>      g_free(to->buffer);
> @@ -99,6 +105,10 @@ void qio_buffer_move(QIOBuffer *to, QIOBuffer *from)
>          return;
>      }
>  
> +    trace_qio_buffer_move(to->name ?: "unnamed",
> +                          from->offset,
> +                          from->name ?: "unnamed");
> +
>      qio_buffer_reserve(to, from->offset);
>      qio_buffer_append(to, from->buffer, from->offset);
>  
> diff --git a/trace-events b/trace-events
> index 88a2f14..0f06b64 100644
> --- a/trace-events
> +++ b/trace-events
> @@ -1376,6 +1376,12 @@ spapr_iommu_new_table(uint64_t liobn, void *tcet, void *table, int fd) "liobn=%"
>  # hw/ppc/ppc.c
>  ppc_tb_adjust(uint64_t offs1, uint64_t offs2, int64_t diff, int64_t seconds) "adjusted from 0x%"PRIx64" to 0x%"PRIx64", diff %"PRId64" (%"PRId64"s)"
>  
> +# io/buffer.c
> +qio_buffer_resize(const char *buf, size_t len) "%s: len %zd"
> +qio_buffer_move_empty(const char *buf, size_t len, const char *from) "%s: %zd bytes from %s"
> +qio_buffer_move(const char *buf, size_t len, const char *from) "%s: %zd bytes from %s"
> +qio_buffer_free(const char *buf) "%s"
> +
>  # util/hbitmap.c
>  hbitmap_iter_skip_words(const void *hb, void *hbi, uint64_t pos, unsigned long cur) "hb %p hbi %p pos %"PRId64" cur 0x%lx"
>  hbitmap_reset(void *hb, uint64_t start, uint64_t count, uint64_t sbit, uint64_t ebit) "hb %p items %"PRIu64",%"PRIu64" bits %"PRIu64"..%"PRIu64

It might be a good idea to add the allocation size (aka capacity) when the buffer is freed:

diff --git a/io/buffer.c b/io/buffer.c
index 05425c2..60a96b0 100644
--- a/io/buffer.c
+++ b/io/buffer.c
@@ -59,7 +59,7 @@ void qio_buffer_reset(QIOBuffer *buffer)
 
 void qio_buffer_free(QIOBuffer *buffer)
 {
-    trace_qio_buffer_free(buffer->name ?: "unnamed");
+    trace_qio_buffer_free(buffer->name ?: "unnamed", buffer->capacity);
     g_free(buffer->buffer);
     g_free(buffer->name);
     buffer->offset = 0;
diff --git a/trace-events b/trace-events
index 0f06b64..53e096b 100644
--- a/trace-events
+++ b/trace-events
@@ -1380,7 +1380,7 @@ ppc_tb_adjust(uint64_t offs1, uint64_t offs2, int64_t diff, int64_t seconds) "ad
 qio_buffer_resize(const char *buf, size_t len) "%s: len %zd"
 qio_buffer_move_empty(const char *buf, size_t len, const char *from) "%s: %zd bytes from %s"
 qio_buffer_move(const char *buf, size_t len, const char *from) "%s: %zd bytes from %s"
-qio_buffer_free(const char *buf) "%s"
+qio_buffer_free(const char *buf, size_t len) "%s: capacity %zd"
 
 # util/hbitmap.c
 hbitmap_iter_skip_words(const void *hb, void *hbi, uint64_t pos, unsigned long cur) "hb %p hbi %p pos %"PRId64" cur 0x%lx"


This reveals that the buffers still increase to a reasonable size so it might still be an option to have some sort of shrinking logic:

3542@1443168337.727259:qio_buffer_free vnc-input/12: capacity 4096
3542@1443168337.727314:qio_buffer_free vnc-output/12: capacity 8388608
3542@1443168337.727935:qio_buffer_free vnc-ws_input/12: capacity 0
3542@1443168337.727960:qio_buffer_free vnc-ws_output/12: capacity 0
3542@1443168337.727978:qio_buffer_free vnc-zlib/12: capacity 0
3542@1443168337.728000:qio_buffer_free vnc-tight/12: capacity 16777216
3542@1443168337.729175:qio_buffer_free vnc-tight-zlib/12: capacity 32768
3542@1443168337.729376:qio_buffer_free vnc-tight-gradient/12: capacity 0
3542@1443168337.729395:qio_buffer_free vnc-tight-png/12: capacity 0
3542@1443168337.729409:qio_buffer_free vnc-zrle/12: capacity 0
3542@1443168337.729429:qio_buffer_free vnc-zrle-fb/12: capacity 0
3542@1443168337.729443:qio_buffer_free vnc-zrle-zlib/12: capacity 0
3542@1443168337.729459:qio_buffer_free vnc-jobs_buffer/12: capacity 0

Peter

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [Qemu-devel] [RfC PATCH 10/10] vnc-jobs: move buffer reset, use new buffer move
  2015-09-24  8:42 ` [Qemu-devel] [RfC PATCH 10/10] vnc-jobs: move buffer reset, use new buffer move Gerd Hoffmann
@ 2015-09-25  9:39   ` Peter Lieven
  0 siblings, 0 replies; 22+ messages in thread
From: Peter Lieven @ 2015-09-25  9:39 UTC (permalink / raw)
  To: Gerd Hoffmann, qemu-devel

Am 24.09.2015 um 10:42 schrieb Gerd Hoffmann:
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  ui/vnc-jobs.c | 10 +++-------
>  1 file changed, 3 insertions(+), 7 deletions(-)
>
> diff --git a/ui/vnc-jobs.c b/ui/vnc-jobs.c
> index 50e6b37..dfc5139 100644
> --- a/ui/vnc-jobs.c
> +++ b/ui/vnc-jobs.c
> @@ -165,8 +165,7 @@ void vnc_jobs_consume_buffer(VncState *vs)
>  
>      vnc_lock_output(vs);
>      if (vs->jobs_buffer.offset) {
> -        vnc_write(vs, vs->jobs_buffer.buffer, vs->jobs_buffer.offset);
> -        qio_buffer_reset(&vs->jobs_buffer);
> +        qio_buffer_move(&vs->output, &vs->jobs_buffer);

vnc_write restores the output handler. you need to add:

diff --git a/ui/vnc-jobs.c b/ui/vnc-jobs.c
index dfc5139..07e0445 100644
--- a/ui/vnc-jobs.c
+++ b/ui/vnc-jobs.c
@@ -29,6 +29,7 @@
 #include "vnc.h"
 #include "vnc-jobs.h"
 #include "qemu/sockets.h"
+#include "qemu/main-loop.h"
 #include "block/aio.h"
 
 /*
@@ -165,6 +166,9 @@ void vnc_jobs_consume_buffer(VncState *vs)
 
     vnc_lock_output(vs);
     if (vs->jobs_buffer.offset) {
+        if (qio_buffer_empty(&vs->output)) {
+            qemu_set_fd_handler(vs->csock, vnc_client_read, vnc_client_write, vs);
+        }
         qio_buffer_move(&vs->output, &vs->jobs_buffer);
     }
     flush = vs->csock != -1 && vs->abort != true;


or you get ocasional hangs.

Peter

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [Qemu-devel] [RfC PATCH 00/10] vnc buffer handling
  2015-09-24  8:41 [Qemu-devel] [RfC PATCH 00/10] vnc buffer handling Gerd Hoffmann
                   ` (10 preceding siblings ...)
  2015-09-24 16:25 ` [Qemu-devel] [RfC PATCH 00/10] vnc buffer handling Daniel P. Berrange
@ 2015-09-25  9:56 ` Peter Lieven
  11 siblings, 0 replies; 22+ messages in thread
From: Peter Lieven @ 2015-09-25  9:56 UTC (permalink / raw)
  To: Gerd Hoffmann, qemu-devel

Am 24.09.2015 um 10:41 schrieb Gerd Hoffmann:
>   Hi,
>
> Here is a patch series to improve the vnc buffer handling.  It picks up
> the qio_buffer patches from Daniel, adds move calls (move data from one
> buffer to another) and tracing, makes vnc use the new features.  Net
> effect should be that (a) vnc copies less data around and (b) buffers
> don't grow forever.
>
> It's RfC because it depends on wip patches.  My plan is to wait for
> Daniels patch series to be merged (which should obsolete patches #1+#2),
> then rebase and repost the series.
>
> Patches are also available from git:
>   git://git.kraxel.org/qemu rebase/ui-vnc-next
>
> please test & review,
>   Gerd

Looks fine except for the missing output handler due to the drop of vnc_write in vnc_jobs_consume_buffer.

However, I would add the following optimization:

diff --git a/ui/vnc-jobs.c b/ui/vnc-jobs.c
index dfc5139..4b384b0 100644
--- a/ui/vnc-jobs.c
+++ b/ui/vnc-jobs.c
@@ -228,6 +232,11 @@ static int vnc_worker_thread_loop(VncJobQueue *queue)
         vnc_unlock_output(job->vs);
         goto disconnected;
     }
+
+    if (qio_buffer_empty(&job->vs->output)) {
+        qio_buffer_move_empty(&vs.output, &job->vs->output);
+    }
+

The idea is that the vs->output is at the end of the queue and will be dropped by the next qio_buffer_move_empty anyway.
So why not reuse it as worker thread output buffer? This reduces reallocs to zero for me after the initial transmission of the desktop.

The buffers however still only allowed to grow during the ongoing session.

Peter

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [Qemu-devel] [RfC PATCH 04/10] io: add qio_buffer_init
  2015-09-24  8:42 ` [Qemu-devel] [RfC PATCH 04/10] io: add qio_buffer_init Gerd Hoffmann
@ 2015-09-25  9:56   ` Peter Lieven
  0 siblings, 0 replies; 22+ messages in thread
From: Peter Lieven @ 2015-09-25  9:56 UTC (permalink / raw)
  To: Gerd Hoffmann, qemu-devel

Am 24.09.2015 um 10:42 schrieb Gerd Hoffmann:
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  include/io/buffer.h | 12 ++++++++++++
>  io/buffer.c         | 11 +++++++++++
>  2 files changed, 23 insertions(+)
>
> diff --git a/include/io/buffer.h b/include/io/buffer.h
> index 2b1b261..cffad19 100644
> --- a/include/io/buffer.h
> +++ b/include/io/buffer.h
> @@ -34,12 +34,24 @@ typedef struct QIOBuffer QIOBuffer;
>   */
>  
>  struct QIOBuffer {
> +    char *name;
>      size_t capacity;
>      size_t offset;
>      uint8_t *buffer;
>  };
>  
>  /**
> + * qio_buffer_init:
> + * @buffer: the buffer object
> + * @name: buffer name
> + *
> + * Optionally attach a name to the buffer, to make it easier
> + * to identify in debug traces.
> + */
> +void qio_buffer_init(QIOBuffer *buffer, const char *name, ...)
> +        GCC_FMT_ATTR(2, 3);
> +
> +/**
>   * qio_buffer_reserve:
>   * @buffer: the buffer object
>   * @len: the minimum required free space
> diff --git a/io/buffer.c b/io/buffer.c
> index 6aa06e7..daa3ebf 100644
> --- a/io/buffer.c
> +++ b/io/buffer.c
> @@ -22,6 +22,15 @@
>  
>  #define QIO_BUFFER_MIN_INIT_SIZE 4096
>  
> +void qio_buffer_init(QIOBuffer *buffer, const char *name, ...)
> +{
> +    va_list ap;
> +
> +    va_start(ap, name);
> +    buffer->name = g_strdup_vprintf(name, ap);
> +    va_end(ap);
> +}
> +
>  void qio_buffer_reserve(QIOBuffer *buffer, size_t len)
>  {
>      if ((buffer->capacity - buffer->offset) < len) {
> @@ -49,9 +58,11 @@ void qio_buffer_reset(QIOBuffer *buffer)
>  void qio_buffer_free(QIOBuffer *buffer)
>  {
>      g_free(buffer->buffer);
> +    g_free(buffer->name);
>      buffer->offset = 0;
>      buffer->capacity = 0;
>      buffer->buffer = NULL;
> +    buffer->name = NULL;
>  }
>  
>  void qio_buffer_append(QIOBuffer *buffer, const void *data, size_t len)

Reviewed-by: Peter Lieven <pl@kamp.de>

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [Qemu-devel] [RfC PATCH 05/10] io: add qio_buffer_move_empty
  2015-09-24  8:42 ` [Qemu-devel] [RfC PATCH 05/10] io: add qio_buffer_move_empty Gerd Hoffmann
@ 2015-09-25  9:56   ` Peter Lieven
  0 siblings, 0 replies; 22+ messages in thread
From: Peter Lieven @ 2015-09-25  9:56 UTC (permalink / raw)
  To: Gerd Hoffmann, qemu-devel

Am 24.09.2015 um 10:42 schrieb Gerd Hoffmann:
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  include/io/buffer.h | 10 ++++++++++
>  io/buffer.c         | 14 ++++++++++++++
>  2 files changed, 24 insertions(+)
>
> diff --git a/include/io/buffer.h b/include/io/buffer.h
> index cffad19..1dddc73 100644
> --- a/include/io/buffer.h
> +++ b/include/io/buffer.h
> @@ -127,4 +127,14 @@ uint8_t *qio_buffer_end(QIOBuffer *buffer);
>   */
>  gboolean qio_buffer_empty(QIOBuffer *buffer);
>  
> +/**
> + * qio_buffer_move_empty:
> + * @to: destination buffer object
> + * @from: source buffer object
> + *
> + * Moves buffer, without copying data.  'to' buffer must be empty.
> + * 'from' buffer is empty and zero-sized on return.
> + */
> +void qio_buffer_move_empty(QIOBuffer *to, QIOBuffer *from);
> +
>  #endif /* QIO_BUFFER_H__ */
> diff --git a/io/buffer.c b/io/buffer.c
> index daa3ebf..09ca321 100644
> --- a/io/buffer.c
> +++ b/io/buffer.c
> @@ -77,3 +77,17 @@ void qio_buffer_advance(QIOBuffer *buffer, size_t len)
>              (buffer->offset - len));
>      buffer->offset -= len;
>  }
> +
> +void qio_buffer_move_empty(QIOBuffer *to, QIOBuffer *from)
> +{
> +    assert(to->offset == 0);
> +
> +    g_free(to->buffer);
> +    to->offset = from->offset;
> +    to->capacity = from->capacity;
> +    to->buffer = from->buffer;
> +
> +    from->offset = 0;
> +    from->capacity = 0;
> +    from->buffer = NULL;
> +}

Reviewed-by: Peter Lieven <pl@kamp.de>

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [Qemu-devel] [RfC PATCH 06/10] io: add qio_buffer_move
  2015-09-24  8:42 ` [Qemu-devel] [RfC PATCH 06/10] io: add qio_buffer_move Gerd Hoffmann
@ 2015-09-25  9:57   ` Peter Lieven
  0 siblings, 0 replies; 22+ messages in thread
From: Peter Lieven @ 2015-09-25  9:57 UTC (permalink / raw)
  To: Gerd Hoffmann, qemu-devel

Am 24.09.2015 um 10:42 schrieb Gerd Hoffmann:
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  include/io/buffer.h | 10 ++++++++++
>  io/buffer.c         | 16 ++++++++++++++++
>  2 files changed, 26 insertions(+)
>
> diff --git a/include/io/buffer.h b/include/io/buffer.h
> index 1dddc73..5676aff 100644
> --- a/include/io/buffer.h
> +++ b/include/io/buffer.h
> @@ -137,4 +137,14 @@ gboolean qio_buffer_empty(QIOBuffer *buffer);
>   */
>  void qio_buffer_move_empty(QIOBuffer *to, QIOBuffer *from);
>  
> +/**
> + * qio_buffer_move:
> + * @to: destination buffer object
> + * @from: source buffer object
> + *
> + * Moves buffer, copying data (unless 'to' buffer happens to be empty).
> + * 'from' buffer is empty and zero-sized on return.
> + */
> +void qio_buffer_move(QIOBuffer *to, QIOBuffer *from);
> +
>  #endif /* QIO_BUFFER_H__ */
> diff --git a/io/buffer.c b/io/buffer.c
> index 09ca321..96077d3 100644
> --- a/io/buffer.c
> +++ b/io/buffer.c
> @@ -91,3 +91,19 @@ void qio_buffer_move_empty(QIOBuffer *to, QIOBuffer *from)
>      from->capacity = 0;
>      from->buffer = NULL;
>  }
> +
> +void qio_buffer_move(QIOBuffer *to, QIOBuffer *from)
> +{
> +    if (to->offset == 0) {
> +        qio_buffer_move_empty(to, from);
> +        return;
> +    }
> +
> +    qio_buffer_reserve(to, from->offset);
> +    qio_buffer_append(to, from->buffer, from->offset);
> +
> +    g_free(from->buffer);
> +    from->offset = 0;
> +    from->capacity = 0;
> +    from->buffer = NULL;
> +}

Reviewed-by: Peter Lieven <pl@kamp.de>

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [Qemu-devel] [RfC PATCH 09/10] vnc: kill jobs queue buffer
  2015-09-24  8:42 ` [Qemu-devel] [RfC PATCH 09/10] vnc: kill jobs queue buffer Gerd Hoffmann
@ 2015-09-25  9:57   ` Peter Lieven
  0 siblings, 0 replies; 22+ messages in thread
From: Peter Lieven @ 2015-09-25  9:57 UTC (permalink / raw)
  To: Gerd Hoffmann, qemu-devel

Am 24.09.2015 um 10:42 schrieb Gerd Hoffmann:
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  ui/vnc-jobs.c | 6 ------
>  1 file changed, 6 deletions(-)
>
> diff --git a/ui/vnc-jobs.c b/ui/vnc-jobs.c
> index 7a234da..50e6b37 100644
> --- a/ui/vnc-jobs.c
> +++ b/ui/vnc-jobs.c
> @@ -54,7 +54,6 @@ struct VncJobQueue {
>      QemuCond cond;
>      QemuMutex mutex;
>      QemuThread thread;
> -    QIOBuffer buffer;
>      bool exit;
>      QTAILQ_HEAD(, VncJob) jobs;
>  };
> @@ -193,7 +192,6 @@ static void vnc_async_encoding_start(VncState *orig, VncState *local)
>      local->zlib = orig->zlib;
>      local->hextile = orig->hextile;
>      local->zrle = orig->zrle;
> -    local->output =  queue->buffer;
>      local->csock = -1; /* Don't do any network work on this thread */
>  
>      qio_buffer_reset(&local->output);
> @@ -206,8 +204,6 @@ static void vnc_async_encoding_end(VncState *orig, VncState *local)
>      orig->hextile = local->hextile;
>      orig->zrle = local->zrle;
>      orig->lossy_rect = local->lossy_rect;
> -
> -    queue->buffer = local->output;
>  }
>  
>  static int vnc_worker_thread_loop(VncJobQueue *queue)
> @@ -303,7 +299,6 @@ static VncJobQueue *vnc_queue_init(void)
>  
>      qemu_cond_init(&queue->cond);
>      qemu_mutex_init(&queue->mutex);
> -    qio_buffer_init(&queue->buffer, "vnc-job-queue");
>      QTAILQ_INIT(&queue->jobs);
>      return queue;
>  }
> @@ -312,7 +307,6 @@ static void vnc_queue_clear(VncJobQueue *q)
>  {
>      qemu_cond_destroy(&queue->cond);
>      qemu_mutex_destroy(&queue->mutex);
> -    qio_buffer_free(&queue->buffer);
>      g_free(q);
>      queue = NULL; /* Unset global queue */
>  }

Reviewed-by: Peter Lieven <pl@kamp.de>

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [Qemu-devel] [RfC PATCH 02/10] io: pull Buffer code out of VNC module
  2015-09-24  8:42 ` [Qemu-devel] [RfC PATCH 02/10] io: pull Buffer code out of VNC module Gerd Hoffmann
@ 2015-09-25  9:57   ` Peter Lieven
  0 siblings, 0 replies; 22+ messages in thread
From: Peter Lieven @ 2015-09-25  9:57 UTC (permalink / raw)
  To: Gerd Hoffmann, qemu-devel

Am 24.09.2015 um 10:42 schrieb Gerd Hoffmann:
> From: "Daniel P. Berrange" <berrange@redhat.com>
>
> The Buffer code in the VNC server is useful for the IO channel
> code, so pull it out into a shared module, QIOBuffer.
>
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
>  include/io/buffer.h | 118 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  io/Makefile.objs    |   1 +
>  io/buffer.c         |  65 +++++++++++++++++++++++++++++
>  ui/vnc-auth-sasl.c  |   4 +-
>  ui/vnc-enc-tight.c  |  38 ++++++++---------
>  ui/vnc-enc-zlib.c   |   6 +--
>  ui/vnc-enc-zrle.c   |  18 ++++----
>  ui/vnc-jobs.c       |  15 +++----
>  ui/vnc-ws.c         |  36 ++++++++--------
>  ui/vnc-ws.h         |   6 +--
>  ui/vnc.c            |  67 ++++++-----------------------
>  ui/vnc.h            |  50 ++++++++--------------
>  12 files changed, 276 insertions(+), 148 deletions(-)
>  create mode 100644 include/io/buffer.h
>  create mode 100644 io/Makefile.objs
>  create mode 100644 io/buffer.c
>
> diff --git a/include/io/buffer.h b/include/io/buffer.h
> new file mode 100644
> index 0000000..2b1b261
> --- /dev/null
> +++ b/include/io/buffer.h
> @@ -0,0 +1,118 @@
> +/*
> + * QEMU I/O buffers
> + *
> + * Copyright (c) 2015 Red Hat, Inc.
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, see <http://www.gnu.org/licenses/>.
> + *
> + */
> +
> +#ifndef QIO_BUFFER_H__
> +#define QIO_BUFFER_H__
> +
> +#include "qemu-common.h"
> +
> +typedef struct QIOBuffer QIOBuffer;
> +
> +/**
> + * QIOBuffer:
> + *
> + * The QIOBuffer object provides a simple dynamically resizing
> + * array, with separate tracking of capacity and usage. This
> + * is typically useful when buffering I/O data.
> + */
> +
> +struct QIOBuffer {
> +    size_t capacity;
> +    size_t offset;
> +    uint8_t *buffer;
> +};
> +
> +/**
> + * qio_buffer_reserve:
> + * @buffer: the buffer object
> + * @len: the minimum required free space
> + *
> + * Ensure that the buffer has space allocated for at least
> + * @len bytes. If the current buffer is too small, it will
> + * be reallocated, possibly to a larger size than requested.
> + */
> +void qio_buffer_reserve(QIOBuffer *buffer, size_t len);
> +
> +/**
> + * qio_buffer_reset:
> + * @buffer: the buffer object
> + *
> + * Reset the length of the stored data to zero, but do
> + * not free / reallocate the memory buffer
> + */
> +void qio_buffer_reset(QIOBuffer *buffer);
> +
> +/**
> + * qio_buffer_free:
> + * @buffer: the buffer object
> + *
> + * Reset the length of the stored data to zero and also
> + * free the internal memory buffer
> + */
> +void qio_buffer_free(QIOBuffer *buffer);
> +
> +/**
> + * qio_buffer_append:
> + * @buffer: the buffer object
> + * @data: the data block to append
> + * @len: the length of @data in bytes
> + *
> + * Append the contents of @data to the end of the buffer.
> + * The caller must ensure that the buffer has sufficient
> + * free space for @len bytes, typically by calling the
> + * qio_buffer_reserve() method prior to appending.
> + */
> +void qio_buffer_append(QIOBuffer *buffer, const void *data, size_t len);
> +
> +/**
> + * qio_buffer_advance:
> + * @buffer: the buffer object
> + * @len: the number of bytes to skip
> + *
> + * Remove @len bytes of data from the head of the buffer.
> + * The internal buffer will not be reallocated, so will
> + * have at least @len bytes of free space after this
> + * call completes
> + */
> +void qio_buffer_advance(QIOBuffer *buffer, size_t len);
> +
> +/**
> + * qio_buffer_end:
> + * @buffer: the buffer object
> + *
> + * Get a pointer to the tail end of the internal buffer
> + * The returned pointer is only valid until the next
> + * call to qio_buffer_reserve().
> + *
> + * Returns: the tail of the buffer
> + */
> +uint8_t *qio_buffer_end(QIOBuffer *buffer);
> +
> +/**
> + * qio_buffer_empty:
> + * @buffer: the buffer object
> + *
> + * Determine if the buffer contains any current data
> + *
> + * Returns: true if the buffer holds data, false otherwise
> + */
> +gboolean qio_buffer_empty(QIOBuffer *buffer);
> +
> +#endif /* QIO_BUFFER_H__ */
> diff --git a/io/Makefile.objs b/io/Makefile.objs
> new file mode 100644
> index 0000000..3de4e47
> --- /dev/null
> +++ b/io/Makefile.objs
> @@ -0,0 +1 @@
> +io-obj-y = buffer.o
> diff --git a/io/buffer.c b/io/buffer.c
> new file mode 100644
> index 0000000..68ae68d
> --- /dev/null
> +++ b/io/buffer.c
> @@ -0,0 +1,65 @@
> +/*
> + * QEMU I/O buffers
> + *
> + * Copyright (c) 2015 Red Hat, Inc.
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, see <http://www.gnu.org/licenses/>.
> + *
> + */
> +
> +#include "io/buffer.h"
> +
> +void qio_buffer_reserve(QIOBuffer *buffer, size_t len)
> +{
> +    if ((buffer->capacity - buffer->offset) < len) {
> +        buffer->capacity += (len + 1024);
> +        buffer->buffer = g_realloc(buffer->buffer, buffer->capacity);
> +    }
> +}
> +
> +gboolean qio_buffer_empty(QIOBuffer *buffer)
> +{
> +    return buffer->offset == 0;
> +}
> +
> +uint8_t *qio_buffer_end(QIOBuffer *buffer)
> +{
> +    return buffer->buffer + buffer->offset;
> +}
> +
> +void qio_buffer_reset(QIOBuffer *buffer)
> +{
> +    buffer->offset = 0;
> +}
> +
> +void qio_buffer_free(QIOBuffer *buffer)
> +{
> +    g_free(buffer->buffer);
> +    buffer->offset = 0;
> +    buffer->capacity = 0;
> +    buffer->buffer = NULL;
> +}
> +
> +void qio_buffer_append(QIOBuffer *buffer, const void *data, size_t len)
> +{
> +    memcpy(buffer->buffer + buffer->offset, data, len);
> +    buffer->offset += len;
> +}
> +
> +void qio_buffer_advance(QIOBuffer *buffer, size_t len)
> +{
> +    memmove(buffer->buffer, buffer->buffer + len,
> +            (buffer->offset - len));
> +    buffer->offset -= len;
> +}
> diff --git a/ui/vnc-auth-sasl.c b/ui/vnc-auth-sasl.c
> index fc732bd..d118266 100644
> --- a/ui/vnc-auth-sasl.c
> +++ b/ui/vnc-auth-sasl.c
> @@ -113,8 +113,8 @@ long vnc_client_read_sasl(VncState *vs)
>          return vnc_client_io_error(vs, -1, -EIO);
>      VNC_DEBUG("Read SASL Encoded %p size %ld Decoded %p size %d\n",
>                encoded, ret, decoded, decodedLen);
> -    buffer_reserve(&vs->input, decodedLen);
> -    buffer_append(&vs->input, decoded, decodedLen);
> +    qio_buffer_reserve(&vs->input, decodedLen);
> +    qio_buffer_append(&vs->input, decoded, decodedLen);
>      return decodedLen;
>  }
>  
> diff --git a/ui/vnc-enc-tight.c b/ui/vnc-enc-tight.c
> index 9a9ddf2..772ec79 100644
> --- a/ui/vnc-enc-tight.c
> +++ b/ui/vnc-enc-tight.c
> @@ -856,7 +856,7 @@ static int tight_compress_data(VncState *vs, int stream_id, size_t bytes,
>      }
>  
>      /* reserve memory in output buffer */
> -    buffer_reserve(&vs->tight.zlib, bytes + 64);
> +    qio_buffer_reserve(&vs->tight.zlib, bytes + 64);
>  
>      /* set pointers */
>      zstream->next_in = vs->tight.tight.buffer;
> @@ -879,7 +879,7 @@ static int tight_compress_data(VncState *vs, int stream_id, size_t bytes,
>      tight_send_compact_size(vs, bytes);
>      vnc_write(vs, vs->tight.zlib.buffer, bytes);
>  
> -    buffer_reset(&vs->tight.zlib);
> +    qio_buffer_reset(&vs->tight.zlib);
>  
>      return bytes;
>  }
> @@ -1053,7 +1053,7 @@ static bool send_gradient_rect(VncState *vs, int x, int y, int w, int h)
>      vnc_write_u8(vs, (stream | VNC_TIGHT_EXPLICIT_FILTER) << 4);
>      vnc_write_u8(vs, VNC_TIGHT_FILTER_GRADIENT);
>  
> -    buffer_reserve(&vs->tight.gradient, w * 3 * sizeof (int));
> +    qio_buffer_reserve(&vs->tight.gradient, w * 3 * sizeof(int));
>  
>      if (vs->tight.pixel24) {
>          tight_filter_gradient24(vs, vs->tight.tight.buffer, w, h);
> @@ -1066,7 +1066,7 @@ static bool send_gradient_rect(VncState *vs, int x, int y, int w, int h)
>          bytes = 2;
>      }
>  
> -    buffer_reset(&vs->tight.gradient);
> +    qio_buffer_reset(&vs->tight.gradient);
>  
>      bytes = w * h * bytes;
>      vs->tight.tight.offset = bytes;
> @@ -1149,7 +1149,7 @@ static int send_palette_rect(VncState *vs, int x, int y,
>  static void jpeg_init_destination(j_compress_ptr cinfo)
>  {
>      VncState *vs = cinfo->client_data;
> -    Buffer *buffer = &vs->tight.jpeg;
> +    QIOBuffer *buffer = &vs->tight.jpeg;
>  
>      cinfo->dest->next_output_byte = (JOCTET *)buffer->buffer + buffer->offset;
>      cinfo->dest->free_in_buffer = (size_t)(buffer->capacity - buffer->offset);
> @@ -1159,10 +1159,10 @@ static void jpeg_init_destination(j_compress_ptr cinfo)
>  static boolean jpeg_empty_output_buffer(j_compress_ptr cinfo)
>  {
>      VncState *vs = cinfo->client_data;
> -    Buffer *buffer = &vs->tight.jpeg;
> +    QIOBuffer *buffer = &vs->tight.jpeg;
>  
>      buffer->offset = buffer->capacity;
> -    buffer_reserve(buffer, 2048);
> +    qio_buffer_reserve(buffer, 2048);
>      jpeg_init_destination(cinfo);
>      return TRUE;
>  }
> @@ -1171,7 +1171,7 @@ static boolean jpeg_empty_output_buffer(j_compress_ptr cinfo)
>  static void jpeg_term_destination(j_compress_ptr cinfo)
>  {
>      VncState *vs = cinfo->client_data;
> -    Buffer *buffer = &vs->tight.jpeg;
> +    QIOBuffer *buffer = &vs->tight.jpeg;
>  
>      buffer->offset = buffer->capacity - cinfo->dest->free_in_buffer;
>  }
> @@ -1190,7 +1190,7 @@ static int send_jpeg_rect(VncState *vs, int x, int y, int w, int h, int quality)
>          return send_full_color_rect(vs, x, y, w, h);
>      }
>  
> -    buffer_reserve(&vs->tight.jpeg, 2048);
> +    qio_buffer_reserve(&vs->tight.jpeg, 2048);
>  
>      cinfo.err = jpeg_std_error(&jerr);
>      jpeg_create_compress(&cinfo);
> @@ -1227,7 +1227,7 @@ static int send_jpeg_rect(VncState *vs, int x, int y, int w, int h, int quality)
>  
>      tight_send_compact_size(vs, vs->tight.jpeg.offset);
>      vnc_write(vs, vs->tight.jpeg.buffer, vs->tight.jpeg.offset);
> -    buffer_reset(&vs->tight.jpeg);
> +    qio_buffer_reset(&vs->tight.jpeg);
>  
>      return 1;
>  }
> @@ -1270,7 +1270,7 @@ static void png_write_data(png_structp png_ptr, png_bytep data,
>  {
>      VncState *vs = png_get_io_ptr(png_ptr);
>  
> -    buffer_reserve(&vs->tight.png, vs->tight.png.offset + length);
> +    qio_buffer_reserve(&vs->tight.png, vs->tight.png.offset + length);
>      memcpy(vs->tight.png.buffer + vs->tight.png.offset, data, length);
>  
>      vs->tight.png.offset += length;
> @@ -1351,7 +1351,7 @@ static int send_png_rect(VncState *vs, int x, int y, int w, int h,
>  
>      png_write_info(png_ptr, info_ptr);
>  
> -    buffer_reserve(&vs->tight.png, 2048);
> +    qio_buffer_reserve(&vs->tight.png, 2048);
>      linebuf = qemu_pixman_linebuf_create(PIXMAN_BE_r8g8b8, w);
>      buf = (uint8_t *)pixman_image_get_data(linebuf);
>      for (dy = 0; dy < h; dy++)
> @@ -1377,14 +1377,14 @@ static int send_png_rect(VncState *vs, int x, int y, int w, int h,
>  
>      tight_send_compact_size(vs, vs->tight.png.offset);
>      vnc_write(vs, vs->tight.png.buffer, vs->tight.png.offset);
> -    buffer_reset(&vs->tight.png);
> +    qio_buffer_reset(&vs->tight.png);
>      return 1;
>  }
>  #endif /* CONFIG_VNC_PNG */
>  
>  static void vnc_tight_start(VncState *vs)
>  {
> -    buffer_reset(&vs->tight.tight);
> +    qio_buffer_reset(&vs->tight.tight);
>  
>      // make the output buffer be the zlib buffer, so we can compress it later
>      vs->tight.tmp = vs->output;
> @@ -1686,13 +1686,13 @@ void vnc_tight_clear(VncState *vs)
>          }
>      }
>  
> -    buffer_free(&vs->tight.tight);
> -    buffer_free(&vs->tight.zlib);
> -    buffer_free(&vs->tight.gradient);
> +    qio_buffer_free(&vs->tight.tight);
> +    qio_buffer_free(&vs->tight.zlib);
> +    qio_buffer_free(&vs->tight.gradient);
>  #ifdef CONFIG_VNC_JPEG
> -    buffer_free(&vs->tight.jpeg);
> +    qio_buffer_free(&vs->tight.jpeg);
>  #endif
>  #ifdef CONFIG_VNC_PNG
> -    buffer_free(&vs->tight.png);
> +    qio_buffer_free(&vs->tight.png);
>  #endif
>  }
> diff --git a/ui/vnc-enc-zlib.c b/ui/vnc-enc-zlib.c
> index d1b97f2..47ba146 100644
> --- a/ui/vnc-enc-zlib.c
> +++ b/ui/vnc-enc-zlib.c
> @@ -47,7 +47,7 @@ void vnc_zlib_zfree(void *x, void *addr)
>  
>  static void vnc_zlib_start(VncState *vs)
>  {
> -    buffer_reset(&vs->zlib.zlib);
> +    qio_buffer_reset(&vs->zlib.zlib);
>  
>      // make the output buffer be the zlib buffer, so we can compress it later
>      vs->zlib.tmp = vs->output;
> @@ -96,7 +96,7 @@ static int vnc_zlib_stop(VncState *vs)
>      }
>  
>      // reserve memory in output buffer
> -    buffer_reserve(&vs->output, vs->zlib.zlib.offset + 64);
> +    qio_buffer_reserve(&vs->output, vs->zlib.zlib.offset + 64);
>  
>      // set pointers
>      zstream->next_in = vs->zlib.zlib.buffer;
> @@ -148,5 +148,5 @@ void vnc_zlib_clear(VncState *vs)
>      if (vs->zlib.stream.opaque) {
>          deflateEnd(&vs->zlib.stream);
>      }
> -    buffer_free(&vs->zlib.zlib);
> +    qio_buffer_free(&vs->zlib.zlib);
>  }
> diff --git a/ui/vnc-enc-zrle.c b/ui/vnc-enc-zrle.c
> index ed3b484..bd1e320 100644
> --- a/ui/vnc-enc-zrle.c
> +++ b/ui/vnc-enc-zrle.c
> @@ -36,7 +36,7 @@ static const int bits_per_packed_pixel[] = {
>  
>  static void vnc_zrle_start(VncState *vs)
>  {
> -    buffer_reset(&vs->zrle.zrle);
> +    qio_buffer_reset(&vs->zrle.zrle);
>  
>      /* make the output buffer be the zlib buffer, so we can compress it later */
>      vs->zrle.tmp = vs->output;
> @@ -53,10 +53,10 @@ static void vnc_zrle_stop(VncState *vs)
>  static void *zrle_convert_fb(VncState *vs, int x, int y, int w, int h,
>                               int bpp)
>  {
> -    Buffer tmp;
> +    QIOBuffer tmp;
>  
> -    buffer_reset(&vs->zrle.fb);
> -    buffer_reserve(&vs->zrle.fb, w * h * bpp + bpp);
> +    qio_buffer_reset(&vs->zrle.fb);
> +    qio_buffer_reserve(&vs->zrle.fb, w * h * bpp + bpp);
>  
>      tmp = vs->output;
>      vs->output = vs->zrle.fb;
> @@ -72,7 +72,7 @@ static int zrle_compress_data(VncState *vs, int level)
>  {
>      z_streamp zstream = &vs->zrle.stream;
>  
> -    buffer_reset(&vs->zrle.zlib);
> +    qio_buffer_reset(&vs->zrle.zlib);
>  
>      if (zstream->opaque != vs) {
>          int err;
> @@ -92,7 +92,7 @@ static int zrle_compress_data(VncState *vs, int level)
>      }
>  
>      /* reserve memory in output buffer */
> -    buffer_reserve(&vs->zrle.zlib, vs->zrle.zrle.offset + 64);
> +    qio_buffer_reserve(&vs->zrle.zlib, vs->zrle.zrle.offset + 64);
>  
>      /* set pointers */
>      zstream->next_in = vs->zrle.zrle.buffer;
> @@ -360,7 +360,7 @@ void vnc_zrle_clear(VncState *vs)
>      if (vs->zrle.stream.opaque) {
>          deflateEnd(&vs->zrle.stream);
>      }
> -    buffer_free(&vs->zrle.zrle);
> -    buffer_free(&vs->zrle.fb);
> -    buffer_free(&vs->zrle.zlib);
> +    qio_buffer_free(&vs->zrle.zrle);
> +    qio_buffer_free(&vs->zrle.fb);
> +    qio_buffer_free(&vs->zrle.zlib);
>  }
> diff --git a/ui/vnc-jobs.c b/ui/vnc-jobs.c
> index 22c9abc..9824c34 100644
> --- a/ui/vnc-jobs.c
> +++ b/ui/vnc-jobs.c
> @@ -54,7 +54,7 @@ struct VncJobQueue {
>      QemuCond cond;
>      QemuMutex mutex;
>      QemuThread thread;
> -    Buffer buffer;
> +    QIOBuffer buffer;
>      bool exit;
>      QTAILQ_HEAD(, VncJob) jobs;
>  };
> @@ -167,7 +167,7 @@ void vnc_jobs_consume_buffer(VncState *vs)
>      vnc_lock_output(vs);
>      if (vs->jobs_buffer.offset) {
>          vnc_write(vs, vs->jobs_buffer.buffer, vs->jobs_buffer.offset);
> -        buffer_reset(&vs->jobs_buffer);
> +        qio_buffer_reset(&vs->jobs_buffer);
>      }
>      flush = vs->csock != -1 && vs->abort != true;
>      vnc_unlock_output(vs);
> @@ -196,7 +196,7 @@ static void vnc_async_encoding_start(VncState *orig, VncState *local)
>      local->output =  queue->buffer;
>      local->csock = -1; /* Don't do any network work on this thread */
>  
> -    buffer_reset(&local->output);
> +    qio_buffer_reset(&local->output);
>  }
>  
>  static void vnc_async_encoding_end(VncState *orig, VncState *local)
> @@ -273,10 +273,11 @@ static int vnc_worker_thread_loop(VncJobQueue *queue)
>      vs.output.buffer[saved_offset + 1] = n_rectangles & 0xFF;
>  
>      vnc_lock_output(job->vs);
> +
>      if (job->vs->csock != -1) {
> -        buffer_reserve(&job->vs->jobs_buffer, vs.output.offset);
> -        buffer_append(&job->vs->jobs_buffer, vs.output.buffer,
> -                      vs.output.offset);
> +        qio_buffer_reserve(&job->vs->jobs_buffer, vs.output.offset);
> +        qio_buffer_append(&job->vs->jobs_buffer, vs.output.buffer,
> +                          vs.output.offset);
>          /* Copy persistent encoding data */
>          vnc_async_encoding_end(job->vs, &vs);
>  
> @@ -310,7 +311,7 @@ static void vnc_queue_clear(VncJobQueue *q)
>  {
>      qemu_cond_destroy(&queue->cond);
>      qemu_mutex_destroy(&queue->mutex);
> -    buffer_free(&queue->buffer);
> +    qio_buffer_free(&queue->buffer);
>      g_free(q);
>      queue = NULL; /* Unset global queue */
>  }
> diff --git a/ui/vnc-ws.c b/ui/vnc-ws.c
> index 175ea50..2fe4476 100644
> --- a/ui/vnc-ws.c
> +++ b/ui/vnc-ws.c
> @@ -95,8 +95,8 @@ void vncws_handshake_read(void *opaque)
>      /* Typical HTTP headers from novnc are 512 bytes, so limiting
>       * total header size to 4096 is easily enough. */
>      size_t want = 4096 - vs->ws_input.offset;
> -    buffer_reserve(&vs->ws_input, want);
> -    ret = vnc_client_read_buf(vs, buffer_end(&vs->ws_input), want);
> +    qio_buffer_reserve(&vs->ws_input, want);
> +    ret = vnc_client_read_buf(vs, qio_buffer_end(&vs->ws_input), want);
>  
>      if (!ret) {
>          if (vs->csock == -1) {
> @@ -111,7 +111,7 @@ void vncws_handshake_read(void *opaque)
>      if (handshake_end) {
>          qemu_set_fd_handler(vs->csock, vnc_client_read, NULL, vs);
>          vncws_process_handshake(vs, vs->ws_input.buffer, vs->ws_input.offset);
> -        buffer_advance(&vs->ws_input, handshake_end - vs->ws_input.buffer +
> +        qio_buffer_advance(&vs->ws_input, handshake_end - vs->ws_input.buffer +
>                  strlen(WS_HANDSHAKE_END));
>      } else if (vs->ws_input.offset >= 4096) {
>          VNC_DEBUG("End of headers not found in first 4096 bytes\n");
> @@ -127,8 +127,8 @@ long vnc_client_read_ws(VncState *vs)
>      size_t payload_size, header_size;
>      VNC_DEBUG("Read websocket %p size %zd offset %zd\n", vs->ws_input.buffer,
>              vs->ws_input.capacity, vs->ws_input.offset);
> -    buffer_reserve(&vs->ws_input, 4096);
> -    ret = vnc_client_read_buf(vs, buffer_end(&vs->ws_input), 4096);
> +    qio_buffer_reserve(&vs->ws_input, 4096);
> +    ret = vnc_client_read_buf(vs, qio_buffer_end(&vs->ws_input), 4096);
>      if (!ret) {
>          return 0;
>      }
> @@ -146,7 +146,7 @@ long vnc_client_read_ws(VncState *vs)
>                  return err;
>              }
>  
> -            buffer_advance(&vs->ws_input, header_size);
> +            qio_buffer_advance(&vs->ws_input, header_size);
>          }
>          if (vs->ws_payload_remain != 0) {
>              err = vncws_decode_frame_payload(&vs->ws_input,
> @@ -162,10 +162,10 @@ long vnc_client_read_ws(VncState *vs)
>              }
>              ret += err;
>  
> -            buffer_reserve(&vs->input, payload_size);
> -            buffer_append(&vs->input, payload, payload_size);
> +            qio_buffer_reserve(&vs->input, payload_size);
> +            qio_buffer_append(&vs->input, payload, payload_size);
>  
> -            buffer_advance(&vs->ws_input, payload_size);
> +            qio_buffer_advance(&vs->ws_input, payload_size);
>          }
>      } while (vs->ws_input.offset > 0);
>  
> @@ -178,13 +178,13 @@ long vnc_client_write_ws(VncState *vs)
>      VNC_DEBUG("Write WS: Pending output %p size %zd offset %zd\n",
>                vs->output.buffer, vs->output.capacity, vs->output.offset);
>      vncws_encode_frame(&vs->ws_output, vs->output.buffer, vs->output.offset);
> -    buffer_reset(&vs->output);
> +    qio_buffer_reset(&vs->output);
>      ret = vnc_client_write_buf(vs, vs->ws_output.buffer, vs->ws_output.offset);
>      if (!ret) {
>          return 0;
>      }
>  
> -    buffer_advance(&vs->ws_output, ret);
> +    qio_buffer_advance(&vs->ws_output, ret);
>  
>      if (vs->ws_output.offset == 0) {
>          qemu_set_fd_handler(vs->csock, vnc_client_read, NULL, vs);
> @@ -267,8 +267,8 @@ void vncws_process_handshake(VncState *vs, uint8_t *line, size_t size)
>      g_free(key);
>  }
>  
> -void vncws_encode_frame(Buffer *output, const void *payload,
> -        const size_t payload_size)
> +void vncws_encode_frame(QIOBuffer *output, const void *payload,
> +                        const size_t payload_size)
>  {
>      size_t header_size = 0;
>      unsigned char opcode = WS_OPCODE_BINARY_FRAME;
> @@ -295,12 +295,12 @@ void vncws_encode_frame(Buffer *output, const void *payload,
>          header_size = 10;
>      }
>  
> -    buffer_reserve(output, header_size + payload_size);
> -    buffer_append(output, header.buf, header_size);
> -    buffer_append(output, payload, payload_size);
> +    qio_buffer_reserve(output, header_size + payload_size);
> +    qio_buffer_append(output, header.buf, header_size);
> +    qio_buffer_append(output, payload, payload_size);
>  }
>  
> -int vncws_decode_frame_header(Buffer *input,
> +int vncws_decode_frame_header(QIOBuffer *input,
>                                size_t *header_size,
>                                size_t *payload_remain,
>                                WsMask *payload_mask)
> @@ -354,7 +354,7 @@ int vncws_decode_frame_header(Buffer *input,
>      return 1;
>  }
>  
> -int vncws_decode_frame_payload(Buffer *input,
> +int vncws_decode_frame_payload(QIOBuffer *input,
>                                 size_t *payload_remain, WsMask *payload_mask,
>                                 uint8_t **payload, size_t *payload_size)
>  {
> diff --git a/ui/vnc-ws.h b/ui/vnc-ws.h
> index 4ab0a8c..2a222a8 100644
> --- a/ui/vnc-ws.h
> +++ b/ui/vnc-ws.h
> @@ -77,13 +77,13 @@ void vncws_handshake_read(void *opaque);
>  long vnc_client_write_ws(VncState *vs);
>  long vnc_client_read_ws(VncState *vs);
>  void vncws_process_handshake(VncState *vs, uint8_t *line, size_t size);
> -void vncws_encode_frame(Buffer *output, const void *payload,
> +void vncws_encode_frame(QIOBuffer *output, const void *payload,
>              const size_t payload_size);
> -int vncws_decode_frame_header(Buffer *input,
> +int vncws_decode_frame_header(QIOBuffer *input,
>                                size_t *header_size,
>                                size_t *payload_remain,
>                                WsMask *payload_mask);
> -int vncws_decode_frame_payload(Buffer *input,
> +int vncws_decode_frame_payload(QIOBuffer *input,
>                                 size_t *payload_remain, WsMask *payload_mask,
>                                 uint8_t **payload, size_t *payload_size);
>  
> diff --git a/ui/vnc.c b/ui/vnc.c
> index bbe1e91..e4b0e3a 100644
> --- a/ui/vnc.c
> +++ b/ui/vnc.c
> @@ -647,49 +647,6 @@ void vnc_framebuffer_update(VncState *vs, int x, int y, int w, int h,
>      vnc_write_s32(vs, encoding);
>  }
>  
> -void buffer_reserve(Buffer *buffer, size_t len)
> -{
> -    if ((buffer->capacity - buffer->offset) < len) {
> -        buffer->capacity += (len + 1024);
> -        buffer->buffer = g_realloc(buffer->buffer, buffer->capacity);
> -    }
> -}
> -
> -static int buffer_empty(Buffer *buffer)
> -{
> -    return buffer->offset == 0;
> -}
> -
> -uint8_t *buffer_end(Buffer *buffer)
> -{
> -    return buffer->buffer + buffer->offset;
> -}
> -
> -void buffer_reset(Buffer *buffer)
> -{
> -        buffer->offset = 0;
> -}
> -
> -void buffer_free(Buffer *buffer)
> -{
> -    g_free(buffer->buffer);
> -    buffer->offset = 0;
> -    buffer->capacity = 0;
> -    buffer->buffer = NULL;
> -}
> -
> -void buffer_append(Buffer *buffer, const void *data, size_t len)
> -{
> -    memcpy(buffer->buffer + buffer->offset, data, len);
> -    buffer->offset += len;
> -}
> -
> -void buffer_advance(Buffer *buf, size_t len)
> -{
> -    memmove(buf->buffer, buf->buffer + len,
> -            (buf->offset - len));
> -    buf->offset -= len;
> -}
>  
>  static void vnc_desktop_resize(VncState *vs)
>  {
> @@ -1236,10 +1193,10 @@ void vnc_disconnect_finish(VncState *vs)
>      vnc_lock_output(vs);
>      vnc_qmp_event(vs, QAPI_EVENT_VNC_DISCONNECTED);
>  
> -    buffer_free(&vs->input);
> -    buffer_free(&vs->output);
> -    buffer_free(&vs->ws_input);
> -    buffer_free(&vs->ws_output);
> +    qio_buffer_free(&vs->input);
> +    qio_buffer_free(&vs->output);
> +    qio_buffer_free(&vs->ws_input);
> +    qio_buffer_free(&vs->ws_output);
>  
>      qapi_free_VncClientInfo(vs->info);
>  
> @@ -1267,7 +1224,7 @@ void vnc_disconnect_finish(VncState *vs)
>      if (vs->bh != NULL) {
>          qemu_bh_delete(vs->bh);
>      }
> -    buffer_free(&vs->jobs_buffer);
> +    qio_buffer_free(&vs->jobs_buffer);
>  
>      for (i = 0; i < VNC_STAT_ROWS; ++i) {
>          g_free(vs->lossy_rect[i]);
> @@ -1409,7 +1366,7 @@ static ssize_t vnc_client_write_plain(VncState *vs)
>      if (!ret)
>          return 0;
>  
> -    buffer_advance(&vs->output, ret);
> +    qio_buffer_advance(&vs->output, ret);
>  
>      if (vs->output.offset == 0) {
>          qemu_set_fd_handler(vs->csock, vnc_client_read, NULL, vs);
> @@ -1512,8 +1469,8 @@ static ssize_t vnc_client_read_plain(VncState *vs)
>      ssize_t ret;
>      VNC_DEBUG("Read plain %p size %zd offset %zd\n",
>                vs->input.buffer, vs->input.capacity, vs->input.offset);
> -    buffer_reserve(&vs->input, 4096);
> -    ret = vnc_client_read_buf(vs, buffer_end(&vs->input), 4096);
> +    qio_buffer_reserve(&vs->input, 4096);
> +    ret = vnc_client_read_buf(vs, qio_buffer_end(&vs->input), 4096);
>      if (!ret)
>          return 0;
>      vs->input.offset += ret;
> @@ -1571,7 +1528,7 @@ void vnc_client_read(void *opaque)
>          }
>  
>          if (!ret) {
> -            buffer_advance(&vs->input, len);
> +            qio_buffer_advance(&vs->input, len);
>          } else {
>              vs->read_handler_expect = ret;
>          }
> @@ -1580,13 +1537,13 @@ void vnc_client_read(void *opaque)
>  
>  void vnc_write(VncState *vs, const void *data, size_t len)
>  {
> -    buffer_reserve(&vs->output, len);
> +    qio_buffer_reserve(&vs->output, len);
>  
> -    if (vs->csock != -1 && buffer_empty(&vs->output)) {
> +    if (vs->csock != -1 && qio_buffer_empty(&vs->output)) {
>          qemu_set_fd_handler(vs->csock, vnc_client_read, vnc_client_write, vs);
>      }
>  
> -    buffer_append(&vs->output, data, len);
> +    qio_buffer_append(&vs->output, data, len);
>  }
>  
>  void vnc_write_s32(VncState *vs, int32_t value)
> diff --git a/ui/vnc.h b/ui/vnc.h
> index 4dd769c..339a1bf 100644
> --- a/ui/vnc.h
> +++ b/ui/vnc.h
> @@ -34,6 +34,7 @@
>  #include "audio/audio.h"
>  #include "qemu/bitmap.h"
>  #include "crypto/tlssession.h"
> +#include "io/buffer.h"
>  #include <zlib.h>
>  #include <stdbool.h>
>  
> @@ -56,13 +57,6 @@
>   *
>   *****************************************************************************/
>  
> -typedef struct Buffer
> -{
> -    size_t capacity;
> -    size_t offset;
> -    uint8_t *buffer;
> -} Buffer;
> -
>  typedef struct VncState VncState;
>  typedef struct VncJob VncJob;
>  typedef struct VncRect VncRect;
> @@ -191,15 +185,15 @@ typedef struct VncTight {
>      uint8_t quality;
>      uint8_t compression;
>      uint8_t pixel24;
> -    Buffer tight;
> -    Buffer tmp;
> -    Buffer zlib;
> -    Buffer gradient;
> +    QIOBuffer tight;
> +    QIOBuffer tmp;
> +    QIOBuffer zlib;
> +    QIOBuffer gradient;
>  #ifdef CONFIG_VNC_JPEG
> -    Buffer jpeg;
> +    QIOBuffer jpeg;
>  #endif
>  #ifdef CONFIG_VNC_PNG
> -    Buffer png;
> +    QIOBuffer png;
>  #endif
>      int levels[4];
>      z_stream stream[4];
> @@ -210,18 +204,18 @@ typedef struct VncHextile {
>  } VncHextile;
>  
>  typedef struct VncZlib {
> -    Buffer zlib;
> -    Buffer tmp;
> +    QIOBuffer zlib;
> +    QIOBuffer tmp;
>      z_stream stream;
>      int level;
>  } VncZlib;
>  
>  typedef struct VncZrle {
>      int type;
> -    Buffer fb;
> -    Buffer zrle;
> -    Buffer tmp;
> -    Buffer zlib;
> +    QIOBuffer fb;
> +    QIOBuffer zrle;
> +    QIOBuffer tmp;
> +    QIOBuffer zlib;
>      z_stream stream;
>      VncPalette palette;
>  } VncZrle;
> @@ -290,10 +284,10 @@ struct VncState
>  
>      VncClientInfo *info;
>  
> -    Buffer output;
> -    Buffer input;
> -    Buffer ws_input;
> -    Buffer ws_output;
> +    QIOBuffer output;
> +    QIOBuffer input;
> +    QIOBuffer ws_input;
> +    QIOBuffer ws_output;
>      size_t ws_payload_remain;
>      WsMask ws_payload_mask;
>      /* current output mode information */
> @@ -315,7 +309,7 @@ struct VncState
>      bool initialized;
>      QemuMutex output_mutex;
>      QEMUBH *bh;
> -    Buffer jobs_buffer;
> +    QIOBuffer jobs_buffer;
>  
>      /* Encoding specific, if you add something here, don't forget to
>       *  update vnc_async_encoding_start()
> @@ -535,14 +529,6 @@ ssize_t vnc_client_io_error(VncState *vs, ssize_t ret, int last_errno);
>  void start_client_init(VncState *vs);
>  void start_auth_vnc(VncState *vs);
>  
> -/* Buffer management */
> -void buffer_reserve(Buffer *buffer, size_t len);
> -void buffer_reset(Buffer *buffer);
> -void buffer_free(Buffer *buffer);
> -void buffer_append(Buffer *buffer, const void *data, size_t len);
> -void buffer_advance(Buffer *buf, size_t len);
> -uint8_t *buffer_end(Buffer *buffer);
> -
>  
>  /* Misc helpers */
>  

Reviewed-by: Peter Lieven <pl@kamp.de>

^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2015-09-25  9:57 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-09-24  8:41 [Qemu-devel] [RfC PATCH 00/10] vnc buffer handling Gerd Hoffmann
2015-09-24  8:41 ` [Qemu-devel] [RfC PATCH 01/10] io/ makefile fluff Gerd Hoffmann
2015-09-24  8:42 ` [Qemu-devel] [RfC PATCH 02/10] io: pull Buffer code out of VNC module Gerd Hoffmann
2015-09-25  9:57   ` Peter Lieven
2015-09-24  8:42 ` [Qemu-devel] [RfC PATCH 03/10] vnc: make the Buffer capacity increase in powers of two Gerd Hoffmann
2015-09-24  8:42 ` [Qemu-devel] [RfC PATCH 04/10] io: add qio_buffer_init Gerd Hoffmann
2015-09-25  9:56   ` Peter Lieven
2015-09-24  8:42 ` [Qemu-devel] [RfC PATCH 05/10] io: add qio_buffer_move_empty Gerd Hoffmann
2015-09-25  9:56   ` Peter Lieven
2015-09-24  8:42 ` [Qemu-devel] [RfC PATCH 06/10] io: add qio_buffer_move Gerd Hoffmann
2015-09-25  9:57   ` Peter Lieven
2015-09-24  8:42 ` [Qemu-devel] [RfC PATCH 07/10] io: add qio_buffer tracing Gerd Hoffmann
2015-09-25  8:10   ` Peter Lieven
2015-09-24  8:42 ` [Qemu-devel] [RfC PATCH 08/10] name vnc buffers Gerd Hoffmann
2015-09-25  7:28   ` Peter Lieven
2015-09-25  7:58   ` Peter Lieven
2015-09-24  8:42 ` [Qemu-devel] [RfC PATCH 09/10] vnc: kill jobs queue buffer Gerd Hoffmann
2015-09-25  9:57   ` Peter Lieven
2015-09-24  8:42 ` [Qemu-devel] [RfC PATCH 10/10] vnc-jobs: move buffer reset, use new buffer move Gerd Hoffmann
2015-09-25  9:39   ` Peter Lieven
2015-09-24 16:25 ` [Qemu-devel] [RfC PATCH 00/10] vnc buffer handling Daniel P. Berrange
2015-09-25  9:56 ` Peter Lieven

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).