Re: [Spice-devel] [spice-gtk PATCH v5 1/6] giopipe: don't fail on create_source
Hi,
> > +static GList *
> > +set_all_sources_ready (GList *sources)
> > +{
> > +GList *it = sources;
> > +while (it != NULL) {
> > +GSource *s = it->data;
> > +GList *next = it->next;
> > +
> > +if (s == NULL || g_source_is_destroyed(s)) {
> > +/* remove */
> > +sources = g_list_delete_link(sources, it);
> > +g_clear_pointer(&s, g_source_unref);
> > +} else {
> > +/* dispatch */
> > +g_source_set_ready_time(s, 0);
> > +}
> > +it = next;
> > +}
> > +return sources;
> > +}
> > +
> > static void
> > pipe_input_stream_check_source (PipeInputStream *self)
> > {
> > -if (self->source && !g_source_is_destroyed(self->source) &&
> > +GSource *source;
> > +
> > +if (self->sources == NULL)
> > +return;
> > +
> > +source = self->sources->data;
> > +if (source && !g_source_is_destroyed(source) &&
> > g_pollable_input_stream_is_readable(G_POLLABLE_INPUT_STREAM(self)))
> > -g_source_set_ready_time(self->source, 0);
> > +self->sources = set_all_sources_ready(self->sources);
>
> No idea why you check the first source here. I think the function could be
> just like:
>
> if (g_pollable_input_stream_is_readable(G_POLLABLE_INPUT_STREAM(self)))
>self->sources = set_all_sources_ready(self->sources)
>
That's true. I'll try to be more careful. Do you want a new version or
the patch (with this change) is in acceptable state?
Thanks,
> > @@ -330,9 +347,14 @@ pipe_output_stream_dispose(GObject *object)
> > static void
> > pipe_output_stream_check_source (PipeOutputStream *self)
> > {
> > -if (self->source && !g_source_is_destroyed(self->source) &&
> > +GSource * source;
> > +if (self->sources == NULL)
> > +return;
> > +
> > +source = self->sources->data;
> > +if (source && !g_source_is_destroyed(source) &&
> >
> > g_pollable_output_stream_is_writable(G_POLLABLE_OUTPUT_STREAM(self)))
> > -g_source_set_ready_time(self->source, 0);
> > +self->sources = set_all_sources_ready(self->sources);
>
> same here
I"ll change as well!
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [spice-gtk PATCH v6 1/6] giopipe: don't fail on create_source
PipeInputStream and PipeOutputStream should not fail when creating
GPollableStream source as this currently does not work with default
write_all and read_all functions;
In order to avoid creating zombie GSource in create_source of both
PipeInputStream and PipeOutputStream, we track all created GSources and
set them to be dispatched when data is available to read/write. It is
worth to mention that concurrent write/read is not possible with current
giopipe and only the last created GSource will read the data as it is
dispatched first.
---
gtk/giopipe.c | 69 +--
1 file changed, 39 insertions(+), 30 deletions(-)
diff --git a/gtk/giopipe.c b/gtk/giopipe.c
index 50edb5b..0e654ce 100644
--- a/gtk/giopipe.c
+++ b/gtk/giopipe.c
@@ -44,7 +44,7 @@ struct _PipeInputStream
* closing.
*/
gboolean peer_closed;
-GSource *source;
+GList *sources;
};
struct _PipeInputStreamClass
@@ -69,7 +69,7 @@ struct _PipeOutputStream
const gchar *buffer;
gsize count;
gboolean peer_closed;
-GSource *source;
+GList *sources;
};
struct _PipeOutputStreamClass
@@ -120,12 +120,35 @@ pipe_input_stream_read (GInputStream *stream,
return count;
}
+static GList *
+set_all_sources_ready (GList *sources)
+{
+GList *it = sources;
+while (it != NULL) {
+GSource *s = it->data;
+GList *next = it->next;
+
+if (s == NULL || g_source_is_destroyed(s)) {
+/* remove */
+sources = g_list_delete_link(sources, it);
+g_clear_pointer(&s, g_source_unref);
+} else {
+/* dispatch */
+g_source_set_ready_time(s, 0);
+}
+it = next;
+}
+return sources;
+}
+
static void
pipe_input_stream_check_source (PipeInputStream *self)
{
-if (self->source && !g_source_is_destroyed(self->source) &&
-g_pollable_input_stream_is_readable(G_POLLABLE_INPUT_STREAM(self)))
-g_source_set_ready_time(self->source, 0);
+if (self->sources == NULL)
+return;
+
+if (g_pollable_input_stream_is_readable(G_POLLABLE_INPUT_STREAM(self)))
+self->sources = set_all_sources_ready(self->sources);
}
static gboolean
@@ -193,10 +216,8 @@ pipe_input_stream_dispose(GObject *object)
self->peer = NULL;
}
-if (self->source) {
-g_source_unref(self->source);
-self->source = NULL;
-}
+g_list_free_full (self->sources, (GDestroyNotify) g_source_unref);
+self->sources = NULL;
G_OBJECT_CLASS(pipe_input_stream_parent_class)->dispose (object);
}
@@ -234,14 +255,8 @@ pipe_input_stream_create_source (GPollableInputStream
*stream,
PipeInputStream *self = PIPE_INPUT_STREAM(stream);
GSource *pollable_source;
-g_return_val_if_fail (self->source == NULL ||
- g_source_is_destroyed (self->source), NULL);
-
-if (self->source && g_source_is_destroyed (self->source))
-g_source_unref (self->source);
-
pollable_source = g_pollable_source_new_full (self, NULL, cancellable);
-self->source = g_source_ref (pollable_source);
+self->sources = g_list_prepend (self->sources, g_source_ref
(pollable_source));
return pollable_source;
}
@@ -319,10 +334,8 @@ pipe_output_stream_dispose(GObject *object)
self->peer = NULL;
}
-if (self->source) {
-g_source_unref(self->source);
-self->source = NULL;
-}
+g_list_free_full (self->sources, (GDestroyNotify) g_source_unref);
+self->sources = NULL;
G_OBJECT_CLASS(pipe_output_stream_parent_class)->dispose (object);
}
@@ -330,9 +343,11 @@ pipe_output_stream_dispose(GObject *object)
static void
pipe_output_stream_check_source (PipeOutputStream *self)
{
-if (self->source && !g_source_is_destroyed(self->source) &&
-g_pollable_output_stream_is_writable(G_POLLABLE_OUTPUT_STREAM(self)))
-g_source_set_ready_time(self->source, 0);
+if (self->sources == NULL)
+return;
+
+if (g_pollable_output_stream_is_writable(G_POLLABLE_OUTPUT_STREAM(self)))
+self->sources = set_all_sources_ready(self->sources);
}
static gboolean
@@ -416,14 +431,8 @@ pipe_output_stream_create_source (GPollableOutputStream
*stream,
PipeOutputStream *self = PIPE_OUTPUT_STREAM(stream);
GSource *pollable_source;
-g_return_val_if_fail (self->source == NULL ||
- g_source_is_destroyed (self->source), NULL);
-
-if (self->source && g_source_is_destroyed (self->source))
-g_source_unref (self->source);
-
pollable_source = g_pollable_source_new_full (self, NULL, cancellable);
-self->source = g_source_ref (pollable_source);
+self->sources = g_list_prepend (self->sources, g_source_ref
(pollable_source));
return pollable_source;
}
--
2.4.2
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedes
Re: [Spice-devel] [spice-gtk PATCH v6 1/6] giopipe: don't fail on create_source
> +if (self->sources == NULL) > +return; I forgot to remove this in both functions :( Oh well.. ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [spice-gtk PATCH v7 1/6] giopipe: don't fail on create_source
PipeInputStream and PipeOutputStream should not fail when creating
GPollableStream source as this currently does not work with default
write_all and read_all functions;
In order to avoid creating zombie GSource in create_source of both
PipeInputStream and PipeOutputStream, we track all created GSources and
set them to be dispatched when data is available to read/write. It is
worth to mention that concurrent write/read is not possible with current
giopipe and only the last created GSource will read the data as it is
dispatched first.
---
gtk/giopipe.c | 63 +++
1 file changed, 33 insertions(+), 30 deletions(-)
diff --git a/gtk/giopipe.c b/gtk/giopipe.c
index 50edb5b..a96783f 100644
--- a/gtk/giopipe.c
+++ b/gtk/giopipe.c
@@ -44,7 +44,7 @@ struct _PipeInputStream
* closing.
*/
gboolean peer_closed;
-GSource *source;
+GList *sources;
};
struct _PipeInputStreamClass
@@ -69,7 +69,7 @@ struct _PipeOutputStream
const gchar *buffer;
gsize count;
gboolean peer_closed;
-GSource *source;
+GList *sources;
};
struct _PipeOutputStreamClass
@@ -120,12 +120,32 @@ pipe_input_stream_read (GInputStream *stream,
return count;
}
+static GList *
+set_all_sources_ready (GList *sources)
+{
+GList *it = sources;
+while (it != NULL) {
+GSource *s = it->data;
+GList *next = it->next;
+
+if (s == NULL || g_source_is_destroyed(s)) {
+/* remove */
+sources = g_list_delete_link(sources, it);
+g_clear_pointer(&s, g_source_unref);
+} else {
+/* dispatch */
+g_source_set_ready_time(s, 0);
+}
+it = next;
+}
+return sources;
+}
+
static void
pipe_input_stream_check_source (PipeInputStream *self)
{
-if (self->source && !g_source_is_destroyed(self->source) &&
-g_pollable_input_stream_is_readable(G_POLLABLE_INPUT_STREAM(self)))
-g_source_set_ready_time(self->source, 0);
+if (g_pollable_input_stream_is_readable(G_POLLABLE_INPUT_STREAM(self)))
+self->sources = set_all_sources_ready(self->sources);
}
static gboolean
@@ -193,10 +213,8 @@ pipe_input_stream_dispose(GObject *object)
self->peer = NULL;
}
-if (self->source) {
-g_source_unref(self->source);
-self->source = NULL;
-}
+g_list_free_full (self->sources, (GDestroyNotify) g_source_unref);
+self->sources = NULL;
G_OBJECT_CLASS(pipe_input_stream_parent_class)->dispose (object);
}
@@ -234,14 +252,8 @@ pipe_input_stream_create_source (GPollableInputStream
*stream,
PipeInputStream *self = PIPE_INPUT_STREAM(stream);
GSource *pollable_source;
-g_return_val_if_fail (self->source == NULL ||
- g_source_is_destroyed (self->source), NULL);
-
-if (self->source && g_source_is_destroyed (self->source))
-g_source_unref (self->source);
-
pollable_source = g_pollable_source_new_full (self, NULL, cancellable);
-self->source = g_source_ref (pollable_source);
+self->sources = g_list_prepend (self->sources, g_source_ref
(pollable_source));
return pollable_source;
}
@@ -319,10 +331,8 @@ pipe_output_stream_dispose(GObject *object)
self->peer = NULL;
}
-if (self->source) {
-g_source_unref(self->source);
-self->source = NULL;
-}
+g_list_free_full (self->sources, (GDestroyNotify) g_source_unref);
+self->sources = NULL;
G_OBJECT_CLASS(pipe_output_stream_parent_class)->dispose (object);
}
@@ -330,9 +340,8 @@ pipe_output_stream_dispose(GObject *object)
static void
pipe_output_stream_check_source (PipeOutputStream *self)
{
-if (self->source && !g_source_is_destroyed(self->source) &&
-g_pollable_output_stream_is_writable(G_POLLABLE_OUTPUT_STREAM(self)))
-g_source_set_ready_time(self->source, 0);
+if (g_pollable_output_stream_is_writable(G_POLLABLE_OUTPUT_STREAM(self)))
+self->sources = set_all_sources_ready(self->sources);
}
static gboolean
@@ -416,14 +425,8 @@ pipe_output_stream_create_source (GPollableOutputStream
*stream,
PipeOutputStream *self = PIPE_OUTPUT_STREAM(stream);
GSource *pollable_source;
-g_return_val_if_fail (self->source == NULL ||
- g_source_is_destroyed (self->source), NULL);
-
-if (self->source && g_source_is_destroyed (self->source))
-g_source_unref (self->source);
-
pollable_source = g_pollable_source_new_full (self, NULL, cancellable);
-self->source = g_source_ref (pollable_source);
+self->sources = g_list_prepend (self->sources, g_source_ref
(pollable_source));
return pollable_source;
}
--
2.4.2
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH 02/11] drm/qxl: Do not leak memory if qxl_release_list_add fails
If the function fails reference counter to the object is not decremented
causing leaks.
This is hard to spot as it happens only on very low memory situations.
Signed-off-by: Frediano Ziglio
Reviewed-by: Dave Airlie
---
drivers/gpu/drm/qxl/qxl_ioctl.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/qxl/qxl_ioctl.c b/drivers/gpu/drm/qxl/qxl_ioctl.c
index afd7297..e8b5207 100644
--- a/drivers/gpu/drm/qxl/qxl_ioctl.c
+++ b/drivers/gpu/drm/qxl/qxl_ioctl.c
@@ -122,8 +122,10 @@ static struct qxl_bo *qxlhw_handle_to_bo(struct qxl_device
*qdev,
qobj = gem_to_qxl_bo(gobj);
ret = qxl_release_list_add(release, qobj);
- if (ret)
+ if (ret) {
+ drm_gem_object_unreference_unlocked(gobj);
return NULL;
+ }
return qobj;
}
--
2.1.0
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH 06/11] drm/qxl: Fix return for qxl_release_alloc
This function return handle to allocated release object which is an int.
Signed-off-by: Frediano Ziglio
Reviewed-by: Dave Airlie
---
drivers/gpu/drm/qxl/qxl_release.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/qxl/qxl_release.c
b/drivers/gpu/drm/qxl/qxl_release.c
index d9b2568..6fd8e50 100644
--- a/drivers/gpu/drm/qxl/qxl_release.c
+++ b/drivers/gpu/drm/qxl/qxl_release.c
@@ -122,7 +122,7 @@ static const struct fence_ops qxl_fence_ops = {
.wait = qxl_fence_wait,
};
-static uint64_t
+static int
qxl_release_alloc(struct qxl_device *qdev, int type,
struct qxl_release **ret)
{
--
2.1.0
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH 01/11] drm/qxl: Do not cause spice-server to clean our objects
If objects are moved back from system memory to VRAM (and spice id created again) memory is already initialized so we need to set flag to not clear memory. If you don't do it after a while using desktop many images turns to black or transparents. Signed-off-by: Frediano Ziglio Reviewed-by: Dave Airlie --- drivers/gpu/drm/qxl/qxl_cmd.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/qxl/qxl_cmd.c b/drivers/gpu/drm/qxl/qxl_cmd.c index bd5404e..85ed5dc 100644 --- a/drivers/gpu/drm/qxl/qxl_cmd.c +++ b/drivers/gpu/drm/qxl/qxl_cmd.c @@ -502,6 +502,7 @@ int qxl_hw_surface_alloc(struct qxl_device *qdev, cmd = (struct qxl_surface_cmd *)qxl_release_map(qdev, release); cmd->type = QXL_SURFACE_CMD_CREATE; + cmd->flags = QXL_SURF_FLAG_KEEP_DATA; cmd->u.surface_create.format = surf->surf.format; cmd->u.surface_create.width = surf->surf.width; cmd->u.surface_create.height = surf->surf.height; -- 2.1.0 ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH 07/11] drm/qxl: Handle correctly failures in qxl_alloc_relase_reserved
Free resources correctly if function fails
Signed-off-by: Frediano Ziglio
Reviewed-by: Dave Airlie
---
drivers/gpu/drm/qxl/qxl_release.c | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/qxl/qxl_release.c
b/drivers/gpu/drm/qxl/qxl_release.c
index 6fd8e50..00604ed 100644
--- a/drivers/gpu/drm/qxl/qxl_release.c
+++ b/drivers/gpu/drm/qxl/qxl_release.c
@@ -363,6 +363,7 @@ int qxl_alloc_release_reserved(struct qxl_device *qdev,
unsigned long size,
ret = qxl_release_bo_alloc(qdev,
&qdev->current_release_bo[cur_idx]);
if (ret) {
mutex_unlock(&qdev->release_mutex);
+ qxl_release_free(qdev, *release);
return ret;
}
}
@@ -377,13 +378,17 @@ int qxl_alloc_release_reserved(struct qxl_device *qdev,
unsigned long size,
mutex_unlock(&qdev->release_mutex);
- qxl_release_list_add(*release, bo);
+ ret = qxl_release_list_add(*release, bo);
+ qxl_bo_unref(&bo);
+ if (ret) {
+ qxl_release_free(qdev, *release);
+ return ret;
+ }
info = qxl_release_map(qdev, *release);
info->id = idr_ret;
qxl_release_unmap(qdev, *release, info);
- qxl_bo_unref(&bo);
return ret;
}
--
2.1.0
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH 04/11] drm/qxl: Avoid double free on error
Is we are not able to get source bo object from handle we free
destination bo object and call cleanup code however destination
object was already inserted in reloc_info array (num_relocs was
already incremented) so on cleanup we free destination again.
Signed-off-by: Frediano Ziglio
Reviewed-by: Dave Airlie
---
drivers/gpu/drm/qxl/qxl_ioctl.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/gpu/drm/qxl/qxl_ioctl.c b/drivers/gpu/drm/qxl/qxl_ioctl.c
index 230ab84..85b3808 100644
--- a/drivers/gpu/drm/qxl/qxl_ioctl.c
+++ b/drivers/gpu/drm/qxl/qxl_ioctl.c
@@ -240,8 +240,6 @@ static int qxl_process_single_command(struct qxl_device
*qdev,
qxlhw_handle_to_bo(qdev, file_priv,
reloc.src_handle, release);
if (!reloc_info[i].src_bo) {
- if (reloc_info[i].dst_bo != cmd_bo)
-
drm_gem_object_unreference_unlocked(&reloc_info[i].dst_bo->gem_base);
ret = -EINVAL;
goto out_free_bos;
}
--
2.1.0
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH 05/11] drm/qxl: Handle all errors in qxl_surface_evict
Only EBUSY error was handled. This could cause code to believe
reserve was successful while it failed.
Signed-off-by: Frediano Ziglio
Reviewed-by: Dave Airlie
---
drivers/gpu/drm/qxl/qxl_cmd.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/qxl/qxl_cmd.c b/drivers/gpu/drm/qxl/qxl_cmd.c
index 85ed5dc..b18f84c 100644
--- a/drivers/gpu/drm/qxl/qxl_cmd.c
+++ b/drivers/gpu/drm/qxl/qxl_cmd.c
@@ -612,8 +612,8 @@ static int qxl_reap_surf(struct qxl_device *qdev, struct
qxl_bo *surf, bool stal
int ret;
ret = qxl_bo_reserve(surf, false);
- if (ret == -EBUSY)
- return -EBUSY;
+ if (ret)
+ return ret;
if (stall)
mutex_unlock(&qdev->surf_evict_mutex);
@@ -622,9 +622,9 @@ static int qxl_reap_surf(struct qxl_device *qdev, struct
qxl_bo *surf, bool stal
if (stall)
mutex_lock(&qdev->surf_evict_mutex);
- if (ret == -EBUSY) {
+ if (ret) {
qxl_bo_unreserve(surf);
- return -EBUSY;
+ return ret;
}
qxl_surface_evict_locked(qdev, surf, true);
--
2.1.0
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH 08/11] drm/qxl: Remove format string errors
Enable format string checks for qxl_io_log and remove resulting warnings
which could lead to memory errors on different platform or just printing
wrong information.
Signed-off-by: Frediano Ziglio
Reviewed-by: Dave Airlie
---
drivers/gpu/drm/qxl/qxl_cmd.c | 2 +-
drivers/gpu/drm/qxl/qxl_display.c | 2 +-
drivers/gpu/drm/qxl/qxl_drv.h | 2 +-
drivers/gpu/drm/qxl/qxl_release.c | 2 +-
4 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/qxl/qxl_cmd.c b/drivers/gpu/drm/qxl/qxl_cmd.c
index b18f84c..edc1eec 100644
--- a/drivers/gpu/drm/qxl/qxl_cmd.c
+++ b/drivers/gpu/drm/qxl/qxl_cmd.c
@@ -248,7 +248,7 @@ int qxl_garbage_collect(struct qxl_device *qdev)
}
}
- QXL_INFO(qdev, "%s: %lld\n", __func__, i);
+ QXL_INFO(qdev, "%s: %d\n", __func__, i);
return i;
}
diff --git a/drivers/gpu/drm/qxl/qxl_display.c
b/drivers/gpu/drm/qxl/qxl_display.c
index 4a0a8b2..a8dbb3e 100644
--- a/drivers/gpu/drm/qxl/qxl_display.c
+++ b/drivers/gpu/drm/qxl/qxl_display.c
@@ -67,7 +67,7 @@ static int qxl_display_copy_rom_client_monitors_config(struct
qxl_device *qdev)
crc = crc32(0, (const uint8_t *)&qdev->rom->client_monitors_config,
sizeof(qdev->rom->client_monitors_config));
if (crc != qdev->rom->client_monitors_config_crc) {
- qxl_io_log(qdev, "crc mismatch: have %X (%d) != %X\n", crc,
+ qxl_io_log(qdev, "crc mismatch: have %X (%zd) != %X\n", crc,
sizeof(qdev->rom->client_monitors_config),
qdev->rom->client_monitors_config_crc);
return 1;
diff --git a/drivers/gpu/drm/qxl/qxl_drv.h b/drivers/gpu/drm/qxl/qxl_drv.h
index 6745c44..62ef8be 100644
--- a/drivers/gpu/drm/qxl/qxl_drv.h
+++ b/drivers/gpu/drm/qxl/qxl_drv.h
@@ -328,7 +328,7 @@ struct qxl_device {
};
/* forward declaration for QXL_INFO_IO */
-void qxl_io_log(struct qxl_device *qdev, const char *fmt, ...);
+__printf(2,3) void qxl_io_log(struct qxl_device *qdev, const char *fmt, ...);
extern const struct drm_ioctl_desc qxl_ioctls[];
extern int qxl_max_ioctl;
diff --git a/drivers/gpu/drm/qxl/qxl_release.c
b/drivers/gpu/drm/qxl/qxl_release.c
index 00604ed..b66ec33 100644
--- a/drivers/gpu/drm/qxl/qxl_release.c
+++ b/drivers/gpu/drm/qxl/qxl_release.c
@@ -153,7 +153,7 @@ qxl_release_alloc(struct qxl_device *qdev, int type,
return handle;
}
*ret = release;
- QXL_INFO(qdev, "allocated release %lld\n", handle);
+ QXL_INFO(qdev, "allocated release %d\n", handle);
release->id = handle;
return handle;
}
--
2.1.0
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH 00/11 v2] Miscellaneous stability patches
This set of patches mainly contains fix for some memory issues using quite aggressively surfaces and other minor problems like images going black after a while. There are no code change from v1, changes: - fix path prefix - rebased on drm-next branch; - add "drm/qxl" prefix on subject; - add reviewed by. About which patches should be applied surely (attempting to put a priority) - "Move main reference counter to GEM object instead of TTM ones" this can causes memory corruption even not wanting to; - "Avoid double free on error" this can be cause leaks in kernel if user space wants, mitigated by the fact that usually DRM inodes are owned by root; - "Handle all errors in qxl_surface_evict" could cause corruption too, not really probable but taking into account that Xorg implementation use a lot signals is not so impossible; - "Handle correctly failures in qxl_alloc_relase_reserved", "Do not leak memory if qxl_release_list_add fails" just cause leaks on situation where memory is already REALLY low, can be omitted; - "Fix print statement not using uninitialized variable", "Remove format string errors" should just print garbage and debugging is disabled by default, not necessary. Frediano Ziglio (11): drm/qxl: Do not cause spice-server to clean our objects drm/qxl: Do not leak memory if qxl_release_list_add fails drm/qxl: Fix print statement not using uninitialized variable drm/qxl: Avoid double free on error drm/qxl: Handle all errors in qxl_surface_evict drm/qxl: Fix return for qxl_release_alloc drm/qxl: Handle correctly failures in qxl_alloc_relase_reserved drm/qxl: Remove format string errors drm/qxl: Move main reference counter to GEM object instead of TTM ones drm/qxl: Simplify cleaning qxl processing command drm/qxl: Propagate correctly errors from qxlhw_handle_to_bo drivers/gpu/drm/qxl/qxl_cmd.c | 11 +- drivers/gpu/drm/qxl/qxl_display.c | 2 +- drivers/gpu/drm/qxl/qxl_drv.h | 2 +- drivers/gpu/drm/qxl/qxl_gem.c | 10 +++-- drivers/gpu/drm/qxl/qxl_ioctl.c | 46 +++ drivers/gpu/drm/qxl/qxl_object.c | 11 -- drivers/gpu/drm/qxl/qxl_release.c | 13 +++ 7 files changed, 46 insertions(+), 49 deletions(-) -- 2.1.0 ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH 03/11] drm/qxl: Fix print statement not using uninitialized variable
reloc_info[i] is not still initialized in the print statement.
Signed-off-by: Frediano Ziglio
Reviewed-by: Dave Airlie
---
drivers/gpu/drm/qxl/qxl_ioctl.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/qxl/qxl_ioctl.c b/drivers/gpu/drm/qxl/qxl_ioctl.c
index e8b5207..230ab84 100644
--- a/drivers/gpu/drm/qxl/qxl_ioctl.c
+++ b/drivers/gpu/drm/qxl/qxl_ioctl.c
@@ -212,7 +212,7 @@ static int qxl_process_single_command(struct qxl_device
*qdev,
/* add the bos to the list of bos to validate -
need to validate first then process relocs? */
if (reloc.reloc_type != QXL_RELOC_TYPE_BO && reloc.reloc_type
!= QXL_RELOC_TYPE_SURF) {
- DRM_DEBUG("unknown reloc type %d\n",
reloc_info[i].type);
+ DRM_DEBUG("unknown reloc type %d\n", reloc.reloc_type);
ret = -EINVAL;
goto out_free_bos;
--
2.1.0
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH 10/11] drm/qxl: Simplify cleaning qxl processing command
In qxlhw_handle_to_bo we incremented counters twice, one time for release object
and one for reloc_info.
In the main function however reloc_info references was drop much earlier than
release so keeping the pointer only on release is safe and make cleaning
process easier.
Signed-off-by: Frediano Ziglio
Reviewed-by: Dave Airlie
---
drivers/gpu/drm/qxl/qxl_ioctl.c | 13 +++--
1 file changed, 3 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/qxl/qxl_ioctl.c b/drivers/gpu/drm/qxl/qxl_ioctl.c
index 85b3808..bb326ff 100644
--- a/drivers/gpu/drm/qxl/qxl_ioctl.c
+++ b/drivers/gpu/drm/qxl/qxl_ioctl.c
@@ -122,10 +122,9 @@ static struct qxl_bo *qxlhw_handle_to_bo(struct qxl_device
*qdev,
qobj = gem_to_qxl_bo(gobj);
ret = qxl_release_list_add(release, qobj);
- if (ret) {
- drm_gem_object_unreference_unlocked(gobj);
+ drm_gem_object_unreference_unlocked(gobj);
+ if (ret)
return NULL;
- }
return qobj;
}
@@ -145,7 +144,7 @@ static int qxl_process_single_command(struct qxl_device
*qdev,
struct qxl_release *release;
struct qxl_bo *cmd_bo;
void *fb_cmd;
- int i, j, ret, num_relocs;
+ int i, ret, num_relocs;
int unwritten;
switch (cmd->type) {
@@ -269,12 +268,6 @@ static int qxl_process_single_command(struct qxl_device
*qdev,
qxl_release_fence_buffer_objects(release);
out_free_bos:
- for (j = 0; j < num_relocs; j++) {
- if (reloc_info[j].dst_bo != cmd_bo)
-
drm_gem_object_unreference_unlocked(&reloc_info[j].dst_bo->gem_base);
- if (reloc_info[j].src_bo && reloc_info[j].src_bo != cmd_bo)
-
drm_gem_object_unreference_unlocked(&reloc_info[j].src_bo->gem_base);
- }
out_free_release:
if (ret)
qxl_release_free(qdev, release);
--
2.1.0
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH 09/11] drm/qxl: Move main reference counter to GEM object instead of TTM ones
qxl_bo structure has two reference counters, one in the GEM object and
another in the TTM object. The GEM object keep a counter to the TTM object
so when GEM counter reached zero the TTM counter (using qxl_bo_unref) was
decremented. The qxl object is fully freed (both GEM and TTM part are cleaned)
when the TTM counter reach zero.
One issue was that surface idr structure has no owning on qxl_bo objects however
it contains a pointer to qxl_bo object. This caused some nasty race condition
for instance qxl_bo object was reaped even after counter was already zero.
This patch fix these races moving main counter (the one used by qxl_bo_(un)ref)
to GEM object which cleanup routine (qxl_gem_object_free) remove the idr pointer
(using qxl_surface_evict) when the counters are still valid.
Signed-off-by: Frediano Ziglio
Reviewed-by: Dave Airlie
---
drivers/gpu/drm/qxl/qxl_gem.c| 10 --
drivers/gpu/drm/qxl/qxl_object.c | 11 ---
2 files changed, 12 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/qxl/qxl_gem.c b/drivers/gpu/drm/qxl/qxl_gem.c
index b96f0c9..d9746e9 100644
--- a/drivers/gpu/drm/qxl/qxl_gem.c
+++ b/drivers/gpu/drm/qxl/qxl_gem.c
@@ -31,9 +31,15 @@
void qxl_gem_object_free(struct drm_gem_object *gobj)
{
struct qxl_bo *qobj = gem_to_qxl_bo(gobj);
+ struct qxl_device *qdev;
+ struct ttm_buffer_object *tbo;
- if (qobj)
- qxl_bo_unref(&qobj);
+ qdev = (struct qxl_device *)gobj->dev->dev_private;
+
+ qxl_surface_evict(qdev, qobj, false);
+
+ tbo = &qobj->tbo;
+ ttm_bo_unref(&tbo);
}
int qxl_gem_object_create(struct qxl_device *qdev, int size,
diff --git a/drivers/gpu/drm/qxl/qxl_object.c b/drivers/gpu/drm/qxl/qxl_object.c
index cdeaf08..6d6f33d 100644
--- a/drivers/gpu/drm/qxl/qxl_object.c
+++ b/drivers/gpu/drm/qxl/qxl_object.c
@@ -208,19 +208,16 @@ void qxl_bo_kunmap_atomic_page(struct qxl_device *qdev,
void qxl_bo_unref(struct qxl_bo **bo)
{
- struct ttm_buffer_object *tbo;
-
if ((*bo) == NULL)
return;
- tbo = &((*bo)->tbo);
- ttm_bo_unref(&tbo);
- if (tbo == NULL)
- *bo = NULL;
+
+ drm_gem_object_unreference_unlocked(&(*bo)->gem_base);
+ *bo = NULL;
}
struct qxl_bo *qxl_bo_ref(struct qxl_bo *bo)
{
- ttm_bo_reference(&bo->tbo);
+ drm_gem_object_reference(&bo->gem_base);
return bo;
}
--
2.1.0
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH 11/11] drm/qxl: Propagate correctly errors from qxlhw_handle_to_bo
This function could return a NULL pointer in case of handle not
present and in case of out of memory conditions however caller
function always returned EINVAL error hiding a possible ENOMEM.
This patch change the function to return the error instead to
be able to propagate the error instead of assuming EINVAL.
Signed-off-by: Frediano Ziglio
Reviewed-by: Dave Airlie
---
drivers/gpu/drm/qxl/qxl_ioctl.c | 33 ++---
1 file changed, 14 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/qxl/qxl_ioctl.c b/drivers/gpu/drm/qxl/qxl_ioctl.c
index bb326ff..37f1faf 100644
--- a/drivers/gpu/drm/qxl/qxl_ioctl.c
+++ b/drivers/gpu/drm/qxl/qxl_ioctl.c
@@ -107,9 +107,9 @@ apply_surf_reloc(struct qxl_device *qdev, struct
qxl_reloc_info *info)
}
/* return holding the reference to this object */
-static struct qxl_bo *qxlhw_handle_to_bo(struct qxl_device *qdev,
-struct drm_file *file_priv, uint64_t
handle,
-struct qxl_release *release)
+static int qxlhw_handle_to_bo(struct qxl_device *qdev,
+ struct drm_file *file_priv, uint64_t handle,
+ struct qxl_release *release, struct qxl_bo
**qbo_p)
{
struct drm_gem_object *gobj;
struct qxl_bo *qobj;
@@ -117,16 +117,17 @@ static struct qxl_bo *qxlhw_handle_to_bo(struct
qxl_device *qdev,
gobj = drm_gem_object_lookup(qdev->ddev, file_priv, handle);
if (!gobj)
- return NULL;
+ return -EINVAL;
qobj = gem_to_qxl_bo(gobj);
ret = qxl_release_list_add(release, qobj);
drm_gem_object_unreference_unlocked(gobj);
if (ret)
- return NULL;
+ return ret;
- return qobj;
+ *qbo_p = qobj;
+ return 0;
}
/*
@@ -219,13 +220,10 @@ static int qxl_process_single_command(struct qxl_device
*qdev,
reloc_info[i].type = reloc.reloc_type;
if (reloc.dst_handle) {
- reloc_info[i].dst_bo = qxlhw_handle_to_bo(qdev,
file_priv,
-
reloc.dst_handle, release);
- if (!reloc_info[i].dst_bo) {
- ret = -EINVAL;
- reloc_info[i].src_bo = NULL;
+ ret = qxlhw_handle_to_bo(qdev, file_priv,
reloc.dst_handle, release,
+&reloc_info[i].dst_bo);
+ if (ret)
goto out_free_bos;
- }
reloc_info[i].dst_offset = reloc.dst_offset;
} else {
reloc_info[i].dst_bo = cmd_bo;
@@ -234,14 +232,11 @@ static int qxl_process_single_command(struct qxl_device
*qdev,
num_relocs++;
/* reserve and validate the reloc dst bo */
- if (reloc.reloc_type == QXL_RELOC_TYPE_BO || reloc.src_handle >
0) {
- reloc_info[i].src_bo =
- qxlhw_handle_to_bo(qdev, file_priv,
- reloc.src_handle, release);
- if (!reloc_info[i].src_bo) {
- ret = -EINVAL;
+ if (reloc.reloc_type == QXL_RELOC_TYPE_BO || reloc.src_handle) {
+ ret = qxlhw_handle_to_bo(qdev, file_priv,
reloc.src_handle, release,
+&reloc_info[i].src_bo);
+ if (ret)
goto out_free_bos;
- }
reloc_info[i].src_offset = reloc.src_offset;
} else {
reloc_info[i].src_bo = NULL;
--
2.1.0
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH spice-server v2 2/2] Handle preferred image compression messages
On Tue, Jun 02, 2015 at 06:26:09PM +0200, Javier Celaya wrote: > El Martes, 2 de junio de 2015 14:33:35 Christophe Fergeau escribió: > > My last remaining comment about this patch is that now the client can > > force the server to use a given compression method. This is something > > the server admin might not want to allow (think "too expensive > > compression methods). > > Maybe image_compression should default to SPICE_IMAGE_COMPRESS_DEFAULT > > rather than _AUTO_GLZ, and we should only allow > > display_channel_handle_preferred_compression() to override > > COMPRESS_DEFAULT, but not other values? > > > > Christophe > > I see. However, currently, QEMU always sets image_compression to the one it > gets > from command-line or _AUTO_GLZ by default. There is no way to know from > within > spice-server whether the admin specified an image compression method or not, > without patching QEMU too. Ah hrm, spice-server has an _AUTO_GLZ default value, but QEMU indeed forces the default too, so we can't change this only with spice-server changes. We could forbid the client from changing compression when it's != AUTO_GLZ, but that probably be weird/unexpected? Christophe pgpJjbsR5lf6S.pgp Description: PGP signature ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH spice-gtk v3 2/3] Cmdline: Get the preferred-compression property
On Tue, Jun 02, 2015 at 05:54:18PM +0200, Javier Celaya wrote: > El Martes, 2 de junio de 2015 14:25:23 usted escribió: > > For what it's worth, once your enum is registered as a GEnum, you can do > > something like gvir_config_genum_get_value > > https://libvirt.org/git/?p=libvirt-glib.git;a=blob;f=libvirt-gconfig/libvirt > > -gconfig- > helpers.c;h=0314a72fd5dfcf8e47df34e7243610071f91b0ee;hb=HEAD#l264 > > in order to go from string representation to integer. As this is only > > needed once here, maybe not worth it. > > > > Christophe > > I supposed so, but I also want to filter out some enum names, like "invalid" > and "lz4" > where not supported, so it seemed more clear in this way.-- Sure, your current patch is fine with me. Christophe pgp0zTO7ZoAdV.pgp Description: PGP signature ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH spice-gtk v3 3/3] Display: Send a preferred compression message on init.
On Tue, Jun 02, 2015 at 06:04:57PM +0200, Javier Celaya wrote: > El Martes, 2 de junio de 2015 14:26:58 Christophe Fergeau escribió: > > I'd deal with the "preferred_compression >= > > SPICE_IMAGE_COMPRESS_ENUM_END" case with a g_warn_if_fail() as > > g_param_spec_enum should guarantee that you won't get a value not > > covered by the registered GEnum. > > > > Christophe > > Then I shouldn't check it at all... After all, there is no check for the > other two properties > in that function (cache-size and glz-window-size). I would rather remove the > "preferred_compression < SPICE_IMAGE_COMPRESS_ENUM_END" condition.-- Sure, fine with me. Christophe pgpYGL8jHtJMh.pgp Description: PGP signature ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [spice-xpi] Workaround firefox-gtk3 bug
Ping ?
Christophe
On Thu, May 28, 2015 at 01:37:24PM +0200, Christophe Fergeau wrote:
> When it's built with gtk3, firefox plugin-container will use some
> LD_PRELOAD hack in order to be able to load the gtk2 flash plugin.
> However, this LD_PRELOAD'ed .so will cause gtk3 remote-viewer to crash
> when it's started by spice-xpi
>
> This commit just cleans up the unwanted LD_PRELOAD variable from the
> environment before starting remote-viewer.
> ---
> SpiceXPI/src/plugin/controller.cpp | 7 +++
> 1 file changed, 7 insertions(+)
>
> diff --git a/SpiceXPI/src/plugin/controller.cpp
> b/SpiceXPI/src/plugin/controller.cpp
> index f8cd39d..63a669d 100644
> --- a/SpiceXPI/src/plugin/controller.cpp
> +++ b/SpiceXPI/src/plugin/controller.cpp
> @@ -169,6 +169,13 @@ gpointer SpiceController::ClientThread(gpointer data)
> else
> env = g_environ_setenv(env, "SPICE_PROXY",
> fake_this->m_proxy.c_str(), TRUE);
>
> +// Work around bug in firefox gtk3 builds, see
> +// https://bugzilla.redhat.com/show_bug.cgi?id=1217076
> +// http://emilio.pozuelo.org/posts/75
> +//
> +// This LD_PRELOAD'ed library causes gtk3 applications to crash
> +env = g_environ_unsetenv(env, "LD_PRELOAD");
> +
> // Try to spawn main client
> client_argv = fake_this->GetClientPath();
> if (client_argv != NULL) {
> --
> 2.4.1
>
> ___
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel
pgpCNqTze5l9j.pgp
Description: PGP signature
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [vdagent-linux] Prepare for 0.16.0 release
On Wed, May 27, 2015 at 03:17:18PM +0200, Christophe Fergeau wrote: > --- > Hey, > > As suggested by poma, it might be a good time to have a 0.16.0 release of > spice-vdagent. Let me know what you think :) Since I haven't heard objections, I'll move forward with this soon. Christophe pgpEhP2aiGvZp.pgp Description: PGP signature ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH v2] Add password length check
Don't allow setting a too long password.
---
Diff to v1: only kept the admin/user password setting check
server/reds.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/server/reds.c b/server/reds.c
index 6d70b68..5579109 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -3503,6 +3503,8 @@ SPICE_GNUC_VISIBLE int
spice_server_set_ticket(SpiceServer *s,
taTicket.expiration_time = now + lifetime;
}
if (passwd != NULL) {
+if (strlen(passwd) > SPICE_MAX_PASSWORD_LENGTH)
+return -1;
g_strlcpy(taTicket.password, passwd, sizeof(taTicket.password));
} else {
memset(taTicket.password, 0, sizeof(taTicket.password));
--
2.1.4
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH v4] Check too long password
Make sure that the password lenght is under the maximum lenght. If not
report it as an authentication failure with an adapted message.
---
Diff to v3:
* Removed the checks on the server side and the corresponding code here
* Removed the new error code to reuse SPICE_CLIENT_ERROR_AUTH_NEEDS_PASSWORD.
* spice_channel_send_spice_ticket returns SpiceChannelEvent instead of a
gboolean:
this helps properly propagating the kind of error to the client.
gtk/spice-channel.c | 80 ++---
1 file changed, 51 insertions(+), 29 deletions(-)
diff --git a/gtk/spice-channel.c b/gtk/spice-channel.c
index 4e7d8b7..d01b147 100644
--- a/gtk/spice-channel.c
+++ b/gtk/spice-channel.c
@@ -1010,7 +1010,34 @@ static int spice_channel_read(SpiceChannel *channel,
void *data, size_t length)
}
/* coroutine context */
-static void spice_channel_send_spice_ticket(SpiceChannel *channel)
+static void spice_channel_failed_authentication(SpiceChannel *channel,
+gboolean invalidPassword)
+{
+SpiceChannelPrivate *c = channel->priv;
+
+if (c->auth_needs_username_and_password)
+g_set_error_literal(&c->error,
+SPICE_CLIENT_ERROR,
+
SPICE_CLIENT_ERROR_AUTH_NEEDS_PASSWORD_AND_USERNAME,
+_("Authentication failed: password and username
are required"));
+else if (invalidPassword)
+g_set_error_literal(&c->error,
+SPICE_CLIENT_ERROR,
+SPICE_CLIENT_ERROR_AUTH_NEEDS_PASSWORD,
+_("Authentication failed: password is too long"));
+else
+g_set_error_literal(&c->error,
+SPICE_CLIENT_ERROR,
+SPICE_CLIENT_ERROR_AUTH_NEEDS_PASSWORD,
+_("Authentication failed: password is required"));
+
+c->event = SPICE_CHANNEL_ERROR_AUTH;
+
+c->has_error = TRUE; /* force disconnect */
+}
+
+/* coroutine context */
+static SpiceChannelEvent spice_channel_send_spice_ticket(SpiceChannel *channel)
{
SpiceChannelPrivate *c = channel->priv;
EVP_PKEY *pubkey;
@@ -1020,13 +1047,14 @@ static void
spice_channel_send_spice_ticket(SpiceChannel *channel)
char *password;
uint8_t *encrypted;
int rc;
+SpiceChannelEvent ret = SPICE_CHANNEL_ERROR_LINK;
bioKey = BIO_new(BIO_s_mem());
-g_return_if_fail(bioKey != NULL);
+g_return_val_if_fail(bioKey != NULL, ret);
BIO_write(bioKey, c->peer_msg->pub_key, SPICE_TICKET_PUBKEY_BYTES);
pubkey = d2i_PUBKEY_bio(bioKey, NULL);
-g_return_if_fail(pubkey != NULL);
+g_return_val_if_fail(pubkey != NULL, ret);
rsa = pubkey->pkey.rsa;
nRSASize = RSA_size(rsa);
@@ -1039,36 +1067,24 @@ static void
spice_channel_send_spice_ticket(SpiceChannel *channel)
g_object_get(c->session, "password", &password, NULL);
if (password == NULL)
password = g_strdup("");
+if (strlen(password) > SPICE_MAX_PASSWORD_LENGTH) {
+spice_channel_failed_authentication(channel, TRUE);
+ret = SPICE_CHANNEL_ERROR_AUTH;
+goto cleanup;
+}
rc = RSA_public_encrypt(strlen(password) + 1, (uint8_t*)password,
encrypted, rsa, RSA_PKCS1_OAEP_PADDING);
g_warn_if_fail(rc > 0);
spice_channel_write(channel, encrypted, nRSASize);
+ret = SPICE_CHANNEL_NONE;
+
+cleanup:
memset(encrypted, 0, nRSASize);
EVP_PKEY_free(pubkey);
BIO_free(bioKey);
g_free(password);
-}
-
-/* coroutine context */
-static void spice_channel_failed_authentication(SpiceChannel *channel)
-{
-SpiceChannelPrivate *c = channel->priv;
-
-if (c->auth_needs_username_and_password)
-g_set_error_literal(&c->error,
-SPICE_CLIENT_ERROR,
-
SPICE_CLIENT_ERROR_AUTH_NEEDS_PASSWORD_AND_USERNAME,
-_("Authentication failed: password and username
are required"));
-else
-g_set_error_literal(&c->error,
-SPICE_CLIENT_ERROR,
-SPICE_CLIENT_ERROR_AUTH_NEEDS_PASSWORD,
-_("Authentication failed: password is required"));
-
-c->event = SPICE_CHANNEL_ERROR_AUTH;
-
-c->has_error = TRUE; /* force disconnect */
+return ret;
}
/* coroutine context */
@@ -1088,7 +1104,7 @@ static gboolean spice_channel_recv_auth(SpiceChannel
*channel)
if (link_res != SPICE_LINK_ERR_OK) {
CHANNEL_DEBUG(channel, "link result: reply %d", link_res);
-spice_channel_failed_authentication(channel);
+spice_channel_failed_authentication(channel, FALSE);
return FALSE;
}
@@ -1662,7 +1678,7 @@ error:
if (saslconn)
sasl_dispose(&saslconn);
-spice_channel_failed_authentication(channel);
+spice_channel_failed_authenticat
Re: [Spice-devel] [PATCH v4] Check too long password
On Wed, Jun 03, 2015 at 04:22:35PM +0200, Cédric Bosdonnat wrote: > Make sure that the password lenght is under the maximum lenght. If not Nit-pick if you re-post, s/lenght/length/g or someone can fixup when merging > report it as an authentication failure with an adapted message. > --- > Diff to v3: > >* Removed the checks on the server side and the corresponding code here >* Removed the new error code to reuse > SPICE_CLIENT_ERROR_AUTH_NEEDS_PASSWORD. >* spice_channel_send_spice_ticket returns SpiceChannelEvent instead of a > gboolean: > this helps properly propagating the kind of error to the client. > Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH spice-gtk v4 2/3] Add a preferred-compression program option
---
gtk/spice-option.c | 37 +
1 file changed, 37 insertions(+)
diff --git a/gtk/spice-option.c b/gtk/spice-option.c
index 958e03c..463a3e3 100644
--- a/gtk/spice-option.c
+++ b/gtk/spice-option.c
@@ -41,6 +41,7 @@ static gint cache_size = 0;
static gint glz_window_size = 0;
static gchar *secure_channels = NULL;
static gchar *shared_dir = NULL;
+static SpiceImageCompress preferred_compression = SPICE_IMAGE_COMPRESS_INVALID;
G_GNUC_NORETURN
static void option_version(void)
@@ -149,6 +150,33 @@ static gboolean parse_usbredir_filter(const gchar
*option_name,
return TRUE;
}
+static gboolean parse_preferred_compression(const gchar *option_name, const
gchar *value,
+gpointer data, GError **error)
+{
+if (!strcmp(value, "auto-glz")) {
+preferred_compression = SPICE_IMAGE_COMPRESS_AUTO_GLZ;
+} else if (!strcmp(value, "auto-lz")) {
+preferred_compression = SPICE_IMAGE_COMPRESS_AUTO_LZ;
+} else if (!strcmp(value, "quic")) {
+preferred_compression = SPICE_IMAGE_COMPRESS_QUIC;
+} else if (!strcmp(value, "glz")) {
+preferred_compression = SPICE_IMAGE_COMPRESS_GLZ;
+} else if (!strcmp(value, "lz")) {
+preferred_compression = SPICE_IMAGE_COMPRESS_LZ;
+#ifdef USE_LZ4
+} else if (!strcmp(value, "lz4")) {
+preferred_compression = SPICE_IMAGE_COMPRESS_LZ4;
+#endif
+} else if (!strcmp(value, "off")) {
+preferred_compression = SPICE_IMAGE_COMPRESS_OFF;
+} else {
+preferred_compression = SPICE_IMAGE_COMPRESS_INVALID;
+g_set_error(error, G_OPTION_ERROR, G_OPTION_ERROR_FAILED,
+_("Image compression algorithm %s not supported"), value);
+return FALSE;
+}
+return TRUE;
+}
/**
* spice_get_option_group: (skip)
@@ -194,6 +222,13 @@ GOptionGroup* spice_get_option_group(void)
N_("Glz compression history size"), N_("") },
{ "spice-shared-dir", '\0', 0, G_OPTION_ARG_FILENAME, &shared_dir,
N_("Shared directory"), N_("") },
+{ "spice-preferred-compression", '\0', 0, G_OPTION_ARG_CALLBACK,
parse_preferred_compression,
+ N_("Preferred image compression algorithm"),
+#ifdef USE_LZ4
+ "" },
+#else
+ "" },
+#endif
{ "spice-debug", '\0', G_OPTION_FLAG_NO_ARG, G_OPTION_ARG_CALLBACK,
option_debug,
N_("Enable Spice-GTK debugging"), NULL },
@@ -281,4 +316,6 @@ void spice_set_session_option(SpiceSession *session)
g_object_set(session, "glz-window-size", glz_window_size, NULL);
if (shared_dir)
g_object_set(session, "shared-dir", shared_dir, NULL);
+if (preferred_compression != SPICE_IMAGE_COMPRESS_INVALID)
+g_object_set(session, "preferred-compression", preferred_compression,
NULL);
}
--
1.9.3
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH spice-gtk v4 3/3] Display: Send a preferred compression message on init.
If the user prefers a specific compression algorithm, report it when
setting up the display channel.
---
gtk/channel-display.c | 11 +++
1 file changed, 11 insertions(+)
diff --git a/gtk/channel-display.c b/gtk/channel-display.c
index efe2259..5dd3f71 100644
--- a/gtk/channel-display.c
+++ b/gtk/channel-display.c
@@ -789,12 +789,15 @@ static void spice_display_channel_up(SpiceChannel
*channel)
SpiceMsgOut *out;
SpiceSession *s = spice_channel_get_session(channel);
SpiceMsgcDisplayInit init;
+SpiceMsgcDisplayPreferredCompression pref_comp_msg;
int cache_size;
int glz_window_size;
+SpiceImageCompress preferred_compression = SPICE_IMAGE_COMPRESS_INVALID;
g_object_get(s,
"cache-size", &cache_size,
"glz-window-size", &glz_window_size,
+ "preferred-compression", &preferred_compression,
NULL);
CHANNEL_DEBUG(channel, "%s: cache_size %d, glz_window_size %d (bytes)",
__FUNCTION__,
cache_size, glz_window_size);
@@ -810,6 +813,14 @@ static void spice_display_channel_up(SpiceChannel *channel)
this monitor */
if (channel->priv->channel_id != 0)
g_coroutine_object_notify(G_OBJECT(channel), "monitors");
+
+if (spice_channel_test_capability(channel,
SPICE_DISPLAY_CAP_PREF_COMPRESSION) &&
+preferred_compression > SPICE_IMAGE_COMPRESS_INVALID) {
+pref_comp_msg.image_compression = preferred_compression;
+out = spice_msg_out_new(channel,
SPICE_MSGC_DISPLAY_PREFERRED_COMPRESSION);
+out->marshallers->msgc_display_preferred_compression(out->marshaller,
&pref_comp_msg);
+spice_msg_out_send_internal(out);
+}
}
#define DRAW(type) {\
--
1.9.3
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH spice-html5 1/3] Minor change to error message formating.
Signed-off-by: Jeremy White
---
display.js | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/display.js b/display.js
index 814ada6..a749dae 100644
--- a/display.js
+++ b/display.js
@@ -469,11 +469,11 @@ SpiceDisplayConn.prototype.process_channel_message =
function(msg)
if (!this.streams)
this.streams = new Array();
if (this.streams[m.id])
-console.log("Stream already exists");
+console.log("Stream " + m.id + " already exists");
else
this.streams[m.id] = m;
if (m.codec_type != SPICE_VIDEO_CODEC_TYPE_MJPEG)
-console.log("Unhandled stream codec: "+m.codec_type);
+console.log("Unhandled stream codec: " + m.codec_type);
return true;
}
--
2.1.4
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH spice-html5 3/3] Add support for stream reports.
This helps video playback do a slightly better job of keeping up
in the browser. It's not a dramatic effect, but enough to start making
video playback almost tolerable.
Signed-off-by: Jeremy White
---
display.js | 67 ++--
enums.js | 1 +
main.js | 3 +++
spiceconn.js | 3 ++-
spicemsg.js | 54
5 files changed, 116 insertions(+), 12 deletions(-)
diff --git a/display.js b/display.js
index 7027230..7938527 100644
--- a/display.js
+++ b/display.js
@@ -477,31 +477,48 @@ SpiceDisplayConn.prototype.process_channel_message =
function(msg)
return true;
}
-if (msg.type == SPICE_MSG_DISPLAY_STREAM_DATA)
+if (msg.type == SPICE_MSG_DISPLAY_STREAM_DATA ||
+msg.type == SPICE_MSG_DISPLAY_STREAM_DATA_SIZED)
{
-var m = new SpiceMsgDisplayStreamData(msg.data);
+var m;
+if (msg.type == SPICE_MSG_DISPLAY_STREAM_DATA_SIZED)
+m = new SpiceMsgDisplayStreamDataSized(msg.data);
+else
+m = new SpiceMsgDisplayStreamData(msg.data);
+
if (!this.streams[m.base.id])
{
console.log("no stream for data");
return false;
}
+
+var mmtime = (Date.now() - this.parent.our_mm_time) +
this.parent.mm_time;
+var latency = m.base.multi_media_time - mmtime;
+
if (this.streams[m.base.id].codec_type ===
SPICE_VIDEO_CODEC_TYPE_MJPEG)
-{
-process_mjpeg_stream_data(this, m);
+process_mjpeg_stream_data(this, m, latency);
+
+if ("report" in this.streams[m.base.id])
+process_stream_data_report(this, m, mmtime, latency);
-}
return true;
}
-if (msg.type == SPICE_MSG_DISPLAY_STREAM_DATA_SIZED)
+if (msg.type == SPICE_MSG_DISPLAY_STREAM_ACTIVATE_REPORT)
{
-var m = new SpiceMsgDisplayStreamDataSized(msg.data);
-if (this.streams[m.base.id].codec_type ===
SPICE_VIDEO_CODEC_TYPE_MJPEG)
-process_mjpeg_stream_data(this, m);
+var m = new SpiceMsgDisplayStreamActivateReport(msg.data);
+
+var report = new SpiceMsgcDisplayStreamReport(m.stream_id,
m.unique_id);
+if (this.streams[m.stream_id])
+{
+this.streams[m.stream_id].report = report;
+this.streams[m.stream_id].max_window_size = m.max_window_size;
+this.streams[m.stream_id].timeout_ms = m.timeout_ms
+}
+
return true;
}
-
if (msg.type == SPICE_MSG_DISPLAY_STREAM_CLIP)
{
var m = new SpiceMsgDisplayStreamClip(msg.data);
@@ -811,8 +828,15 @@ function handle_draw_jpeg_onload()
}
}
-function process_mjpeg_stream_data(sc, m)
+function process_mjpeg_stream_data(sc, m, latency)
{
+if (latency < 0)
+{
+if ("report" in sc.streams[m.base.id])
+sc.streams[m.base.id].report.num_drops++;
+return;
+}
+
var tmpstr = "data:image/jpeg,";
var img = new Image;
var i;
@@ -837,3 +861,24 @@ function process_mjpeg_stream_data(sc, m)
img.src = tmpstr;
}
+function process_stream_data_report(sc, m, mmtime, latency)
+{
+sc.streams[m.base.id].report.num_frames++;
+if (sc.streams[m.base.id].report.start_frame_mm_time == 0)
+sc.streams[m.base.id].report.start_frame_mm_time =
m.base.multi_media_time;
+
+if (sc.streams[m.base.id].report.num_frames >
sc.streams[m.base.id].max_window_size ||
+(m.base.multi_media_time -
sc.streams[m.base.id].report.start_frame_mm_time) >
sc.streams[m.base.id].timeout_ms)
+{
+sc.streams[m.base.id].report.end_frame_mm_time =
m.base.multi_media_time;
+sc.streams[m.base.id].report.last_frame_delay = latency;
+
+var msg = new SpiceMiniData();
+msg.build_msg(SPICE_MSGC_DISPLAY_STREAM_REPORT,
sc.streams[m.base.id].report);
+sc.send_msg(msg);
+
+sc.streams[m.base.id].report.start_frame_mm_time = 0;
+sc.streams[m.base.id].report.num_frames = 0;
+sc.streams[m.base.id].report.num_drops = 0;
+}
+}
diff --git a/enums.js b/enums.js
index 7f55e46..6e94025 100644
--- a/enums.js
+++ b/enums.js
@@ -133,6 +133,7 @@ var SPICE_MSG_DISPLAY_DRAW_COMPOSITE= 318;
var SPICE_MSG_DISPLAY_STREAM_ACTIVATE_REPORT = 319;
var SPICE_MSGC_DISPLAY_INIT = 101;
+var SPICE_MSGC_DISPLAY_STREAM_REPORT= 102;
var SPICE_MSG_INPUTS_INIT = 101;
var SPICE_MSG_INPUTS_KEY_MODIFIERS = 102;
diff --git a/main.js b/main.js
index bfc102a..99b2274 100644
--- a/main.js
+++ b/main.js
@@ -86,6 +86,9 @@ SpiceMainConn.prototype.process_channel_message =
function(msg)
" ; ram_hint "+
this.main_init.ram_hint);
}
+this.our_mm_time = Date.now();
+this.mm_time = this.main_init.multi_media_time;
+
this.handle_mouse_mode(this.main_init.current_mouse_mode,
[Spice-devel] [PATCH spice-html5 2/3] Implement support for sized data streams.
Signed-off-by: Jeremy White
---
display.js | 60 ++--
enums.js | 11 +++
spiceconn.js | 4
spicemsg.js | 23 +++
4 files changed, 76 insertions(+), 22 deletions(-)
diff --git a/display.js b/display.js
index a749dae..7027230 100644
--- a/display.js
+++ b/display.js
@@ -487,32 +487,21 @@ SpiceDisplayConn.prototype.process_channel_message =
function(msg)
}
if (this.streams[m.base.id].codec_type ===
SPICE_VIDEO_CODEC_TYPE_MJPEG)
{
-var tmpstr = "data:image/jpeg,";
-var img = new Image;
-var i;
-for (i = 0; i < m.data.length; i++)
-{
-tmpstr += '%';
-if (m.data[i] < 16)
-tmpstr += '0';
-tmpstr += m.data[i].toString(16);
-}
-var strm_base = new SpiceMsgDisplayBase();
-strm_base.surface_id = this.streams[m.base.id].surface_id;
-strm_base.box = this.streams[m.base.id].dest;
-strm_base.clip = this.streams[m.base.id].clip;
-img.o =
-{ base: strm_base,
- tag: "mjpeg." + m.base.id,
- descriptor: null,
- sc : this,
-};
-img.onload = handle_draw_jpeg_onload;
-img.src = tmpstr;
+process_mjpeg_stream_data(this, m);
+
}
return true;
}
+if (msg.type == SPICE_MSG_DISPLAY_STREAM_DATA_SIZED)
+{
+var m = new SpiceMsgDisplayStreamDataSized(msg.data);
+if (this.streams[m.base.id].codec_type ===
SPICE_VIDEO_CODEC_TYPE_MJPEG)
+process_mjpeg_stream_data(this, m);
+return true;
+}
+
+
if (msg.type == SPICE_MSG_DISPLAY_STREAM_CLIP)
{
var m = new SpiceMsgDisplayStreamClip(msg.data);
@@ -821,3 +810,30 @@ function handle_draw_jpeg_onload()
this.o.sc.surfaces[this.o.base.surface_id].draw_count++;
}
}
+
+function process_mjpeg_stream_data(sc, m)
+{
+var tmpstr = "data:image/jpeg,";
+var img = new Image;
+var i;
+for (i = 0; i < m.data.length; i++)
+{
+tmpstr += '%';
+if (m.data[i] < 16)
+tmpstr += '0';
+tmpstr += m.data[i].toString(16);
+}
+var strm_base = new SpiceMsgDisplayBase();
+strm_base.surface_id = sc.streams[m.base.id].surface_id;
+strm_base.box = m.dest || sc.streams[m.base.id].dest;
+strm_base.clip = sc.streams[m.base.id].clip;
+img.o =
+{ base: strm_base,
+ tag: "mjpeg." + m.base.id,
+ descriptor: null,
+ sc : sc,
+};
+img.onload = handle_draw_jpeg_onload;
+img.src = tmpstr;
+}
+
diff --git a/enums.js b/enums.js
index 17d77cb..7f55e46 100644
--- a/enums.js
+++ b/enums.js
@@ -127,6 +127,10 @@ var SPICE_MSG_DISPLAY_DRAW_TRANSPARENT = 312;
var SPICE_MSG_DISPLAY_DRAW_ALPHA_BLEND = 313;
var SPICE_MSG_DISPLAY_SURFACE_CREATE= 314;
var SPICE_MSG_DISPLAY_SURFACE_DESTROY = 315;
+var SPICE_MSG_DISPLAY_STREAM_DATA_SIZED = 316;
+var SPICE_MSG_DISPLAY_MONITORS_CONFIG = 317;
+var SPICE_MSG_DISPLAY_DRAW_COMPOSITE= 318;
+var SPICE_MSG_DISPLAY_STREAM_ACTIVATE_REPORT = 319;
var SPICE_MSGC_DISPLAY_INIT = 101;
@@ -171,6 +175,13 @@ var SPICE_MAIN_CAP_NAME_AND_UUID = 1;
var SPICE_MAIN_CAP_AGENT_CONNECTED_TOKENS = 2;
var SPICE_MAIN_CAP_SEAMLESS_MIGRATE = 3;
+var SPICE_DISPLAY_CAP_SIZED_STREAM= 0;
+var SPICE_DISPLAY_CAP_MONITORS_CONFIG = 1;
+var SPICE_DISPLAY_CAP_COMPOSITE = 2;
+var SPICE_DISPLAY_CAP_A8_SURFACE = 3;
+var SPICE_DISPLAY_CAP_STREAM_REPORT = 4;
+var SPICE_DISPLAY_CAP_LZ4_COMPRESSION = 5;
+
var SPICE_AUDIO_DATA_MODE_INVALID = 0;
var SPICE_AUDIO_DATA_MODE_RAW = 1;
var SPICE_AUDIO_DATA_MODE_CELT_0_5_1= 2;
diff --git a/spiceconn.js b/spiceconn.js
index ec42d8d..b7043c0 100644
--- a/spiceconn.js
+++ b/spiceconn.js
@@ -133,6 +133,10 @@ SpiceConn.prototype =
msg.channel_caps.push(
(1 << SPICE_MAIN_CAP_AGENT_CONNECTED_TOKENS)
);
+else if (msg.channel_type == SPICE_CHANNEL_DISPLAY)
+msg.channel_caps.push(
+(1 << SPICE_DISPLAY_CAP_SIZED_STREAM)
+);
hdr.size = msg.buffer_size();
diff --git a/spicemsg.js b/spicemsg.js
index e167b3d..4b78d94 100644
--- a/spicemsg.js
+++ b/spicemsg.js
@@ -1146,6 +1146,29 @@ SpiceMsgDisplayStreamData.prototype =
},
}
+function SpiceMsgDisplayStreamDataSized(a, at)
+{
+this.from_buffer(a, at);
+}
+
+SpiceMsgDisplayStreamDataSized.prototype =
+{
+from_buffer: function(a, at)
+{
+at = at || 0;
+var dv = new SpiceDataView(a);
+this.base = new SpiceStreamDataHeader;
+at = this.base.from_dv(dv, at, a);
+this.width = dv.getUint32(at, true); at += 4;
+this.height = dv.getUint32
