Re: [Spice-devel] Spice agent and LXC

2014-11-24 Thread Charles Ricketts
Using the "Spice for Newbies" (which doesn't really seem to fit its
title IMO) documentation I was able to answer some of my questions
regarding data flow. However, I'm not closer to solving my problem.
After building from the git repositories I get the same results in a
Fedora LXC while an Ubuntu LXC segfaults during startup.

(II) spiceqxl(0): 249: 1080x7680, 16 bits, stride 2160, 317mm x 2255mm,
orientation 1
(II) spiceqxl(0): 250: 7680x1080, 16 bits, stride 15360, 2255mm x 317mm,
orientation 2
(II) spiceqxl(0): 251: 1080x7680, 16 bits, stride 2160, 317mm x 2255mm,
orientation 3
(II) spiceqxl(0): PreInit complete
(--) Depth 24 pixmap format is 32 bpp
(II) spiceqxl(0): framebuffer at 0x7f708425d010 (16384 KB)
(II) spiceqxl(0): command ram at 0x7f708525d010 (114680 KB)
(II) spiceqxl(0): vram at 0x7f707c25c010 (131072 KB)
(II) spiceqxl(0): rom at 0x7f70945498a0
resizing surface0 to 16777216
memory space from 0x7f708525d010 to 0x7f708c25a010
memory space from 0x7f707c25c010 to 0x7f708425c010
(EE)
(EE) Backtrace:
(EE) 0: X (xorg_backtrace+0x48) [0x7f7093a48d28]
(EE) 1: X (0x7f70938a+0x1aca19) [0x7f7093a4ca19]
(EE) 2: /lib/x86_64-linux-gnu/libpthread.so.0 (0x7f709299d000+0x10340)
[0x7f70929ad340]
(EE) 3: /usr/lib/x86_64-linux-gnu/libspice-server.so.1
(0x7f708d21a000+0x20d3c) [0x7f708d23ad3c]
(EE) 4: /usr/lib/x86_64-linux-gnu/libspice-server.so.1
(0x7f708d21a000+0x210dc) [0x7f708d23b0dc]
(EE) 5: /usr/lib/xorg/modules/drivers/spiceqxl_drv.so
(0x7f708d537000+0x97b5) [0x7f708d5407b5]
(EE) 6: /usr/lib/x86_64-linux-gnu/libspice-server.so.1
(0x7f708d21a000+0x22dde) [0x7f708d23cdde]
(EE) 7: /usr/lib/x86_64-linux-gnu/libspice-server.so.1
(spice_server_add_interface+0x3a6) [0x7f708d263206]
(EE) 8: /usr/lib/xorg/modules/drivers/spiceqxl_drv.so
(0x7f708d537000+0xc193) [0x7f708d543193]
(EE) 9: X (AddScreen+0x71) [0x7f70938f5ca1]
(EE) 10: X (InitOutput+0x3c8) [0x7f7093936ce8]
(EE) 11: X (0x7f70938a+0x596bb) [0x7f70938f96bb]
(EE) 12: /lib/x86_64-linux-gnu/libc.so.6 (__libc_start_main+0xf5)
[0x7f70913dcec5]
(EE) 13: X (0x7f70938a+0x44dde) [0x7f70938e4dde]
(EE)
(EE) Segmentation fault at address 0xd8
(EE)
Fatal server error:
(EE) Caught signal 11 (Segmentation fault). Server aborting
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel


Re: [Spice-devel] [PATCH 1/2][spice-gtk] audio: new-sample callback must return GST_FLOW_OK

2014-11-24 Thread Fabiano Fidêncio
On Mon, Nov 24, 2014 at 6:57 PM, Victor Toso  wrote:
> This fix is necessary when using windows-client with gstreamer 1.0 to
> record-audio in the target machine.
>
> ---
>  gtk/spice-gstaudio.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/gtk/spice-gstaudio.c b/gtk/spice-gstaudio.c
> index 6173783..df2fc8e 100644
> --- a/gtk/spice-gstaudio.c
> +++ b/gtk/spice-gstaudio.c
> @@ -120,7 +120,7 @@ static void spice_gstaudio_class_init(SpiceGstaudioClass 
> *klass)
>  g_type_class_add_private(klass, sizeof(SpiceGstaudioPrivate));
>  }
>
> -static void record_new_buffer(GstAppSink *appsink, gpointer data)
> +static GstFlowReturn record_new_buffer(GstAppSink *appsink, gpointer data)
>  {
>  SpiceGstaudio *gstaudio = data;
>  SpiceGstaudioPrivate *p = gstaudio->priv;
> @@ -135,6 +135,7 @@ static void record_new_buffer(GstAppSink *appsink, 
> gpointer data)
>  msg = gst_message_new_application(GST_OBJECT(p->record.pipe), NULL);
>  #endif
>  gst_element_post_message(p->record.pipe, msg);
> +return GST_FLOW_OK;
>  }
>
>  static void record_stop(SpiceGstaudio *gstaudio)
> --
> 2.1.0
>
> ___
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel


ACK!
-- 
Fabiano Fidêncio
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel


Re: [Spice-devel] [PATCH 2/2] audio: Avoid bad pipelines from gst_parse_launch

2014-11-24 Thread Fabiano Fidêncio
On Mon, Nov 24, 2014 at 6:57 PM, Victor Toso  wrote:
> gst_parse_launch may return not NULL even when error is set.
> This can lead to data loss when playing or recording audio.

