[Spice-devel] [PATCH v2 5/8] replay: Remove useless check
Same check is done inside replay_fscanf
Signed-off-by: Frediano Ziglio
---
server/red-replay-qxl.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/server/red-replay-qxl.c b/server/red-replay-qxl.c
index 28da13d..ef5569e 100644
--- a/server/red-replay-qxl.c
+++ b/server/red-replay-qxl.c
@@ -1063,9 +1063,6 @@ static QXLCompatDrawable
*red_replay_compat_drawable(SpiceReplay *replay, uint32
static QXLPHYSICAL red_replay_drawable(SpiceReplay *replay, uint32_t flags)
{
-if (replay->error) {
-return 0;
-}
replay_fscanf(replay, "drawable\n");
if (flags & QXL_COMMAND_FLAG_COMPAT) {
return QXLPHYSICAL_FROM_PTR(red_replay_compat_drawable(replay, flags));
--
2.7.4
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH v2 6/8] replay: Assure read_binary receives a NULL pointer
read_binary do not allocate a buffer for no-NULL pointers.
Avoid using uninitialized data and allocate proper buffer.
Signed-off-by: Frediano Ziglio
---
server/red-replay-qxl.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/server/red-replay-qxl.c b/server/red-replay-qxl.c
index ef5569e..45c105c 100644
--- a/server/red-replay-qxl.c
+++ b/server/red-replay-qxl.c
@@ -1089,7 +1089,7 @@ static QXLUpdateCmd *red_replay_update_cmd(SpiceReplay
*replay)
static QXLMessage *red_replay_message(SpiceReplay *replay)
{
-QXLMessage *qxl;
+QXLMessage *qxl = NULL;
size_t size;
read_binary(replay, "message", &size, (uint8_t**)&qxl, sizeof(QXLMessage));
--
2.7.4
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH v2 8/8] replay: Propagate error correctly in replay_fread
Signed-off-by: Frediano Ziglio
---
server/red-replay-qxl.c | 11 +--
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/server/red-replay-qxl.c b/server/red-replay-qxl.c
index 73f9cd4..fe4f7a9 100644
--- a/server/red-replay-qxl.c
+++ b/server/red-replay-qxl.c
@@ -57,16 +57,15 @@ struct SpiceReplay {
pthread_cond_t cond;
};
-static int replay_fread(SpiceReplay *replay, uint8_t *buf, size_t size)
+static ssize_t replay_fread(SpiceReplay *replay, uint8_t *buf, size_t size)
{
-if (replay->error) {
-return 0;
-}
-if (feof(replay->fd)) {
+if (replay->error
+|| feof(replay->fd)
+|| fread(buf, 1, size, replay->fd) != size) {
replay->error = TRUE;
return 0;
}
-return fread(buf, size, 1, replay->fd);
+return size;
}
__attribute__((format(scanf, 2, 3)))
--
2.7.4
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH v2 7/8] replay: Update pointer in allocated list
Avoid to free invalid pointer.
Signed-off-by: Frediano Ziglio
---
server/red-replay-qxl.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/server/red-replay-qxl.c b/server/red-replay-qxl.c
index 45c105c..73f9cd4 100644
--- a/server/red-replay-qxl.c
+++ b/server/red-replay-qxl.c
@@ -413,6 +413,7 @@ static QXLImage *red_replay_image(SpiceReplay *replay,
uint32_t flags)
int temp;
int has_palette;
int has_image;
+GList *elem;
replay_fscanf(replay, "image %d\n", &has_image);
if (replay->error) {
@@ -423,6 +424,7 @@ static QXLImage *red_replay_image(SpiceReplay *replay,
uint32_t flags)
}
qxl = (QXLImage*)replay_malloc0(replay, sizeof(QXLImage));
+elem = replay->allocated;
replay_fscanf(replay, "descriptor.id %"PRIu64"\n", &qxl->descriptor.id);
replay_fscanf(replay, "descriptor.type %d\n", &temp); qxl->descriptor.type
= temp;
replay_fscanf(replay, "descriptor.flags %d\n", &temp);
qxl->descriptor.flags = temp;
@@ -485,8 +487,9 @@ static QXLImage *red_replay_image(SpiceReplay *replay,
uint32_t flags)
if (replay->error) {
return NULL;
}
-qxl = realloc(qxl, sizeof(QXLImageDescriptor) + sizeof(QXLQUICData) +
- qxl->quic.data_size);
+qxl = spice_realloc(qxl, sizeof(QXLImageDescriptor) +
sizeof(QXLQUICData) +
+qxl->quic.data_size);
+elem->data = qxl;
size = red_replay_data_chunks(replay, "quic.data",
(uint8_t**)&qxl->quic.data, 0);
spice_assert(size == qxl->quic.data_size);
break;
--
2.7.4
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH v2 4/8] replay: Detect errors from red_replay_data_chunks
Change the return to ssize_t to be able to distinguish from
empty buffer to error.
Check result returned and avoid continuing potentially
deferencing NULL pointers.
Signed-off-by: Frediano Ziglio
---
server/red-replay-qxl.c | 37 ++---
1 file changed, 26 insertions(+), 11 deletions(-)
diff --git a/server/red-replay-qxl.c b/server/red-replay-qxl.c
index dbaa995..28da13d 100644
--- a/server/red-replay-qxl.c
+++ b/server/red-replay-qxl.c
@@ -279,8 +279,8 @@ static replay_t read_binary(SpiceReplay *replay, const char
*prefix, size_t *siz
return replay_fscanf(replay, "\n");
}
-static size_t red_replay_data_chunks(SpiceReplay *replay, const char *prefix,
- uint8_t **mem, size_t base_size)
+static ssize_t red_replay_data_chunks(SpiceReplay *replay, const char *prefix,
+ uint8_t **mem, size_t base_size)
{
size_t data_size;
int count_chunks;
@@ -288,12 +288,15 @@ static size_t red_replay_data_chunks(SpiceReplay *replay,
const char *prefix,
QXLDataChunk *cur, *next;
replay_fscanf(replay, "data_chunks %d %zu\n", &count_chunks, &data_size);
+if (replay->error) {
+return -1;
+}
if (base_size == 0) {
base_size = sizeof(QXLDataChunk);
}
if (read_binary(replay, prefix, &next_data_size, mem, base_size) ==
REPLAY_ERROR) {
-return 0;
+return -1;
}
cur = (QXLDataChunk*)(*mem + base_size - sizeof(QXLDataChunk));
cur->data_size = next_data_size;
@@ -302,7 +305,7 @@ static size_t red_replay_data_chunks(SpiceReplay *replay,
const char *prefix,
while (count_chunks-- > 0) {
if (read_binary(replay, prefix, &next_data_size,
(uint8_t**)&cur->next_chunk,
sizeof(QXLDataChunk)) == REPLAY_ERROR) {
-return 0;
+return -1;
}
data_size += next_data_size;
next = QXLPHYSICAL_TO_PTR(cur->next_chunk);
@@ -355,9 +358,12 @@ static void red_replay_rect_ptr(SpiceReplay *replay, const
char *prefix, QXLRect
static QXLPath *red_replay_path(SpiceReplay *replay)
{
QXLPath *qxl = NULL;
-size_t data_size;
+ssize_t data_size;
data_size = red_replay_data_chunks(replay, "path", (uint8_t**)&qxl,
sizeof(QXLPath));
+if (data_size < 0) {
+return NULL;
+}
qxl->data_size = data_size;
return qxl;
}
@@ -378,7 +384,9 @@ static QXLClipRects *red_replay_clip_rects(SpiceReplay
*replay)
if (replay->error) {
return NULL;
}
-red_replay_data_chunks(replay, "clip_rects", (uint8_t**)&qxl,
sizeof(QXLClipRects));
+if (red_replay_data_chunks(replay, "clip_rects", (uint8_t**)&qxl,
sizeof(QXLClipRects)) < 0) {
+return NULL;
+}
qxl->num_rects = num_rects;
return qxl;
}
@@ -399,7 +407,8 @@ static uint8_t *red_replay_image_data_flat(SpiceReplay
*replay, size_t *size)
static QXLImage *red_replay_image(SpiceReplay *replay, uint32_t flags)
{
QXLImage* qxl = NULL;
-size_t bitmap_size, size;
+size_t bitmap_size;
+ssize_t size;
uint8_t qxl_flags;
int temp;
int has_palette;
@@ -714,13 +723,16 @@ static QXLString *red_replay_string(SpiceReplay *replay)
uint32_t data_size;
uint16_t length;
uint16_t flags;
-size_t chunk_size;
+ssize_t chunk_size;
QXLString *qxl = NULL;
replay_fscanf(replay, "data_size %d\n", &data_size);
replay_fscanf(replay, "length %d\n", &temp); length = temp;
replay_fscanf(replay, "flags %d\n", &temp); flags = temp;
chunk_size = red_replay_data_chunks(replay, "string", (uint8_t**)&qxl,
sizeof(QXLString));
+if (chunk_size < 0) {
+return NULL;
+}
qxl->data_size = data_size;
qxl->length = length;
qxl->flags = flags;
@@ -1143,6 +1155,7 @@ static QXLCursor *red_replay_cursor(SpiceReplay *replay)
{
int temp;
QXLCursor cursor, *qxl = NULL;
+ssize_t data_size;
replay_fscanf(replay, "header.unique %"SCNu64"\n", &cursor.header.unique);
replay_fscanf(replay, "header.type %d\n", &temp);
@@ -1160,10 +1173,12 @@ static QXLCursor *red_replay_cursor(SpiceReplay *replay)
if (replay->error) {
return NULL;
}
-cursor.data_size = temp;
-cursor.data_size = red_replay_data_chunks(replay, "cursor",
(uint8_t**)&qxl, sizeof(QXLCursor));
+data_size = red_replay_data_chunks(replay, "cursor", (uint8_t**)&qxl,
sizeof(QXLCursor));
+if (data_size < 0) {
+return NULL;
+}
qxl->header = cursor.header;
-qxl->data_size = cursor.data_size;
+qxl->data_size = data_size;
return qxl;
}
--
2.7.4
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH v2 2/8] replay: Record allocations in a GList to handle errors
Allocations are kept into a GList to be able to free in case some
errors happened.
Signed-off-by: Frediano Ziglio
---
server/red-replay-qxl.c | 68 -
1 file changed, 56 insertions(+), 12 deletions(-)
diff --git a/server/red-replay-qxl.c b/server/red-replay-qxl.c
index 7170292..518d5bc 100644
--- a/server/red-replay-qxl.c
+++ b/server/red-replay-qxl.c
@@ -50,6 +50,8 @@ struct SpiceReplay {
GArray *id_free; // free list
int nsurfaces;
+GList *allocated;
+
pthread_mutex_t mutex;
pthread_cond_t cond;
};
@@ -88,6 +90,20 @@ static replay_t replay_fscanf(SpiceReplay *replay, const
char *fmt, ...)
return replay->error ? REPLAY_ERROR : REPLAY_OK;
}
+static inline void *replay_malloc(SpiceReplay *replay, size_t size)
+{
+void *mem = spice_malloc(size);
+replay->allocated = g_list_prepend(replay->allocated, mem);
+return mem;
+}
+
+static inline void *replay_malloc0(SpiceReplay *replay, size_t size)
+{
+void *mem = replay_malloc(replay, size);
+memset(mem, 0, size);
+return mem;
+}
+
static uint32_t replay_id_get(SpiceReplay *replay, uint32_t id)
{
uint32_t newid = 0;
@@ -199,7 +215,7 @@ static replay_t read_binary(SpiceReplay *replay, const char
*prefix, size_t *siz
return REPLAY_ERROR;
if (*buf == NULL) {
-*buf = spice_malloc(*size + base_size);
+*buf = replay_malloc(replay, *size + base_size);
}
#if 0
{
@@ -211,9 +227,11 @@ static replay_t read_binary(SpiceReplay *replay, const
char *prefix, size_t *siz
spice_return_val_if_fail(with_zlib != -1, REPLAY_ERROR);
if (with_zlib) {
int ret;
+GList *elem;
replay_fscanf(replay, "%d:", &zlib_size);
-zlib_buffer = spice_malloc(zlib_size);
+zlib_buffer = replay_malloc(replay, zlib_size);
+elem = replay->allocated;
replay_fread(replay, zlib_buffer, zlib_size);
strm.zalloc = Z_NULL;
strm.zfree = Z_NULL;
@@ -241,6 +259,7 @@ static replay_t read_binary(SpiceReplay *replay, const char
*prefix, size_t *siz
}
}
(void)inflateEnd(&strm);
+replay->allocated = g_list_remove_link(replay->allocated, elem);
free(zlib_buffer); // TODO - avoid repeat alloc/dealloc by keeping last
} else {
replay_fread(replay, *buf + base_size, *size);
@@ -377,7 +396,7 @@ static QXLImage *red_replay_image(SpiceReplay *replay,
uint32_t flags)
return NULL;
}
-qxl = spice_new0(QXLImage, 1);
+qxl = (QXLImage*)replay_malloc0(replay, sizeof(QXLImage));
replay_fscanf(replay, "descriptor.id %"PRIu64"\n", &qxl->descriptor.id);
replay_fscanf(replay, "descriptor.type %d\n", &temp); qxl->descriptor.type
= temp;
replay_fscanf(replay, "descriptor.flags %d\n", &temp);
qxl->descriptor.flags = temp;
@@ -398,7 +417,7 @@ static QXLImage *red_replay_image(SpiceReplay *replay,
uint32_t flags)
int i, num_ents;
replay_fscanf(replay, "qp.num_ents %d\n", &num_ents);
-qp = spice_malloc(sizeof(QXLPalette) + num_ents *
sizeof(qp->ents[0]));
+qp = replay_malloc(replay, sizeof(QXLPalette) + num_ents *
sizeof(qp->ents[0]));
qp->num_ents = num_ents;
qxl->bitmap.palette = QXLPHYSICAL_FROM_PTR(qp);
replay_fscanf(replay, "unique %"PRIu64"\n", &qp->unique);
@@ -788,7 +807,7 @@ static void red_replay_composite_free(SpiceReplay *replay,
QXLComposite *qxl, ui
static QXLDrawable *red_replay_native_drawable(SpiceReplay *replay, uint32_t
flags)
{
-QXLDrawable *qxl = spice_malloc0(sizeof(QXLDrawable)); // TODO - this is
too large usually
+QXLDrawable *qxl = replay_malloc0(replay, sizeof(QXLDrawable)); // TODO -
this is too large usually
int i;
int temp;
@@ -919,7 +938,7 @@ static void red_replay_native_drawable_free(SpiceReplay
*replay, QXLDrawable *qx
static QXLCompatDrawable *red_replay_compat_drawable(SpiceReplay *replay,
uint32_t flags)
{
int temp;
-QXLCompatDrawable *qxl = spice_malloc0(sizeof(QXLCompatDrawable)); // TODO
- too large usually
+QXLCompatDrawable *qxl = replay_malloc0(replay,
sizeof(QXLCompatDrawable)); // TODO - too large usually
red_replay_rect_ptr(replay, "bbox", &qxl->bbox);
red_replay_clip_ptr(replay, &qxl->clip);
@@ -994,7 +1013,7 @@ static QXLPHYSICAL red_replay_drawable(SpiceReplay
*replay, uint32_t flags)
static QXLUpdateCmd *red_replay_update_cmd(SpiceReplay *replay)
{
-QXLUpdateCmd *qxl = spice_malloc0(sizeof(QXLUpdateCmd));
+QXLUpdateCmd *qxl = replay_malloc0(replay, sizeof(QXLUpdateCmd));
replay_fscanf(replay, "update\n");
red_replay_rect_ptr(replay, "area", &qxl->area);
@@ -1019,7 +1038,7 @@ static QXLSurfaceCmd *red_replay_surface_cmd(SpiceReplay
*replay)
size_t size;
size_t read_size;
int temp;
-QXLSurfaceCmd *qxl = spice_malloc0(sizeof(QXLSurfaceCmd));
+QXLSurfaceCm
[Spice-devel] [PATCH v2 3/8] replay: Handle errors reading strings from record file
To check fscanf read all needed information a dummy "%n" is appended
to any string and the value stored there is tested. This as scanf family
could return a valid value but not entirely process the string so
adding a "%n" and checking this was processed make sure all expected
string is found.
The code to check for a specific string is now a bit more complicated
as replay_fscanf use a macro which append a constant string.
The "error" field is used to mark any error happened, so in most cases
there is no explicit check beside when this could cause a problem
(for instance results of replay_fscanf used which would result in
uninitialised variable usage).
Signed-off-by: Frediano Ziglio
---
server/red-replay-qxl.c | 91 +++--
1 file changed, 80 insertions(+), 11 deletions(-)
diff --git a/server/red-replay-qxl.c b/server/red-replay-qxl.c
index 518d5bc..dbaa995 100644
--- a/server/red-replay-qxl.c
+++ b/server/red-replay-qxl.c
@@ -49,6 +49,7 @@ struct SpiceReplay {
GArray *id_map_inv; // replay id -> record id
GArray *id_free; // free list
int nsurfaces;
+int end_pos;
GList *allocated;
@@ -69,11 +70,13 @@ static int replay_fread(SpiceReplay *replay, uint8_t *buf,
size_t size)
}
__attribute__((format(scanf, 2, 3)))
-static replay_t replay_fscanf(SpiceReplay *replay, const char *fmt, ...)
+static replay_t replay_fscanf_check(SpiceReplay *replay, const char *fmt, ...)
{
va_list ap;
int ret;
+replay->end_pos = -1;
+
if (replay->error) {
return REPLAY_ERROR;
}
@@ -84,11 +87,13 @@ static replay_t replay_fscanf(SpiceReplay *replay, const
char *fmt, ...)
va_start(ap, fmt);
ret = vfscanf(replay->fd, fmt, ap);
va_end(ap);
-if (ret == EOF) {
+if (ret == EOF || replay->end_pos < 0) {
replay->error = TRUE;
}
return replay->error ? REPLAY_ERROR : REPLAY_OK;
}
+#define replay_fscanf(r, fmt, ...) \
+replay_fscanf_check(r, fmt "%n", ## __VA_ARGS__, &r->end_pos)
static inline void *replay_malloc(SpiceReplay *replay, size_t size)
{
@@ -210,9 +215,11 @@ static replay_t read_binary(SpiceReplay *replay, const
char *prefix, size_t *siz
uint8_t *zlib_buffer;
z_stream strm;
-snprintf(template, sizeof(template), "binary %%d %s %%ld:", prefix);
-if (replay_fscanf(replay, template, &with_zlib, size) == REPLAY_ERROR)
+snprintf(template, sizeof(template), "binary %%d %s %%ld:%%n", prefix);
+replay_fscanf_check(replay, template, &with_zlib, size, &replay->end_pos);
+if (replay->error) {
return REPLAY_ERROR;
+}
if (*buf == NULL) {
*buf = replay_malloc(replay, *size + base_size);
@@ -224,15 +231,19 @@ static replay_t read_binary(SpiceReplay *replay, const
char *prefix, size_t *siz
hexdump(*buf + base_size, *size);
}
#else
-spice_return_val_if_fail(with_zlib != -1, REPLAY_ERROR);
if (with_zlib) {
int ret;
GList *elem;
replay_fscanf(replay, "%d:", &zlib_size);
+if (replay->error) {
+return REPLAY_ERROR;
+}
zlib_buffer = replay_malloc(replay, zlib_size);
elem = replay->allocated;
-replay_fread(replay, zlib_buffer, zlib_size);
+if (replay_fread(replay, zlib_buffer, zlib_size) != zlib_size) {
+return REPLAY_ERROR;
+}
strm.zalloc = Z_NULL;
strm.zfree = Z_NULL;
strm.opaque = Z_NULL;
@@ -265,8 +276,7 @@ static replay_t read_binary(SpiceReplay *replay, const char
*prefix, size_t *siz
replay_fread(replay, *buf + base_size, *size);
}
#endif
-replay_fscanf(replay, "\n");
-return REPLAY_OK;
+return replay_fscanf(replay, "\n");
}
static size_t red_replay_data_chunks(SpiceReplay *replay, const char *prefix,
@@ -337,8 +347,9 @@ static void red_replay_rect_ptr(SpiceReplay *replay, const
char *prefix, QXLRect
{
char template[1024];
-snprintf(template, sizeof(template), "rect %s %%d %%d %%d %%d\n", prefix);
-replay_fscanf(replay, template, &qxl->top, &qxl->left, &qxl->bottom,
&qxl->right);
+snprintf(template, sizeof(template), "rect %s %%d %%d %%d %%d\n%%n",
prefix);
+replay_fscanf_check(replay, template, &qxl->top, &qxl->left, &qxl->bottom,
&qxl->right,
+&replay->end_pos);
}
static QXLPath *red_replay_path(SpiceReplay *replay)
@@ -364,6 +375,9 @@ static QXLClipRects *red_replay_clip_rects(SpiceReplay
*replay)
int num_rects;
replay_fscanf(replay, "num_rects %d\n", &num_rects);
+if (replay->error) {
+return NULL;
+}
red_replay_data_chunks(replay, "clip_rects", (uint8_t**)&qxl,
sizeof(QXLClipRects));
qxl->num_rects = num_rects;
return qxl;
@@ -392,6 +406,9 @@ static QXLImage *red_replay_image(SpiceReplay *replay,
uint32_t flags)
int has_image;
replay_fscanf(replay, "image %d\n", &has_image);
+if (replay->error) {
+return NULL
[Spice-devel] [PATCH v2 0/8] Improve error handling in red-replay-qxl.c
Detect error in files and handle more gracefully. This version: - split the previous one; - update it; - fix different observations from Christophe. The last patch still contains the "old style". Could be true that if chaining could be not that readable but I'm not a big fun of too many goto either. Frediano Ziglio (8): replay: Rename eof to error replay: Record allocations in a GList to handle errors replay: Handle errors reading strings from record file replay: Detect errors from red_replay_data_chunks replay: Remove useless check replay: Assure read_binary receives a NULL pointer replay: Update pointer in allocated list replay: Propagate error correctly in replay_fread server/red-replay-qxl.c | 249 1 file changed, 188 insertions(+), 61 deletions(-) -- 2.7.4 ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH v2 1/8] replay: Rename eof to error
The eof variable and enumeration will be used for all errors
so avoid confusion.
Signed-off-by: Frediano Ziglio
---
server/red-replay-qxl.c | 38 +++---
1 file changed, 19 insertions(+), 19 deletions(-)
diff --git a/server/red-replay-qxl.c b/server/red-replay-qxl.c
index b17c38b..7170292 100644
--- a/server/red-replay-qxl.c
+++ b/server/red-replay-qxl.c
@@ -36,12 +36,12 @@
typedef enum {
REPLAY_OK = 0,
-REPLAY_EOF,
+REPLAY_ERROR,
} replay_t;
struct SpiceReplay {
FILE *fd;
-int eof;
+gboolean error;
int counter;
bool created_primary;
@@ -56,11 +56,11 @@ struct SpiceReplay {
static int replay_fread(SpiceReplay *replay, uint8_t *buf, size_t size)
{
-if (replay->eof) {
+if (replay->error) {
return 0;
}
if (feof(replay->fd)) {
-replay->eof = 1;
+replay->error = TRUE;
return 0;
}
return fread(buf, size, 1, replay->fd);
@@ -72,20 +72,20 @@ static replay_t replay_fscanf(SpiceReplay *replay, const
char *fmt, ...)
va_list ap;
int ret;
-if (replay->eof) {
-return REPLAY_EOF;
+if (replay->error) {
+return REPLAY_ERROR;
}
if (feof(replay->fd)) {
-replay->eof = 1;
-return REPLAY_EOF;
+replay->error = TRUE;
+return REPLAY_ERROR;
}
va_start(ap, fmt);
ret = vfscanf(replay->fd, fmt, ap);
va_end(ap);
if (ret == EOF) {
-replay->eof = 1;
+replay->error = TRUE;
}
-return replay->eof ? REPLAY_EOF : REPLAY_OK;
+return replay->error ? REPLAY_ERROR : REPLAY_OK;
}
static uint32_t replay_id_get(SpiceReplay *replay, uint32_t id)
@@ -195,8 +195,8 @@ static replay_t read_binary(SpiceReplay *replay, const char
*prefix, size_t *siz
z_stream strm;
snprintf(template, sizeof(template), "binary %%d %s %%ld:", prefix);
-if (replay_fscanf(replay, template, &with_zlib, size) == REPLAY_EOF)
-return REPLAY_EOF;
+if (replay_fscanf(replay, template, &with_zlib, size) == REPLAY_ERROR)
+return REPLAY_ERROR;
if (*buf == NULL) {
*buf = spice_malloc(*size + base_size);
@@ -208,7 +208,7 @@ static replay_t read_binary(SpiceReplay *replay, const char
*prefix, size_t *siz
hexdump(*buf + base_size, *size);
}
#else
-spice_return_val_if_fail(with_zlib != -1, REPLAY_EOF);
+spice_return_val_if_fail(with_zlib != -1, REPLAY_ERROR);
if (with_zlib) {
int ret;
@@ -234,7 +234,7 @@ static replay_t read_binary(SpiceReplay *replay, const char
*prefix, size_t *siz
* it seems it may kill the red_worker thread (so a chunk is
* left hanging and the rest of the message is never written).
* Let it pass */
-return REPLAY_EOF;
+return REPLAY_ERROR;
}
if (ret != Z_OK) {
spice_warn_if_reached();
@@ -263,7 +263,7 @@ static size_t red_replay_data_chunks(SpiceReplay *replay,
const char *prefix,
base_size = sizeof(QXLDataChunk);
}
-if (read_binary(replay, prefix, &next_data_size, mem, base_size) ==
REPLAY_EOF) {
+if (read_binary(replay, prefix, &next_data_size, mem, base_size) ==
REPLAY_ERROR) {
return 0;
}
cur = (QXLDataChunk*)(*mem + base_size - sizeof(QXLDataChunk));
@@ -272,7 +272,7 @@ static size_t red_replay_data_chunks(SpiceReplay *replay,
const char *prefix,
cur->next_chunk = cur->prev_chunk = 0;
while (count_chunks-- > 0) {
if (read_binary(replay, prefix, &next_data_size,
(uint8_t**)&cur->next_chunk,
-sizeof(QXLDataChunk)) == REPLAY_EOF) {
+sizeof(QXLDataChunk)) == REPLAY_ERROR) {
return 0;
}
data_size += next_data_size;
@@ -981,7 +981,7 @@ static QXLCompatDrawable
*red_replay_compat_drawable(SpiceReplay *replay, uint32
static QXLPHYSICAL red_replay_drawable(SpiceReplay *replay, uint32_t flags)
{
-if (replay->eof) {
+if (replay->error) {
return 0;
}
replay_fscanf(replay, "drawable\n");
@@ -1194,7 +1194,7 @@ SPICE_GNUC_VISIBLE QXLCommandExt*
spice_replay_next_cmd(SpiceReplay *replay,
while (what != 0) {
replay_fscanf(replay, "event %d %d %d %"PRIu64"\n", &counter,
&what, &type, ×tamp);
-if (replay->eof) {
+if (replay->error) {
return NULL;
}
if (what == 1) {
@@ -1296,7 +1296,7 @@ SpiceReplay *spice_replay_new(FILE *file, int nsurfaces)
replay = spice_malloc0(sizeof(SpiceReplay));
-replay->eof = 0;
+replay->error = FALSE;
replay->fd = file;
replay->created_primary = FALSE;
pthread_mutex_init(&replay->mutex, NULL);
--
2.7.4
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [vdagent-linux v2 0/5] Reorganize vdagent Makefile and directory
Series looks good to me.
Jonathon
On Thu, 2016-09-15 at 18:51 +0200, Victor Toso wrote:
> Hi, second version.
>
> In the first patch, I've added the $(NULL), diff is here:
> http://paste.fedoraproject.org/428487/73955986/
>
> We now have a 4th patch that we might want to merge with the 3rd
> patch... Having
> an extra patch helps with the review, I guess.
>
> The 5th patch is a small typo that I've just found.
>
> Keeping the acks from Jonathon and Frediano in the first three
> patches.
>
> Cheers,
> toso
>
> Marc-André Lureau (2):
> build-sys: get rid of noinst_HEADERS
> build-sys: move user/system to respective dir
>
> Victor Toso (3):
> build-sys: reformat Makefile.am
> build-sys: remove prefix from filenames
> buildsys: statis typo in configure option
>
> .gitignore | 6 ++
> Makefile.am| 109
> ++---
> configure.ac | 2 +-
> src/{vdagent-audio.c => vdagent/audio.c} | 2 +-
> src/{vdagent-audio.h => vdagent/audio.h} | 4 +-
> src/{vdagent-file-xfers.c => vdagent/file-xfers.c} | 4 +-
> src/{vdagent-file-xfers.h => vdagent/file-xfers.h} | 0
> src/{ => vdagent}/vdagent.c| 6 +-
> src/{vdagent-x11-priv.h => vdagent/x11-priv.h} | 0
> src/{vdagent-x11-randr.c => vdagent/x11-randr.c} | 4 +-
> src/{vdagent-x11.c => vdagent/x11.c} | 4 +-
> src/{vdagent-x11.h => vdagent/x11.h} | 4 +-
> src/{ => vdagentd}/console-kit.c | 0
> src/{ => vdagentd}/dummy-session-info.c| 0
> src/{ => vdagentd}/session-info.h | 0
> src/{ => vdagentd}/systemd-login.c | 0
> src/{vdagentd-uinput.c => vdagentd/uinput.c} | 4 +-
> src/{vdagentd-uinput.h => vdagentd/uinput.h} | 0
> src/{ => vdagentd}/vdagentd.c | 6 +-
> .../virtio-port.c} | 2 +-
> .../virtio-port.h} | 4 +-
> src/{vdagentd-xorg-conf.c => vdagentd/xorg-conf.c} | 2 +-
> src/{vdagentd-xorg-conf.h => vdagentd/xorg-conf.h} | 0
> 23 files changed, 104 insertions(+), 59 deletions(-)
> rename src/{vdagent-audio.c => vdagent/audio.c} (99%)
> rename src/{vdagent-audio.h => vdagent/audio.h} (91%)
> rename src/{vdagent-file-xfers.c => vdagent/file-xfers.c} (99%)
> rename src/{vdagent-file-xfers.h => vdagent/file-xfers.h} (100%)
> rename src/{ => vdagent}/vdagent.c (99%)
> rename src/{vdagent-x11-priv.h => vdagent/x11-priv.h} (100%)
> rename src/{vdagent-x11-randr.c => vdagent/x11-randr.c} (99%)
> rename src/{vdagent-x11.c => vdagent/x11.c} (99%)
> rename src/{vdagent-x11.h => vdagent/x11.h} (95%)
> rename src/{ => vdagentd}/console-kit.c (100%)
> rename src/{ => vdagentd}/dummy-session-info.c (100%)
> rename src/{ => vdagentd}/session-info.h (100%)
> rename src/{ => vdagentd}/systemd-login.c (100%)
> rename src/{vdagentd-uinput.c => vdagentd/uinput.c} (99%)
> rename src/{vdagentd-uinput.h => vdagentd/uinput.h} (100%)
> rename src/{ => vdagentd}/vdagentd.c (99%)
> rename src/{vdagent-virtio-port.c => vdagentd/virtio-port.c} (99%)
> rename src/{vdagent-virtio-port.h => vdagentd/virtio-port.h} (97%)
> rename src/{vdagentd-xorg-conf.c => vdagentd/xorg-conf.c} (99%)
> rename src/{vdagentd-xorg-conf.h => vdagentd/xorg-conf.h} (100%)
>
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH v2 2/2] Add DisplayChannelPrivate struct
On Thu, 2016-09-15 at 11:22 -0500, Jonathon Jongsma wrote:
> Move all of the DisplayChannel data memembers into a private struct
> to
> encapsulate things better. This necessitated a few new 'public'
> methods
> and a small bit of refactoring to avoid poking into DisplayChannel
> internals from too many places. The DisplayChannel and the
> DisplayChannelClient are still far too intertwined to completely
> avoid
> accessing private data, so at the moment the private struct is
> defined
> in display-channel.h and the DisplayChannelClient implementation
> still accesses it sometimes.
> ---
> server/dcc-send.c| 16 +--
> server/dcc.c | 33 ++---
> server/display-channel.c | 314 -
> --
> server/display-channel.h | 13 +-
> server/red-worker.c | 6 +
> server/stream.c | 53
> 6 files changed, 227 insertions(+), 208 deletions(-)
>
> diff --git a/server/dcc-send.c b/server/dcc-send.c
> index 0afa25c..0635afa 100644
> --- a/server/dcc-send.c
> +++ b/server/dcc-send.c
> @@ -96,7 +96,7 @@ static int
> is_surface_area_lossy(DisplayChannelClient *dcc, uint32_t surface_id,
>
> spice_return_val_if_fail(display_channel_validate_surface(displa
> y, surface_id), FALSE);
>
> -surface = &display->surfaces[surface_id];
> +surface = &display->priv->surfaces[surface_id];
> surface_lossy_region = &dcc->priv-
> >surface_client_lossy_region[surface_id];
>
> if (!area) {
> @@ -208,13 +208,13 @@ static void
> red_display_add_image_to_pixmap_cache(RedChannelClient *rcc,
> io_image->descriptor.flags |=
> SPICE_IMAGE_FLAGS_CACHE_ME;
> dcc->priv->send_data.pixmap_cache_items[dcc->priv-
> >send_data.num_pixmap_cache_items++] =
>
> image->descriptor.id;
> -stat_inc_counter(reds, display_channel-
> >add_to_cache_counter, 1);
> +stat_inc_counter(reds, display_channel->priv-
> >add_to_cache_counter, 1);
> }
> }
> }
>
> if (!(io_image->descriptor.flags & SPICE_IMAGE_FLAGS_CACHE_ME))
> {
> -stat_inc_counter(reds, display_channel->non_cache_counter,
> 1);
> +stat_inc_counter(reds, display_channel->priv-
> >non_cache_counter, 1);
> }
> }
>
> @@ -385,7 +385,7 @@ static FillBitsType
> fill_bits(DisplayChannelClient *dcc, SpiceMarshaller *m,
> dcc->priv->send_data.pixmap_cache_items[dcc->priv-
> >send_data.num_pixmap_cache_items++] =
> image.descriptor.id;
> if (can_lossy || !lossy_cache_item) {
> -if (!display->enable_jpeg || lossy_cache_item) {
> +if (!display->priv->enable_jpeg || lossy_cache_item)
> {
> image.descriptor.type =
> SPICE_IMAGE_TYPE_FROM_CACHE;
> } else {
> // making sure, in multiple monitor scenario,
> that lossy items that
> @@ -397,7 +397,7 @@ static FillBitsType
> fill_bits(DisplayChannelClient *dcc, SpiceMarshaller *m,
> &bitmap_palette_out,
> &lzplt_palette_out);
> spice_assert(bitmap_palette_out == NULL);
> spice_assert(lzplt_palette_out == NULL);
> -stat_inc_counter(reds, display->cache_hits_counter,
> 1);
> +stat_inc_counter(reds, display->priv-
> >cache_hits_counter, 1);
> pthread_mutex_unlock(&dcc->priv->pixmap_cache-
> >lock);
> return FILL_BITS_TYPE_CACHE;
> } else {
> @@ -420,7 +420,7 @@ static FillBitsType
> fill_bits(DisplayChannelClient *dcc, SpiceMarshaller *m,
> return FILL_BITS_TYPE_SURFACE;
> }
>
> -surface = &display->surfaces[surface_id];
> +surface = &display->priv->surfaces[surface_id];
> image.descriptor.type = SPICE_IMAGE_TYPE_SURFACE;
> image.descriptor.flags = 0;
> image.descriptor.width = surface->context.width;
> @@ -1862,7 +1862,7 @@ static void
> display_channel_marshall_migrate_data(RedChannelClient *rcc,
> spice_marshaller_add(base_marshaller,
> (uint8_t *)&display_data,
> sizeof(display_data) - sizeof(uint32_t));
> display_channel_marshall_migrate_data_surfaces(dcc,
> base_marshaller,
> - display_channel-
> >enable_jpeg);
> + display_channel-
> >priv->enable_jpeg);
> }
>
> static void display_channel_marshall_pixmap_sync(RedChannelClient
> *rcc,
> @@ -2148,7 +2148,7 @@ static void
> marshall_qxl_drawable(RedChannelClient *rcc,
> if (item->stream && red_marshall_stream_data(rcc, m, item)) {
> return;
> }
> -if (display->enable_jpeg)
> +if (display->priv->enable_jpeg)
> marshall_lossy_qxl_drawable(rcc, m, dpi);
> else
> marshall_lossless_qxl_d
[Spice-devel] [PATCH v2] Change RedCharDevice::write_queue to GQueue
Change a couple more Rings to GQueue
---
Changes in v2:
- use GQueue instead of GQueue*
server/char-device.c | 73 +---
server/char-device.h | 1 -
2 files changed, 29 insertions(+), 45 deletions(-)
diff --git a/server/char-device.c b/server/char-device.c
index 957fb26..a03867f 100644
--- a/server/char-device.c
+++ b/server/char-device.c
@@ -50,8 +50,8 @@ struct RedCharDevicePrivate {
int active; /* has read/write been performed since the device was started
*/
int wait_for_migrate_data;
-Ring write_queue;
-Ring write_bufs_pool;
+GQueue write_queue;
+GQueue write_bufs_pool;
uint64_t cur_pool_size;
RedCharDeviceWriteBuffer *cur_write_buf;
uint8_t *cur_write_buf_pos;
@@ -150,16 +150,11 @@ static void
red_char_device_write_buffer_free(RedCharDeviceWriteBuffer *buf)
free(buf);
}
-static void write_buffers_queue_free(Ring *write_queue)
+static void write_buffers_queue_free(GQueue *write_queue)
{
-while (!ring_is_empty(write_queue)) {
-RingItem *item = ring_get_tail(write_queue);
-RedCharDeviceWriteBuffer *buf;
-
-ring_remove(item);
-buf = SPICE_CONTAINEROF(item, RedCharDeviceWriteBuffer, link);
+RedCharDeviceWriteBuffer *buf;
+while ((buf = g_queue_pop_tail(write_queue)))
red_char_device_write_buffer_free(buf);
-}
}
static void red_char_device_write_buffer_pool_add(RedCharDevice *dev,
@@ -171,7 +166,7 @@ static void
red_char_device_write_buffer_pool_add(RedCharDevice *dev,
buf->origin = WRITE_BUFFER_ORIGIN_NONE;
buf->client = NULL;
dev->priv->cur_pool_size += buf->buf_size;
-ring_add(&dev->priv->write_bufs_pool, &buf->link);
+g_queue_push_head(&dev->priv->write_bufs_pool, buf);
return;
}
@@ -182,7 +177,7 @@ static void
red_char_device_write_buffer_pool_add(RedCharDevice *dev,
static void red_char_device_client_free(RedCharDevice *dev,
RedCharDeviceClient *dev_client)
{
-RingItem *item, *next;
+GList *l, *next;
if (dev_client->wait_for_tokens_timer) {
reds_core_timer_remove(dev->priv->reds,
dev_client->wait_for_tokens_timer);
@@ -192,16 +187,18 @@ static void red_char_device_client_free(RedCharDevice
*dev,
g_queue_free_full(dev_client->send_queue,
(GDestroyNotify)red_pipe_item_unref);
/* remove write buffers that are associated with the client */
-spice_debug("write_queue_is_empty %d",
ring_is_empty(&dev->priv->write_queue) && !dev->priv->cur_write_buf);
-RING_FOREACH_SAFE(item, next, &dev->priv->write_queue) {
-RedCharDeviceWriteBuffer *write_buf;
+spice_debug("write_queue_is_empty %d",
g_queue_is_empty(&dev->priv->write_queue) && !dev->priv->cur_write_buf);
+l = g_queue_peek_head_link(&dev->priv->write_queue);
+while (l) {
+RedCharDeviceWriteBuffer *write_buf = l->data;
+next = l->next;
-write_buf = SPICE_CONTAINEROF(item, RedCharDeviceWriteBuffer, link);
if (write_buf->origin == WRITE_BUFFER_ORIGIN_CLIENT &&
write_buf->client == dev_client->client) {
-ring_remove(item);
+g_queue_delete_link(&dev->priv->write_queue, l);
red_char_device_write_buffer_pool_add(dev, write_buf);
}
+l = next;
}
if (dev->priv->cur_write_buf && dev->priv->cur_write_buf->origin ==
WRITE_BUFFER_ORIGIN_CLIENT &&
@@ -483,13 +480,10 @@ static int red_char_device_write_to_device(RedCharDevice
*dev)
uint32_t write_len;
if (!dev->priv->cur_write_buf) {
-RingItem *item = ring_get_tail(&dev->priv->write_queue);
-if (!item) {
+dev->priv->cur_write_buf =
g_queue_pop_tail(&dev->priv->write_queue);
+if (!dev->priv->cur_write_buf)
break;
-}
-dev->priv->cur_write_buf = SPICE_CONTAINEROF(item,
RedCharDeviceWriteBuffer, link);
dev->priv->cur_write_buf_pos = dev->priv->cur_write_buf->buf;
-ring_remove(item);
}
write_len = dev->priv->cur_write_buf->buf +
dev->priv->cur_write_buf->buf_used -
@@ -519,7 +513,7 @@ static int red_char_device_write_to_device(RedCharDevice
*dev)
CHAR_DEVICE_WRITE_TO_TIMEOUT);
}
} else {
-spice_assert(ring_is_empty(&dev->priv->write_queue));
+spice_assert(g_queue_is_empty(&dev->priv->write_queue));
}
dev->priv->active = dev->priv->active || total;
}
@@ -542,16 +536,14 @@ static RedCharDeviceWriteBuffer
*__red_char_device_write_buffer_get(
RedCharDevice *dev, RedClient *client,
int size, int origin, int migrated_data_tokens)
{
-RingItem *item;
RedCharDeviceWriteBuffer *ret;
if (origin == WRITE_BUFFER_ORIGIN_SERVER && !dev->priv->num_self_tokens) {
return NULL;
}
-if ((item =
Re: [Spice-devel] [PATCH v2 8/9] Change RedCharDevice::write_queue to GQueue
On Thu, 2016-09-15 at 16:55 +0200, Pavel Grunt wrote: > Hey, > > On Wed, 2016-09-14 at 11:53 -0500, Jonathon Jongsma wrote: > > > > Change a couple more Rings to GQueue > > --- > > Changes in v2: > > - use GQueue, not GList > > > > server/char-device.c | 79 +-- > > - > > > > @@ -1119,8 +1104,8 @@ red_char_device_finalize(GObject *object) > > reds_core_timer_remove(self->priv->reds, self->priv- > > > > > > write_to_dev_timer); > > self->priv->write_to_dev_timer = NULL; > > } > > -write_buffers_queue_free(&self->priv->write_queue); > > -write_buffers_queue_free(&self->priv->write_bufs_pool); > > +write_buffers_queue_free(self->priv->write_queue); > > +write_buffers_queue_free(self->priv->write_bufs_pool); > > Did you consider using g_queue_free_full() ? > > Thanks, > Pavel > I did consider it, but that would free the GQueue structure as well as all of the elements in the queue. I don't want to do that here. We want a valid GQueue, but it should be empty. Jonathon ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH v2] Change RedCharDevicePrivate::clients to GList
More Ring cleanup. At the moment, we only support a single client, so
this is only a one-element list
---
Changes in v2:
- use safe loop in red_char_device_send_msg_to_clients()
server/char-device.c | 71 +---
1 file changed, 29 insertions(+), 42 deletions(-)
diff --git a/server/char-device.c b/server/char-device.c
index f826524..508d154 100644
--- a/server/char-device.c
+++ b/server/char-device.c
@@ -32,7 +32,6 @@
typedef struct RedCharDeviceClient RedCharDeviceClient;
struct RedCharDeviceClient {
-RingItem link;
RedCharDevice *dev;
RedClient *client;
int do_flow_control;
@@ -58,8 +57,7 @@ struct RedCharDevicePrivate {
SpiceTimer *write_to_dev_timer;
uint64_t num_self_tokens;
-Ring clients; /* list of RedCharDeviceClient */
-uint32_t num_clients;
+GList *clients; /* list of RedCharDeviceClient */
uint64_t client_tokens_interval; /* frequency of returning tokens to the
client */
SpiceCharDeviceInstance *sin;
@@ -207,8 +205,7 @@ static void red_char_device_client_free(RedCharDevice *dev,
dev->priv->cur_write_buf->client = NULL;
}
-dev->priv->num_clients--;
-ring_remove(&dev_client->link);
+dev->priv->clients = g_list_remove(dev->priv->clients, dev_client);
free(dev_client);
}
@@ -222,12 +219,11 @@ static void
red_char_device_handle_client_overflow(RedCharDeviceClient *dev_clie
static RedCharDeviceClient *red_char_device_client_find(RedCharDevice *dev,
RedClient *client)
{
-RingItem *item;
+GList *item;
-RING_FOREACH(item, &dev->priv->clients) {
-RedCharDeviceClient *dev_client;
+for (item = dev->priv->clients; item != NULL; item = item->next) {
+RedCharDeviceClient *dev_client = item->data;
-dev_client = SPICE_CONTAINEROF(item, RedCharDeviceClient, link);
if (dev_client->client == client) {
return dev_client;
}
@@ -253,13 +249,11 @@ static int
red_char_device_can_send_to_client(RedCharDeviceClient *dev_client)
static uint64_t red_char_device_max_send_tokens(RedCharDevice *dev)
{
-RingItem *item;
+GList *item;
uint64_t max = 0;
-RING_FOREACH(item, &dev->priv->clients) {
-RedCharDeviceClient *dev_client;
-
-dev_client = SPICE_CONTAINEROF(item, RedCharDeviceClient, link);
+for (item = dev->priv->clients; item != NULL; item = item->next) {
+RedCharDeviceClient *dev_client = item->data;
if (!dev_client->do_flow_control) {
max = ~0;
@@ -295,12 +289,13 @@ static void
red_char_device_add_msg_to_client_queue(RedCharDeviceClient *dev_cli
static void red_char_device_send_msg_to_clients(RedCharDevice *dev,
RedPipeItem *msg)
{
-RingItem *item, *next;
+GList *l;
-RING_FOREACH_SAFE(item, next, &dev->priv->clients) {
-RedCharDeviceClient *dev_client;
+l = dev->priv->clients;
+while (l) {
+GList *next = l->next;
+RedCharDeviceClient *dev_client = l->data;
-dev_client = SPICE_CONTAINEROF(item, RedCharDeviceClient, link);
if (red_char_device_can_send_to_client(dev_client)) {
dev_client->num_send_tokens--;
spice_assert(g_queue_is_empty(dev_client->send_queue));
@@ -310,6 +305,7 @@ static void
red_char_device_send_msg_to_clients(RedCharDevice *dev,
} else {
red_char_device_add_msg_to_client_queue(dev_client, msg);
}
+l = next;
}
}
@@ -338,7 +334,7 @@ static int red_char_device_read_from_device(RedCharDevice
*dev)
* Reading from the device only in case at least one of the clients have a
free token.
* All messages will be discarded if no client is attached to the device
*/
-while ((max_send_tokens || ring_is_empty(&dev->priv->clients)) &&
dev->priv->running) {
+while ((max_send_tokens || (dev->priv->clients == NULL)) &&
dev->priv->running) {
RedPipeItem *msg;
msg = red_char_device_read_one_msg_from_device(dev);
@@ -743,7 +739,7 @@ int red_char_device_client_add(RedCharDevice *dev,
spice_assert(dev);
spice_assert(client);
-if (wait_for_migrate_data && (dev->priv->num_clients > 0 ||
dev->priv->active)) {
+if (wait_for_migrate_data && (dev->priv->clients != NULL ||
dev->priv->active)) {
spice_warning("can't restore device %p from migration data. The device
"
"has already been active", dev);
return FALSE;
@@ -757,8 +753,7 @@ int red_char_device_client_add(RedCharDevice *dev,
num_client_tokens,
num_send_tokens);
dev_client->dev = dev;
-ring_add(&dev->priv->clients, &dev_client->link);
-dev->priv->num_clients++;
+dev->priv->clients = g_list_prepend(dev->priv->clients, dev_client)
Re: [Spice-devel] [PATCH v2 9/9] Change RedCharDevicePrivate::clients to GList
On Thu, 2016-09-15 at 11:16 -0400, Frediano Ziglio wrote:
> >
> >
> > More Ring cleanup. At the moment, we only support a single client,
> > so
> > this is only a one-element list
> > ---
> > Changes in v2:
> > - simplified red_char_device_finalize() to avoid using
> > g_list_last()
> >
> > server/char-device.c | 68
> >
> > 1 file changed, 26 insertions(+), 42 deletions(-)
> >
> > diff --git a/server/char-device.c b/server/char-device.c
> > index f826524..1b99aa9 100644
> > --- a/server/char-device.c
> > +++ b/server/char-device.c
> > @@ -32,7 +32,6 @@
> >
> > typedef struct RedCharDeviceClient RedCharDeviceClient;
> > struct RedCharDeviceClient {
> > -RingItem link;
> > RedCharDevice *dev;
> > RedClient *client;
> > int do_flow_control;
> > @@ -58,8 +57,7 @@ struct RedCharDevicePrivate {
> > SpiceTimer *write_to_dev_timer;
> > uint64_t num_self_tokens;
> >
> > -Ring clients; /* list of RedCharDeviceClient */
> > -uint32_t num_clients;
> > +GList *clients; /* list of RedCharDeviceClient */
> >
> > uint64_t client_tokens_interval; /* frequency of returning
> > tokens to the
> > client */
> > SpiceCharDeviceInstance *sin;
> > @@ -207,8 +205,7 @@ static void
> > red_char_device_client_free(RedCharDevice
> > *dev,
> > dev->priv->cur_write_buf->client = NULL;
> > }
> >
> > -dev->priv->num_clients--;
> > -ring_remove(&dev_client->link);
> > +dev->priv->clients = g_list_remove(dev->priv->clients,
> > dev_client);
> > free(dev_client);
> > }
> >
> > @@ -222,12 +219,11 @@ static void
> > red_char_device_handle_client_overflow(RedCharDeviceClient
> > *dev_clie
> > static RedCharDeviceClient
> > *red_char_device_client_find(RedCharDevice *dev,
> > RedClient
> > *client)
> > {
> > -RingItem *item;
> > +GList *item;
> >
> > -RING_FOREACH(item, &dev->priv->clients) {
> > -RedCharDeviceClient *dev_client;
> > +for (item = dev->priv->clients; item != NULL; item = item-
> > >next) {
> > +RedCharDeviceClient *dev_client = item->data;
> >
> > -dev_client = SPICE_CONTAINEROF(item, RedCharDeviceClient,
> > link);
> > if (dev_client->client == client) {
> > return dev_client;
> > }
> > @@ -253,13 +249,11 @@ static int
> > red_char_device_can_send_to_client(RedCharDeviceClient *dev_client)
> >
> > static uint64_t red_char_device_max_send_tokens(RedCharDevice
> > *dev)
> > {
> > -RingItem *item;
> > +GList *item;
> > uint64_t max = 0;
> >
> > -RING_FOREACH(item, &dev->priv->clients) {
> > -RedCharDeviceClient *dev_client;
> > -
> > -dev_client = SPICE_CONTAINEROF(item, RedCharDeviceClient,
> > link);
> > +for (item = dev->priv->clients; item != NULL; item = item-
> > >next) {
> > +RedCharDeviceClient *dev_client = item->data;
> >
> > if (!dev_client->do_flow_control) {
> > max = ~0;
> > @@ -295,12 +289,11 @@ static void
> > red_char_device_add_msg_to_client_queue(RedCharDeviceClient
> > *dev_cli
> > static void red_char_device_send_msg_to_clients(RedCharDevice
> > *dev,
> > RedPipeItem *msg)
> > {
> > -RingItem *item, *next;
> > +GList *l;
> >
> > -RING_FOREACH_SAFE(item, next, &dev->priv->clients) {
> > -RedCharDeviceClient *dev_client;
> > +for (l = dev->priv->clients; l != NULL; l = l->next) {
> > +RedCharDeviceClient *dev_client = l->data;
> >
> > -dev_client = SPICE_CONTAINEROF(item, RedCharDeviceClient,
> > link);
> > if (red_char_device_can_send_to_client(dev_client)) {
> > dev_client->num_send_tokens--;
> > spice_assert(g_queue_is_empty(dev_client-
> > >send_queue));
>
> The _SAFE loop here was necessary as there are combinations were
> the client is removed from the list during the loop.
> This will make l vanish before l->next is read.
> Even if there is only a client this can happen.
Hmm, I couldn't find a code path where this happened. But there is a
comment within the loop suggesting that it can happen, so it's better
to use the safer loop style. Sorry for changing it without enough
diligence.
jonathon
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [RFC PATCH 2/2] Start writing some documentation on protocol
On Thu, 2016-09-15 at 07:08 -0400, Frediano Ziglio wrote:
> >
> >
> > On Fri, 2016-09-09 at 10:44 +0100, Frediano Ziglio wrote:
> > >
> > > Signed-off-by: Frediano Ziglio
> > > ---
> > > docs/spice_protocol.txt | 48
> > >
> > > 1 file changed, 48 insertions(+)
> > >
> > > diff --git a/docs/spice_protocol.txt b/docs/spice_protocol.txt
> > > index b62da25..b0ba568 100644
> > > --- a/docs/spice_protocol.txt
> > > +++ b/docs/spice_protocol.txt
> > > @@ -1,2 +1,50 @@
> > > Spice protocol format file
> > > ==
> > > +
> > > +Copyright (C) 2016 Red Hat, Inc.
> > > +Licensed under a Creative Commons Attribution-Share Alike 3.0
> > > +United States License (see http://creativecommons.org/licenses/b
> > > y-sa
> > > /3.0/us/legalcode).
> > > +
> > > +Basic
> > > +-
> > > +The spice protocol format file defines the network protocol used
> > > by
> > > spice.
> > > +It resemble the C format.
> > > +
> > > +file ::=
> > > +definitions ::= |
> > > +definition ::=
> > > |
> > > +protocol ::= "protocol" "{"
> > > "}"
> > > ";"
> > > +protocol_channels ::=
> > > |
> > > +protocol_channel ::= [ "="
> > > ]
> > > ";"
> > > +integer ::= |
> > > +dec ::= [+-][0-9]+
> > > +hex ::= "0x" [0-9a-f]+
> > > +identifier ::= [a-z][a-z0-9_]*
> > > +
> > > +(here BNF with some regular expression are used).
> > > +
> > > +Example:
> > > +
> > > +channel ExampleChannel {
> > > + message {
> > > + uint32 dummy;
> > > + } Dummy;
> > > +};
> > > +
> > > +protocol Example {
> > > +ExampleChannel first = 1001;
> > > +};
> > > +
> > > +As you can see brackets like C are used and structures looks
> > > like C
> > > but you
> > > +can also see that keyworks like `channel`, `protocol`, `message`
> > > or
> > > some
> > > +predefined types like `uint32` are proper of the protocol.
> > > +
> > > +Comments
> > > +
> > > +Both C and C++ style comments are supported
> > > +
> > > +// this is a comment
> > > +/* this is a comment too
> > > + but can be split in multiple lines */
> > > +
> > > +
> >
> >
> > I think that documenting the protocol is badly needed, but I fear
> > that
> > unless this documentation is connected to the protocol definition
> > in
> > some way, it will get out-of-sync pretty quickly.
> >
> > Jonathon
> >
>
> out-of-sync can be a problem but it's also true that the sync
> cannot move far away as this would break compatibility.
> Also we could extend the tests Christophe did to make sure that
> syntax is still working the same way.
> I was thinking about some small tools to extract documentation from
> spice_parser.py and other files but didn't manage to think any good
> way in a timely fashion. I wouldn't create another "project"
> (spend too much time) writing a tool that generate the documentation
> from source, looks like the information we need are quite spread all
> over the sources. I also tried to look at pyparsing (not really
> familiar with) to get a list the overall syntax documentation but
> didn't find nothing either.
>
> Do you think I (I would prefer a WE) should investigate more on
> getting documentation from source? I think not, unless someone with
> better knowledge came with an easy solution.
No, at the moment I think it's probably not worth spending time on a
new tool to extract documentation.
>
> Do you think it's worth if I continue this branch/patches or should
> I discard them?
I don't think they need to be discarded.
Jonathon
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [vdagent-linux v2 2/5] build-sys: get rid of noinst_HEADERS
From: Marc-André Lureau Regular headers file should be listed in SOURCES Signed-off-by: Victor Toso Acked-by: Jonathon Jongsma Acked-by: Frediano Ziglio --- Makefile.am | 26 -- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/Makefile.am b/Makefile.am index d27ed13..9930450 100644 --- a/Makefile.am +++ b/Makefile.am @@ -21,10 +21,15 @@ src_spice_vdagent_LDADD = \ src_spice_vdagent_SOURCES =\ src/udscs.c \ + src/udscs.h \ src/vdagent-audio.c \ + src/vdagent-audio.h \ src/vdagent-file-xfers.c\ + src/vdagent-file-xfers.h\ + src/vdagent-x11-priv.h \ src/vdagent-x11-randr.c \ src/vdagent-x11.c \ + src/vdagent-x11.h \ src/vdagent.c \ $(NULL) @@ -48,10 +53,17 @@ src_spice_vdagentd_LDADD = \ src_spice_vdagentd_SOURCES = \ src/vdagentd.c \ + src/session-info.h \ + src/vdagentd-proto-strings.h\ + src/vdagentd-proto.h\ src/vdagentd-uinput.c \ + src/vdagentd-uinput.h \ src/vdagentd-xorg-conf.c\ + src/vdagentd-xorg-conf.h\ src/vdagent-virtio-port.c \ + src/vdagent-virtio-port.h \ src/udscs.c \ + src/udscs.h \ $(NULL) if HAVE_CONSOLE_KIT @@ -64,20 +76,6 @@ src_spice_vdagentd_SOURCES += src/dummy-session-info.c endif endif -noinst_HEADERS = \ - src/session-info.h \ - src/udscs.h \ - src/vdagent-audio.h \ - src/vdagent-file-xfers.h\ - src/vdagent-virtio-port.h \ - src/vdagent-x11-priv.h \ - src/vdagent-x11.h \ - src/vdagentd-proto-strings.h\ - src/vdagentd-proto.h\ - src/vdagentd-uinput.h \ - src/vdagentd-xorg-conf.h\ - $(NULL) - xdgautostartdir = $(sysconfdir)/xdg/autostart xdgautostart_DATA = $(top_srcdir)/data/spice-vdagent.desktop -- 2.9.3 ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [vdagent-linux v2 3/5] build-sys: move user/system to respective dir
From: Marc-André Lureau
Signed-off-by: Victor Toso
Acked-by: Jonathon Jongsma
Acked-by: Frediano Ziglio
---
Makefile.am | 57 +---
src/{ => vdagent}/vdagent-audio.c| 0
src/{ => vdagent}/vdagent-audio.h| 0
src/{ => vdagent}/vdagent-file-xfers.c | 0
src/{ => vdagent}/vdagent-file-xfers.h | 0
src/{ => vdagent}/vdagent-x11-priv.h | 0
src/{ => vdagent}/vdagent-x11-randr.c| 0
src/{ => vdagent}/vdagent-x11.c | 0
src/{ => vdagent}/vdagent-x11.h | 0
src/{ => vdagent}/vdagent.c | 0
src/{ => vdagentd}/console-kit.c | 0
src/{ => vdagentd}/dummy-session-info.c | 0
src/{ => vdagentd}/session-info.h| 0
src/{ => vdagentd}/systemd-login.c | 0
src/{ => vdagentd}/vdagent-virtio-port.c | 0
src/{ => vdagentd}/vdagent-virtio-port.h | 0
src/{ => vdagentd}/vdagentd-uinput.c | 0
src/{ => vdagentd}/vdagentd-uinput.h | 0
src/{ => vdagentd}/vdagentd-xorg-conf.c | 0
src/{ => vdagentd}/vdagentd-xorg-conf.h | 0
src/{ => vdagentd}/vdagentd.c| 0
21 files changed, 31 insertions(+), 26 deletions(-)
rename src/{ => vdagent}/vdagent-audio.c (100%)
rename src/{ => vdagent}/vdagent-audio.h (100%)
rename src/{ => vdagent}/vdagent-file-xfers.c (100%)
rename src/{ => vdagent}/vdagent-file-xfers.h (100%)
rename src/{ => vdagent}/vdagent-x11-priv.h (100%)
rename src/{ => vdagent}/vdagent-x11-randr.c (100%)
rename src/{ => vdagent}/vdagent-x11.c (100%)
rename src/{ => vdagent}/vdagent-x11.h (100%)
rename src/{ => vdagent}/vdagent.c (100%)
rename src/{ => vdagentd}/console-kit.c (100%)
rename src/{ => vdagentd}/dummy-session-info.c (100%)
rename src/{ => vdagentd}/session-info.h (100%)
rename src/{ => vdagentd}/systemd-login.c (100%)
rename src/{ => vdagentd}/vdagent-virtio-port.c (100%)
rename src/{ => vdagentd}/vdagent-virtio-port.h (100%)
rename src/{ => vdagentd}/vdagentd-uinput.c (100%)
rename src/{ => vdagentd}/vdagentd-uinput.h (100%)
rename src/{ => vdagentd}/vdagentd-xorg-conf.c (100%)
rename src/{ => vdagentd}/vdagentd-xorg-conf.h (100%)
rename src/{ => vdagentd}/vdagentd.c (100%)
diff --git a/Makefile.am b/Makefile.am
index 9930450..861d806 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -4,11 +4,19 @@ NULL =
bin_PROGRAMS = src/spice-vdagent
sbin_PROGRAMS = src/spice-vdagentd
+common_sources = \
+ src/udscs.c \
+ src/udscs.h \
+ src/vdagentd-proto-strings.h\
+ src/vdagentd-proto.h\
+ $(NULL)
+
src_spice_vdagent_CFLAGS = \
$(X_CFLAGS) \
$(SPICE_CFLAGS) \
$(GLIB2_CFLAGS) \
$(ALSA_CFLAGS) \
+ -I$(srcdir)/src \
-DUDSCS_NO_SERVER \
$(NULL)
@@ -20,17 +28,16 @@ src_spice_vdagent_LDADD = \
$(NULL)
src_spice_vdagent_SOURCES =\
- src/udscs.c \
- src/udscs.h \
- src/vdagent-audio.c \
- src/vdagent-audio.h \
- src/vdagent-file-xfers.c\
- src/vdagent-file-xfers.h\
- src/vdagent-x11-priv.h \
- src/vdagent-x11-randr.c \
- src/vdagent-x11.c \
- src/vdagent-x11.h \
- src/vdagent.c \
+ $(common_sources) \
+ src/vdagent/vdagent-audio.c \
+ src/vdagent/vdagent-audio.h \
+ src/vdagent/vdagent-file-xfers.c\
+ src/vdagent/vdagent-file-xfers.h\
+ src/vdagent/vdagent-x11-priv.h \
+ src/vdagent/vdagent-x11-randr.c \
+ src/vdagent/vdagent-x11.c \
+ src/vdagent/vdagent-x11.h \
+ src/vdagent/vdagent.c \
$(NULL)
src_spice_vdagentd_CFLAGS =\
@@ -40,6 +47,7 @@ src_spice_vdagentd_CFLAGS = \
$(SPICE_CFLAGS) \
$(GLIB2_CFLAGS) \
$(PIE_CFLAGS) \
+ -I$(srcdir)/src \
$(NULL)
src_spice_vdagentd_LDADD = \
@@ -52,27 +60,24 @@ src_spice_vdagentd_LDADD = \
$(NULL)
src_spice_vdagentd_SOURCES = \
- src/vdagentd.c \
- src/session-info.h \
- src/vdagentd-proto-strings.h\
- src/vdagentd-proto.h\
- src/vdagentd-uinput.c
[Spice-devel] [vdagent-linux v2 5/5] buildsys: statis typo in configure option
--- configure.ac | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index 2106085..0c2cfd8 100644 --- a/configure.ac +++ b/configure.ac @@ -73,7 +73,7 @@ AC_ARG_ENABLE([pciaccess], [enable_pciaccess="yes"]) AC_ARG_ENABLE([static-uinput], - [AS_HELP_STRING([--enable-statis-uinput], [Enable use of a fixed, static uinput device for X-servers without hotplug support (default: no)])], + [AS_HELP_STRING([--enable-static-uinput], [Enable use of a fixed, static uinput device for X-servers without hotplug support (default: no)])], [enable_static_uinput="$enableval"], [enable_static_uinput="no"]) -- 2.9.3 ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [vdagent-linux v2 1/5] build-sys: reformat Makefile.am
Makes the makefile more readable and easy to change in the next few commits. Based on Marc-André Lureau patch. Acked-by: Jonathon Jongsma Acked-by: Frediano Ziglio --- Makefile.am | 98 ++--- 1 file changed, 67 insertions(+), 31 deletions(-) diff --git a/Makefile.am b/Makefile.am index 680ef83..d27ed13 100644 --- a/Makefile.am +++ b/Makefile.am @@ -4,24 +4,56 @@ NULL = bin_PROGRAMS = src/spice-vdagent sbin_PROGRAMS = src/spice-vdagentd -src_spice_vdagent_CFLAGS = $(X_CFLAGS) $(SPICE_CFLAGS) $(GLIB2_CFLAGS) $(ALSA_CFLAGS) -DUDSCS_NO_SERVER -src_spice_vdagent_LDADD = $(X_LIBS) $(SPICE_LIBS) $(GLIB2_LIBS) $(ALSA_LIBS) -src_spice_vdagent_SOURCES = src/vdagent.c \ -src/vdagent-x11.c \ -src/vdagent-x11-randr.c \ -src/vdagent-file-xfers.c \ -src/vdagent-audio.c \ -src/udscs.c - -src_spice_vdagentd_CFLAGS = $(DBUS_CFLAGS) $(LIBSYSTEMD_LOGIN_CFLAGS) \ - $(PCIACCESS_CFLAGS) $(SPICE_CFLAGS) $(GLIB2_CFLAGS) $(PIE_CFLAGS) -src_spice_vdagentd_LDADD = $(DBUS_LIBS) $(LIBSYSTEMD_LOGIN_LIBS) \ - $(PCIACCESS_LIBS) $(SPICE_LIBS) $(GLIB2_LIBS) $(PIE_LDFLAGS) -src_spice_vdagentd_SOURCES = src/vdagentd.c \ - src/vdagentd-uinput.c \ - src/vdagentd-xorg-conf.c \ - src/vdagent-virtio-port.c \ - src/udscs.c +src_spice_vdagent_CFLAGS = \ + $(X_CFLAGS) \ + $(SPICE_CFLAGS) \ + $(GLIB2_CFLAGS) \ + $(ALSA_CFLAGS) \ + -DUDSCS_NO_SERVER \ + $(NULL) + +src_spice_vdagent_LDADD = \ + $(X_LIBS) \ + $(SPICE_LIBS) \ + $(GLIB2_LIBS) \ + $(ALSA_LIBS)\ + $(NULL) + +src_spice_vdagent_SOURCES =\ + src/udscs.c \ + src/vdagent-audio.c \ + src/vdagent-file-xfers.c\ + src/vdagent-x11-randr.c \ + src/vdagent-x11.c \ + src/vdagent.c \ + $(NULL) + +src_spice_vdagentd_CFLAGS =\ + $(DBUS_CFLAGS) \ + $(LIBSYSTEMD_LOGIN_CFLAGS) \ + $(PCIACCESS_CFLAGS) \ + $(SPICE_CFLAGS) \ + $(GLIB2_CFLAGS) \ + $(PIE_CFLAGS) \ + $(NULL) + +src_spice_vdagentd_LDADD = \ + $(DBUS_LIBS)\ + $(LIBSYSTEMD_LOGIN_LIBS)\ + $(PCIACCESS_LIBS) \ + $(SPICE_LIBS) \ + $(GLIB2_LIBS) \ + $(PIE_LDFLAGS) \ + $(NULL) + +src_spice_vdagentd_SOURCES = \ + src/vdagentd.c \ + src/vdagentd-uinput.c \ + src/vdagentd-xorg-conf.c\ + src/vdagent-virtio-port.c \ + src/udscs.c \ + $(NULL) + if HAVE_CONSOLE_KIT src_spice_vdagentd_SOURCES += src/console-kit.c else @@ -32,17 +64,19 @@ src_spice_vdagentd_SOURCES += src/dummy-session-info.c endif endif -noinst_HEADERS = src/session-info.h \ - src/udscs.h \ - src/vdagent-audio.h \ - src/vdagent-file-xfers.h \ - src/vdagent-virtio-port.h \ - src/vdagent-x11.h \ - src/vdagent-x11-priv.h \ - src/vdagentd-proto.h \ - src/vdagentd-proto-strings.h \ - src/vdagentd-uinput.h \ - src/vdagentd-xorg-conf.h +noinst_HEADERS = \ + src/session-info.h \ + src/udscs.h \ + src/vdagent-audio.h \ + src/vdagent-file-xfers.h\ + src/vdagent-virtio-port.h \ + src/vdagent-x11-priv.h \ + src/vdagent-x11.h \ + src/vdagentd-proto-strings.h\ + src/vdagentd-proto.h\ + src/vdagentd-uinput.h \ + src/vdagentd-xorg-conf.h\ + $(NULL) xdgautostartdir = $(sysconfdir)/xdg/autostart xdgautostart_DATA = $(top_srcdir)/data/spice-vdagent.desktop @@ -75,8 +109,10 @@ tmpfiles_DATA = $(top_srcdir)/data/tmpfiles.d/spice-vdagentd.conf endif manpagedir = $(mandir
[Spice-devel] [vdagent-linux v2 4/5] build-sys: remove prefix from filenames
---
.gitignore | 6 +
Makefile.am| 28 +++---
src/vdagent/{vdagent-audio.c => audio.c} | 2 +-
src/vdagent/{vdagent-audio.h => audio.h} | 4 ++--
src/vdagent/{vdagent-file-xfers.c => file-xfers.c} | 4 ++--
src/vdagent/{vdagent-file-xfers.h => file-xfers.h} | 0
src/vdagent/vdagent.c | 6 ++---
src/vdagent/{vdagent-x11-priv.h => x11-priv.h} | 0
src/vdagent/{vdagent-x11-randr.c => x11-randr.c} | 4 ++--
src/vdagent/{vdagent-x11.c => x11.c} | 4 ++--
src/vdagent/{vdagent-x11.h => x11.h} | 4 ++--
src/vdagentd/{vdagentd-uinput.c => uinput.c} | 4 ++--
src/vdagentd/{vdagentd-uinput.h => uinput.h} | 0
src/vdagentd/vdagentd.c| 6 ++---
.../{vdagent-virtio-port.c => virtio-port.c} | 2 +-
.../{vdagent-virtio-port.h => virtio-port.h} | 4 ++--
src/vdagentd/{vdagentd-xorg-conf.c => xorg-conf.c} | 2 +-
src/vdagentd/{vdagentd-xorg-conf.h => xorg-conf.h} | 0
18 files changed, 43 insertions(+), 37 deletions(-)
rename src/vdagent/{vdagent-audio.c => audio.c} (99%)
rename src/vdagent/{vdagent-audio.h => audio.h} (91%)
rename src/vdagent/{vdagent-file-xfers.c => file-xfers.c} (99%)
rename src/vdagent/{vdagent-file-xfers.h => file-xfers.h} (100%)
rename src/vdagent/{vdagent-x11-priv.h => x11-priv.h} (100%)
rename src/vdagent/{vdagent-x11-randr.c => x11-randr.c} (99%)
rename src/vdagent/{vdagent-x11.c => x11.c} (99%)
rename src/vdagent/{vdagent-x11.h => x11.h} (95%)
rename src/vdagentd/{vdagentd-uinput.c => uinput.c} (99%)
rename src/vdagentd/{vdagentd-uinput.h => uinput.h} (100%)
rename src/vdagentd/{vdagent-virtio-port.c => virtio-port.c} (99%)
rename src/vdagentd/{vdagent-virtio-port.h => virtio-port.h} (97%)
rename src/vdagentd/{vdagentd-xorg-conf.c => xorg-conf.c} (99%)
rename src/vdagentd/{vdagentd-xorg-conf.h => xorg-conf.h} (100%)
diff --git a/.gitignore b/.gitignore
index 4c039ee..ae47a90 100644
--- a/.gitignore
+++ b/.gitignore
@@ -8,6 +8,12 @@ src/stamp-h1
src/*.o
src/.deps
src/.dirstamp
+src/vdagent/*.o
+src/vdagent/.deps
+src/vdagent/.dirstamp
+src/vdagentd/*.o
+src/vdagentd/.deps
+src/vdagentd/.dirstamp
config.log
config.status
aclocal.m4
diff --git a/Makefile.am b/Makefile.am
index 861d806..7755f09 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -29,14 +29,14 @@ src_spice_vdagent_LDADD = \
src_spice_vdagent_SOURCES =\
$(common_sources) \
- src/vdagent/vdagent-audio.c \
- src/vdagent/vdagent-audio.h \
- src/vdagent/vdagent-file-xfers.c\
- src/vdagent/vdagent-file-xfers.h\
- src/vdagent/vdagent-x11-priv.h \
- src/vdagent/vdagent-x11-randr.c \
- src/vdagent/vdagent-x11.c \
- src/vdagent/vdagent-x11.h \
+ src/vdagent/audio.c \
+ src/vdagent/audio.h \
+ src/vdagent/file-xfers.c\
+ src/vdagent/file-xfers.h\
+ src/vdagent/x11-priv.h \
+ src/vdagent/x11-randr.c \
+ src/vdagent/x11.c \
+ src/vdagent/x11.h \
src/vdagent/vdagent.c \
$(NULL)
@@ -63,12 +63,12 @@ src_spice_vdagentd_SOURCES =\
$(common_sources) \
src/vdagentd/vdagentd.c \
src/vdagentd/session-info.h \
- src/vdagentd/vdagentd-uinput.c \
- src/vdagentd/vdagentd-uinput.h \
- src/vdagentd/vdagentd-xorg-conf.c \
- src/vdagentd/vdagentd-xorg-conf.h \
- src/vdagentd/vdagent-virtio-port.c \
- src/vdagentd/vdagent-virtio-port.h \
+ src/vdagentd/uinput.c \
+ src/vdagentd/uinput.h \
+ src/vdagentd/xorg-conf.c\
+ src/vdagentd/xorg-conf.h\
+ src/vdagentd/virtio-port.c \
+ src/vdagentd/virtio-port.h \
$(NULL)
if HAVE_CONSOLE_KIT
diff --git a/src/vdagent/vdagent-audio.c b/src/vdagent/audio.c
similarity index 99%
rename from src/vdagent/vdagent-audio.c
rename to src/vdagent/audio.c
index 6b11cd8..e823569 100644
--- a/src/vdagent/vdagent-audio.c
+++ b/src/vdagent/audio.c
@@ -26,7 +26,7 @@
#include
#include
#include
-#include "vdagent-audio.h"
+#include "audio.h"
#define ALSA_MUTE 0
#define ALSA_UNMUTE 1
diff --git a/src/vdagent/vdagent-audio.h b/src/vdagent/audio.h
similarity index 91%
rename from src/vdagent/vdagent-audio.h
rename to src/vdagent/audio.h
index 6f29d4b..7d0ae35 100644
--- a/src/vdagent/vdagent-audio.h
+++ b/src/vdagent/audio.h
@@ -1,6 +1,
[Spice-devel] [vdagent-linux v2 0/5] Reorganize vdagent Makefile and directory
Hi, second version.
In the first patch, I've added the $(NULL), diff is here:
http://paste.fedoraproject.org/428487/73955986/
We now have a 4th patch that we might want to merge with the 3rd patch... Having
an extra patch helps with the review, I guess.
The 5th patch is a small typo that I've just found.
Keeping the acks from Jonathon and Frediano in the first three patches.
Cheers,
toso
Marc-André Lureau (2):
build-sys: get rid of noinst_HEADERS
build-sys: move user/system to respective dir
Victor Toso (3):
build-sys: reformat Makefile.am
build-sys: remove prefix from filenames
buildsys: statis typo in configure option
.gitignore | 6 ++
Makefile.am| 109 ++---
configure.ac | 2 +-
src/{vdagent-audio.c => vdagent/audio.c} | 2 +-
src/{vdagent-audio.h => vdagent/audio.h} | 4 +-
src/{vdagent-file-xfers.c => vdagent/file-xfers.c} | 4 +-
src/{vdagent-file-xfers.h => vdagent/file-xfers.h} | 0
src/{ => vdagent}/vdagent.c| 6 +-
src/{vdagent-x11-priv.h => vdagent/x11-priv.h} | 0
src/{vdagent-x11-randr.c => vdagent/x11-randr.c} | 4 +-
src/{vdagent-x11.c => vdagent/x11.c} | 4 +-
src/{vdagent-x11.h => vdagent/x11.h} | 4 +-
src/{ => vdagentd}/console-kit.c | 0
src/{ => vdagentd}/dummy-session-info.c| 0
src/{ => vdagentd}/session-info.h | 0
src/{ => vdagentd}/systemd-login.c | 0
src/{vdagentd-uinput.c => vdagentd/uinput.c} | 4 +-
src/{vdagentd-uinput.h => vdagentd/uinput.h} | 0
src/{ => vdagentd}/vdagentd.c | 6 +-
.../virtio-port.c} | 2 +-
.../virtio-port.h} | 4 +-
src/{vdagentd-xorg-conf.c => vdagentd/xorg-conf.c} | 2 +-
src/{vdagentd-xorg-conf.h => vdagentd/xorg-conf.h} | 0
23 files changed, 104 insertions(+), 59 deletions(-)
rename src/{vdagent-audio.c => vdagent/audio.c} (99%)
rename src/{vdagent-audio.h => vdagent/audio.h} (91%)
rename src/{vdagent-file-xfers.c => vdagent/file-xfers.c} (99%)
rename src/{vdagent-file-xfers.h => vdagent/file-xfers.h} (100%)
rename src/{ => vdagent}/vdagent.c (99%)
rename src/{vdagent-x11-priv.h => vdagent/x11-priv.h} (100%)
rename src/{vdagent-x11-randr.c => vdagent/x11-randr.c} (99%)
rename src/{vdagent-x11.c => vdagent/x11.c} (99%)
rename src/{vdagent-x11.h => vdagent/x11.h} (95%)
rename src/{ => vdagentd}/console-kit.c (100%)
rename src/{ => vdagentd}/dummy-session-info.c (100%)
rename src/{ => vdagentd}/session-info.h (100%)
rename src/{ => vdagentd}/systemd-login.c (100%)
rename src/{vdagentd-uinput.c => vdagentd/uinput.c} (99%)
rename src/{vdagentd-uinput.h => vdagentd/uinput.h} (100%)
rename src/{ => vdagentd}/vdagentd.c (99%)
rename src/{vdagent-virtio-port.c => vdagentd/virtio-port.c} (99%)
rename src/{vdagent-virtio-port.h => vdagentd/virtio-port.h} (97%)
rename src/{vdagentd-xorg-conf.c => vdagentd/xorg-conf.c} (99%)
rename src/{vdagentd-xorg-conf.h => vdagentd/xorg-conf.h} (100%)
--
2.9.3
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH v2 2/2] Add DisplayChannelPrivate struct
Move all of the DisplayChannel data memembers into a private struct to
encapsulate things better. This necessitated a few new 'public' methods
and a small bit of refactoring to avoid poking into DisplayChannel
internals from too many places. The DisplayChannel and the
DisplayChannelClient are still far too intertwined to completely avoid
accessing private data, so at the moment the private struct is defined
in display-channel.h and the DisplayChannelClient implementation
still accesses it sometimes.
---
server/dcc-send.c| 16 +--
server/dcc.c | 33 ++---
server/display-channel.c | 314 ---
server/display-channel.h | 13 +-
server/red-worker.c | 6 +
server/stream.c | 53
6 files changed, 227 insertions(+), 208 deletions(-)
diff --git a/server/dcc-send.c b/server/dcc-send.c
index 0afa25c..0635afa 100644
--- a/server/dcc-send.c
+++ b/server/dcc-send.c
@@ -96,7 +96,7 @@ static int is_surface_area_lossy(DisplayChannelClient *dcc,
uint32_t surface_id,
spice_return_val_if_fail(display_channel_validate_surface(display,
surface_id), FALSE);
-surface = &display->surfaces[surface_id];
+surface = &display->priv->surfaces[surface_id];
surface_lossy_region = &dcc->priv->surface_client_lossy_region[surface_id];
if (!area) {
@@ -208,13 +208,13 @@ static void
red_display_add_image_to_pixmap_cache(RedChannelClient *rcc,
io_image->descriptor.flags |= SPICE_IMAGE_FLAGS_CACHE_ME;
dcc->priv->send_data.pixmap_cache_items[dcc->priv->send_data.num_pixmap_cache_items++]
=
image->descriptor.id;
-stat_inc_counter(reds, display_channel->add_to_cache_counter,
1);
+stat_inc_counter(reds,
display_channel->priv->add_to_cache_counter, 1);
}
}
}
if (!(io_image->descriptor.flags & SPICE_IMAGE_FLAGS_CACHE_ME)) {
-stat_inc_counter(reds, display_channel->non_cache_counter, 1);
+stat_inc_counter(reds, display_channel->priv->non_cache_counter, 1);
}
}
@@ -385,7 +385,7 @@ static FillBitsType fill_bits(DisplayChannelClient *dcc,
SpiceMarshaller *m,
dcc->priv->send_data.pixmap_cache_items[dcc->priv->send_data.num_pixmap_cache_items++]
=
image.descriptor.id;
if (can_lossy || !lossy_cache_item) {
-if (!display->enable_jpeg || lossy_cache_item) {
+if (!display->priv->enable_jpeg || lossy_cache_item) {
image.descriptor.type = SPICE_IMAGE_TYPE_FROM_CACHE;
} else {
// making sure, in multiple monitor scenario, that lossy
items that
@@ -397,7 +397,7 @@ static FillBitsType fill_bits(DisplayChannelClient *dcc,
SpiceMarshaller *m,
&bitmap_palette_out, &lzplt_palette_out);
spice_assert(bitmap_palette_out == NULL);
spice_assert(lzplt_palette_out == NULL);
-stat_inc_counter(reds, display->cache_hits_counter, 1);
+stat_inc_counter(reds, display->priv->cache_hits_counter, 1);
pthread_mutex_unlock(&dcc->priv->pixmap_cache->lock);
return FILL_BITS_TYPE_CACHE;
} else {
@@ -420,7 +420,7 @@ static FillBitsType fill_bits(DisplayChannelClient *dcc,
SpiceMarshaller *m,
return FILL_BITS_TYPE_SURFACE;
}
-surface = &display->surfaces[surface_id];
+surface = &display->priv->surfaces[surface_id];
image.descriptor.type = SPICE_IMAGE_TYPE_SURFACE;
image.descriptor.flags = 0;
image.descriptor.width = surface->context.width;
@@ -1862,7 +1862,7 @@ static void
display_channel_marshall_migrate_data(RedChannelClient *rcc,
spice_marshaller_add(base_marshaller,
(uint8_t *)&display_data, sizeof(display_data) -
sizeof(uint32_t));
display_channel_marshall_migrate_data_surfaces(dcc, base_marshaller,
-
display_channel->enable_jpeg);
+
display_channel->priv->enable_jpeg);
}
static void display_channel_marshall_pixmap_sync(RedChannelClient *rcc,
@@ -2148,7 +2148,7 @@ static void marshall_qxl_drawable(RedChannelClient *rcc,
if (item->stream && red_marshall_stream_data(rcc, m, item)) {
return;
}
-if (display->enable_jpeg)
+if (display->priv->enable_jpeg)
marshall_lossy_qxl_drawable(rcc, m, dpi);
else
marshall_lossless_qxl_drawable(rcc, m, dpi);
diff --git a/server/dcc.c b/server/dcc.c
index 36c9c1d..62e32c4 100644
--- a/server/dcc.c
+++ b/server/dcc.c
@@ -21,6 +21,7 @@
#include "dcc-private.h"
#include "display-channel.h"
+#include "dcc.h"
#include "red-channel-client-private.h"
#define DISPLAY_CLIENT_SHORT_TIMEOUT
[Spice-devel] [PATCH v2 1/2] Improve encapsulation of DisplayChannel
Add a couple new functions to the header so that they can be called by
other objects rather than poking into the internals of the struct.
---
server/dcc-send.c| 16 +--
server/display-channel.c | 71
server/display-channel.h | 37 +
server/red-worker.c | 44 ++
server/stream.c | 18 ++--
5 files changed, 109 insertions(+), 77 deletions(-)
diff --git a/server/dcc-send.c b/server/dcc-send.c
index 521e6a2..0afa25c 100644
--- a/server/dcc-send.c
+++ b/server/dcc-send.c
@@ -94,7 +94,7 @@ static int is_surface_area_lossy(DisplayChannelClient *dcc,
uint32_t surface_id,
QRegion lossy_region;
DisplayChannel *display = DCC_TO_DC(dcc);
-spice_return_val_if_fail(validate_surface(display, surface_id), FALSE);
+spice_return_val_if_fail(display_channel_validate_surface(display,
surface_id), FALSE);
surface = &display->surfaces[surface_id];
surface_lossy_region = &dcc->priv->surface_client_lossy_region[surface_id];
@@ -414,7 +414,7 @@ static FillBitsType fill_bits(DisplayChannelClient *dcc,
SpiceMarshaller *m,
RedSurface *surface;
surface_id = simage->u.surface.surface_id;
-if (!validate_surface(display, surface_id)) {
+if (!display_channel_validate_surface(display, surface_id)) {
spice_warning("Invalid surface in SPICE_IMAGE_TYPE_SURFACE");
pthread_mutex_unlock(&dcc->priv->pixmap_cache->lock);
return FILL_BITS_TYPE_SURFACE;
@@ -1706,7 +1706,7 @@ static int red_marshall_stream_data(RedChannelClient *rcc,
return FALSE;
}
-StreamAgent *agent = &dcc->priv->stream_agents[get_stream_id(display,
stream)];
+StreamAgent *agent =
&dcc->priv->stream_agents[display_channel_get_stream_id(display, stream)];
uint64_t time_now = spice_get_monotonic_time_ns();
if (!dcc->priv->use_video_encoder_rate_control) {
@@ -1752,7 +1752,7 @@ static int red_marshall_stream_data(RedChannelClient *rcc,
red_channel_client_init_send_data(rcc, SPICE_MSG_DISPLAY_STREAM_DATA,
NULL);
-stream_data.base.id = get_stream_id(display, stream);
+stream_data.base.id = display_channel_get_stream_id(display, stream);
stream_data.base.multi_media_time = frame_mm_time;
stream_data.data_size = outbuf->size;
@@ -1762,7 +1762,7 @@ static int red_marshall_stream_data(RedChannelClient *rcc,
red_channel_client_init_send_data(rcc,
SPICE_MSG_DISPLAY_STREAM_DATA_SIZED, NULL);
-stream_data.base.id = get_stream_id(display, stream);
+stream_data.base.id = display_channel_get_stream_id(display, stream);
stream_data.base.multi_media_time = frame_mm_time;
stream_data.data_size = outbuf->size;
stream_data.width = copy->src_area.right - copy->src_area.left;
@@ -2171,7 +2171,7 @@ static void marshall_stream_start(RedChannelClient *rcc,
SpiceClipRects clip_rects;
stream_create.surface_id = 0;
-stream_create.id = get_stream_id(DCC_TO_DC(dcc), stream);
+stream_create.id = display_channel_get_stream_id(DCC_TO_DC(dcc), stream);
stream_create.flags = stream->top_down ? SPICE_STREAM_FLAGS_TOP_DOWN : 0;
stream_create.codec_type = agent->video_encoder->codec_type;
@@ -2207,7 +2207,7 @@ static void marshall_stream_clip(RedChannelClient *rcc,
red_channel_client_init_send_data(rcc, SPICE_MSG_DISPLAY_STREAM_CLIP,
&item->base);
SpiceMsgDisplayStreamClip stream_clip;
-stream_clip.id = get_stream_id(DCC_TO_DC(dcc), agent->stream);
+stream_clip.id = display_channel_get_stream_id(DCC_TO_DC(dcc),
agent->stream);
stream_clip.clip.type = item->clip_type;
stream_clip.clip.rects = item->rects;
@@ -2221,7 +2221,7 @@ static void marshall_stream_end(RedChannelClient *rcc,
SpiceMsgDisplayStreamDestroy destroy;
red_channel_client_init_send_data(rcc, SPICE_MSG_DISPLAY_STREAM_DESTROY,
NULL);
-destroy.id = get_stream_id(DCC_TO_DC(dcc), agent->stream);
+destroy.id = display_channel_get_stream_id(DCC_TO_DC(dcc), agent->stream);
stream_agent_stop(agent);
spice_marshall_msg_display_stream_destroy(base_marshaller, &destroy);
}
diff --git a/server/display-channel.c b/server/display-channel.c
index 108e69b..56bb029 100644
--- a/server/display-channel.c
+++ b/server/display-channel.c
@@ -203,6 +203,13 @@ void display_channel_surface_unref(DisplayChannel
*display, uint32_t surface_id)
spice_warn_if_fail(ring_is_empty(&surface->depend_on_me));
}
+/* TODO: perhaps rename to "ready" or "realized" ? */
+bool display_channel_surface_has_canvas(DisplayChannel *display,
+uint32_t surface_id)
+{
+return display->surfaces[surface_id].context.canvas != NULL;
+}
+
static void streams_update_visible_region(DisplayChannel *display, Drawable
*drawable)
{
Ring *ring;
@@ -232,7 +239,7 @@ static vo
Re: [Spice-devel] [PATCH v2 8/9] Change RedCharDevice::write_queue to GQueue
>
> Change a couple more Rings to GQueue
> ---
> Changes in v2:
> - use GQueue, not GList
>
> server/char-device.c | 79
> +---
> server/char-device.h | 1 -
> 2 files changed, 32 insertions(+), 48 deletions(-)
>
> diff --git a/server/char-device.c b/server/char-device.c
> index 957fb26..f826524 100644
> --- a/server/char-device.c
> +++ b/server/char-device.c
> @@ -50,8 +50,8 @@ struct RedCharDevicePrivate {
> int active; /* has read/write been performed since the device was
> started */
> int wait_for_migrate_data;
>
> -Ring write_queue;
> -Ring write_bufs_pool;
> +GQueue *write_queue;
> +GQueue *write_bufs_pool;
> uint64_t cur_pool_size;
> RedCharDeviceWriteBuffer *cur_write_buf;
> uint8_t *cur_write_buf_pos;
> @@ -1195,8 +1180,8 @@ red_char_device_init(RedCharDevice *self)
> {
> self->priv = RED_CHAR_DEVICE_PRIVATE(self);
>
> -ring_init(&self->priv->write_queue);
> -ring_init(&self->priv->write_bufs_pool);
> +self->priv->write_queue = g_queue_new();
> +self->priv->write_bufs_pool = g_queue_new();
> ring_init(&self->priv->clients);
>
> g_signal_connect(self, "notify::sin",
> G_CALLBACK(red_char_device_on_sin_changed), NULL);
Why not using "GQueue write_queue"/"GQueue write_bufs_pool" and g_queue_init
instead to avoid the pointer dereference every time?
Frediano
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH] replay: Handle errors in record file
Hey,
On Mon, Sep 12, 2016 at 12:56:05PM +0100, Frediano Ziglio wrote:
> Detect errors in record file. This can happen from a wrong version or
> corruption of files.
> Allocations are kept into a GList to be able to free in case some
> errors happened.
It would be nice to have this bit in a separate commit
> To check fscanf read all needed information a dummy "%n" is appended
> to any string and the value stored there is tested. This as scanf family
> could return a valid value but not entirely process the string so
> adding a "%n" and checking this was processed make sure all expected
> string is found.
And this one in a separate one too
> The "eof" field is used to mark any error happened, so in most cases
> there is no explicit check beside when this could cause a problem
> (for instance results of replay_fscanf used which would result in
> uninitialised variable usage).
Should we rename that field then? If it's no longer 'end-of-file', but
'error-occurred', 'eof' is very misleading.
>
> Signed-off-by: Frediano Ziglio
> ---
> server/red-replay-qxl.c | 256
> ++--
> 1 file changed, 185 insertions(+), 71 deletions(-)
>
> Don't remember which version of the patch is.
> If I remember it should fix latests comments.
>
> diff --git a/server/red-replay-qxl.c b/server/red-replay-qxl.c
> index b17c38b..f916fa8 100644
> --- a/server/red-replay-qxl.c
> +++ b/server/red-replay-qxl.c
> @@ -49,29 +49,33 @@ struct SpiceReplay {
> GArray *id_map_inv; // replay id -> record id
> GArray *id_free; // free list
> int nsurfaces;
> +int end_pos;
> +
> +GList *allocated;
>
> pthread_mutex_t mutex;
> pthread_cond_t cond;
> };
>
> -static int replay_fread(SpiceReplay *replay, uint8_t *buf, size_t size)
> +static ssize_t replay_fread(SpiceReplay *replay, uint8_t *buf, size_t size)
> {
> -if (replay->eof) {
> -return 0;
> -}
> -if (feof(replay->fd)) {
> +if (replay->eof
> +|| feof(replay->fd)
> +|| fread(buf, 1, size, replay->fd) != size) {
Really not a big fan of chaining calls in an if, what about something
like this?
if (replay->eof)
goto error;
if (feof(replay->fd)
goto error;
if (fread(...) ..)
goto error;
return size;
error:
...
> replay->eof = 1;
> return 0;
> }
> -return fread(buf, size, 1, replay->fd);
> +return size;
> }
>
> __attribute__((format(scanf, 2, 3)))
> -static replay_t replay_fscanf(SpiceReplay *replay, const char *fmt, ...)
> +static replay_t replay_fscanf_check(SpiceReplay *replay, const char *fmt,
> ...)
> {
> va_list ap;
> int ret;
>
> +replay->end_pos = -1;
> +
> if (replay->eof) {
> return REPLAY_EOF;
> }
> @@ -82,11 +86,27 @@ static replay_t replay_fscanf(SpiceReplay *replay, const
> char *fmt, ...)
> va_start(ap, fmt);
> ret = vfscanf(replay->fd, fmt, ap);
> va_end(ap);
> -if (ret == EOF) {
> +if (ret == EOF || replay->end_pos < 0) {
> replay->eof = 1;
> }
> return replay->eof ? REPLAY_EOF : REPLAY_OK;
> }
> +#define replay_fscanf(r, fmt, ...) \
> +replay_fscanf_check(r, fmt "%n", ## __VA_ARGS__, &r->end_pos)
> +
> +static inline void *replay_malloc(SpiceReplay *replay, size_t size)
> +{
> +void *p = spice_malloc(size);
Can we get a more expressive name than 'p' ? :) mem, allocation or
allocated_mem or whatever
> +replay->allocated = g_list_prepend(replay->allocated, p);
> +return p;
> +}
> +
> +static inline void *replay_malloc0(SpiceReplay *replay, size_t size)
> +{
> +void *p = spice_malloc0(size);
> +replay->allocated = g_list_prepend(replay->allocated, p);
> +return p;
> +}
replay_malloc0 could be:
void *mem = replay_malloc(replay, size);
memset(mem, '\0', size);
>
> static uint32_t replay_id_get(SpiceReplay *replay, uint32_t id)
> {
> @@ -188,18 +208,20 @@ static void hexdump(uint8_t *hex, uint8_t bytes)
> static replay_t read_binary(SpiceReplay *replay, const char *prefix, size_t
> *size, uint8_t
> **buf, size_t base_size)
> {
> -char template[1024];
> -int with_zlib = -1;
> -int zlib_size;
> -uint8_t *zlib_buffer;
> +char file_prefix[1024];
> +int with_zlib;
> z_stream strm;
>
> -snprintf(template, sizeof(template), "binary %%d %s %%ld:", prefix);
> -if (replay_fscanf(replay, template, &with_zlib, size) == REPLAY_EOF)
> +if (replay_fscanf(replay, "binary %d %1000s %ld:", &with_zlib,
> file_prefix, size) == REPLAY_EOF) {
file_prefix is 1024 bytes, but you only use 1000 here?
> return REPLAY_EOF;
> +}
> +if (strcmp(file_prefix, prefix) != 0) {
> +replay->eof = 1;
> +return REPLAY_EOF;
> +}
>
> if (*buf == NULL) {
> -*buf = spice_malloc(*size + base_size);
> +*buf = replay_malloc(replay, *size + base_size);
> }
> #if 0
> {
> @@ -211,10 +233,18 @@ sta
Re: [Spice-devel] [PATCH v2 9/9] Change RedCharDevicePrivate::clients to GList
>
> More Ring cleanup. At the moment, we only support a single client, so
> this is only a one-element list
> ---
> Changes in v2:
> - simplified red_char_device_finalize() to avoid using g_list_last()
>
> server/char-device.c | 68
>
> 1 file changed, 26 insertions(+), 42 deletions(-)
>
> diff --git a/server/char-device.c b/server/char-device.c
> index f826524..1b99aa9 100644
> --- a/server/char-device.c
> +++ b/server/char-device.c
> @@ -32,7 +32,6 @@
>
> typedef struct RedCharDeviceClient RedCharDeviceClient;
> struct RedCharDeviceClient {
> -RingItem link;
> RedCharDevice *dev;
> RedClient *client;
> int do_flow_control;
> @@ -58,8 +57,7 @@ struct RedCharDevicePrivate {
> SpiceTimer *write_to_dev_timer;
> uint64_t num_self_tokens;
>
> -Ring clients; /* list of RedCharDeviceClient */
> -uint32_t num_clients;
> +GList *clients; /* list of RedCharDeviceClient */
>
> uint64_t client_tokens_interval; /* frequency of returning tokens to the
> client */
> SpiceCharDeviceInstance *sin;
> @@ -207,8 +205,7 @@ static void red_char_device_client_free(RedCharDevice
> *dev,
> dev->priv->cur_write_buf->client = NULL;
> }
>
> -dev->priv->num_clients--;
> -ring_remove(&dev_client->link);
> +dev->priv->clients = g_list_remove(dev->priv->clients, dev_client);
> free(dev_client);
> }
>
> @@ -222,12 +219,11 @@ static void
> red_char_device_handle_client_overflow(RedCharDeviceClient *dev_clie
> static RedCharDeviceClient *red_char_device_client_find(RedCharDevice *dev,
> RedClient *client)
> {
> -RingItem *item;
> +GList *item;
>
> -RING_FOREACH(item, &dev->priv->clients) {
> -RedCharDeviceClient *dev_client;
> +for (item = dev->priv->clients; item != NULL; item = item->next) {
> +RedCharDeviceClient *dev_client = item->data;
>
> -dev_client = SPICE_CONTAINEROF(item, RedCharDeviceClient, link);
> if (dev_client->client == client) {
> return dev_client;
> }
> @@ -253,13 +249,11 @@ static int
> red_char_device_can_send_to_client(RedCharDeviceClient *dev_client)
>
> static uint64_t red_char_device_max_send_tokens(RedCharDevice *dev)
> {
> -RingItem *item;
> +GList *item;
> uint64_t max = 0;
>
> -RING_FOREACH(item, &dev->priv->clients) {
> -RedCharDeviceClient *dev_client;
> -
> -dev_client = SPICE_CONTAINEROF(item, RedCharDeviceClient, link);
> +for (item = dev->priv->clients; item != NULL; item = item->next) {
> +RedCharDeviceClient *dev_client = item->data;
>
> if (!dev_client->do_flow_control) {
> max = ~0;
> @@ -295,12 +289,11 @@ static void
> red_char_device_add_msg_to_client_queue(RedCharDeviceClient *dev_cli
> static void red_char_device_send_msg_to_clients(RedCharDevice *dev,
> RedPipeItem *msg)
> {
> -RingItem *item, *next;
> +GList *l;
>
> -RING_FOREACH_SAFE(item, next, &dev->priv->clients) {
> -RedCharDeviceClient *dev_client;
> +for (l = dev->priv->clients; l != NULL; l = l->next) {
> +RedCharDeviceClient *dev_client = l->data;
>
> -dev_client = SPICE_CONTAINEROF(item, RedCharDeviceClient, link);
> if (red_char_device_can_send_to_client(dev_client)) {
> dev_client->num_send_tokens--;
> spice_assert(g_queue_is_empty(dev_client->send_queue));
The _SAFE loop here was necessary as there are combinations were
the client is removed from the list during the loop.
This will make l vanish before l->next is read.
Even if there is only a client this can happen.
> @@ -338,7 +331,7 @@ static int red_char_device_read_from_device(RedCharDevice
> *dev)
> * Reading from the device only in case at least one of the clients have
> a free token.
> * All messages will be discarded if no client is attached to the device
> */
> -while ((max_send_tokens || ring_is_empty(&dev->priv->clients)) &&
> dev->priv->running) {
> +while ((max_send_tokens || (dev->priv->clients == NULL)) &&
> dev->priv->running) {
> RedPipeItem *msg;
>
> msg = red_char_device_read_one_msg_from_device(dev);
> @@ -743,7 +736,7 @@ int red_char_device_client_add(RedCharDevice *dev,
> spice_assert(dev);
> spice_assert(client);
>
> -if (wait_for_migrate_data && (dev->priv->num_clients > 0 ||
> dev->priv->active)) {
> +if (wait_for_migrate_data && (dev->priv->clients != NULL ||
> dev->priv->active)) {
> spice_warning("can't restore device %p from migration data. The
> device "
>"has already been active", dev);
> return FALSE;
> @@ -757,8 +750,7 @@ int red_char_device_client_add(RedCharDevice *dev,
> num_client_tokens,
Re: [Spice-devel] [PATCH v2 7/9] Make glz_dictionary_list a GList
>
> Removing more intrusive RingItems from various structures and improving
> readibility.
> ---
> no changes
Acked-by: Frediano Ziglio
Frediano
>
> server/image-encoders.c | 17 +++--
> 1 file changed, 7 insertions(+), 10 deletions(-)
>
> diff --git a/server/image-encoders.c b/server/image-encoders.c
> index 5759230..39aca6c 100644
> --- a/server/image-encoders.c
> +++ b/server/image-encoders.c
> @@ -36,7 +36,6 @@ typedef struct RedGlzDrawable RedGlzDrawable;
> typedef struct GlzDrawableInstanceItem GlzDrawableInstanceItem;
>
> struct GlzSharedDictionary {
> -RingItem base;
> GlzEncDictContext *dict;
> uint32_t refs;
> uint8_t id;
> @@ -694,23 +693,21 @@ static GlzSharedDictionary
> *glz_shared_dictionary_new(RedClient *client, uint8_t
> shared_dict->refs = 1;
> shared_dict->migrate_freeze = FALSE;
> shared_dict->client = client;
> -ring_item_init(&shared_dict->base);
> pthread_rwlock_init(&shared_dict->encode_lock, NULL);
>
> return shared_dict;
> }
>
> static pthread_mutex_t glz_dictionary_list_lock = PTHREAD_MUTEX_INITIALIZER;
> -static Ring glz_dictionary_list = {&glz_dictionary_list,
> &glz_dictionary_list};
> +static GList *glz_dictionary_list;
>
> static GlzSharedDictionary *find_glz_dictionary(RedClient *client, uint8_t
> dict_id)
> {
> -RingItem *now;
> +GList *l;
> GlzSharedDictionary *ret = NULL;
>
> -now = &glz_dictionary_list;
> -while ((now = ring_next(&glz_dictionary_list, now))) {
> -GlzSharedDictionary *dict = SPICE_UPCAST(GlzSharedDictionary, now);
> +for (l = glz_dictionary_list; l != NULL; l = l->next) {
> +GlzSharedDictionary *dict = l->data;
> if ((dict->client == client) && (dict->id == dict_id)) {
> ret = dict;
> break;
> @@ -749,7 +746,7 @@ gboolean image_encoders_get_glz_dictionary(ImageEncoders
> *enc,
> shared_dict->refs++;
> } else {
> shared_dict = create_glz_dictionary(enc, client, id, window_size);
> -ring_add(&glz_dictionary_list, &shared_dict->base);
> +glz_dictionary_list = g_list_prepend(glz_dictionary_list,
> shared_dict);
> }
>
> pthread_mutex_unlock(&glz_dictionary_list_lock);
> @@ -785,7 +782,7 @@ gboolean
> image_encoders_restore_glz_dictionary(ImageEncoders *enc,
> shared_dict->refs++;
> } else {
> shared_dict = restore_glz_dictionary(enc, client, id, restore_data);
> -ring_add(&glz_dictionary_list, &shared_dict->base);
> +glz_dictionary_list = g_list_prepend(glz_dictionary_list,
> shared_dict);
> }
>
> pthread_mutex_unlock(&glz_dictionary_list_lock);
> @@ -819,7 +816,7 @@ static void image_encoders_release_glz(ImageEncoders
> *enc)
> pthread_mutex_unlock(&glz_dictionary_list_lock);
> return;
> }
> -ring_remove(&shared_dict->base);
> +glz_dictionary_list = g_list_remove(glz_dictionary_list, shared_dict);
> pthread_mutex_unlock(&glz_dictionary_list_lock);
> glz_enc_dictionary_destroy(shared_dict->dict, &enc->glz_data.usr);
> pthread_rwlock_destroy(&shared_dict->encode_lock);
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH v2 8/9] Change RedCharDevice::write_queue to GQueue
Hey,
On Wed, 2016-09-14 at 11:53 -0500, Jonathon Jongsma wrote:
> Change a couple more Rings to GQueue
> ---
> Changes in v2:
> - use GQueue, not GList
>
> server/char-device.c | 79 +--
> -
> server/char-device.h | 1 -
> 2 files changed, 32 insertions(+), 48 deletions(-)
>
> diff --git a/server/char-device.c b/server/char-device.c
> index 957fb26..f826524 100644
> --- a/server/char-device.c
> +++ b/server/char-device.c
> @@ -50,8 +50,8 @@ struct RedCharDevicePrivate {
> int active; /* has read/write been performed since the device
> was started */
> int wait_for_migrate_data;
>
> -Ring write_queue;
> -Ring write_bufs_pool;
> +GQueue *write_queue;
> +GQueue *write_bufs_pool;
> uint64_t cur_pool_size;
> RedCharDeviceWriteBuffer *cur_write_buf;
> uint8_t *cur_write_buf_pos;
> @@ -150,16 +150,11 @@ static void
> red_char_device_write_buffer_free(RedCharDeviceWriteBuffer *buf)
> free(buf);
> }
>
> -static void write_buffers_queue_free(Ring *write_queue)
> +static void write_buffers_queue_free(GQueue *write_queue)
> {
> -while (!ring_is_empty(write_queue)) {
> -RingItem *item = ring_get_tail(write_queue);
> -RedCharDeviceWriteBuffer *buf;
> -
> -ring_remove(item);
> -buf = SPICE_CONTAINEROF(item, RedCharDeviceWriteBuffer,
> link);
> +RedCharDeviceWriteBuffer *buf;
> +while ((buf = g_queue_pop_tail(write_queue)))
> red_char_device_write_buffer_free(buf);
> -}
What about freeing the queue itself ?
> }
>
> static void red_char_device_write_buffer_pool_add(RedCharDevice
> *dev,
> @@ -171,7 +166,7 @@ static void
> red_char_device_write_buffer_pool_add(RedCharDevice *dev,
> buf->origin = WRITE_BUFFER_ORIGIN_NONE;
> buf->client = NULL;
> dev->priv->cur_pool_size += buf->buf_size;
> -ring_add(&dev->priv->write_bufs_pool, &buf->link);
> +g_queue_push_head(dev->priv->write_bufs_pool, buf);
> return;
> }
>
> @@ -182,7 +177,7 @@ static void
> red_char_device_write_buffer_pool_add(RedCharDevice *dev,
> static void red_char_device_client_free(RedCharDevice *dev,
> RedCharDeviceClient
> *dev_client)
> {
> -RingItem *item, *next;
> +GList *l, *next;
>
> if (dev_client->wait_for_tokens_timer) {
> reds_core_timer_remove(dev->priv->reds, dev_client-
> >wait_for_tokens_timer);
> @@ -192,16 +187,18 @@ static void
> red_char_device_client_free(RedCharDevice *dev,
> g_queue_free_full(dev_client->send_queue,
> (GDestroyNotify)red_pipe_item_unref);
>
> /* remove write buffers that are associated with the client */
> -spice_debug("write_queue_is_empty %d", ring_is_empty(&dev-
> >priv->write_queue) && !dev->priv->cur_write_buf);
> -RING_FOREACH_SAFE(item, next, &dev->priv->write_queue) {
> -RedCharDeviceWriteBuffer *write_buf;
> +spice_debug("write_queue_is_empty %d", g_queue_is_empty(dev-
> >priv->write_queue) && !dev->priv->cur_write_buf);
> +l = g_queue_peek_head_link(dev->priv->write_queue);
> +while (l) {
> +RedCharDeviceWriteBuffer *write_buf = l->data;
> +next = l->next;
>
> -write_buf = SPICE_CONTAINEROF(item,
> RedCharDeviceWriteBuffer, link);
> if (write_buf->origin == WRITE_BUFFER_ORIGIN_CLIENT &&
> write_buf->client == dev_client->client) {
> -ring_remove(item);
> +g_queue_delete_link(dev->priv->write_queue, l);
> red_char_device_write_buffer_pool_add(dev, write_buf);
> }
> +l = next;
> }
>
> if (dev->priv->cur_write_buf && dev->priv->cur_write_buf-
> >origin == WRITE_BUFFER_ORIGIN_CLIENT &&
> @@ -483,13 +480,10 @@ static int
> red_char_device_write_to_device(RedCharDevice *dev)
> uint32_t write_len;
>
> if (!dev->priv->cur_write_buf) {
> -RingItem *item = ring_get_tail(&dev->priv-
> >write_queue);
> -if (!item) {
> +dev->priv->cur_write_buf = g_queue_pop_tail(dev->priv-
> >write_queue);
> +if (!dev->priv->cur_write_buf)
> break;
> -}
> -dev->priv->cur_write_buf = SPICE_CONTAINEROF(item,
> RedCharDeviceWriteBuffer, link);
> dev->priv->cur_write_buf_pos = dev->priv-
> >cur_write_buf->buf;
> -ring_remove(item);
> }
>
> write_len = dev->priv->cur_write_buf->buf + dev->priv-
> >cur_write_buf->buf_used -
> @@ -519,7 +513,7 @@ static int
> red_char_device_write_to_device(RedCharDevice *dev)
> CHAR_DEVICE_WRITE_TO_TIMEOUT)
> ;
> }
> } else {
> -spice_assert(ring_is_empty(&dev->priv->write_queue));
> +spice_assert(g_queue_is_empty(dev->priv->write_queue));
> }
> dev->priv->active = dev->priv->active || total;
> }
> @@ -542,16 +536,14 @
Re: [Spice-devel] spice performance tweaking
Hi, I tried a virtual rhel7.3beta (server with gui) on a rhel7.3beta host. The host was a laptop to which I setup to use my old wifi router that only has 54Mbit so the bandwith was poor and unstable. Without the compression options the spice display was really bad. You could see the screen being rendered bit by bit. I tried this with the custom rpm and did not really see an improvement. I downgraded the spice-server and rebooted I added the compression options: Now the spice display was very usable and almost no artifacts. Only when moving the windows you can see tearing at the edges. Also started a glxgears window to see how it deals with that I applied the custom rpm and rebooted the host. After restarting the vm and logging into spice There is less tearing when moving the window. So there is an improvement, but it's hard to tell how much by simply watching when draging the window. some details from our openvpn config the protocol has to be tcp. centos 7.2 openvpn server settings: port secret-port-number proto tcp dev tun tun-mtu 1500 ca /path/to/ca cert /path/to/cert key /path/to/key dh /path/to/dh server some.ip.address some.subnet.mask ifconfig-pool-persist ipp.txt keepalive 10 120 comp-lzo user nobody group nobody persist-key persist-tun status /path/to/log verb 3 mute 20 fedora 24 NetworkManager settings: [connection] id=our id for our top secret vpn uuid=..xxx.xxx.xx type=vpn permissions=user:secretusernottellingyou:; secondaries= [vpn] remote-random=no connection-type=tls remote=remote.ip.address tunnel-mtu=1500 comp-lzo=yes cert-pass-flags=1 proto-tcp=yes port= mssfix=yes dev-type=tun ca=/path/to/ca key=/path/to/key cert=/path/to/crt service-type=org.freedesktop.NetworkManager.openvpn [ipv4] dns-search= ignore-auto-routes=true method=auto never-default=true route1=additional.route/mask route2=additional.route/mask [ipv6] addr-gen-mode=stable-privacy dns-search= method=auto Cheers Rob Verduijn. 2016-09-15 16:04 GMT+02:00 Rob Verduijn : > Hello, > > The performance fixes sound awesome. > I'm afraid I cannot test them in the environment with the low bandwith > setup soon (maybe next week, but don't hold your breath) > I'm going to build a local test setup to see if this is improving some of > the issues. I got a laptop running the rhel7.3 beta, let's see how that > performs. > > Rob Verduijn > > > 2016-09-15 13:31 GMT+02:00 Frediano Ziglio : > >> I saw you are using CentOS 7. I built the package with RHEL 7 (they are >> binary compatible). >> About the testing just which normal usage you should see improvements in >> bandwidth and >> reactivity. >> >> Changes from current CentOS package: >> - used a newer version, there are couple of changes that decrease latency; >> - additional patches to improve bandwidth usage (for small drawing this >> should decrease >> bandwidth usage by a 15-20%); >> - additional patch to decrease a bandwidth limitation due to a peculiar >> half-duplex usage >> of spice protocol (this is clearly visible with high latency >> connections); >> - additional patch to decrease packet fragmentation due to TCP_NODELAY >> usage. >> >> Alternatively would be helpful for us to get a local reproduction of the >> problem. >> OpenVPN configuration files would be helpful (we don't need any security >> detail like keys, ip, host or system names, just to understand the type >> of VPN, >> encryption parameters, compression, additional latency introduced and so >> on). >> >> The fact that you are not able to get a record from the guest means that >> the QXL >> (guest <-> server) protocol how the spice-server is handling guest >> command is >> fine. The fact that on the client you can see clearly such slowness is >> due to spice >> protocol, the connection/vpn, some spice-server implementation and >> possibly >> client implementation too. Unfortunately too much stuff to be able to >> point the >> finger to one of them. >> I tried some test and did this: >> - opened task manager on Windows 7; >> - switched to performance tab; >> - maximized task manager; >> - double clicked on CPU usage to get only CPU usage and history. >> When CPU usage change I can see the flickering on CPU usage but not on >> the history graphs. It this the kind of flickering you are noticing? >> >> Frediano >> >> >> For which distro is that package ? >> Centos 7.2 ? rhel7.3beta or fedora24 ? >> >> Rob Verduijn >> >> 2016-09-14 15:59 GMT+02:00 Frediano Ziglio : >> >>> Could you test at least? Would be very helpful. We could then backport >>> some improvements. >>> >>> Frediano >>> >>> >>> thanx,I'll stick with the centos packages, >>> >>> I need a very good reason before I start using beta packages. >>> And a nice to have feature is not one of them. >>> >>> Also I dug in to the openvpn tweaks and it seems that all of them are >>> related to udp tunnels. >>> Performance is sadly rather low when you have to use tcp (like me) >>> because the fire
Re: [Spice-devel] [PATCH v2 1/9] Add DisplayChannelPrivate struct
Hi,
On Wed, 2016-09-14 at 11:53 -0500, Jonathon Jongsma wrote:
> Move all of the DisplayChannel data memembers into a private struct
> to
> encapsulate things better. This necessitated a few new 'public'
> methods
the public methods could be introduced in a different patch, but it is
not a big deal
> and a small bit of refactoring to avoid poking into DisplayChannel
> internals from too many places. The DisplayChannel and the
> DisplayChannelClient are still far too intertwined to completely
> avoid
> accessing private data, so at the moment the private struct is
> defined
> in display-channel.h and the DisplayChannelClient implementation
> still accesses it sometimes.
> ---
> Changes in v2:
> - use priv[1] to avoid memory leak
>
> server/dcc-send.c| 32 ++---
> server/dcc.c | 33 ++---
> server/display-channel.c | 357 +++-
> ---
> server/display-channel.h | 51 ---
> server/red-worker.c | 50 ++-
> server/stream.c | 71 +-
> 6 files changed, 323 insertions(+), 271 deletions(-)
>
> diff --git a/server/dcc-send.c b/server/dcc-send.c
> index 521e6a2..0635afa 100644
> --- a/server/dcc-send.c
> +++ b/server/dcc-send.c
> @@ -94,9 +94,9 @@ static int
> is_surface_area_lossy(DisplayChannelClient *dcc, uint32_t
> surface_id,
> QRegion lossy_region;
> DisplayChannel *display = DCC_TO_DC(dcc);
>
> -spice_return_val_if_fail(validate_surface(display, surface_id),
> FALSE);
> +spice_return_val_if_fail(display_channel_validate_surface(displ
> ay, surface_id), FALSE);
>
> -surface = &display->surfaces[surface_id];
> +surface = &display->priv->surfaces[surface_id];
> surface_lossy_region = &dcc->priv-
> >surface_client_lossy_region[surface_id];
>
> if (!area) {
> @@ -208,13 +208,13 @@ static void
> red_display_add_image_to_pixmap_cache(RedChannelClient *rcc,
> io_image->descriptor.flags |=
> SPICE_IMAGE_FLAGS_CACHE_ME;
> dcc->priv->send_data.pixmap_cache_items[dcc->priv-
> >send_data.num_pixmap_cache_items++] =
>
> image->descriptor.id;
> -stat_inc_counter(reds, display_channel-
> >add_to_cache_counter, 1);
> +stat_inc_counter(reds, display_channel->priv-
> >add_to_cache_counter, 1);
> }
> }
> }
>
> if (!(io_image->descriptor.flags & SPICE_IMAGE_FLAGS_CACHE_ME))
> {
> -stat_inc_counter(reds, display_channel->non_cache_counter,
> 1);
> +stat_inc_counter(reds, display_channel->priv-
> >non_cache_counter, 1);
> }
> }
>
> @@ -385,7 +385,7 @@ static FillBitsType
> fill_bits(DisplayChannelClient *dcc, SpiceMarshaller *m,
> dcc->priv->send_data.pixmap_cache_items[dcc->priv-
> >send_data.num_pixmap_cache_items++] =
> image.descriptor.id;
> if (can_lossy || !lossy_cache_item) {
> -if (!display->enable_jpeg || lossy_cache_item) {
> +if (!display->priv->enable_jpeg ||
> lossy_cache_item) {
> image.descriptor.type =
> SPICE_IMAGE_TYPE_FROM_CACHE;
> } else {
> // making sure, in multiple monitor scenario,
> that lossy items that
> @@ -397,7 +397,7 @@ static FillBitsType
> fill_bits(DisplayChannelClient *dcc, SpiceMarshaller *m,
> &bitmap_palette_out,
> &lzplt_palette_out);
> spice_assert(bitmap_palette_out == NULL);
> spice_assert(lzplt_palette_out == NULL);
> -stat_inc_counter(reds, display->cache_hits_counter,
> 1);
> +stat_inc_counter(reds, display->priv-
> >cache_hits_counter, 1);
> pthread_mutex_unlock(&dcc->priv->pixmap_cache-
> >lock);
> return FILL_BITS_TYPE_CACHE;
> } else {
> @@ -414,13 +414,13 @@ static FillBitsType
> fill_bits(DisplayChannelClient *dcc, SpiceMarshaller *m,
> RedSurface *surface;
>
> surface_id = simage->u.surface.surface_id;
> -if (!validate_surface(display, surface_id)) {
> +if (!display_channel_validate_surface(display, surface_id))
> {
> spice_warning("Invalid surface in
> SPICE_IMAGE_TYPE_SURFACE");
> pthread_mutex_unlock(&dcc->priv->pixmap_cache->lock);
> return FILL_BITS_TYPE_SURFACE;
> }
>
> -surface = &display->surfaces[surface_id];
> +surface = &display->priv->surfaces[surface_id];
> image.descriptor.type = SPICE_IMAGE_TYPE_SURFACE;
> image.descriptor.flags = 0;
> image.descriptor.width = surface->context.width;
> @@ -1706,7 +1706,7 @@ static int
> red_marshall_stream_data(RedChannelClient *rcc,
> return FALSE;
> }
>
> -StreamAgent *agent = &dcc->priv-
> >stream_agents[get_stream_id(display, stream)];
> +StreamAgent *agent
Re: [Spice-devel] [PATCH v2 2/9] Add CursorChannelPrivate struct
On Wed, 2016-09-14 at 11:53 -0500, Jonathon Jongsma wrote:
> Encapsulate private data of CursorChannel in a private struct. This
> isn't very useful at the moment, but it will help prepare the way
> for
> porting the RedChannel heirarchy to GObject.
Acked-by: Pavel Grunt
> ---
> Changes in v2:
> - use priv[1] to avoid memory leak
>
> server/cursor-channel.c | 57 +++---
> ---
> 1 file changed, 31 insertions(+), 26 deletions(-)
>
> diff --git a/server/cursor-channel.c b/server/cursor-channel.c
> index 080b22b..52c8e2d 100644
> --- a/server/cursor-channel.c
> +++ b/server/cursor-channel.c
> @@ -47,9 +47,8 @@ typedef struct RedCursorPipeItem {
> CursorItem *cursor_item;
> } RedCursorPipeItem;
>
> -struct CursorChannel {
> -CommonGraphicsChannel common; // Must be the first thing
> -
> +typedef struct CursorChannelPrivate CursorChannelPrivate;
> +struct CursorChannelPrivate {
> CursorItem *item;
> int cursor_visible;
> SpicePoint16 cursor_position;
> @@ -62,6 +61,12 @@ struct CursorChannel {
> #endif
> };
>
> +struct CursorChannel {
> +CommonGraphicsChannel common; // Must be the first thing
> +
> +CursorChannelPrivate priv[1];
> +};
> +
> static void cursor_pipe_item_free(RedPipeItem *pipe_item);
>
> static CursorItem *cursor_item_new(QXLInstance *qxl, RedCursorCmd
> *cmd)
> @@ -108,10 +113,10 @@ static void cursor_item_unref(CursorItem
> *item)
>
> static void cursor_set_item(CursorChannel *cursor, CursorItem
> *item)
> {
> -if (cursor->item)
> -cursor_item_unref(cursor->item);
> +if (cursor->priv->item)
> +cursor_item_unref(cursor->priv->item);
>
> -cursor->item = item ? cursor_item_ref(item) : NULL;
> +cursor->priv->item = item ? cursor_item_ref(item) : NULL;
> }
>
> static RedPipeItem *new_cursor_pipe_item(RedChannelClient *rcc,
> void *data, int num)
> @@ -202,12 +207,12 @@ static void
> red_marshall_cursor_init(CursorChannelClient *ccc, SpiceMarshaller *
> cursor_channel =
> (CursorChannel*)red_channel_client_get_channel(rcc);
>
> red_channel_client_init_send_data(rcc, SPICE_MSG_CURSOR_INIT,
> NULL);
> -msg.visible = cursor_channel->cursor_visible;
> -msg.position = cursor_channel->cursor_position;
> -msg.trail_length = cursor_channel->cursor_trail_length;
> -msg.trail_frequency = cursor_channel->cursor_trail_frequency;
> +msg.visible = cursor_channel->priv->cursor_visible;
> +msg.position = cursor_channel->priv->cursor_position;
> +msg.trail_length = cursor_channel->priv->cursor_trail_length;
> +msg.trail_frequency = cursor_channel->priv-
> >cursor_trail_frequency;
>
> -cursor_fill(ccc, &msg.cursor, cursor_channel->item, &info);
> +cursor_fill(ccc, &msg.cursor, cursor_channel->priv->item,
> &info);
> spice_marshall_msg_cursor_init(base_marshaller, &msg);
> add_buf_from_info(base_marshaller, &info);
> }
> @@ -242,7 +247,7 @@ static void cursor_marshall(CursorChannelClient
> *ccc,
>
> red_channel_client_init_send_data(rcc,
> SPICE_MSG_CURSOR_SET, pipe_item);
> cursor_set.position = cmd->u.set.position;
> -cursor_set.visible = cursor_channel->cursor_visible;
> +cursor_set.visible = cursor_channel->priv-
> >cursor_visible;
>
> cursor_fill(ccc, &cursor_set.cursor, item, &info);
> spice_marshall_msg_cursor_set(m, &cursor_set);
> @@ -324,8 +329,8 @@ CursorChannel* cursor_channel_new(RedWorker
> *worker)
> &cbs,
> red_channel_client_handle_message);
>
> cursor_channel = (CursorChannel *)channel;
> -cursor_channel->cursor_visible = TRUE;
> -cursor_channel->mouse_mode = SPICE_MOUSE_MODE_SERVER;
> +cursor_channel->priv->cursor_visible = TRUE;
> +cursor_channel->priv->mouse_mode = SPICE_MOUSE_MODE_SERVER;
>
> return cursor_channel;
> }
> @@ -342,20 +347,20 @@ void cursor_channel_process_cmd(CursorChannel
> *cursor, RedCursorCmd *cursor_cmd)
>
> switch (cursor_cmd->type) {
> case QXL_CURSOR_SET:
> -cursor->cursor_visible = cursor_cmd->u.set.visible;
> +cursor->priv->cursor_visible = cursor_cmd->u.set.visible;
> cursor_set_item(cursor, cursor_item);
> break;
> case QXL_CURSOR_MOVE:
> -cursor_show = !cursor->cursor_visible;
> -cursor->cursor_visible = TRUE;
> -cursor->cursor_position = cursor_cmd->u.position;
> +cursor_show = !cursor->priv->cursor_visible;
> +cursor->priv->cursor_visible = TRUE;
> +cursor->priv->cursor_position = cursor_cmd->u.position;
> break;
> case QXL_CURSOR_HIDE:
> -cursor->cursor_visible = FALSE;
> +cursor->priv->cursor_visible = FALSE;
> break;
> case QXL_CURSOR_TRAIL:
> -cursor->cursor_trail_length = cursor_cmd->u.trail.length;
> -cursor->cursor_trail_frequency = cursor_cmd-
> >u.trail.frequency;
Re: [Spice-devel] spice performance tweaking
Hello, The performance fixes sound awesome. I'm afraid I cannot test them in the environment with the low bandwith setup soon (maybe next week, but don't hold your breath) I'm going to build a local test setup to see if this is improving some of the issues. I got a laptop running the rhel7.3 beta, let's see how that performs. Rob Verduijn 2016-09-15 13:31 GMT+02:00 Frediano Ziglio : > I saw you are using CentOS 7. I built the package with RHEL 7 (they are > binary compatible). > About the testing just which normal usage you should see improvements in > bandwidth and > reactivity. > > Changes from current CentOS package: > - used a newer version, there are couple of changes that decrease latency; > - additional patches to improve bandwidth usage (for small drawing this > should decrease > bandwidth usage by a 15-20%); > - additional patch to decrease a bandwidth limitation due to a peculiar > half-duplex usage > of spice protocol (this is clearly visible with high latency > connections); > - additional patch to decrease packet fragmentation due to TCP_NODELAY > usage. > > Alternatively would be helpful for us to get a local reproduction of the > problem. > OpenVPN configuration files would be helpful (we don't need any security > detail like keys, ip, host or system names, just to understand the type of > VPN, > encryption parameters, compression, additional latency introduced and so > on). > > The fact that you are not able to get a record from the guest means that > the QXL > (guest <-> server) protocol how the spice-server is handling guest command > is > fine. The fact that on the client you can see clearly such slowness is due > to spice > protocol, the connection/vpn, some spice-server implementation and possibly > client implementation too. Unfortunately too much stuff to be able to > point the > finger to one of them. > I tried some test and did this: > - opened task manager on Windows 7; > - switched to performance tab; > - maximized task manager; > - double clicked on CPU usage to get only CPU usage and history. > When CPU usage change I can see the flickering on CPU usage but not on > the history graphs. It this the kind of flickering you are noticing? > > Frediano > > > For which distro is that package ? > Centos 7.2 ? rhel7.3beta or fedora24 ? > > Rob Verduijn > > 2016-09-14 15:59 GMT+02:00 Frediano Ziglio : > >> Could you test at least? Would be very helpful. We could then backport >> some improvements. >> >> Frediano >> >> >> thanx,I'll stick with the centos packages, >> >> I need a very good reason before I start using beta packages. >> And a nice to have feature is not one of them. >> >> Also I dug in to the openvpn tweaks and it seems that all of them are >> related to udp tunnels. >> Performance is sadly rather low when you have to use tcp (like me) >> because the firewall is managed by a third party. >> >> Rob Verduijn >> >> 2016-09-14 15:49 GMT+02:00 Frediano Ziglio : >> >>> >>> Hello, >>> >>> I'm trying to improve my spice performance on a kvm host/guest. >>> It's currently rather slow and I can see screens beeing build up, and >>> delays when draging windows. >>> >>> It's being tunneled through openvpn, which is set to use tcp. >>> tcp required because of the firewall which is maintained by 3rd party. >>> >>> I have full access to the kvm host, kvm guest and openvpn server. >>> >>> Have you got any tips so that I can improve spice performance ? >>> I alrready am running tuned with the virtual-guest profile for guests >>> and host profile for the host. >>> All systems are runnning CentOS 7 >>> >>> Any tips for : >>> - the KVM host ? >>> - the KVM guest ? >>> - the openvpn server ? >>> >>> Cheers >>> Rob Verduijn >>> >>> Hi, >>> can you try version at https://www.datafilehost.com/d/b07f008e ? >>> >>> The sha1 hash (please check it) is 0e2191c363e109475aeb2bff401e69 >>> 9f0a07a795. >>> >>> Be prepare for the rollback, it's not a version meant for production >>> usage. >>> >>> Frediano >>> >>> >> >> > > ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH spice-common] proto: Add agent features message
Hi - Original Message - > Hi, > > On Thu, Sep 15, 2016 at 09:01:30AM -0400, Marc-André Lureau wrote: > > Hi > > > > - Original Message - > > > Hi, > > > > > > On Thu, Sep 15, 2016 at 08:18:38AM -0400, Marc-André Lureau wrote: > > > > > > Actually, there is agent capabilities, I think that's what the > > > > > > server should be overriding instead. > > > > > > > > > > I know that is possible but imo it is hack. It would be needed to > > > > > filter VD_AGENT_ANNOUNCE_CAPABILITIES from the agent, insert > > > > > something > > > > > like VD_AGENT_CAP_FILE_XFER_DISABLED, > > > > > VD_AGENT_CAP_COPY_PASTE_DISABLED > > > > > and also in the case that the filter is changed on the fly, it would > > > > > be needed to generate complete VD_AGENT_ANNOUNCE_CAPABILITIES (or > > > > > request agent to send them). I think it would be more complicated... > > > > > > > > I don't think it is so complicated, but I might be wrong. The server > > > > already parses some agents messages for filtering. > > > > > > > > At least I think it would be cleaner from the protocol POV. I don't > > > > see much benefit for the client to know that the server disabled > > > > something explicitely vs the agent not having the capability. > > > > > > I think we could do some distinction about agent capabilities and > > > features that are disabled on host and for that reason I think this > > > message makes sense from protocol POV. > > > > What differences does that make from a client? Do you think it helps > > much for the client to say "the feature foo has been disabled by the > > server" vs "the feature foo is not available"? > > Well, with "feature is disabled" user can request a sysadmin to enable > it while with "feature not available" only, user will spam us saying > that vdagent is running fine, why it does not work? :) Features could also be disabled from the agent itself (although we lack configuration for that), or by the client. Ie, there are many reasons why a feature may not be available. It would be endless to extend the protocol to teach all the ways :) > > > The are already many caps: common caps, channel caps, vdagent caps.. > > Does the protocol need to grow yet another "agent_features"? That > > really seems redundant with vdagent caps. > > As you said, it *can* be implemented with capabilities but that changes > the meaning of it. Agent is capable of doing file-transfer but the fact > that it is disabled by QEMU should be handled differently, IMHO. To me it doesn't change the meaning, it simply states it's not available, not the reason why. > AFAICT, what happens today is that client tries to file-transfer even if > file-transfer is disabled on server. Server ignores it. We could reply > with some server message saying that feature is disabled? We would still > need to improve protocol for that, I think. I think that would be an interesting extension (perhaps we already have such error replies?) But it's still better to state upfront the the feature is unavailable. ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH spice-common] proto: Add agent features message
Hi, On Thu, Sep 15, 2016 at 09:01:30AM -0400, Marc-André Lureau wrote: > Hi > > - Original Message - > > Hi, > > > > On Thu, Sep 15, 2016 at 08:18:38AM -0400, Marc-André Lureau wrote: > > > > > Actually, there is agent capabilities, I think that's what the > > > > > server should be overriding instead. > > > > > > > > I know that is possible but imo it is hack. It would be needed to > > > > filter VD_AGENT_ANNOUNCE_CAPABILITIES from the agent, insert something > > > > like VD_AGENT_CAP_FILE_XFER_DISABLED, VD_AGENT_CAP_COPY_PASTE_DISABLED > > > > and also in the case that the filter is changed on the fly, it would > > > > be needed to generate complete VD_AGENT_ANNOUNCE_CAPABILITIES (or > > > > request agent to send them). I think it would be more complicated... > > > > > > I don't think it is so complicated, but I might be wrong. The server > > > already parses some agents messages for filtering. > > > > > > At least I think it would be cleaner from the protocol POV. I don't > > > see much benefit for the client to know that the server disabled > > > something explicitely vs the agent not having the capability. > > > > I think we could do some distinction about agent capabilities and > > features that are disabled on host and for that reason I think this > > message makes sense from protocol POV. > > What differences does that make from a client? Do you think it helps > much for the client to say "the feature foo has been disabled by the > server" vs "the feature foo is not available"? Well, with "feature is disabled" user can request a sysadmin to enable it while with "feature not available" only, user will spam us saying that vdagent is running fine, why it does not work? :) > The are already many caps: common caps, channel caps, vdagent caps.. > Does the protocol need to grow yet another "agent_features"? That > really seems redundant with vdagent caps. As you said, it *can* be implemented with capabilities but that changes the meaning of it. Agent is capable of doing file-transfer but the fact that it is disabled by QEMU should be handled differently, IMHO. AFAICT, what happens today is that client tries to file-transfer even if file-transfer is disabled on server. Server ignores it. We could reply with some server message saying that feature is disabled? We would still need to improve protocol for that, I think. toso signature.asc Description: PGP signature ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH spice-common] proto: Add agent features message
Hi - Original Message - > Hi, > > On Thu, Sep 15, 2016 at 08:18:38AM -0400, Marc-André Lureau wrote: > > > > Actually, there is agent capabilities, I think that's what the > > > > server should be overriding instead. > > > > > > I know that is possible but imo it is hack. It would be needed to > > > filter VD_AGENT_ANNOUNCE_CAPABILITIES from the agent, insert something > > > like VD_AGENT_CAP_FILE_XFER_DISABLED, VD_AGENT_CAP_COPY_PASTE_DISABLED > > > and also in the case that the filter is changed on the fly, it would > > > be needed to generate complete VD_AGENT_ANNOUNCE_CAPABILITIES (or > > > request agent to send them). I think it would be more complicated... > > > > I don't think it is so complicated, but I might be wrong. The server > > already parses some agents messages for filtering. > > > > At least I think it would be cleaner from the protocol POV. I don't > > see much benefit for the client to know that the server disabled > > something explicitely vs the agent not having the capability. > > I think we could do some distinction about agent capabilities and > features that are disabled on host and for that reason I think this > message makes sense from protocol POV. What differences does that make from a client? Do you think it helps much for the client to say "the feature foo has been disabled by the server" vs "the feature foo is not available"? The are already many caps: common caps, channel caps, vdagent caps.. Does the protocol need to grow yet another "agent_features"? That really seems redundant with vdagent caps. ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH spice-common] proto: Add agent features message
Hi, On Thu, Sep 15, 2016 at 08:18:38AM -0400, Marc-André Lureau wrote: > > > Actually, there is agent capabilities, I think that's what the > > > server should be overriding instead. > > > > I know that is possible but imo it is hack. It would be needed to > > filter VD_AGENT_ANNOUNCE_CAPABILITIES from the agent, insert something > > like VD_AGENT_CAP_FILE_XFER_DISABLED, VD_AGENT_CAP_COPY_PASTE_DISABLED > > and also in the case that the filter is changed on the fly, it would > > be needed to generate complete VD_AGENT_ANNOUNCE_CAPABILITIES (or > > request agent to send them). I think it would be more complicated... > > I don't think it is so complicated, but I might be wrong. The server > already parses some agents messages for filtering. > > At least I think it would be cleaner from the protocol POV. I don't > see much benefit for the client to know that the server disabled > something explicitely vs the agent not having the capability. I think we could do some distinction about agent capabilities and features that are disabled on host and for that reason I think this message makes sense from protocol POV. Although, this is more host features then guest/agent features? toso signature.asc Description: PGP signature ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH spice-common] proto: Add agent features message
On Thu, 2016-09-15 at 08:18 -0400, Marc-André Lureau wrote:
> Hi
>
> - Original Message -
> > Hi Marc-André,
> >
> > On Thu, 2016-09-15 at 06:28 -0400, Marc-André Lureau wrote:
> > > Hi
> > >
> > > - Original Message -
> > > > Hi
> > > >
> > > > - Original Message -
> > > > > The message is sent by server to the client to indicate
> > > > > which agent features are enabled or disabled by using flags.
> > > > >
> > > > > If a flag is set, then the corresponding feature is enabled.
> > > > >
> > > > > The message currently supports info about copy & paste and
> > > > > file
> > > > > transfer.
> > > >
> > > >
> > > >
> > > > Have you considered using channel capabilities? it seems to me
> > > > it
> > > > would fit
> > > > nicely, beside there is already APIs, and the flag are not
> > > > limited
> > > > to 32
> > > > bits.
> > > >
> >
> >
> > Yes, I have, but the channel caps are only sent once, no? The
> > filter
> > can be changed on the fly by calling
> > spice_server_set_agent_file_xfer() etc. and client should be
> > informed
> > about that.
> >
>
>
> ok
>
> > >
> > >
> > > Actually, there is agent capabilities, I think that's what the
> > > server should be overriding instead.
> >
> >
> > I know that is possible but imo it is hack. It would be needed to
> > filter VD_AGENT_ANNOUNCE_CAPABILITIES from the agent, insert
> > something
> > like VD_AGENT_CAP_FILE_XFER_DISABLED, VD_AGENT_CAP_COPY_PASTE_DISA
> > BLED
> > and also in the case that the filter is changed on the fly, it
> > would
> > be needed to generate complete VD_AGENT_ANNOUNCE_CAPABILITIES (or
> > request agent to send them). I think it would be more
> > complicated...
>
>
> I don't think it is so complicated, but I might be wrong. The server
> already parses some agents messages for filtering.
It would just need to catch VD_AGENT_ANNOUNCE_CAPABILITIES and modify
it. And when the stuff changes on the fly, server should create the
agent messages
>
> At least I think it would be cleaner from the protocol POV.
It would be the agent capability set by the server, it doesn't sound
clean.
> I don't see much benefit for the client to know that the server
> disabled something explicitely vs the agent not having the
> capability.
>
I will think more about it,
Pavel
> >
> > Thanks,
> > Pavel
> >
> > >
> > > > >
> > > > > Related:
> > > > > https://bugzilla.redhat.com/show_bug.cgi?id=1373725
> > > > > ---
> > > > > common/messages.h | 4
> > > > > configure.ac | 2 +-
> > > > > spice.proto | 9 +
> > > > > 3 files changed, 14 insertions(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/common/messages.h b/common/messages.h
> > > > > index 516a345..f8648b6 100644
> > > > > --- a/common/messages.h
> > > > > +++ b/common/messages.h
> > > > > @@ -99,6 +99,10 @@ typedef struct
> > > > > SpiceMsgMainMigrateBeginSeamless {
> > > > > uint32_t src_mig_version;
> > > > > } SpiceMsgMainMigrateBeginSeamless;
> > > > >
> > > > > +typedef struct SpiceMsgMainAgentFeatures {
> > > > > +uint32_t flags;
> > > > > +} SpiceMsgMainAgentFeatures;
> > > > > +
> > > > > typedef struct SpiceMsgcMainMigrateDstDoSeamless {
> > > > > uint32_t src_version;
> > > > > } SpiceMsgcMainMigrateDstDoSeamless;
> > > > > diff --git a/configure.ac b/configure.ac
> > > > > index c3ad5a4..c0e49b7 100644
> > > > > --- a/configure.ac
> > > > > +++ b/configure.ac
> > > > > @@ -28,7 +28,7 @@ AM_PROG_CC_C_O
> > > > > SPICE_CHECK_SYSDEPS
> > > > >
> > > > > # Checks for libraries
> > > > > -PKG_CHECK_MODULES([PROTOCOL], [spice-protocol >= 0.12.12])
> > > > > +PKG_CHECK_MODULES([PROTOCOL], [spice-protocol >= 0.12.13])
> > > > >
> > > > > SPICE_CHECK_PYTHON_MODULES()
> > > > >
> > > > > diff --git a/spice.proto b/spice.proto
> > > > > index 0bfc515..ea4dfc2 100644
> > > > > --- a/spice.proto
> > > > > +++ b/spice.proto
> > > > > @@ -226,6 +226,11 @@ struct DstInfo {
> > > > > uint8 *cert_subject_data[cert_subject_size]
> > > > > @zero_terminated @marshall;
> > > > > } @ctype(SpiceMigrationDstInfo);
> > > > >
> > > > > +flags32 agent_features_flags {
> > > > > +COPY_PASTE,
> > > > > +FILE_TRANSFER
> > > > > +} @prefix(SPICE_AGENT_FEATURE_);
> > > > > +
> > > > > channel MainChannel : BaseChannel {
> > > > > server:
> > > > > message {
> > > > > @@ -303,6 +308,10 @@ channel MainChannel : BaseChannel {
> > > > > Empty migrate_dst_seamless_ack;
> > > > > Empty migrate_dst_seamless_nack;
> > > > >
> > > > > +message {
> > > > > +agent_features_flags flags;
> > > > > +} agent_features;
> > > > > +
> > > > > client:
> > > > > message {
> > > > > uint64 cache_size;
> > > > > --
> > > > > 2.10.0
> > > > >
> > > > > ___
> > > > > Spice-devel mailing list
> > > > > Spice-devel@lists.freedesktop.org
> > > > > https://lists.freedesktop.org/mailman/listinfo/spice-devel
> > > > >
> > > >
> > >
Re: [Spice-devel] [PATCH spice-common] proto: Add agent features message
Hi
- Original Message -
> Hi Marc-André,
>
> On Thu, 2016-09-15 at 06:28 -0400, Marc-André Lureau wrote:
> > Hi
> >
> > - Original Message -
> > > Hi
> > >
> > > - Original Message -
> > > > The message is sent by server to the client to indicate
> > > > which agent features are enabled or disabled by using flags.
> > > >
> > > > If a flag is set, then the corresponding feature is enabled.
> > > >
> > > > The message currently supports info about copy & paste and file
> > > > transfer.
> > >
> > >
> > > Have you considered using channel capabilities? it seems to me it
> > > would fit
> > > nicely, beside there is already APIs, and the flag are not limited
> > > to 32
> > > bits.
> > >
>
> Yes, I have, but the channel caps are only sent once, no? The filter
> can be changed on the fly by calling
> spice_server_set_agent_file_xfer() etc. and client should be informed
> about that.
>
ok
> >
> >
> > Actually, there is agent capabilities, I think that's what the
> > server should be overriding instead.
>
> I know that is possible but imo it is hack. It would be needed to
> filter VD_AGENT_ANNOUNCE_CAPABILITIES from the agent, insert something
> like VD_AGENT_CAP_FILE_XFER_DISABLED, VD_AGENT_CAP_COPY_PASTE_DISABLED
> and also in the case that the filter is changed on the fly, it would
> be needed to generate complete VD_AGENT_ANNOUNCE_CAPABILITIES (or
> request agent to send them). I think it would be more complicated...
I don't think it is so complicated, but I might be wrong. The server already
parses some agents messages for filtering.
At least I think it would be cleaner from the protocol POV. I don't see much
benefit for the client to know that the server disabled something explicitely
vs the agent not having the capability.
>
> Thanks,
> Pavel
>
> >
> > > >
> > > > Related:
> > > > https://bugzilla.redhat.com/show_bug.cgi?id=1373725
> > > > ---
> > > > common/messages.h | 4
> > > > configure.ac | 2 +-
> > > > spice.proto | 9 +
> > > > 3 files changed, 14 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/common/messages.h b/common/messages.h
> > > > index 516a345..f8648b6 100644
> > > > --- a/common/messages.h
> > > > +++ b/common/messages.h
> > > > @@ -99,6 +99,10 @@ typedef struct
> > > > SpiceMsgMainMigrateBeginSeamless {
> > > > uint32_t src_mig_version;
> > > > } SpiceMsgMainMigrateBeginSeamless;
> > > >
> > > > +typedef struct SpiceMsgMainAgentFeatures {
> > > > +uint32_t flags;
> > > > +} SpiceMsgMainAgentFeatures;
> > > > +
> > > > typedef struct SpiceMsgcMainMigrateDstDoSeamless {
> > > > uint32_t src_version;
> > > > } SpiceMsgcMainMigrateDstDoSeamless;
> > > > diff --git a/configure.ac b/configure.ac
> > > > index c3ad5a4..c0e49b7 100644
> > > > --- a/configure.ac
> > > > +++ b/configure.ac
> > > > @@ -28,7 +28,7 @@ AM_PROG_CC_C_O
> > > > SPICE_CHECK_SYSDEPS
> > > >
> > > > # Checks for libraries
> > > > -PKG_CHECK_MODULES([PROTOCOL], [spice-protocol >= 0.12.12])
> > > > +PKG_CHECK_MODULES([PROTOCOL], [spice-protocol >= 0.12.13])
> > > >
> > > > SPICE_CHECK_PYTHON_MODULES()
> > > >
> > > > diff --git a/spice.proto b/spice.proto
> > > > index 0bfc515..ea4dfc2 100644
> > > > --- a/spice.proto
> > > > +++ b/spice.proto
> > > > @@ -226,6 +226,11 @@ struct DstInfo {
> > > > uint8 *cert_subject_data[cert_subject_size]
> > > > @zero_terminated @marshall;
> > > > } @ctype(SpiceMigrationDstInfo);
> > > >
> > > > +flags32 agent_features_flags {
> > > > +COPY_PASTE,
> > > > +FILE_TRANSFER
> > > > +} @prefix(SPICE_AGENT_FEATURE_);
> > > > +
> > > > channel MainChannel : BaseChannel {
> > > > server:
> > > > message {
> > > > @@ -303,6 +308,10 @@ channel MainChannel : BaseChannel {
> > > > Empty migrate_dst_seamless_ack;
> > > > Empty migrate_dst_seamless_nack;
> > > >
> > > > +message {
> > > > +agent_features_flags flags;
> > > > +} agent_features;
> > > > +
> > > > client:
> > > > message {
> > > > uint64 cache_size;
> > > > --
> > > > 2.10.0
> > > >
> > > > ___
> > > > Spice-devel mailing list
> > > > Spice-devel@lists.freedesktop.org
> > > > https://lists.freedesktop.org/mailman/listinfo/spice-devel
> > > >
> > >
> > > ___
> > > Spice-devel mailing list
> > > Spice-devel@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/spice-devel
> > >
> ___
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
>
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH spice-common] proto: Add agent features message
Hi Marc-André,
On Thu, 2016-09-15 at 06:28 -0400, Marc-André Lureau wrote:
> Hi
>
> - Original Message -
> > Hi
> >
> > - Original Message -
> > > The message is sent by server to the client to indicate
> > > which agent features are enabled or disabled by using flags.
> > >
> > > If a flag is set, then the corresponding feature is enabled.
> > >
> > > The message currently supports info about copy & paste and file
> > > transfer.
> >
> >
> > Have you considered using channel capabilities? it seems to me it
> > would fit
> > nicely, beside there is already APIs, and the flag are not limited
> > to 32
> > bits.
> >
Yes, I have, but the channel caps are only sent once, no? The filter
can be changed on the fly by calling
spice_server_set_agent_file_xfer() etc. and client should be informed
about that.
>
>
> Actually, there is agent capabilities, I think that's what the
> server should be overriding instead.
I know that is possible but imo it is hack. It would be needed to
filter VD_AGENT_ANNOUNCE_CAPABILITIES from the agent, insert something
like VD_AGENT_CAP_FILE_XFER_DISABLED, VD_AGENT_CAP_COPY_PASTE_DISABLED
and also in the case that the filter is changed on the fly, it would
be needed to generate complete VD_AGENT_ANNOUNCE_CAPABILITIES (or
request agent to send them). I think it would be more complicated...
Thanks,
Pavel
>
> > >
> > > Related:
> > > https://bugzilla.redhat.com/show_bug.cgi?id=1373725
> > > ---
> > > common/messages.h | 4
> > > configure.ac | 2 +-
> > > spice.proto | 9 +
> > > 3 files changed, 14 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/common/messages.h b/common/messages.h
> > > index 516a345..f8648b6 100644
> > > --- a/common/messages.h
> > > +++ b/common/messages.h
> > > @@ -99,6 +99,10 @@ typedef struct
> > > SpiceMsgMainMigrateBeginSeamless {
> > > uint32_t src_mig_version;
> > > } SpiceMsgMainMigrateBeginSeamless;
> > >
> > > +typedef struct SpiceMsgMainAgentFeatures {
> > > +uint32_t flags;
> > > +} SpiceMsgMainAgentFeatures;
> > > +
> > > typedef struct SpiceMsgcMainMigrateDstDoSeamless {
> > > uint32_t src_version;
> > > } SpiceMsgcMainMigrateDstDoSeamless;
> > > diff --git a/configure.ac b/configure.ac
> > > index c3ad5a4..c0e49b7 100644
> > > --- a/configure.ac
> > > +++ b/configure.ac
> > > @@ -28,7 +28,7 @@ AM_PROG_CC_C_O
> > > SPICE_CHECK_SYSDEPS
> > >
> > > # Checks for libraries
> > > -PKG_CHECK_MODULES([PROTOCOL], [spice-protocol >= 0.12.12])
> > > +PKG_CHECK_MODULES([PROTOCOL], [spice-protocol >= 0.12.13])
> > >
> > > SPICE_CHECK_PYTHON_MODULES()
> > >
> > > diff --git a/spice.proto b/spice.proto
> > > index 0bfc515..ea4dfc2 100644
> > > --- a/spice.proto
> > > +++ b/spice.proto
> > > @@ -226,6 +226,11 @@ struct DstInfo {
> > > uint8 *cert_subject_data[cert_subject_size]
> > > @zero_terminated @marshall;
> > > } @ctype(SpiceMigrationDstInfo);
> > >
> > > +flags32 agent_features_flags {
> > > +COPY_PASTE,
> > > +FILE_TRANSFER
> > > +} @prefix(SPICE_AGENT_FEATURE_);
> > > +
> > > channel MainChannel : BaseChannel {
> > > server:
> > > message {
> > > @@ -303,6 +308,10 @@ channel MainChannel : BaseChannel {
> > > Empty migrate_dst_seamless_ack;
> > > Empty migrate_dst_seamless_nack;
> > >
> > > +message {
> > > +agent_features_flags flags;
> > > +} agent_features;
> > > +
> > > client:
> > > message {
> > > uint64 cache_size;
> > > --
> > > 2.10.0
> > >
> > > ___
> > > Spice-devel mailing list
> > > Spice-devel@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/spice-devel
> > >
> >
> > ___
> > Spice-devel mailing list
> > Spice-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/spice-devel
> >
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] spice performance tweaking
I saw you are using CentOS 7. I built the package with RHEL 7 (they are binary compatible). About the testing just which normal usage you should see improvements in bandwidth and reactivity. Changes from current CentOS package: - used a newer version, there are couple of changes that decrease latency; - additional patches to improve bandwidth usage (for small drawing this should decrease bandwidth usage by a 15-20%); - additional patch to decrease a bandwidth limitation due to a peculiar half-duplex usage of spice protocol (this is clearly visible with high latency connections); - additional patch to decrease packet fragmentation due to TCP_NODELAY usage. Alternatively would be helpful for us to get a local reproduction of the problem. OpenVPN configuration files would be helpful (we don't need any security detail like keys, ip, host or system names, just to understand the type of VPN, encryption parameters, compression, additional latency introduced and so on). The fact that you are not able to get a record from the guest means that the QXL (guest <-> server) protocol how the spice-server is handling guest command is fine. The fact that on the client you can see clearly such slowness is due to spice protocol, the connection/vpn, some spice-server implementation and possibly client implementation too. Unfortunately too much stuff to be able to point the finger to one of them. I tried some test and did this: - opened task manager on Windows 7; - switched to performance tab; - maximized task manager; - double clicked on CPU usage to get only CPU usage and history. When CPU usage change I can see the flickering on CPU usage but not on the history graphs. It this the kind of flickering you are noticing? Frediano > For which distro is that package ? > Centos 7.2 ? rhel7.3beta or fedora24 ? > Rob Verduijn > 2016-09-14 15:59 GMT+02:00 Frediano Ziglio < fzig...@redhat.com > : > > Could you test at least? Would be very helpful. We could then backport some > > improvements. > > > Frediano > > > > thanx,I'll stick with the centos packages, > > > > > > I need a very good reason before I start using beta packages. > > > > > > And a nice to have feature is not one of them. > > > > > > Also I dug in to the openvpn tweaks and it seems that all of them are > > > related > > > to udp tunnels. > > > > > > Performance is sadly rather low when you have to use tcp (like me) > > > because > > > the firewall is managed by a third party. > > > > > > Rob Verduijn > > > > > > 2016-09-14 15:49 GMT+02:00 Frediano Ziglio < fzig...@redhat.com > : > > > > > > > > Hello, > > > > > > > > > > > > > > > I'm trying to improve my spice performance on a kvm host/guest. > > > > > > > > > > > > > > > It's currently rather slow and I can see screens beeing build up, and > > > > > delays > > > > > when draging windows. > > > > > > > > > > > > > > > It's being tunneled through openvpn, which is set to use tcp. > > > > > > > > > > > > > > > tcp required because of the firewall which is maintained by 3rd > > > > > party. > > > > > > > > > > > > > > > I have full access to the kvm host, kvm guest and openvpn server. > > > > > > > > > > > > > > > Have you got any tips so that I can improve spice performance ? > > > > > > > > > > > > > > > I alrready am running tuned with the virtual-guest profile for guests > > > > > and > > > > > host profile for the host. > > > > > > > > > > > > > > > All systems are runnning CentOS 7 > > > > > > > > > > > > > > > Any tips for : > > > > > > > > > > > > > > > - the KVM host ? > > > > > > > > > > > > > > > - the KVM guest ? > > > > > > > > > > > > > > > - the openvpn server ? > > > > > > > > > > > > > > > Cheers > > > > > > > > > > > > > > > Rob Verduijn > > > > > > > > > > > > > > Hi, > > > > > > > > > > can you try version at https://www.datafilehost.com/d/b07f008e ? > > > > > > > > > > The sha1 hash (please check it) is > > > > 0e2191c363e109475aeb2bff401e699f0a07a795. > > > > > > > > > > Be prepare for the rollback, it's not a version meant for production > > > > usage. > > > > > > > > > > Frediano > > > > > > ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [vdagent-linux v1 3/3] build-sys: move user/system to respective dir
Hi,
On Thu, Sep 15, 2016 at 06:56:06AM -0400, Frediano Ziglio wrote:
> >
> > From: Marc-André Lureau
> >
> > Signed-off-by: Victor Toso
> > ---
> > Makefile.am | 59
> > +---
> > src/{ => vdagent}/vdagent-audio.c| 0
> > src/{ => vdagent}/vdagent-audio.h| 0
> > src/{ => vdagent}/vdagent-file-xfers.c | 0
> > src/{ => vdagent}/vdagent-file-xfers.h | 0
> > src/{ => vdagent}/vdagent-x11-priv.h | 0
> > src/{ => vdagent}/vdagent-x11-randr.c| 0
> > src/{ => vdagent}/vdagent-x11.c | 0
> > src/{ => vdagent}/vdagent-x11.h | 0
> > src/{ => vdagent}/vdagent.c | 0
> > src/{ => vdagentd}/console-kit.c | 0
> > src/{ => vdagentd}/dummy-session-info.c | 0
> > src/{ => vdagentd}/session-info.h| 0
> > src/{ => vdagentd}/systemd-login.c | 0
> > src/{ => vdagentd}/vdagent-virtio-port.c | 0
> > src/{ => vdagentd}/vdagent-virtio-port.h | 0
> > src/{ => vdagentd}/vdagentd-uinput.c | 0
> > src/{ => vdagentd}/vdagentd-uinput.h | 0
> > src/{ => vdagentd}/vdagentd-xorg-conf.c | 0
> > src/{ => vdagentd}/vdagentd-xorg-conf.h | 0
> > src/{ => vdagentd}/vdagentd.c| 0
> > 21 files changed, 32 insertions(+), 27 deletions(-)
> > rename src/{ => vdagent}/vdagent-audio.c (100%)
> > rename src/{ => vdagent}/vdagent-audio.h (100%)
> > rename src/{ => vdagent}/vdagent-file-xfers.c (100%)
> > rename src/{ => vdagent}/vdagent-file-xfers.h (100%)
> > rename src/{ => vdagent}/vdagent-x11-priv.h (100%)
> > rename src/{ => vdagent}/vdagent-x11-randr.c (100%)
> > rename src/{ => vdagent}/vdagent-x11.c (100%)
> > rename src/{ => vdagent}/vdagent-x11.h (100%)
> > rename src/{ => vdagent}/vdagent.c (100%)
> > rename src/{ => vdagentd}/console-kit.c (100%)
> > rename src/{ => vdagentd}/dummy-session-info.c (100%)
> > rename src/{ => vdagentd}/session-info.h (100%)
> > rename src/{ => vdagentd}/systemd-login.c (100%)
> > rename src/{ => vdagentd}/vdagent-virtio-port.c (100%)
> > rename src/{ => vdagentd}/vdagent-virtio-port.h (100%)
> > rename src/{ => vdagentd}/vdagentd-uinput.c (100%)
> > rename src/{ => vdagentd}/vdagentd-uinput.h (100%)
> > rename src/{ => vdagentd}/vdagentd-xorg-conf.c (100%)
> > rename src/{ => vdagentd}/vdagentd-xorg-conf.h (100%)
> > rename src/{ => vdagentd}/vdagentd.c (100%)
> >
> > diff --git a/Makefile.am b/Makefile.am
> > index 9bc9dd8..33518a0 100644
> > --- a/Makefile.am
> > +++ b/Makefile.am
> > @@ -4,11 +4,19 @@ NULL =
> > bin_PROGRAMS = src/spice-vdagent
> > sbin_PROGRAMS = src/spice-vdagentd
> >
> > +common_sources = \
> > + src/udscs.c \
> > + src/udscs.h \
> > + src/vdagentd-proto-strings.h\
> > + src/vdagentd-proto.h\
> > + $(NULL)
> > +
> > src_spice_vdagent_CFLAGS = \
> > $(X_CFLAGS) \
> > $(SPICE_CFLAGS) \
> > $(GLIB2_CFLAGS) \
> > $(ALSA_CFLAGS) \
> > + -I$(srcdir)/src \
> > -DUDSCS_NO_SERVER
> >
> > src_spice_vdagent_LDADD = \
> > @@ -18,17 +26,16 @@ src_spice_vdagent_LDADD = \
> > $(ALSA_LIBS)
> >
> > src_spice_vdagent_SOURCES =\
> > - src/udscs.c \
> > - src/udscs.h \
> > - src/vdagent-audio.c \
> > - src/vdagent-audio.h \
> > - src/vdagent-file-xfers.c\
> > - src/vdagent-file-xfers.h\
> > - src/vdagent-x11-priv.h \
> > - src/vdagent-x11-randr.c \
> > - src/vdagent-x11.c \
> > - src/vdagent-x11.h \
> > - src/vdagent.c \
> > + $(common_sources) \
> > + src/vdagent/vdagent-audio.c \
> > + src/vdagent/vdagent-audio.h \
> > + src/vdagent/vdagent-file-xfers.c\
> > + src/vdagent/vdagent-file-xfers.h\
> > + src/vdagent/vdagent-x11-priv.h \
> > + src/vdagent/vdagent-x11-randr.c \
> > + src/vdagent/vdagent-x11.c \
> > + src/vdagent/vdagent-x11.h \
> > + src/vdagent/vdagent.c \
> > $(NULL)
> >
> > src_spice_vdagentd_CFLAGS =\
> > @@ -37,7 +44,8 @@ src_spice_vdagentd_CFLAGS = \
> > $(PCIACCESS_CFLAGS) \
> > $(SPICE_CFLAGS) \
> > $(GLIB2_CFLAGS) \
> > - $(PIE_CFLAGS)
> > + $(PIE_CFLAGS) \
> > + -I$(srcdir)/src
> >
> > src_spice_vdagentd_LDADD = \
> > $(DBUS_LIBS)
Re: [Spice-devel] [vdagent-linux v1 1/3] build-sys: reformat Makefile.am
Hi, On Thu, Sep 15, 2016 at 06:51:25AM -0400, Frediano Ziglio wrote: > > > > Makes the makefile more readable and easy to change in the next few > > commits. > > > > Based on Marc-André Lureau patch. > > --- > > Makefile.am | 94 > > + > > 1 file changed, 63 insertions(+), 31 deletions(-) > > > > diff --git a/Makefile.am b/Makefile.am > > index 680ef83..4739c44 100644 > > --- a/Makefile.am > > +++ b/Makefile.am > > @@ -4,24 +4,52 @@ NULL = > > bin_PROGRAMS = src/spice-vdagent > > sbin_PROGRAMS = src/spice-vdagentd > > > > -src_spice_vdagent_CFLAGS = $(X_CFLAGS) $(SPICE_CFLAGS) $(GLIB2_CFLAGS) > > $(ALSA_CFLAGS) -DUDSCS_NO_SERVER > > -src_spice_vdagent_LDADD = $(X_LIBS) $(SPICE_LIBS) $(GLIB2_LIBS) > > $(ALSA_LIBS) > > -src_spice_vdagent_SOURCES = src/vdagent.c \ > > -src/vdagent-x11.c \ > > -src/vdagent-x11-randr.c \ > > -src/vdagent-file-xfers.c \ > > -src/vdagent-audio.c \ > > -src/udscs.c > > - > > -src_spice_vdagentd_CFLAGS = $(DBUS_CFLAGS) $(LIBSYSTEMD_LOGIN_CFLAGS) \ > > - $(PCIACCESS_CFLAGS) $(SPICE_CFLAGS) $(GLIB2_CFLAGS) $(PIE_CFLAGS) > > -src_spice_vdagentd_LDADD = $(DBUS_LIBS) $(LIBSYSTEMD_LOGIN_LIBS) \ > > - $(PCIACCESS_LIBS) $(SPICE_LIBS) $(GLIB2_LIBS) $(PIE_LDFLAGS) > > -src_spice_vdagentd_SOURCES = src/vdagentd.c \ > > - src/vdagentd-uinput.c \ > > - src/vdagentd-xorg-conf.c \ > > - src/vdagent-virtio-port.c \ > > - src/udscs.c > > +src_spice_vdagent_CFLAGS = \ > > + $(X_CFLAGS) \ > > + $(SPICE_CFLAGS) \ > > + $(GLIB2_CFLAGS) \ > > + $(ALSA_CFLAGS) \ > > + -DUDSCS_NO_SERVER > > + > > Why not always use the final $(NULL) line terminator? > This would reduce line changes in the near (later patches in > this patchset) future. Agreed > > > +src_spice_vdagent_LDADD = \ > > + $(X_LIBS) \ > > + $(SPICE_LIBS) \ > > + $(GLIB2_LIBS) \ > > + $(ALSA_LIBS) > > + > > +src_spice_vdagent_SOURCES =\ > > + src/udscs.c \ > > + src/vdagent-audio.c \ > > + src/vdagent-file-xfers.c\ > > + src/vdagent-x11-randr.c \ > > + src/vdagent-x11.c \ > > + src/vdagent.c \ > > + $(NULL) > > + > > +src_spice_vdagentd_CFLAGS =\ > > + $(DBUS_CFLAGS) \ > > + $(LIBSYSTEMD_LOGIN_CFLAGS) \ > > + $(PCIACCESS_CFLAGS) \ > > + $(SPICE_CFLAGS) \ > > + $(GLIB2_CFLAGS) \ > > + $(PIE_CFLAGS) > > + > > +src_spice_vdagentd_LDADD = \ > > + $(DBUS_LIBS)\ > > + $(LIBSYSTEMD_LOGIN_LIBS)\ > > + $(PCIACCESS_LIBS) \ > > + $(SPICE_LIBS) \ > > + $(GLIB2_LIBS) \ > > + $(PIE_LDFLAGS) > > + > > +src_spice_vdagentd_SOURCES = \ > > + src/vdagentd.c \ > > + src/vdagentd-uinput.c \ > > + src/vdagentd-xorg-conf.c\ > > + src/vdagent-virtio-port.c \ > > + src/udscs.c \ > > + $(NULL) > > + > > if HAVE_CONSOLE_KIT > > src_spice_vdagentd_SOURCES += src/console-kit.c > > else > > @@ -32,17 +60,19 @@ src_spice_vdagentd_SOURCES += src/dummy-session-info.c > > endif > > endif > > > > -noinst_HEADERS = src/session-info.h \ > > - src/udscs.h \ > > - src/vdagent-audio.h \ > > - src/vdagent-file-xfers.h \ > > - src/vdagent-virtio-port.h \ > > - src/vdagent-x11.h \ > > - src/vdagent-x11-priv.h \ > > - src/vdagentd-proto.h \ > > - src/vdagentd-proto-strings.h \ > > - src/vdagentd-uinput.h \ > > - src/vdagentd-xorg-conf.h > > +noinst_HEADERS = \ > > + src/session-info.h \ > > + src/udscs.h \ > > + src/vdagent-audio.h \ > > + src/vdagent-file-xfers.h\ > > + src/vdagent-virtio-port.h \ > > + src/vdagent-x11-priv.h \ > > + src/vdagent-x11.h \ > > + src/vdagentd-proto-strings.h\ > > + src/vdagentd-proto.h\ > > + src/vdagentd-uinput.h \ > > + src/vdagentd-xo
Re: [Spice-devel] [RFC PATCH 2/2] Start writing some documentation on protocol
>
> On Fri, 2016-09-09 at 10:44 +0100, Frediano Ziglio wrote:
> > Signed-off-by: Frediano Ziglio
> > ---
> > docs/spice_protocol.txt | 48
> >
> > 1 file changed, 48 insertions(+)
> >
> > diff --git a/docs/spice_protocol.txt b/docs/spice_protocol.txt
> > index b62da25..b0ba568 100644
> > --- a/docs/spice_protocol.txt
> > +++ b/docs/spice_protocol.txt
> > @@ -1,2 +1,50 @@
> > Spice protocol format file
> > ==
> > +
> > +Copyright (C) 2016 Red Hat, Inc.
> > +Licensed under a Creative Commons Attribution-Share Alike 3.0
> > +United States License (see http://creativecommons.org/licenses/by-sa
> > /3.0/us/legalcode).
> > +
> > +Basic
> > +-
> > +The spice protocol format file defines the network protocol used by
> > spice.
> > +It resemble the C format.
> > +
> > +file ::=
> > +definitions ::= |
> > +definition ::=
> > |
> > +protocol ::= "protocol" "{" "}"
> > ";"
> > +protocol_channels ::=
> > |
> > +protocol_channel ::= [ "=" ]
> > ";"
> > +integer ::= |
> > +dec ::= [+-][0-9]+
> > +hex ::= "0x" [0-9a-f]+
> > +identifier ::= [a-z][a-z0-9_]*
> > +
> > +(here BNF with some regular expression are used).
> > +
> > +Example:
> > +
> > +channel ExampleChannel {
> > + message {
> > + uint32 dummy;
> > + } Dummy;
> > +};
> > +
> > +protocol Example {
> > +ExampleChannel first = 1001;
> > +};
> > +
> > +As you can see brackets like C are used and structures looks like C
> > but you
> > +can also see that keyworks like `channel`, `protocol`, `message` or
> > some
> > +predefined types like `uint32` are proper of the protocol.
> > +
> > +Comments
> > +
> > +Both C and C++ style comments are supported
> > +
> > +// this is a comment
> > +/* this is a comment too
> > + but can be split in multiple lines */
> > +
> > +
>
>
> I think that documenting the protocol is badly needed, but I fear that
> unless this documentation is connected to the protocol definition in
> some way, it will get out-of-sync pretty quickly.
>
> Jonathon
>
out-of-sync can be a problem but it's also true that the sync
cannot move far away as this would break compatibility.
Also we could extend the tests Christophe did to make sure that
syntax is still working the same way.
I was thinking about some small tools to extract documentation from
spice_parser.py and other files but didn't manage to think any good
way in a timely fashion. I wouldn't create another "project"
(spend too much time) writing a tool that generate the documentation
from source, looks like the information we need are quite spread all
over the sources. I also tried to look at pyparsing (not really
familiar with) to get a list the overall syntax documentation but
didn't find nothing either.
Do you think I (I would prefer a WE) should investigate more on
getting documentation from source? I think not, unless someone with
better knowledge came with an easy solution.
Do you think it's worth if I continue this branch/patches or should
I discard them?
Frediano
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [vdagent-linux v1 3/3] build-sys: move user/system to respective dir
>
> From: Marc-André Lureau
>
> Signed-off-by: Victor Toso
> ---
> Makefile.am | 59
> +---
> src/{ => vdagent}/vdagent-audio.c| 0
> src/{ => vdagent}/vdagent-audio.h| 0
> src/{ => vdagent}/vdagent-file-xfers.c | 0
> src/{ => vdagent}/vdagent-file-xfers.h | 0
> src/{ => vdagent}/vdagent-x11-priv.h | 0
> src/{ => vdagent}/vdagent-x11-randr.c| 0
> src/{ => vdagent}/vdagent-x11.c | 0
> src/{ => vdagent}/vdagent-x11.h | 0
> src/{ => vdagent}/vdagent.c | 0
> src/{ => vdagentd}/console-kit.c | 0
> src/{ => vdagentd}/dummy-session-info.c | 0
> src/{ => vdagentd}/session-info.h| 0
> src/{ => vdagentd}/systemd-login.c | 0
> src/{ => vdagentd}/vdagent-virtio-port.c | 0
> src/{ => vdagentd}/vdagent-virtio-port.h | 0
> src/{ => vdagentd}/vdagentd-uinput.c | 0
> src/{ => vdagentd}/vdagentd-uinput.h | 0
> src/{ => vdagentd}/vdagentd-xorg-conf.c | 0
> src/{ => vdagentd}/vdagentd-xorg-conf.h | 0
> src/{ => vdagentd}/vdagentd.c| 0
> 21 files changed, 32 insertions(+), 27 deletions(-)
> rename src/{ => vdagent}/vdagent-audio.c (100%)
> rename src/{ => vdagent}/vdagent-audio.h (100%)
> rename src/{ => vdagent}/vdagent-file-xfers.c (100%)
> rename src/{ => vdagent}/vdagent-file-xfers.h (100%)
> rename src/{ => vdagent}/vdagent-x11-priv.h (100%)
> rename src/{ => vdagent}/vdagent-x11-randr.c (100%)
> rename src/{ => vdagent}/vdagent-x11.c (100%)
> rename src/{ => vdagent}/vdagent-x11.h (100%)
> rename src/{ => vdagent}/vdagent.c (100%)
> rename src/{ => vdagentd}/console-kit.c (100%)
> rename src/{ => vdagentd}/dummy-session-info.c (100%)
> rename src/{ => vdagentd}/session-info.h (100%)
> rename src/{ => vdagentd}/systemd-login.c (100%)
> rename src/{ => vdagentd}/vdagent-virtio-port.c (100%)
> rename src/{ => vdagentd}/vdagent-virtio-port.h (100%)
> rename src/{ => vdagentd}/vdagentd-uinput.c (100%)
> rename src/{ => vdagentd}/vdagentd-uinput.h (100%)
> rename src/{ => vdagentd}/vdagentd-xorg-conf.c (100%)
> rename src/{ => vdagentd}/vdagentd-xorg-conf.h (100%)
> rename src/{ => vdagentd}/vdagentd.c (100%)
>
> diff --git a/Makefile.am b/Makefile.am
> index 9bc9dd8..33518a0 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -4,11 +4,19 @@ NULL =
> bin_PROGRAMS = src/spice-vdagent
> sbin_PROGRAMS = src/spice-vdagentd
>
> +common_sources = \
> + src/udscs.c \
> + src/udscs.h \
> + src/vdagentd-proto-strings.h\
> + src/vdagentd-proto.h\
> + $(NULL)
> +
> src_spice_vdagent_CFLAGS = \
> $(X_CFLAGS) \
> $(SPICE_CFLAGS) \
> $(GLIB2_CFLAGS) \
> $(ALSA_CFLAGS) \
> + -I$(srcdir)/src \
> -DUDSCS_NO_SERVER
>
> src_spice_vdagent_LDADD =\
> @@ -18,17 +26,16 @@ src_spice_vdagent_LDADD = \
> $(ALSA_LIBS)
>
> src_spice_vdagent_SOURCES = \
> - src/udscs.c \
> - src/udscs.h \
> - src/vdagent-audio.c \
> - src/vdagent-audio.h \
> - src/vdagent-file-xfers.c\
> - src/vdagent-file-xfers.h\
> - src/vdagent-x11-priv.h \
> - src/vdagent-x11-randr.c \
> - src/vdagent-x11.c \
> - src/vdagent-x11.h \
> - src/vdagent.c \
> + $(common_sources) \
> + src/vdagent/vdagent-audio.c \
> + src/vdagent/vdagent-audio.h \
> + src/vdagent/vdagent-file-xfers.c\
> + src/vdagent/vdagent-file-xfers.h\
> + src/vdagent/vdagent-x11-priv.h \
> + src/vdagent/vdagent-x11-randr.c \
> + src/vdagent/vdagent-x11.c \
> + src/vdagent/vdagent-x11.h \
> + src/vdagent/vdagent.c \
> $(NULL)
>
> src_spice_vdagentd_CFLAGS = \
> @@ -37,7 +44,8 @@ src_spice_vdagentd_CFLAGS = \
> $(PCIACCESS_CFLAGS) \
> $(SPICE_CFLAGS) \
> $(GLIB2_CFLAGS) \
> - $(PIE_CFLAGS)
> + $(PIE_CFLAGS) \
> + -I$(srcdir)/src
>
> src_spice_vdagentd_LDADD = \
> $(DBUS_LIBS)\
> @@ -48,27 +56,24 @@ src_spice_vdagentd_LDADD =\
> $(PIE_LDFLAGS)
>
> src_spice_vdagentd_SOURCES = \
> - src/vdagentd.c \
Re: [Spice-devel] [vdagent-linux v1 1/3] build-sys: reformat Makefile.am
> > Makes the makefile more readable and easy to change in the next few > commits. > > Based on Marc-André Lureau patch. > --- > Makefile.am | 94 > + > 1 file changed, 63 insertions(+), 31 deletions(-) > > diff --git a/Makefile.am b/Makefile.am > index 680ef83..4739c44 100644 > --- a/Makefile.am > +++ b/Makefile.am > @@ -4,24 +4,52 @@ NULL = > bin_PROGRAMS = src/spice-vdagent > sbin_PROGRAMS = src/spice-vdagentd > > -src_spice_vdagent_CFLAGS = $(X_CFLAGS) $(SPICE_CFLAGS) $(GLIB2_CFLAGS) > $(ALSA_CFLAGS) -DUDSCS_NO_SERVER > -src_spice_vdagent_LDADD = $(X_LIBS) $(SPICE_LIBS) $(GLIB2_LIBS) $(ALSA_LIBS) > -src_spice_vdagent_SOURCES = src/vdagent.c \ > -src/vdagent-x11.c \ > -src/vdagent-x11-randr.c \ > -src/vdagent-file-xfers.c \ > -src/vdagent-audio.c \ > -src/udscs.c > - > -src_spice_vdagentd_CFLAGS = $(DBUS_CFLAGS) $(LIBSYSTEMD_LOGIN_CFLAGS) \ > - $(PCIACCESS_CFLAGS) $(SPICE_CFLAGS) $(GLIB2_CFLAGS) $(PIE_CFLAGS) > -src_spice_vdagentd_LDADD = $(DBUS_LIBS) $(LIBSYSTEMD_LOGIN_LIBS) \ > - $(PCIACCESS_LIBS) $(SPICE_LIBS) $(GLIB2_LIBS) $(PIE_LDFLAGS) > -src_spice_vdagentd_SOURCES = src/vdagentd.c \ > - src/vdagentd-uinput.c \ > - src/vdagentd-xorg-conf.c \ > - src/vdagent-virtio-port.c \ > - src/udscs.c > +src_spice_vdagent_CFLAGS = \ > + $(X_CFLAGS) \ > + $(SPICE_CFLAGS) \ > + $(GLIB2_CFLAGS) \ > + $(ALSA_CFLAGS) \ > + -DUDSCS_NO_SERVER > + Why not always use the final $(NULL) line terminator? This would reduce line changes in the near (later patches in this patchset) future. > +src_spice_vdagent_LDADD =\ > + $(X_LIBS) \ > + $(SPICE_LIBS) \ > + $(GLIB2_LIBS) \ > + $(ALSA_LIBS) > + > +src_spice_vdagent_SOURCES = \ > + src/udscs.c \ > + src/vdagent-audio.c \ > + src/vdagent-file-xfers.c\ > + src/vdagent-x11-randr.c \ > + src/vdagent-x11.c \ > + src/vdagent.c \ > + $(NULL) > + > +src_spice_vdagentd_CFLAGS = \ > + $(DBUS_CFLAGS) \ > + $(LIBSYSTEMD_LOGIN_CFLAGS) \ > + $(PCIACCESS_CFLAGS) \ > + $(SPICE_CFLAGS) \ > + $(GLIB2_CFLAGS) \ > + $(PIE_CFLAGS) > + > +src_spice_vdagentd_LDADD = \ > + $(DBUS_LIBS)\ > + $(LIBSYSTEMD_LOGIN_LIBS)\ > + $(PCIACCESS_LIBS) \ > + $(SPICE_LIBS) \ > + $(GLIB2_LIBS) \ > + $(PIE_LDFLAGS) > + > +src_spice_vdagentd_SOURCES = \ > + src/vdagentd.c \ > + src/vdagentd-uinput.c \ > + src/vdagentd-xorg-conf.c\ > + src/vdagent-virtio-port.c \ > + src/udscs.c \ > + $(NULL) > + > if HAVE_CONSOLE_KIT > src_spice_vdagentd_SOURCES += src/console-kit.c > else > @@ -32,17 +60,19 @@ src_spice_vdagentd_SOURCES += src/dummy-session-info.c > endif > endif > > -noinst_HEADERS = src/session-info.h \ > - src/udscs.h \ > - src/vdagent-audio.h \ > - src/vdagent-file-xfers.h \ > - src/vdagent-virtio-port.h \ > - src/vdagent-x11.h \ > - src/vdagent-x11-priv.h \ > - src/vdagentd-proto.h \ > - src/vdagentd-proto-strings.h \ > - src/vdagentd-uinput.h \ > - src/vdagentd-xorg-conf.h > +noinst_HEADERS = \ > + src/session-info.h \ > + src/udscs.h \ > + src/vdagent-audio.h \ > + src/vdagent-file-xfers.h\ > + src/vdagent-virtio-port.h \ > + src/vdagent-x11-priv.h \ > + src/vdagent-x11.h \ > + src/vdagentd-proto-strings.h\ > + src/vdagentd-proto.h\ > + src/vdagentd-uinput.h \ > + src/vdagentd-xorg-conf.h\ > + $(NULL) > > xdgautostartdir = $(sysconfdir)/xdg/autostart > xdgautostart_DATA = $(top_srcdir)/data/spice-vdagent.desktop > @@ -75,8 +105,10 @@ tmpfiles_DATA = > $(top_srcdir)/data/tmpfiles.d/spice-v
Re: [Spice-devel] [PATCH spice-common] proto: Add agent features message
Hi
- Original Message -
> Hi
>
> - Original Message -
> > The message is sent by server to the client to indicate
> > which agent features are enabled or disabled by using flags.
> >
> > If a flag is set, then the corresponding feature is enabled.
> >
> > The message currently supports info about copy & paste and file transfer.
>
> Have you considered using channel capabilities? it seems to me it would fit
> nicely, beside there is already APIs, and the flag are not limited to 32
> bits.
>
Actually, there is agent capabilities, I think that's what the server should be
overriding instead.
> >
> > Related:
> > https://bugzilla.redhat.com/show_bug.cgi?id=1373725
> > ---
> > common/messages.h | 4
> > configure.ac | 2 +-
> > spice.proto | 9 +
> > 3 files changed, 14 insertions(+), 1 deletion(-)
> >
> > diff --git a/common/messages.h b/common/messages.h
> > index 516a345..f8648b6 100644
> > --- a/common/messages.h
> > +++ b/common/messages.h
> > @@ -99,6 +99,10 @@ typedef struct SpiceMsgMainMigrateBeginSeamless {
> > uint32_t src_mig_version;
> > } SpiceMsgMainMigrateBeginSeamless;
> >
> > +typedef struct SpiceMsgMainAgentFeatures {
> > +uint32_t flags;
> > +} SpiceMsgMainAgentFeatures;
> > +
> > typedef struct SpiceMsgcMainMigrateDstDoSeamless {
> > uint32_t src_version;
> > } SpiceMsgcMainMigrateDstDoSeamless;
> > diff --git a/configure.ac b/configure.ac
> > index c3ad5a4..c0e49b7 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -28,7 +28,7 @@ AM_PROG_CC_C_O
> > SPICE_CHECK_SYSDEPS
> >
> > # Checks for libraries
> > -PKG_CHECK_MODULES([PROTOCOL], [spice-protocol >= 0.12.12])
> > +PKG_CHECK_MODULES([PROTOCOL], [spice-protocol >= 0.12.13])
> >
> > SPICE_CHECK_PYTHON_MODULES()
> >
> > diff --git a/spice.proto b/spice.proto
> > index 0bfc515..ea4dfc2 100644
> > --- a/spice.proto
> > +++ b/spice.proto
> > @@ -226,6 +226,11 @@ struct DstInfo {
> > uint8 *cert_subject_data[cert_subject_size] @zero_terminated @marshall;
> > } @ctype(SpiceMigrationDstInfo);
> >
> > +flags32 agent_features_flags {
> > +COPY_PASTE,
> > +FILE_TRANSFER
> > +} @prefix(SPICE_AGENT_FEATURE_);
> > +
> > channel MainChannel : BaseChannel {
> > server:
> > message {
> > @@ -303,6 +308,10 @@ channel MainChannel : BaseChannel {
> > Empty migrate_dst_seamless_ack;
> > Empty migrate_dst_seamless_nack;
> >
> > +message {
> > +agent_features_flags flags;
> > +} agent_features;
> > +
> > client:
> > message {
> > uint64 cache_size;
> > --
> > 2.10.0
> >
> > ___
> > Spice-devel mailing list
> > Spice-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/spice-devel
> >
> ___
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
>
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH spice-common] proto: Add agent features message
Hi
- Original Message -
> The message is sent by server to the client to indicate
> which agent features are enabled or disabled by using flags.
>
> If a flag is set, then the corresponding feature is enabled.
>
> The message currently supports info about copy & paste and file transfer.
Have you considered using channel capabilities? it seems to me it would fit
nicely, beside there is already APIs, and the flag are not limited to 32 bits.
>
> Related:
> https://bugzilla.redhat.com/show_bug.cgi?id=1373725
> ---
> common/messages.h | 4
> configure.ac | 2 +-
> spice.proto | 9 +
> 3 files changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/common/messages.h b/common/messages.h
> index 516a345..f8648b6 100644
> --- a/common/messages.h
> +++ b/common/messages.h
> @@ -99,6 +99,10 @@ typedef struct SpiceMsgMainMigrateBeginSeamless {
> uint32_t src_mig_version;
> } SpiceMsgMainMigrateBeginSeamless;
>
> +typedef struct SpiceMsgMainAgentFeatures {
> +uint32_t flags;
> +} SpiceMsgMainAgentFeatures;
> +
> typedef struct SpiceMsgcMainMigrateDstDoSeamless {
> uint32_t src_version;
> } SpiceMsgcMainMigrateDstDoSeamless;
> diff --git a/configure.ac b/configure.ac
> index c3ad5a4..c0e49b7 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -28,7 +28,7 @@ AM_PROG_CC_C_O
> SPICE_CHECK_SYSDEPS
>
> # Checks for libraries
> -PKG_CHECK_MODULES([PROTOCOL], [spice-protocol >= 0.12.12])
> +PKG_CHECK_MODULES([PROTOCOL], [spice-protocol >= 0.12.13])
>
> SPICE_CHECK_PYTHON_MODULES()
>
> diff --git a/spice.proto b/spice.proto
> index 0bfc515..ea4dfc2 100644
> --- a/spice.proto
> +++ b/spice.proto
> @@ -226,6 +226,11 @@ struct DstInfo {
> uint8 *cert_subject_data[cert_subject_size] @zero_terminated @marshall;
> } @ctype(SpiceMigrationDstInfo);
>
> +flags32 agent_features_flags {
> +COPY_PASTE,
> +FILE_TRANSFER
> +} @prefix(SPICE_AGENT_FEATURE_);
> +
> channel MainChannel : BaseChannel {
> server:
> message {
> @@ -303,6 +308,10 @@ channel MainChannel : BaseChannel {
> Empty migrate_dst_seamless_ack;
> Empty migrate_dst_seamless_nack;
>
> +message {
> +agent_features_flags flags;
> +} agent_features;
> +
> client:
> message {
> uint64 cache_size;
> --
> 2.10.0
>
> ___
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
>
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH spice-protocol 2/2] Add support for reporting availability of agent features
Some agent features can be disabled on the server:
* Copy & Paste
* File transfer
Add a message to give this info to the client, so it doesn't
try to use them.
Related:
https://bugzilla.redhat.com/show_bug.cgi?id=1373725
---
spice/enums.h | 8
1 file changed, 8 insertions(+)
diff --git a/spice/enums.h b/spice/enums.h
index 1d0c2db..dc68fce 100644
--- a/spice/enums.h
+++ b/spice/enums.h
@@ -128,6 +128,13 @@ typedef enum SpicePubkeyType {
SPICE_PUBKEY_TYPE_ENUM_END
} SpicePubkeyType;
+typedef enum SpiceAgentFeaturesFlags {
+SPICE_AGENT_FEATURE_COPY_PASTE = (1 << 0),
+SPICE_AGENT_FEATURE_FILE_TRANSFER = (1 << 1),
+
+SPICE_AGENT_FEATURES_FLAGS_MASK = 0x3
+} SpiceAgentFeaturesFlags;
+
typedef enum SpiceClipType {
SPICE_CLIP_TYPE_NONE,
SPICE_CLIP_TYPE_RECTS,
@@ -466,6 +473,7 @@ enum {
SPICE_MSG_MAIN_MIGRATE_BEGIN_SEAMLESS,
SPICE_MSG_MAIN_MIGRATE_DST_SEAMLESS_ACK,
SPICE_MSG_MAIN_MIGRATE_DST_SEAMLESS_NACK,
+SPICE_MSG_MAIN_AGENT_FEATURES,
SPICE_MSG_END_MAIN
};
--
2.10.0
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [RFC] Inform client about disabled features on server
Hi! On the server it is possible to disabled some agent features: Copy & Paste and File transfer. Server then filters messages related to this features and discards. Since client is not aware that a feature is disabled, it keeps sending them. The issue is more obvious with File transfer which has a progress dialog since v0.31 of spice-gtk. These patches introduce the SPICE_MSG_MAIN_AGENT_FEATURES messages to inform client which features are enabled. Client uses this knowledge to avoid sending disabled vdagent messages and also it doesn't open the file transfer dialog. You can also see commits on my agentfeatures branches: https://cgit.freedesktop.org/~pgrunt/spice-common/?h=agentfeatures https://cgit.freedesktop.org/~pgrunt/spice-protocol/?h=agentfeatures https://cgit.freedesktop.org/~pgrunt/spice/?h=agentfeatures https://cgit.freedesktop.org/~pgrunt/spice-gtk/?h=agentfeatures Thanks for comments, Pavel -- 2.10.0 ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH spice-common] proto: Add agent features message
The message is sent by server to the client to indicate
which agent features are enabled or disabled by using flags.
If a flag is set, then the corresponding feature is enabled.
The message currently supports info about copy & paste and file transfer.
Related:
https://bugzilla.redhat.com/show_bug.cgi?id=1373725
---
common/messages.h | 4
configure.ac | 2 +-
spice.proto | 9 +
3 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/common/messages.h b/common/messages.h
index 516a345..f8648b6 100644
--- a/common/messages.h
+++ b/common/messages.h
@@ -99,6 +99,10 @@ typedef struct SpiceMsgMainMigrateBeginSeamless {
uint32_t src_mig_version;
} SpiceMsgMainMigrateBeginSeamless;
+typedef struct SpiceMsgMainAgentFeatures {
+uint32_t flags;
+} SpiceMsgMainAgentFeatures;
+
typedef struct SpiceMsgcMainMigrateDstDoSeamless {
uint32_t src_version;
} SpiceMsgcMainMigrateDstDoSeamless;
diff --git a/configure.ac b/configure.ac
index c3ad5a4..c0e49b7 100644
--- a/configure.ac
+++ b/configure.ac
@@ -28,7 +28,7 @@ AM_PROG_CC_C_O
SPICE_CHECK_SYSDEPS
# Checks for libraries
-PKG_CHECK_MODULES([PROTOCOL], [spice-protocol >= 0.12.12])
+PKG_CHECK_MODULES([PROTOCOL], [spice-protocol >= 0.12.13])
SPICE_CHECK_PYTHON_MODULES()
diff --git a/spice.proto b/spice.proto
index 0bfc515..ea4dfc2 100644
--- a/spice.proto
+++ b/spice.proto
@@ -226,6 +226,11 @@ struct DstInfo {
uint8 *cert_subject_data[cert_subject_size] @zero_terminated @marshall;
} @ctype(SpiceMigrationDstInfo);
+flags32 agent_features_flags {
+COPY_PASTE,
+FILE_TRANSFER
+} @prefix(SPICE_AGENT_FEATURE_);
+
channel MainChannel : BaseChannel {
server:
message {
@@ -303,6 +308,10 @@ channel MainChannel : BaseChannel {
Empty migrate_dst_seamless_ack;
Empty migrate_dst_seamless_nack;
+message {
+agent_features_flags flags;
+} agent_features;
+
client:
message {
uint64 cache_size;
--
2.10.0
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH spice] Inform client about agent features
Send the SPICE_MSG_MAIN_AGENT_FEATURES message containing information
about enabled/disabled agent features when client connects.
Currently supported features:
* Copy & Paste
* File transfer
Related:
https://bugzilla.redhat.com/show_bug.cgi?id=1373725
---
configure.ac | 2 +-
server/main-channel-client.c | 32
server/main-channel-client.h | 2 ++
server/main-channel.c| 5 +
server/main-channel.h| 1 +
server/reds.c| 16
spice-common | 2 +-
7 files changed, 58 insertions(+), 2 deletions(-)
diff --git a/configure.ac b/configure.ac
index f8284f6..71da32b 100644
--- a/configure.ac
+++ b/configure.ac
@@ -143,7 +143,7 @@ AS_IF([test x"$have_smartcard" = "xyes"], [
AS_VAR_APPEND([SPICE_REQUIRES], [" libcacard >= 0.1.2"])
])
-SPICE_PROTOCOL_MIN_VER=0.12.12
+SPICE_PROTOCOL_MIN_VER=0.12.13
PKG_CHECK_MODULES([SPICE_PROTOCOL], [spice-protocol >=
$SPICE_PROTOCOL_MIN_VER])
AC_SUBST([SPICE_PROTOCOL_MIN_VER])
diff --git a/server/main-channel-client.c b/server/main-channel-client.c
index 236a2e7..75c1496 100644
--- a/server/main-channel-client.c
+++ b/server/main-channel-client.c
@@ -122,6 +122,11 @@ typedef struct RedMultiMediaTimePipeItem {
int time;
} RedMultiMediaTimePipeItem;
+typedef struct RedAgentFeaturesPipeItem {
+RedPipeItem base;
+uint32_t flags;
+} RedAgentFeaturesPipeItem;
+
#define ZERO_BUF_SIZE 4096
static const uint8_t zero_page[ZERO_BUF_SIZE] = {0};
@@ -342,6 +347,18 @@ RedPipeItem
*main_multi_media_time_item_new(RedChannelClient *rcc,
return &item->base;
}
+RedPipeItem *main_agent_features_item_new(RedChannelClient *rcc,
+ void *data, int num G_GNUC_UNUSED)
+{
+uint32_t *flags = data;
+RedAgentFeaturesPipeItem *item;
+
+item = spice_malloc(sizeof(RedAgentFeaturesPipeItem));
+red_pipe_item_init(&item->base, RED_PIPE_ITEM_TYPE_MAIN_AGENT_FEATURES);
+item->flags = *flags;
+return &item->base;
+}
+
void main_channel_client_handle_migrate_connected(MainChannelClient *mcc,
int success,
int seamless)
@@ -833,6 +850,17 @@ static void
main_channel_marshall_multi_media_time(RedChannelClient *rcc,
spice_marshall_msg_main_multi_media_time(m, &time_mes);
}
+static void main_channel_marshall_agent_features(RedChannelClient *rcc,
+ SpiceMarshaller *m,
+ RedAgentFeaturesPipeItem
*item)
+{
+SpiceMsgMainAgentFeatures agent_features_msg;
+
+red_channel_client_init_send_data(rcc, SPICE_MSG_MAIN_AGENT_FEATURES,
&item->base);
+agent_features_msg.flags = item->flags;
+spice_marshall_msg_main_agent_features(m, &agent_features_msg);
+}
+
static void main_channel_marshall_migrate_switch(SpiceMarshaller *m,
RedChannelClient *rcc,
RedPipeItem *item)
{
@@ -944,6 +972,10 @@ void main_channel_client_send_item(RedChannelClient *rcc,
RedPipeItem *base)
case RED_PIPE_ITEM_TYPE_MAIN_AGENT_CONNECTED_TOKENS:
main_channel_marshall_agent_connected(m, rcc, base);
break;
+case RED_PIPE_ITEM_TYPE_MAIN_AGENT_FEATURES:
+main_channel_marshall_agent_features(rcc, m,
+
SPICE_UPCAST(RedAgentFeaturesPipeItem, base));
+break;
default:
break;
};
diff --git a/server/main-channel-client.h b/server/main-channel-client.h
index 9f05af3..5254916 100644
--- a/server/main-channel-client.h
+++ b/server/main-channel-client.h
@@ -89,6 +89,7 @@ enum {
RED_PIPE_ITEM_TYPE_MAIN_NAME,
RED_PIPE_ITEM_TYPE_MAIN_UUID,
RED_PIPE_ITEM_TYPE_MAIN_AGENT_CONNECTED_TOKENS,
+RED_PIPE_ITEM_TYPE_MAIN_AGENT_FEATURES,
};
typedef struct MainMouseModeItemInfo {
@@ -104,5 +105,6 @@ typedef struct MainMultiMediaTimeItemInfo {
RedPipeItem *main_multi_media_time_item_new(RedChannelClient *rcc,
void *data, int num);
+RedPipeItem *main_agent_features_item_new(RedChannelClient *rcc, void *data,
int num);
#endif /* __MAIN_CHANNEL_CLIENT_H__ */
diff --git a/server/main-channel.c b/server/main-channel.c
index 4670315..2cc425b 100644
--- a/server/main-channel.c
+++ b/server/main-channel.c
@@ -131,6 +131,11 @@ void main_channel_push_multi_media_time(MainChannel
*main_chan, int time)
main_multi_media_time_item_new, &info);
}
+void main_channel_push_agent_features(MainChannel *main_chan, uint32_t flags)
+{
+red_channel_pipes_new_add_push(&main_chan->base,
main_agent_features_item_new, &flags);
+}
+
static void main_channel_fill_mig_target(MainChannel *main_channel,
RedsMigSpice *mig_target)
{
spice_assert(mig_target);
diff --git a/server/main-channel.h b/s
[Spice-devel] [PATCH spice-protocol 1/2] build-sys: post-release bump
--- configure.ac | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index dd2d373..17f48d1 100644 --- a/configure.ac +++ b/configure.ac @@ -2,7 +2,7 @@ AC_PREREQ([2.57]) m4_define([SPICE_MAJOR], 0) m4_define([SPICE_MINOR], 12) -m4_define([SPICE_MICRO], 12) +m4_define([SPICE_MICRO], 13) AC_INIT(spice-protocol, [SPICE_MAJOR.SPICE_MINOR.SPICE_MICRO], [], spice-protocol) -- 2.10.0 ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH spice-gtk] main: Handle agent features message
The message indicates that some agent features are enabled or disabled.
Currently supported features:
* Copy & Paste
* File transfer
It allows to avoid sending unnecessary agent messages.
For compatibility reason all the features are considered enabled by default
Resolves:
https://bugzilla.redhat.com/show_bug.cgi?id=1373725
---
configure.ac | 2 +-
spice-common | 2 +-
src/channel-main.c | 51 +++
3 files changed, 53 insertions(+), 2 deletions(-)
diff --git a/configure.ac b/configure.ac
index f3e7f8d..aa60e91 100644
--- a/configure.ac
+++ b/configure.ac
@@ -69,7 +69,7 @@ AC_CHECK_LIBM
AC_SUBST(LIBM)
AC_CONFIG_SUBDIRS([spice-common])
-PKG_CHECK_MODULES([SPICE_PROTOCOL], [spice-protocol >= 0.12.12])
+PKG_CHECK_MODULES([SPICE_PROTOCOL], [spice-protocol >= 0.12.13])
COMMON_CFLAGS='-I${top_builddir}/spice-common/ -I${top_srcdir}/spice-common/
${SPICE_PROTOCOL_CFLAGS}'
AC_SUBST(COMMON_CFLAGS)
diff --git a/spice-common b/spice-common
index 62f3024..ee8a6c3 16
--- a/spice-common
+++ b/spice-common
@@ -1 +1 @@
-Subproject commit 62f3024f4220766761269618bf3df143ff5c9956
+Subproject commit ee8a6c3a98ab5aff9f54133c3ec315e5298b34cd
diff --git a/src/channel-main.c b/src/channel-main.c
index 990a06a..341df4f 100644
--- a/src/channel-main.c
+++ b/src/channel-main.c
@@ -105,6 +105,7 @@ struct _SpiceMainChannelPrivate {
guint agent_msg_pos;
uint8_t agent_msg_size;
uint32_tagent_caps[VD_AGENT_CAPS_SIZE];
+uint32_tagent_features;
SpiceDisplayConfig display[MAX_DISPLAY];
ginttimer_id;
GQueue *agent_msg_queue;
@@ -447,6 +448,7 @@ static void spice_main_constructed(GObject *object)
/* update default value */
c->max_clipboard = spice_main_get_max_clipboard(self);
+c->agent_features = SPICE_AGENT_FEATURES_FLAGS_MASK;
if (G_OBJECT_CLASS(spice_main_channel_parent_class)->constructed)
G_OBJECT_CLASS(spice_main_channel_parent_class)->constructed(object);
@@ -2099,6 +2101,17 @@ static void main_handle_agent_token(SpiceChannel
*channel, SpiceMsgIn *in)
agent_send_msg_queue(SPICE_MAIN_CHANNEL(channel));
}
+/* coroutine context */
+static void main_handle_agent_features(SpiceChannel *channel, SpiceMsgIn *in)
+{
+SpiceMsgMainAgentFeatures *features = spice_msg_in_parsed(in);
+SpiceMainChannelPrivate *c = SPICE_MAIN_CHANNEL(channel)->priv;
+
+CHANNEL_DEBUG(channel, "received agent features: %x", features->flags);
+
+c->agent_features = features->flags;
+}
+
/* main context */
static void migrate_channel_new_cb(SpiceSession *s, SpiceChannel *channel,
gpointer data)
{
@@ -2490,6 +2503,7 @@ static void channel_set_handlers(SpiceChannelClass *klass)
[ SPICE_MSG_MAIN_AGENT_DISCONNECTED ] =
main_handle_agent_disconnected,
[ SPICE_MSG_MAIN_AGENT_DATA ] = main_handle_agent_data,
[ SPICE_MSG_MAIN_AGENT_TOKEN ] = main_handle_agent_token,
+[ SPICE_MSG_MAIN_AGENT_FEATURES ] = main_handle_agent_features,
[ SPICE_MSG_MAIN_MIGRATE_BEGIN ] = main_handle_migrate_begin,
[ SPICE_MSG_MAIN_MIGRATE_END ] = main_handle_migrate_end,
@@ -2636,9 +2650,16 @@ void spice_main_clipboard_grab(SpiceMainChannel
*channel, guint32 *types, int nt
void spice_main_clipboard_selection_grab(SpiceMainChannel *channel, guint
selection,
guint32 *types, int ntypes)
{
+SpiceMainChannelPrivate *c;
g_return_if_fail(channel != NULL);
g_return_if_fail(SPICE_IS_MAIN_CHANNEL(channel));
+c = channel->priv;
+if (!(c->agent_features & SPICE_AGENT_FEATURE_COPY_PASTE)) {
+CHANNEL_DEBUG(channel, "Copy & Paste feature is disabled");
+return;
+}
+
agent_clipboard_grab(channel, selection, types, ntypes);
spice_channel_wakeup(SPICE_CHANNEL(channel), FALSE);
}
@@ -2677,6 +2698,11 @@ void
spice_main_clipboard_selection_release(SpiceMainChannel *channel, guint sel
if (!c->agent_connected)
return;
+if (!(c->agent_features & SPICE_AGENT_FEATURE_COPY_PASTE)) {
+CHANNEL_DEBUG(channel, "Copy & Paste feature is disabled");
+return;
+}
+
agent_clipboard_release(channel, selection);
spice_channel_wakeup(SPICE_CHANNEL(channel), FALSE);
}
@@ -2714,9 +2740,16 @@ void spice_main_clipboard_notify(SpiceMainChannel
*channel,
void spice_main_clipboard_selection_notify(SpiceMainChannel *channel, guint
selection,
guint32 type, const guchar *data,
size_t size)
{
+SpiceMainChannelPrivate *c;
g_return_if_fail(channel != NULL);
g_return_if_fail(SPICE_IS_MAIN_CHANNEL(channel));
+c = channel->priv;
+if (!(c->agent_features & SPICE_AGENT_FEATURE_COPY_PASTE)) {
+CHANNEL_DEBUG(channel, "Copy & Paste f
Re: [Spice-devel] [PATCH 2/2] OpenSSL from 1.1.0 is thread safe by default
Hey,
On Thu, Sep 15, 2016 at 12:24:15AM +0200, Sebastian Andrzej Siewior wrote:
> On 2016-08-11 14:22:59 [+0100], Frediano Ziglio wrote:
> > +++ b/server/reds.c
> > @@ -2771,6 +2771,7 @@ static int ssl_password_cb(char *buf, int size, int
> > flags, void *userdata)
> > return (strlen(pass));
> > }
> >
> > +#if OPENSSL_VERSION_NUMBER < 0x101FL
> > static unsigned long pthreads_thread_id(void)
> > {
> > unsigned long ret;
>
> This seems to be part of the spice git. I am looking at spice-gtk and
> this seems to lack 1.1.0 support. Is someone looking at this by any
> chance? :)
I took a look at that some time ago, and hit some issues porting
spice_channel_load_ca() which uses X509_LOOKUP/X509_LOOKUP_METHOD.
I got the impression from the thread at
https://mta.openssl.org/pipermail/openssl-dev/2016-April/006654.html
that openssl 1.1.0 was missing some methods to let us do what we need
(or maybe it's there but I got lost in the API :)
So I stop trying the port after finding that email. This was 2.5 months
ago, maybe these accessors have been added before the final release(?)
Christophe
signature.asc
Description: PGP signature
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH 2/2] OpenSSL from 1.1.0 is thread safe by default
On 2016-08-11 14:22:59 [+0100], Frediano Ziglio wrote:
> +++ b/server/reds.c
> @@ -2771,6 +2771,7 @@ static int ssl_password_cb(char *buf, int size, int
> flags, void *userdata)
> return (strlen(pass));
> }
>
> +#if OPENSSL_VERSION_NUMBER < 0x101FL
> static unsigned long pthreads_thread_id(void)
> {
> unsigned long ret;
This seems to be part of the spice git. I am looking at spice-gtk and
this seems to lack 1.1.0 support. Is someone looking at this by any
chance? :)
Sebastian
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