Is there any case where gst_parse_launch returns NULL but does not set
the error?
If no, I'd go for a simple:
if (error != NULL) {
 
}


Best Regards,
-- 
Fabiano Fidêncio
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel


Re: [Spice-devel] [xf86-video-qxl v2] Enable smartcard support for XSpice.

2014-11-24 Thread Jeremy White

Sorry; this was wrong.  v3 should hopefully be correct.

Cheers,

Jeremy


___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel


[Spice-devel] [xf86-video-qxl v3] Enable smartcard support for XSpice.

2014-11-24 Thread Jeremy White
This is done by creating a Unix domain socket to which smartcard
messages are transferred, using the vscard protocol.

A further system library, spiceccid, is used to provide an interface into
pcsc-lite, specifically the pcsc-lite daemon, so that regular Unix applications
can access the passed through smartcard information.

Signed-off-by: Jeremy White 
---
 configure.ac   |   21 ++
 examples/spiceqxl.xorg.conf.example|3 +
 src/Makefile.am|3 +-
 src/qxl.h  |2 +
 src/qxl_driver.c   |   14 +-
 src/spiceccid/Makefile.am  |   29 ++
 src/spiceccid/spice.pcsc.conf.template |7 +
 src/spiceccid/spiceccid.c  |  455 
 src/spiceqxl_smartcard.c   |  193 ++
 src/spiceqxl_smartcard.h   |   31 +++
 10 files changed, 756 insertions(+), 2 deletions(-)
 create mode 100644 src/spiceccid/Makefile.am
 create mode 100644 src/spiceccid/spice.pcsc.conf.template
 create mode 100644 src/spiceccid/spiceccid.c
 create mode 100644 src/spiceqxl_smartcard.c
 create mode 100644 src/spiceqxl_smartcard.h

diff --git a/configure.ac b/configure.ac
index 14e0597..d9da852 100644
--- a/configure.ac
+++ b/configure.ac
@@ -137,8 +137,27 @@ if test "x$enable_xspice" = "xyes"; then
 else
 enable_xspice=no
 fi
+
+AC_ARG_ENABLE([ccid],
+[AS_HELP_STRING([--enable-ccid],
+[Build the spiceccid SmartCard driver (default is no)])],
+[enable_ccid=$enableval],
+[enable_ccid=no])
+AC_ARG_WITH(ccid-module-dir,
+[AS_HELP_STRING([--with-ccid-module-dir=DIR ],
+[Specify the install path for spiceccid driver (default is 
$libdir/pcsc/drivers/serial)])],
+[ cciddir="$withval" ],
+[ cciddir="$libdir/pcsc/drivers/serial" ])
+AC_SUBST(cciddir)
+if test "x$enable_ccid" != "xno"; then
+PKG_CHECK_MODULES(LIBPCSCLITE, [libpcsclite])
+PKG_CHECK_MODULES(LIBCACARD, [libcacard])
+fi
+
+
 AM_CONDITIONAL(BUILD_XSPICE, test "x$enable_xspice" = "xyes")
 AM_CONDITIONAL(BUILD_QXL, test "x$enable_qxl" = "xyes")
+AM_CONDITIONAL(BUILD_SPICECCID, test "x$enable_ccid" = "xyes")
 
 AC_ARG_ENABLE([udev],
AS_HELP_STRING([--disable-udev], [Disable libudev support 
[default=auto]]),
@@ -168,6 +187,7 @@ fi
 AC_CONFIG_FILES([
 Makefile
 src/Makefile
+src/spiceccid/Makefile
 src/uxa/Makefile
 scripts/Makefile
 examples/Makefile
@@ -187,4 +207,5 @@ echo "
 KMS:  ${DRM_MODE}
 Build qxl:${enable_qxl}
 Build xspice: ${enable_xspice}
+Build spiceccid:  ${enable_ccid}
 "
diff --git a/examples/spiceqxl.xorg.conf.example 
b/examples/spiceqxl.xorg.conf.example
index 597a5bd..d15f7f2 100644
--- a/examples/spiceqxl.xorg.conf.example
+++ b/examples/spiceqxl.xorg.conf.example
@@ -143,6 +143,9 @@ Section "Device"
 #  to the client.   Default is no mixing.
 #Option "SpicePlaybackFIFODir"  "/tmp/"
 
+# A unix domain name for a unix domain socket
+#  to communicate with a spiceccid smartcard driver
+#Option "SpiceSmartCardFile"  "/tmp/spice.pcsc.comm"
 EndSection
 
 Section "InputDevice"
diff --git a/src/Makefile.am b/src/Makefile.am
index bf50ae1..6c72bbd 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -25,7 +25,7 @@
 # _ladir passes a dummy rpath to libtool so the thing will actually link
 # TODO: -nostdlib/-Bstatic/-lgcc platform magic, not installing the .a, etc.
 
-SUBDIRS=uxa
+SUBDIRS=uxa spiceccid
 
 AM_CFLAGS = $(SPICE_PROTOCOL_CFLAGS) $(XORG_CFLAGS) $(PCIACCESS_CFLAGS) 
$(CWARNFLAGS) $(DRM_CFLAGS) @LIBUDEV_CFLAGS@
 
@@ -96,6 +96,7 @@ spiceqxl_drv_la_SOURCES = \
spiceqxl_uinput.c   \
spiceqxl_uinput.h   \
spiceqxl_audio.c\
+   spiceqxl_smartcard.c\
spiceqxl_audio.h\
spiceqxl_inputs.c   \
spiceqxl_inputs.h   \
diff --git a/src/qxl.h b/src/qxl.h
index 603faca..54995cf 100644
--- a/src/qxl.h
+++ b/src/qxl.h
@@ -157,6 +157,7 @@ enum {
 OPTION_FRAME_BUFFER_SIZE,
 OPTION_SURFACE_BUFFER_SIZE,
 OPTION_COMMAND_BUFFER_SIZE,
+OPTION_SPICE_SMARTCARD_FILE,
 #endif
 OPTION_COUNT,
 };
@@ -352,6 +353,7 @@ struct _qxl_screen_t
 
 char playback_fifo_dir[PATH_MAX];
 void *playback_opaque;
+char smartcard_file[PATH_MAX];
 #endif /* XSPICE */
 
 uint32_t deferred_fps;
diff --git a/src/qxl_driver.c b/src/qxl_driver.c
index 165f468..9ad8921 100644
--- a/src/qxl_driver.c
+++ b/src/qxl_driver.c
@@ -55,6 +55,7 @@
 #include "spiceqxl_io_port.h"
 #include "spiceqxl_spice_server.h"
 #include "spiceqxl_audio.h"
+#include "spiceqxl_smartcard.

[Spice-devel] [PATCH 2/2] audio: Avoid bad pipelines from gst_parse_launch

2014-11-24 Thread Victor Toso
gst_parse_launch may return not NULL even when error is set.
This can lead to data loss when playing or recording audio.
---
 gtk/spice-gstaudio.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/gtk/spice-gstaudio.c b/gtk/spice-gstaudio.c
index df2fc8e..2b7f683 100644
--- a/gtk/spice-gstaudio.c
+++ b/gtk/spice-gstaudio.c
@@ -245,7 +245,7 @@ static void record_start(SpiceRecordChannel *channel, gint 
format, gint channels
 "appsink caps=\"%s\" name=appsink", audio_caps);
 
 p->record.pipe = gst_parse_launch(pipeline, &error);
-if (p->record.pipe == NULL) {
+if (error != NULL || p->record.pipe == NULL) {
 g_warning("Failed to create pipeline: %s", error->message);
 goto lerr;
 }
@@ -345,7 +345,7 @@ static void playback_start(SpicePlaybackChannel *channel, 
gint format, gint chan
"audioconvert ! audioresample ! 
autoaudiosink name=\"audiosink\"", audio_caps);
 SPICE_DEBUG("audio pipeline: %s", pipeline);
 p->playback.pipe = gst_parse_launch(pipeline, &error);
-if (p->playback.pipe == NULL) {
+if (error != NULL || p->playback.pipe == NULL) {
 g_warning("Failed to create pipeline: %s", error->message);
 goto lerr;
 }
-- 
2.1.0

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel


[Spice-devel] [PATCH 1/2][spice-gtk] audio: new-sample callback must return GST_FLOW_OK

2014-11-24 Thread Victor Toso
This fix is necessary when using windows-client with gstreamer 1.0 to
record-audio in the target machine.

---
 gtk/spice-gstaudio.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/gtk/spice-gstaudio.c b/gtk/spice-gstaudio.c
index 6173783..df2fc8e 100644
--- a/gtk/spice-gstaudio.c
+++ b/gtk/spice-gstaudio.c
@@ -120,7 +120,7 @@ static void spice_gstaudio_class_init(SpiceGstaudioClass 
*klass)
 g_type_class_add_private(klass, sizeof(SpiceGstaudioPrivate));
 }
 
-static void record_new_buffer(GstAppSink *appsink, gpointer data)
+static GstFlowReturn record_new_buffer(GstAppSink *appsink, gpointer data)
 {
 SpiceGstaudio *gstaudio = data;
 SpiceGstaudioPrivate *p = gstaudio->priv;
@@ -135,6 +135,7 @@ static void record_new_buffer(GstAppSink *appsink, gpointer 
data)
 msg = gst_message_new_application(GST_OBJECT(p->record.pipe), NULL);
 #endif
 gst_element_post_message(p->record.pipe, msg);
+return GST_FLOW_OK;
 }
 
 static void record_stop(SpiceGstaudio *gstaudio)
-- 
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] chardev: remove write polling

2014-11-24 Thread Marc-André Lureau
Hi

- Original Message -
> Hey,
> 
> On Mon, Nov 24, 2014 at 04:56:26PM +0100, Marc-André Lureau wrote:
> > In an effort to reduce the wakeups per second, get rid of the
> > "write_to_dev" timer when the implementation supports
> > SPICE_CHAR_DEVICE_NOTIFY_WRITABLE.
> 
> Do you know what is the status of getting support for this in QEMU? Did
> the char device changes this feature depends on get committed to master?

It is in Gerd spice-next branch, I assume it will be merge soon, so it would be 
better to make sure it is indeed in 0.12.6.
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel


Re: [Spice-devel] [PATCH spice] chardev: remove write polling

2014-11-24 Thread Christophe Fergeau
Hey,

On Mon, Nov 24, 2014 at 04:56:26PM +0100, Marc-André Lureau wrote:
> In an effort to reduce the wakeups per second, get rid of the
> "write_to_dev" timer when the implementation supports
> SPICE_CHAR_DEVICE_NOTIFY_WRITABLE.

Do you know what is the status of getting support for this in QEMU? Did
the char device changes this feature depends on get committed to master?

Christophe


pgpnj_W1ZrDAY.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] Remove guest side video time-stamping

2014-11-24 Thread Marc-André Lureau
On Mon, Nov 24, 2014 at 4:46 PM, Christophe Fergeau  wrote:
> This could be renamed to reds_enable_mm_time in a followup patch as
> there is no longer a timer


It took me some time to understand, all mm_timer could be renamed to
mm_time after the patch, I agree.

-- 
Marc-André Lureau
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel


Re: [Spice-devel] [spice-gtk][PATCH] spice-widget: Don't crash when Display is NULL

2014-11-24 Thread Fabiano Fidêncio
On Mon, Nov 24, 2014 at 4:28 PM, Christophe Fergeau  wrote:
> On Mon, Nov 24, 2014 at 01:25:51AM +0100, Fabiano Fidêncio wrote:
>> Return early or warn when SpiceDisplay is NULL instead of crash, avoiding
>> segfaults when running on windows using GTK3
>> ---
>>  gtk/spice-widget.c | 16 +---
>>  1 file changed, 13 insertions(+), 3 deletions(-)
>>
>> diff --git a/gtk/spice-widget.c b/gtk/spice-widget.c
>> index ae11073..67429f0 100644
>> --- a/gtk/spice-widget.c
>> +++ b/gtk/spice-widget.c
>> @@ -701,9 +701,12 @@ SpiceGrabSequence 
>> *spice_display_get_grab_keys(SpiceDisplay *display)
>>  static void try_keyboard_grab(SpiceDisplay *display)
>>  {
>>  GtkWidget *widget = GTK_WIDGET(display);
>> +GdkWindow *window = gtk_widget_get_window(widget);
>>  SpiceDisplayPrivate *d = display->priv;
>>  GdkGrabStatus status;
>>
>> +g_return_if_fail(window != NULL);
>> +
>>  if (g_getenv("SPICE_NOGRAB"))
>>  return;
>>  if (d->disable_inputs)
>> @@ -731,8 +734,8 @@ static void try_keyboard_grab(SpiceDisplay *display)
>>  GetModuleHandle(NULL), 0);
>>  g_warn_if_fail(d->keyboard_hook != NULL);
>>  #endif
>> -status = gdk_keyboard_grab(gtk_widget_get_window(widget), FALSE,
>> -   GDK_CURRENT_TIME);
>> +
>> +status = gdk_keyboard_grab(window, FALSE, GDK_CURRENT_TIME);
>>  if (status != GDK_GRAB_SUCCESS) {
>>  g_warning("keyboard grab failed %d", status);
>>  d->keyboard_grab_active = false;
>> @@ -1294,7 +1297,14 @@ static gboolean check_for_grab_key(SpiceDisplay 
>> *display, int type, int keyval)
>>  static void update_display(SpiceDisplay *display)
>>  {
>>  #ifdef G_OS_WIN32
>> -win32_window = display ? 
>> GDK_WINDOW_HWND(gtk_widget_get_window(GTK_WIDGET(display))) : NULL;
>> +GdkWindow *window = NULL;
>> +
>> +if (display != NULL) {
>> +window = gtk_widget_get_window(GTK_WIDGET(display));
>> +g_warn_if_fail (window != NULL);
>> +}
>> +
>> +win32_window = window ? GDK_WINDOW_HWND(window) : NULL;
>>  #endif
>
> For what it's worth, this hunk seems to be more about protecting against
> a NULL window (which could come from
> gtk_widget_get_window(GTK_WIDGET(NULL))) than protecting against a NULL
> display.
>

 I see your point. Would be better a different patch for this part.

Best Regards,
-- 
Fabiano Fidêncio
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel


[Spice-devel] [PATCH spice] chardev: remove write polling

2014-11-24 Thread Marc-André Lureau
In an effort to reduce the wakeups per second, get rid of the
"write_to_dev" timer when the implementation supports
SPICE_CHAR_DEVICE_NOTIFY_WRITABLE.

When this flag is set, the frontend instance is responsible for calling
spice_char_device_wakeup() when the device is ready to perform IO.

Related to:
https://bugzilla.redhat.com/show_bug.cgi?id=912763
---

v3: fix interface version check, spotted by Uri

server/char_device.c | 36 +++-
 server/spice.h   |  9 +++--
 2 files changed, 34 insertions(+), 11 deletions(-)

diff --git a/server/char_device.c b/server/char_device.c
index 6d2339e..5f1b32d 100644
--- a/server/char_device.c
+++ b/server/char_device.c
@@ -438,7 +438,10 @@ static int 
spice_char_device_write_to_device(SpiceCharDeviceState *dev)
 }
 
 spice_char_device_state_ref(dev);
-core->timer_cancel(dev->write_to_dev_timer);
+
+if (dev->write_to_dev_timer) {
+core->timer_cancel(dev->write_to_dev_timer);
+}
 
 sif = SPICE_CONTAINEROF(dev->sin->base.sif, SpiceCharDeviceInterface, 
base);
 while (dev->running) {
@@ -473,8 +476,10 @@ static int 
spice_char_device_write_to_device(SpiceCharDeviceState *dev)
 /* retry writing as long as the write queue is not empty */
 if (dev->running) {
 if (dev->cur_write_buf) {
-core->timer_start(dev->write_to_dev_timer,
-  CHAR_DEVICE_WRITE_TO_TIMEOUT);
+if (dev->write_to_dev_timer) {
+core->timer_start(dev->write_to_dev_timer,
+  CHAR_DEVICE_WRITE_TO_TIMEOUT);
+}
 } else {
 spice_assert(ring_is_empty(&dev->write_queue));
 }
@@ -488,7 +493,9 @@ static void spice_char_dev_write_retry(void *opaque)
 {
 SpiceCharDeviceState *dev = opaque;
 
-core->timer_cancel(dev->write_to_dev_timer);
+if (dev->write_to_dev_timer) {
+core->timer_cancel(dev->write_to_dev_timer);
+}
 spice_char_device_write_to_device(dev);
 }
 
@@ -635,6 +642,7 @@ SpiceCharDeviceState 
*spice_char_device_state_create(SpiceCharDeviceInstance *si
  void *opaque)
 {
 SpiceCharDeviceState *char_dev;
+SpiceCharDeviceInterface *sif;
 
 spice_assert(sin);
 spice_assert(cbs->read_one_msg_from_device && cbs->ref_msg_to_client &&
@@ -652,10 +660,15 @@ SpiceCharDeviceState 
*spice_char_device_state_create(SpiceCharDeviceInstance *si
 ring_init(&char_dev->write_bufs_pool);
 ring_init(&char_dev->clients);
 
-char_dev->write_to_dev_timer = core->timer_add(spice_char_dev_write_retry, 
char_dev);
-if (!char_dev->write_to_dev_timer) {
-spice_error("failed creating char dev write timer");
+sif = SPICE_CONTAINEROF(char_dev->sin->base.sif, SpiceCharDeviceInterface, 
base);
+if ((sif->base.minor_version <= 1 && sif->base.minor_version <= 2) ||
+!(sif->flags & SPICE_CHAR_DEVICE_NOTIFY_WRITABLE)) {
+char_dev->write_to_dev_timer = 
core->timer_add(spice_char_dev_write_retry, char_dev);
+if (!char_dev->write_to_dev_timer) {
+spice_error("failed creating char dev write timer");
+}
 }
+
 char_dev->refs = 1;
 sin->st = char_dev;
 spice_debug("sin %p dev_state %p", sin, char_dev);
@@ -697,7 +710,9 @@ static void 
spice_char_device_state_unref(SpiceCharDeviceState *char_dev)
 void spice_char_device_state_destroy(SpiceCharDeviceState *char_dev)
 {
 reds_on_char_device_state_destroy(char_dev);
-core->timer_remove(char_dev->write_to_dev_timer);
+if (char_dev->write_to_dev_timer) {
+core->timer_remove(char_dev->write_to_dev_timer);
+}
 write_buffers_queue_free(&char_dev->write_queue);
 write_buffers_queue_free(&char_dev->write_bufs_pool);
 if (char_dev->cur_write_buf) {
@@ -805,7 +820,9 @@ void spice_char_device_stop(SpiceCharDeviceState *dev)
 spice_debug("dev_state %p", dev);
 dev->running = FALSE;
 dev->active = FALSE;
-core->timer_cancel(dev->write_to_dev_timer);
+if (dev->write_to_dev_timer) {
+core->timer_cancel(dev->write_to_dev_timer);
+}
 }
 
 void spice_char_device_reset(SpiceCharDeviceState *dev)
@@ -842,6 +859,7 @@ void spice_char_device_reset(SpiceCharDeviceState *dev)
 
 void spice_char_device_wakeup(SpiceCharDeviceState *dev)
 {
+spice_char_device_write_to_device(dev);
 spice_char_device_read_from_device(dev);
 }
 
diff --git a/server/spice.h b/server/spice.h
index 58700d1..bd5bba8 100644
--- a/server/spice.h
+++ b/server/spice.h
@@ -24,7 +24,7 @@
 #include 
 #include 
 
-#define SPICE_SERVER_VERSION 0x000c05 /* release 0.12.5 */
+#define SPICE_SERVER_VERSION 0x000c06 /* release 0.12.6 */
 
 #ifdef SPICE_SERVER_INTERNAL
 #undef SPICE_GNUC_DEPRECATED
@@ -402,11 +402,15 @@ void spice_server_set_record_rate(SpiceRecordInstance 
*sin, uint32_t frequen
 
 #define SPICE_INTERFACE_CHAR_DEVICE "char_device"
 #define SPICE_INTERFACE_CHAR_DEVIC

Re: [Spice-devel] [PATCH spice] Remove guest side video time-stamping

2014-11-24 Thread Christophe Fergeau
On Fri, Nov 14, 2014 at 01:55:27PM +0100, Marc-André Lureau wrote:
> The multimedia time is defined by the server side monotonic time [1],
> but the drawing time-stamp is done in guest side, so it requires
> synchronization between host and guest. This is expensive, when no audio
> is playing, there is a ~30x/sec wakeup to update the qxl device mmtime,
> and it requires marking dirty the rom region.
> 
> Instead, the video timestamping can be done more efficiently on server
> side, without visible drawbacks.
> 
> [1] a better timestamp could be the audio time, since audio players are
> usually sync with audio time)
> 
> Related to:
> https://bugzilla.redhat.com/show_bug.cgi?id=912763
> ---
> 
> This is a resend, as a non-RFC version this time.
> 
>  server/red_dispatcher.c |  9 -
>  server/red_worker.c |  1 +
>  server/reds-private.h   |  2 --
>  server/reds.c   | 13 -
>  server/snd_worker.c |  1 -
>  server/spice-qxl.h  |  2 +-

This file does not exist upstream

>  6 files changed, 2 insertions(+), 26 deletions(-)
> 
> diff --git a/server/red_dispatcher.c b/server/red_dispatcher.c
> index a6ffe7b..d8389bc 100644
> --- a/server/red_dispatcher.c
> +++ b/server/red_dispatcher.c
> @@ -751,15 +751,6 @@ static void qxl_worker_loadvm_commands(QXLWorker 
> *qxl_worker,
>  red_dispatcher_loadvm_commands((RedDispatcher*)qxl_worker, ext, count);
>  }
>  
> -void red_dispatcher_set_mm_time(uint32_t mm_time)
> -{
> -RedDispatcher *now = dispatchers;
> -while (now) {
> -now->qxl->st->qif->set_mm_time(now->qxl, mm_time);
> -now = now->next;
> -}
> -}
> -
>  static inline int calc_compression_level(void)
>  {
>  spice_assert(streaming_video != STREAM_VIDEO_INVALID);
> diff --git a/server/red_worker.c b/server/red_worker.c
> index 9f18495..cbb78a2 100644
> --- a/server/red_worker.c
> +++ b/server/red_worker.c
> @@ -4195,6 +4195,7 @@ static inline void red_process_drawable(RedWorker 
> *worker, RedDrawable *red_draw
>  return;
>  }
>  
> +red_drawable->mm_time = reds_get_mm_time();
>  surface_id = drawable->surface_id;
>  
>  worker->surfaces[surface_id].refs++;
> diff --git a/server/reds-private.h b/server/reds-private.h
> index ee09e7c..0bfbf3b 100644
> --- a/server/reds-private.h
> +++ b/server/reds-private.h
> @@ -6,7 +6,6 @@
>  #include 
>  
>  #define MIGRATE_TIMEOUT (1000 * 10) /* 10sec */
> -#define MM_TIMER_GRANULARITY_MS (1000 / 30)
>  #define MM_TIME_DELTA 400 /*ms*/
>  
>  typedef struct TicketAuthentication {
> @@ -159,7 +158,6 @@ typedef struct RedsState {
>  int dispatcher_allows_client_mouse;
>  MonitorMode monitor_mode;
>  SpiceTimer *mig_timer;
> -SpiceTimer *mm_timer;
>  
>  int vm_running;
>  Ring char_devs_states; /* list of SpiceCharDeviceStateItem */
> diff --git a/server/reds.c b/server/reds.c
> index 505bacd..c2d71f6 100644
> --- a/server/reds.c
> +++ b/server/reds.c
> @@ -2799,7 +2799,6 @@ uint32_t reds_get_mm_time(void)
>  
>  void reds_enable_mm_timer(void)

This could be renamed to reds_enable_mm_time in a followup patch as
there is no longer a timer

>  {
> -core->timer_start(reds->mm_timer, MM_TIMER_GRANULARITY_MS);
>  reds->mm_timer_enabled = TRUE;
>  reds->mm_time_latency = MM_TIME_DELTA;
>  reds_send_mm_time();
> @@ -2807,16 +2806,9 @@ void reds_enable_mm_timer(void)
>  
>  void reds_disable_mm_timer(void)

Same here.

ACK otherwise.

Christophe


pgp10pmHGziMq.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-gtk][PATCH] spice-widget: Don't crash when Display is NULL

2014-11-24 Thread Christophe Fergeau
On Mon, Nov 24, 2014 at 01:25:51AM +0100, Fabiano Fidêncio wrote:
> Return early or warn when SpiceDisplay is NULL instead of crash, avoiding
> segfaults when running on windows using GTK3
> ---
>  gtk/spice-widget.c | 16 +---
>  1 file changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/gtk/spice-widget.c b/gtk/spice-widget.c
> index ae11073..67429f0 100644
> --- a/gtk/spice-widget.c
> +++ b/gtk/spice-widget.c
> @@ -701,9 +701,12 @@ SpiceGrabSequence 
> *spice_display_get_grab_keys(SpiceDisplay *display)
>  static void try_keyboard_grab(SpiceDisplay *display)
>  {
>  GtkWidget *widget = GTK_WIDGET(display);
> +GdkWindow *window = gtk_widget_get_window(widget);
>  SpiceDisplayPrivate *d = display->priv;
>  GdkGrabStatus status;
>  
> +g_return_if_fail(window != NULL);
> +
>  if (g_getenv("SPICE_NOGRAB"))
>  return;
>  if (d->disable_inputs)
> @@ -731,8 +734,8 @@ static void try_keyboard_grab(SpiceDisplay *display)
>  GetModuleHandle(NULL), 0);
>  g_warn_if_fail(d->keyboard_hook != NULL);
>  #endif
> -status = gdk_keyboard_grab(gtk_widget_get_window(widget), FALSE,
> -   GDK_CURRENT_TIME);
> +
> +status = gdk_keyboard_grab(window, FALSE, GDK_CURRENT_TIME);
>  if (status != GDK_GRAB_SUCCESS) {
>  g_warning("keyboard grab failed %d", status);
>  d->keyboard_grab_active = false;
> @@ -1294,7 +1297,14 @@ static gboolean check_for_grab_key(SpiceDisplay 
> *display, int type, int keyval)
>  static void update_display(SpiceDisplay *display)
>  {
>  #ifdef G_OS_WIN32
> -win32_window = display ? 
> GDK_WINDOW_HWND(gtk_widget_get_window(GTK_WIDGET(display))) : NULL;
> +GdkWindow *window = NULL;
> +
> +if (display != NULL) {
> +window = gtk_widget_get_window(GTK_WIDGET(display));
> +g_warn_if_fail (window != NULL);
> +}
> +
> +win32_window = window ? GDK_WINDOW_HWND(window) : NULL;
>  #endif

For what it's worth, this hunk seems to be more about protecting against
a NULL window (which could come from
gtk_widget_get_window(GTK_WIDGET(NULL))) than protecting against a NULL
display.

Christophe



pgpI1jqQcLdaW.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-gtk][PATCH] spice-widget: Don't crash when Display is NULL

2014-11-24 Thread Fabiano Fidêncio
On Mon, Nov 24, 2014 at 3:35 PM, Marc-André Lureau
 wrote:
> On Mon, Nov 24, 2014 at 3:32 PM, Fabiano Fidêncio  
> wrote:
>> When opening the very first window and when a any other display was being 
>> added.
>> I know this is probably a virt-viewer fault that we have to fix there,
>> I just think that if we can avoid the crash here and warn the user is
>> better than just crash.
>
>
> I mean, could you provide a backtrace where the crash happens?

Not so easily (thanks to the windows builds). I'll generate the
packages in a way I can have the debug info on not stripped out.
I will do it soon, but probably not today :-(

Best Regards,
-- 
Fabiano Fidêncio
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel


Re: [Spice-devel] [spice-gtk][PATCH] spice-widget: Don't crash when Display is NULL

2014-11-24 Thread Marc-André Lureau
On Mon, Nov 24, 2014 at 3:32 PM, Fabiano Fidêncio  wrote:
> When opening the very first window and when a any other display was being 
> added.
> I know this is probably a virt-viewer fault that we have to fix there,
> I just think that if we can avoid the crash here and warn the user is
> better than just crash.


I mean, could you provide a backtrace where the crash happens?

-- 
Marc-André Lureau
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel


Re: [Spice-devel] [spice-gtk][PATCH] spice-widget: Don't crash when Display is NULL

2014-11-24 Thread Fabiano Fidêncio
On Mon, Nov 24, 2014 at 3:27 PM, Marc-André Lureau
 wrote:
> On Mon, Nov 24, 2014 at 1:25 AM, Fabiano Fidêncio  wrote:
>> Return early or warn when SpiceDisplay is NULL instead of crash, avoiding
>> segfaults when running on windows using GTK3
>
>
> Where did the crash happen?

When opening the very first window and when a any other display was being added.
I know this is probably a virt-viewer fault that we have to fix there,
I just think that if we can avoid the crash here and warn the user is
better than just crash.

Best Regards,
-- 
Fabiano Fidêncio
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel


Re: [Spice-devel] [spice-gtk][PATCH] spice-widget: Don't crash when Display is NULL

2014-11-24 Thread Marc-André Lureau
On Mon, Nov 24, 2014 at 1:25 AM, Fabiano Fidêncio  wrote:
> Return early or warn when SpiceDisplay is NULL instead of crash, avoiding
> segfaults when running on windows using GTK3


Where did the crash happen?

-- 
Marc-André Lureau
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel


Re: [Spice-devel] [PATCH spice-gtk v4] Release keyboard grab using keyboard shortcut

2014-11-24 Thread Marc-André Lureau
It works now, ack

On Mon, Nov 24, 2014 at 8:24 AM, Pavel Grunt  wrote:
> This commit adds the ability to release the keyboard grab when
> the release keys (ctrl+alt) are pressed and released. It allows
> to use keyboard shortcuts (eg alt+tab, alt+f4) on the client.
>
> The keyboard is grabbed again when the release keys are pressed
> and released or when the mouse moves.
>
> https://bugs.freedesktop.org/show_bug.cgi?id=85331
> ---
> v4:
>  - when 'focus in' clear activeseq, because it can wrongly contain keys that 
> were pressed
>but missed the event they were released
> v3:
>  - don't grab when the keyboard doesn't have the focus
> v2:
>  - add missing 'release_keys(display)' calls
> ---
>  gtk/spice-widget-priv.h |  2 ++
>  gtk/spice-widget.c  | 52 
> ++---
>  2 files changed, 47 insertions(+), 7 deletions(-)
>
> diff --git a/gtk/spice-widget-priv.h b/gtk/spice-widget-priv.h
> index 9c38e2e..0e1f661 100644
> --- a/gtk/spice-widget-priv.h
> +++ b/gtk/spice-widget-priv.h
> @@ -109,6 +109,8 @@ struct _SpiceDisplayPrivate {
>  guint   key_delayed_id;
>  SpiceGrabSequence *grabseq; /* the configured key sequence */
>  gboolean*activeseq; /* the currently pressed keys */
> +gbooleanseq_pressed;
> +gbooleankeyboard_grab_released;
>  gintmark;
>  #ifdef WIN32
>  HHOOK   keyboard_hook;
> diff --git a/gtk/spice-widget.c b/gtk/spice-widget.c
> index ae11073..87d2bb0 100644
> --- a/gtk/spice-widget.c
> +++ b/gtk/spice-widget.c
> @@ -719,6 +719,8 @@ static void try_keyboard_grab(SpiceDisplay *display)
>  return;
>  if (!d->mouse_have_pointer)
>  return;
> +if (d->keyboard_grab_released)
> +return;
>
>  g_return_if_fail(gtk_widget_is_focus(widget));
>
> @@ -1198,6 +1200,9 @@ static void send_key(SpiceDisplay *display, int 
> scancode, SendKeyType type, gboo
>  if (d->disable_inputs)
>  return;
>
> +if (d->keyboard_grab_released)
> +return;
> +
>  i = scancode / 32;
>  b = scancode % 32;
>  m = (1 << b);
> @@ -1259,7 +1264,8 @@ static void release_keys(SpiceDisplay *display)
>  }
>  }
>
> -static gboolean check_for_grab_key(SpiceDisplay *display, int type, int 
> keyval)
> +static gboolean check_for_grab_key(SpiceDisplay *display, int type, int 
> keyval,
> +   int check_type, int reset_type)
>  {
>  SpiceDisplayPrivate *d = display->priv;
>  int i;
> @@ -1267,13 +1273,13 @@ static gboolean check_for_grab_key(SpiceDisplay 
> *display, int type, int keyval)
>  if (!d->grabseq->nkeysyms)
>  return FALSE;
>
> -if (type == GDK_KEY_PRESS) {
> -/* Record the new key press */
> +if (type == check_type) {
> +/* Record the new key */
>  for (i = 0 ; i < d->grabseq->nkeysyms ; i++)
>  if (d->grabseq->keysyms[i] == keyval)
>  d->activeseq[i] = TRUE;
>
> -/* Return if any key is not pressed */
> +/* Return if any key is missing */
>  for (i = 0 ; i < d->grabseq->nkeysyms ; i++)
>  if (d->activeseq[i] == FALSE)
>  return FALSE;
> @@ -1281,9 +1287,10 @@ static gboolean check_for_grab_key(SpiceDisplay 
> *display, int type, int keyval)
>  /* resets the whole grab sequence on success */
>  memset(d->activeseq, 0, sizeof(gboolean) * d->grabseq->nkeysyms);
>  return TRUE;
> -} else if (type == GDK_KEY_RELEASE) {
> -/* Any key release resets the whole grab sequence */
> +} else if (type == reset_type) {
> +/* reset key event type resets the whole grab sequence */
>  memset(d->activeseq, 0, sizeof(gboolean) * d->grabseq->nkeysyms);
> +d->seq_pressed = FALSE;
>  return FALSE;
>  } else
>  g_warn_if_reached();
> @@ -1291,6 +1298,16 @@ static gboolean check_for_grab_key(SpiceDisplay 
> *display, int type, int keyval)
>  return FALSE;
>  }
>
> +static gboolean check_for_grab_key_pressed(SpiceDisplay *display, int type, 
> int keyval)
> +{
> +return check_for_grab_key(display, type, keyval, GDK_KEY_PRESS, 
> GDK_KEY_RELEASE);
> +}
> +
> +static gboolean check_for_grab_key_released(SpiceDisplay *display, int type, 
> int keyval)
> +{
> +return check_for_grab_key(display, type, keyval, GDK_KEY_RELEASE, 
> GDK_KEY_PRESS);
> +}
> +
>  static void update_display(SpiceDisplay *display)
>  {
>  #ifdef G_OS_WIN32
> @@ -1321,7 +1338,7 @@ static gboolean key_event(GtkWidget *widget, 
> GdkEventKey *key)
>  __FUNCTION__, key->type == GDK_KEY_PRESS ? "press" : "release",
>  key->hardware_keycode, key->state, key->group, key->is_modifier);
>
> -if (check_for_grab_key(display, key->type, key->keyval)) {
> +if (!d->seq_pressed && check_for_grab_key_pressed(display, key->type, 
> key->keyval)) {
>  g_sig

Re: [Spice-devel] [spice-gtk] Remove channels from SpiceSession::channels on session disconnect

2014-11-24 Thread Christophe Fergeau
On Fri, Nov 21, 2014 at 02:13:20PM +0100, Marc-André Lureau wrote:
> Fundamentally, it doesn't change much from my patch, I guess removing
> g_clear_object(channel->priv->session) and ignoring the case where the
> channel isn't in the session (like you do here) would have been the
> same.

Yup, that's very likely, it was easier for me to restart from scratch (after
learning from your patches) than to try to modify your patches, but
they probably don't need to be much different indeed.

> 
> I still think it's a better idea to clear the channel->session if the
> channel has been disconnect from the session, although I agree that it
> requires a bit more testing and fixing for some warnings/criticals.

Well, clearing the session would indeed be cleaner, but it has its share
of issues with existing code. Ideally it would be possible not to set
the session to NULL for now, and warn when something tries to use the
session after it has been disconnected. This would allow to fix the
various issues that show up without risking a crash on a NULL session,
but I don't really see how to achieve that except by adding some
test+g_warning to all function acting on a session.

Christophe


pgpr22bl8yUIR.pgp
Description: PGP signature
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel