[Spice-devel] spice can't transfer files from gust OS to client

2018-11-20 Thread wangjiedong
I installed spice-vdagent in guest OS Ubuntu 1804 , then i connect guest with 
virt-viewer .
I can drag files to guest Ubuntu from client , but i can't drag files out from 
guest Ubuntu.
Then i try it with guest OS windows installed windows-spice-tools , it can't 
transfer files from guest to client too !

Do we only transfer file from client to guest or any configuration i ignore?
Thanks for help !___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel


[Spice-devel] [PATCH spice-gtk] spice-channel: Adjust endianness in a single place for link message

2018-11-20 Thread Frediano Ziglio
Make sure all fields have right endian. "error" field was not adjusted.

Signed-off-by: Frediano Ziglio 
---
 src/spice-channel.c | 12 +---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/src/spice-channel.c b/src/spice-channel.c
index 1b3578cd..1486137e 100644
--- a/src/spice-channel.c
+++ b/src/spice-channel.c
@@ -1913,6 +1913,12 @@ static gboolean spice_channel_recv_link_msg(SpiceChannel 
*channel)
   c->name, __FUNCTION__, rc, c->peer_hdr.size);
 goto error;
 }
+
+c->peer_msg->error = GUINT32_FROM_LE(c->peer_msg->error);
+c->peer_msg->num_common_caps = 
GUINT32_FROM_LE(c->peer_msg->num_common_caps);
+c->peer_msg->num_channel_caps = 
GUINT32_FROM_LE(c->peer_msg->num_channel_caps);
+c->peer_msg->caps_offset = GUINT32_FROM_LE(c->peer_msg->caps_offset);
+
 switch (c->peer_msg->error) {
 case SPICE_LINK_ERR_OK:
 /* nothing */
@@ -1928,8 +1934,8 @@ static gboolean spice_channel_recv_link_msg(SpiceChannel 
*channel)
 goto error;
 }
 
-num_channel_caps = GUINT32_FROM_LE(c->peer_msg->num_channel_caps);
-num_common_caps = GUINT32_FROM_LE(c->peer_msg->num_common_caps);
+num_channel_caps = c->peer_msg->num_channel_caps;
+num_common_caps = c->peer_msg->num_common_caps;
 
 num_caps = num_channel_caps + num_common_caps;
 CHANNEL_DEBUG(channel, "%s: %u caps", __FUNCTION__, num_caps);
@@ -1937,7 +1943,7 @@ static gboolean spice_channel_recv_link_msg(SpiceChannel 
*channel)
 /* see original spice/client code: */
 /* g_return_if_fail(c->peer_msg + c->peer_msg->caps_offset * 
sizeof(uint32_t) > c->peer_msg + c->peer_hdr.size); */
 
-caps_src = (uint8_t *)c->peer_msg + 
GUINT32_FROM_LE(c->peer_msg->caps_offset);
+caps_src = (uint8_t *)c->peer_msg + c->peer_msg->caps_offset;
 CHANNEL_DEBUG(channel, "got remote common caps:");
 store_caps(caps_src, num_common_caps, c->remote_common_caps);
 
-- 
2.17.2

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


Re: [Spice-devel] [PATCH spice-gtk 1/2] spice-channel: Remove useless peer_pos

2018-11-20 Thread Christophe Fergeau
On Tue, Nov 20, 2018 at 01:25:44PM +, Frediano Ziglio wrote:
> With coroutine the spice_channel_recv_link_msg function reads the
> entire message, no reason to store partial position.

This reverts 9a36e853d which predates coroutines indeed.

Acked-by: Christophe Fergeau 

> 
> Signed-off-by: Frediano Ziglio 
> ---
>  src/spice-channel-priv.h | 1 -
>  src/spice-channel.c  | 7 ++-
>  2 files changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/src/spice-channel-priv.h b/src/spice-channel-priv.h
> index 091c1163..5984ca56 100644
> --- a/src/spice-channel-priv.h
> +++ b/src/spice-channel-priv.h
> @@ -129,7 +129,6 @@ struct _SpiceChannelPrivate {
>  SpiceLinkHeader link_hdr;
>  SpiceLinkHeader peer_hdr;
>  SpiceLinkReply* peer_msg;
> -int peer_pos;
>  
>  int message_ack_window;
>  int message_ack_count;
> diff --git a/src/spice-channel.c b/src/spice-channel.c
> index 2aa0826d..dc3e2950 100644
> --- a/src/spice-channel.c
> +++ b/src/spice-channel.c
> @@ -1906,10 +1906,8 @@ static gboolean 
> spice_channel_recv_link_msg(SpiceChannel *channel)
>  
>  c = channel->priv;
>  
> -rc = spice_channel_read(channel, (uint8_t*)c->peer_msg + c->peer_pos,
> -c->peer_hdr.size - c->peer_pos);
> -c->peer_pos += rc;
> -if (c->peer_pos != c->peer_hdr.size) {
> +rc = spice_channel_read(channel, (uint8_t*)c->peer_msg, 
> c->peer_hdr.size);
> +if (rc != c->peer_hdr.size) {
>  g_critical("%s: %s: incomplete link reply (%d/%u)",
>c->name, __FUNCTION__, rc, c->peer_hdr.size);
>  goto error;
> @@ -2821,7 +2819,6 @@ static void channel_reset(SpiceChannel *channel, 
> gboolean migrating)
>  c->auth_needs_password = FALSE;
>  
>  g_clear_pointer(&c->peer_msg, g_free);
> -c->peer_pos = 0;
>  
>  g_mutex_lock(&c->xmit_queue_lock);
>  c->xmit_queue_blocked = TRUE; /* Disallow queuing new messages */
> -- 
> 2.17.2
> 
> ___
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel


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-gtk 2/2] spice-channel: Use unsigned for num_caps

2018-11-20 Thread Christophe Fergeau

Acked-by: Christophe Fergeau 

On Tue, Nov 20, 2018 at 01:25:45PM +, Frediano Ziglio wrote:
> Is just a sum of 2 unsigned numbers
> 
> Signed-off-by: Frediano Ziglio 
> ---
>  src/spice-channel.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/src/spice-channel.c b/src/spice-channel.c
> index dc3e2950..1b3578cd 100644
> --- a/src/spice-channel.c
> +++ b/src/spice-channel.c
> @@ -1896,7 +1896,8 @@ static void store_caps(const uint8_t *caps_src, 
> uint32_t ncaps,
>  static gboolean spice_channel_recv_link_msg(SpiceChannel *channel)
>  {
>  SpiceChannelPrivate *c;
> -int rc, num_caps;
> +int rc;
> +uint32_t num_caps;
>  uint32_t num_channel_caps, num_common_caps;
>  uint8_t *caps_src;
>  SpiceChannelEvent event = SPICE_CHANNEL_ERROR_LINK;
> @@ -1931,7 +1932,7 @@ static gboolean 
> spice_channel_recv_link_msg(SpiceChannel *channel)
>  num_common_caps = GUINT32_FROM_LE(c->peer_msg->num_common_caps);
>  
>  num_caps = num_channel_caps + num_common_caps;
> -CHANNEL_DEBUG(channel, "%s: %d caps", __FUNCTION__, num_caps);
> +CHANNEL_DEBUG(channel, "%s: %u caps", __FUNCTION__, num_caps);
>  
>  /* see original spice/client code: */
>  /* g_return_if_fail(c->peer_msg + c->peer_msg->caps_offset * 
> sizeof(uint32_t) > c->peer_msg + c->peer_hdr.size); */
> -- 
> 2.17.2
> 
> ___
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel


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] Support for toggling Menubar visibility on spicy

2018-11-20 Thread Christophe Fergeau
Hey,

spicy really is a test tool for the spice-gtk widget. As such, we
usually don't add such UI polishing patches to it as we don't want to
encourage its use outside of spice development.
If you need something similar based on spice-gtk, I'd recommend looking
at virt-viewer/remote-viewer.

Thanks,

Christophe

On Mon, Nov 19, 2018 at 02:10:08PM -0200, Silvio Rhatto wrote:
> This patch adds a submenu item at the Views menu
> item which allows to hide and show the Menubar.
> 
> The functionality is also available using Shift+F6
> keyboard combination.
> 
> This feature makes possible a more seamless
> integration between spice guests and the host's
> window manager.
> ---
>  tools/spicy.c | 25 -
>  1 file changed, 24 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/spicy.c b/tools/spicy.c
> index 263c15f..6c4f1e3 100644
> --- a/tools/spicy.c
> +++ b/tools/spicy.c
> @@ -469,6 +469,15 @@ static void menu_cb_statusbar(GtkToggleAction *action, 
> gpointer data)
>  g_key_file_set_boolean(keyfile, "ui", "statusbar", state);
>  }
>  
> +static void menu_cb_menubar(GtkToggleAction *action, gpointer data)
> +{
> +SpiceWindow *win = data;
> +gboolean state = gtk_toggle_action_get_active(action);
> +
> +gtk_widget_set_visible(win->menubar, state);
> +g_key_file_set_boolean(keyfile, "ui", "menubar", state);
> +}
> +
>  static void menu_cb_about(GtkAction *action, void *data)
>  {
>  char *comments = "gtk test client app for the\n"
> @@ -518,10 +527,14 @@ static gboolean window_state_cb(GtkWidget *widget, 
> GdkEventWindowState *event,
>  gboolean state;
>  GtkAction *toggle;
>  
> -gtk_widget_show(win->menubar);
> +toggle = gtk_action_group_get_action(win->ag, "Menubar");
> +state = gtk_toggle_action_get_active(GTK_TOGGLE_ACTION(toggle));
> +gtk_widget_set_visible(win->menubar, state);
> +
>  toggle = gtk_action_group_get_action(win->ag, "Toolbar");
>  state = gtk_toggle_action_get_active(GTK_TOGGLE_ACTION(toggle));
>  gtk_widget_set_visible(win->toolbar, state);
> +
>  toggle = gtk_action_group_get_action(win->ag, "Statusbar");
>  state = gtk_toggle_action_get_active(GTK_TOGGLE_ACTION(toggle));
>  gtk_widget_set_visible(win->statusbar, state);
> @@ -677,6 +690,10 @@ static void restore_configuration(SpiceWindow *win)
>  }
>  g_clear_error(&error);
>  
> +state = g_key_file_get_boolean(keyfile, "ui", "menubar", &error);
> +if (error == NULL)
> +gtk_widget_set_visible(win->menubar, state);
> +g_clear_error(&error);
>  
>  state = g_key_file_get_boolean(keyfile, "ui", "toolbar", &error);
>  if (error == NULL)
> @@ -850,6 +867,11 @@ static const GtkToggleActionEntry tentries[] = {
>  .name= "Toolbar",
>  .label   = "Toolbar",
>  .callback= G_CALLBACK(menu_cb_toolbar),
> +},{
> +.name= "Menubar",
> +.label   = "Menubar",
> +.callback= G_CALLBACK(menu_cb_menubar),
> +.accelerator = "F6",
>  }
>  };
>  
> @@ -924,6 +946,7 @@ static char ui_xml[] =
>  "  \n"
>  "  \n"
>  "  \n"
> +"  \n"
>  "\n"
>  "\n"
>  #ifdef USE_SMARTCARD
> -- 
> 2.11.0
> 
> ___
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel


signature.asc
Description: PGP signature
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel


[Spice-devel] [PATCH] Support for toggling Menubar visibility on spicy

2018-11-20 Thread Silvio Rhatto
This patch adds a submenu item at the Views menu
item which allows to hide and show the Menubar.

The functionality is also available using Shift+F6
keyboard combination.

This feature makes possible a more seamless
integration between spice guests and the host's
window manager.
---
 tools/spicy.c | 25 -
 1 file changed, 24 insertions(+), 1 deletion(-)

diff --git a/tools/spicy.c b/tools/spicy.c
index 263c15f..6c4f1e3 100644
--- a/tools/spicy.c
+++ b/tools/spicy.c
@@ -469,6 +469,15 @@ static void menu_cb_statusbar(GtkToggleAction *action, 
gpointer data)
 g_key_file_set_boolean(keyfile, "ui", "statusbar", state);
 }
 
+static void menu_cb_menubar(GtkToggleAction *action, gpointer data)
+{
+SpiceWindow *win = data;
+gboolean state = gtk_toggle_action_get_active(action);
+
+gtk_widget_set_visible(win->menubar, state);
+g_key_file_set_boolean(keyfile, "ui", "menubar", state);
+}
+
 static void menu_cb_about(GtkAction *action, void *data)
 {
 char *comments = "gtk test client app for the\n"
@@ -518,10 +527,14 @@ static gboolean window_state_cb(GtkWidget *widget, 
GdkEventWindowState *event,
 gboolean state;
 GtkAction *toggle;
 
-gtk_widget_show(win->menubar);
+toggle = gtk_action_group_get_action(win->ag, "Menubar");
+state = gtk_toggle_action_get_active(GTK_TOGGLE_ACTION(toggle));
+gtk_widget_set_visible(win->menubar, state);
+
 toggle = gtk_action_group_get_action(win->ag, "Toolbar");
 state = gtk_toggle_action_get_active(GTK_TOGGLE_ACTION(toggle));
 gtk_widget_set_visible(win->toolbar, state);
+
 toggle = gtk_action_group_get_action(win->ag, "Statusbar");
 state = gtk_toggle_action_get_active(GTK_TOGGLE_ACTION(toggle));
 gtk_widget_set_visible(win->statusbar, state);
@@ -677,6 +690,10 @@ static void restore_configuration(SpiceWindow *win)
 }
 g_clear_error(&error);
 
+state = g_key_file_get_boolean(keyfile, "ui", "menubar", &error);
+if (error == NULL)
+gtk_widget_set_visible(win->menubar, state);
+g_clear_error(&error);
 
 state = g_key_file_get_boolean(keyfile, "ui", "toolbar", &error);
 if (error == NULL)
@@ -850,6 +867,11 @@ static const GtkToggleActionEntry tentries[] = {
 .name= "Toolbar",
 .label   = "Toolbar",
 .callback= G_CALLBACK(menu_cb_toolbar),
+},{
+.name= "Menubar",
+.label   = "Menubar",
+.callback= G_CALLBACK(menu_cb_menubar),
+.accelerator = "F6",
 }
 };
 
@@ -924,6 +946,7 @@ static char ui_xml[] =
 "  \n"
 "  \n"
 "  \n"
+"  \n"
 "\n"
 "\n"
 #ifdef USE_SMARTCARD
-- 
2.11.0

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


[Spice-devel] [PATCH spice-gtk 1/2] spice-channel: Remove useless peer_pos

2018-11-20 Thread Frediano Ziglio
With coroutine the spice_channel_recv_link_msg function reads the
entire message, no reason to store partial position.

Signed-off-by: Frediano Ziglio 
---
 src/spice-channel-priv.h | 1 -
 src/spice-channel.c  | 7 ++-
 2 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/src/spice-channel-priv.h b/src/spice-channel-priv.h
index 091c1163..5984ca56 100644
--- a/src/spice-channel-priv.h
+++ b/src/spice-channel-priv.h
@@ -129,7 +129,6 @@ struct _SpiceChannelPrivate {
 SpiceLinkHeader link_hdr;
 SpiceLinkHeader peer_hdr;
 SpiceLinkReply* peer_msg;
-int peer_pos;
 
 int message_ack_window;
 int message_ack_count;
diff --git a/src/spice-channel.c b/src/spice-channel.c
index 2aa0826d..dc3e2950 100644
--- a/src/spice-channel.c
+++ b/src/spice-channel.c
@@ -1906,10 +1906,8 @@ static gboolean spice_channel_recv_link_msg(SpiceChannel 
*channel)
 
 c = channel->priv;
 
-rc = spice_channel_read(channel, (uint8_t*)c->peer_msg + c->peer_pos,
-c->peer_hdr.size - c->peer_pos);
-c->peer_pos += rc;
-if (c->peer_pos != c->peer_hdr.size) {
+rc = spice_channel_read(channel, (uint8_t*)c->peer_msg, c->peer_hdr.size);
+if (rc != c->peer_hdr.size) {
 g_critical("%s: %s: incomplete link reply (%d/%u)",
   c->name, __FUNCTION__, rc, c->peer_hdr.size);
 goto error;
@@ -2821,7 +2819,6 @@ static void channel_reset(SpiceChannel *channel, gboolean 
migrating)
 c->auth_needs_password = FALSE;
 
 g_clear_pointer(&c->peer_msg, g_free);
-c->peer_pos = 0;
 
 g_mutex_lock(&c->xmit_queue_lock);
 c->xmit_queue_blocked = TRUE; /* Disallow queuing new messages */
-- 
2.17.2

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


[Spice-devel] [PATCH spice-gtk 2/2] spice-channel: Use unsigned for num_caps

2018-11-20 Thread Frediano Ziglio
Is just a sum of 2 unsigned numbers

Signed-off-by: Frediano Ziglio 
---
 src/spice-channel.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/src/spice-channel.c b/src/spice-channel.c
index dc3e2950..1b3578cd 100644
--- a/src/spice-channel.c
+++ b/src/spice-channel.c
@@ -1896,7 +1896,8 @@ static void store_caps(const uint8_t *caps_src, uint32_t 
ncaps,
 static gboolean spice_channel_recv_link_msg(SpiceChannel *channel)
 {
 SpiceChannelPrivate *c;
-int rc, num_caps;
+int rc;
+uint32_t num_caps;
 uint32_t num_channel_caps, num_common_caps;
 uint8_t *caps_src;
 SpiceChannelEvent event = SPICE_CHANNEL_ERROR_LINK;
@@ -1931,7 +1932,7 @@ static gboolean spice_channel_recv_link_msg(SpiceChannel 
*channel)
 num_common_caps = GUINT32_FROM_LE(c->peer_msg->num_common_caps);
 
 num_caps = num_channel_caps + num_common_caps;
-CHANNEL_DEBUG(channel, "%s: %d caps", __FUNCTION__, num_caps);
+CHANNEL_DEBUG(channel, "%s: %u caps", __FUNCTION__, num_caps);
 
 /* see original spice/client code: */
 /* g_return_if_fail(c->peer_msg + c->peer_msg->caps_offset * 
sizeof(uint32_t) > c->peer_msg + c->peer_hdr.size); */
-- 
2.17.2

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


Re: [Spice-devel] [PATCH spice-common] Integrate recorder library

2018-11-20 Thread Christophe de Dinechin


> On 20 Nov 2018, at 13:49, Frediano Ziglio  wrote:
> 
>> 
>> Thanks Frediano,
>> 
>> 
>> Reviewed-by: Christophe de Dinechin 
>> 
>> 
>>> On 19 Nov 2018, at 21:46, Frediano Ziglio  wrote:
>>> 
>>> From: Christophe de Dinechin 
>> 
>> Replace with “Suggested-by: “ :-)
>> 
>> 
>>> 
>>> Allow to use recorder library. See https://github.com/c3d/recorder for
>>> details.
>>> The main usage will be to collect statistics while the programs will run.
>>> By default the recorder will be disabled at compile time.
>>> Both autoconf and Meson are supported.
>>> Autoconf requires the addition of SPICE_CHECK_RECORDER call in
>>> configure.ac.
>>> Meson requires to add recorder option.
>>> 
>>> Signed-off-by: Frediano Ziglio 
>>> ---
>>> .gitmodules |  3 ++
>>> common/Makefile.am  | 10 +
>>> common/log.c|  3 ++
>>> common/meson.build  | 12 +-
>>> common/recorder |  1 +
>>> common/recorder.h   | 75 +
>>> configure.ac|  8 +++-
>>> m4/spice-deps.m4| 15 
>>> meson.build | 12 +-
>>> meson_options.txt   |  6 +++
>>> tests/Makefile.am   | 12 ++
>>> tests/test-dummy-recorder.c | 53 ++
>>> 12 files changed, 206 insertions(+), 4 deletions(-)
>>> create mode 16 common/recorder
>>> create mode 100644 common/recorder.h
>>> create mode 100644 tests/test-dummy-recorder.c
>>> 
>>> diff --git a/.gitmodules b/.gitmodules
>>> index e69de29..a7ea8b1 100644
>>> --- a/.gitmodules
>>> +++ b/.gitmodules
>>> @@ -0,0 +1,3 @@
>>> +[submodule "common/recorder"]
>>> +   path = common/recorder
>>> +   url = https://github.com/c3d/recorder.git
>> 
>> Shouldn’t you keep a clone somewhere in the SPICE repo?
>> 
> 
> Sure, if no complain I would clone to https://gitlab.freedesktop.org/spice 
> group
> and change url to "../recorder.git".
> 
>> 
>>> diff --git a/common/Makefile.am b/common/Makefile.am
>>> index da0d941..4c0a6cd 100644
>>> --- a/common/Makefile.am
>>> +++ b/common/Makefile.am
>>> @@ -51,8 +51,18 @@ libspice_common_la_SOURCES = \
>>> snd_codec.c \
>>> snd_codec.h \
>>> verify.h\
>>> +   recorder.h  \
>>> $(NULL)
>>> 
>>> +if ENABLE_RECORDER
>>> +libspice_common_la_SOURCES += \
>>> +   recorder/recorder.c \
>>> +   recorder/recorder.h \
>>> +   recorder/recorder_ring.c\
>>> +   recorder/recorder_ring.h\
>>> +   $(NULL)
>>> +endif
>> 
>> I see you decided not to build it as a separate DLL after all.
>> 
> 
> Was already "ready", but can be changed.

Can be done later too.

> 
>>> +
>>> # These 2 files are not build as part of spice-common
>>> # build system, but modules using spice-common will build
>>> # them with the appropriate options. We need to let automake
>>> diff --git a/common/log.c b/common/log.c
>>> index b59c8a8..a7806c5 100644
>>> --- a/common/log.c
>>> +++ b/common/log.c
>>> @@ -27,6 +27,8 @@
>>> #include 
>>> #endif
>>> 
>>> +#include 
>>> +
>>> #include "log.h"
>>> #include "backtrace.h"
>>> 
>>> @@ -154,6 +156,7 @@ SPICE_CONSTRUCTOR_FUNC(spice_log_init)
>>>if (!g_thread_supported())
>>>g_thread_init(NULL);
>>> #endif
>>> +recorder_dump_on_common_signals(0, 0);
>>> }
>>> 
>>> static void spice_logv(const char *log_domain,
>>> diff --git a/common/meson.build b/common/meson.build
>>> index 6ac55dc..e715e31 100644
>>> --- a/common/meson.build
>>> +++ b/common/meson.build
>>> @@ -35,9 +35,19 @@ spice_common_sources = [
>>>  'rop3.h',
>>>  'snd_codec.c',
>>>  'snd_codec.h',
>>> -  'verify.h'
>>> +  'verify.h',
>>> +  'recorder.h'
>>> ]
>>> 
>>> +if get_option('recorder')
>>> +  spice_common_sources += [
>>> +'recorder/recorder.c',
>>> +'recorder/recorder.h',
>>> +'recorder/recorder_ring.c',
>>> +'recorder/recorder_ring.h'
>>> +  ]
>>> +endif
>>> +
>>> spice_common_lib = static_library('spice-common', spice_common_sources,
>>>  install : false,
>>>  include_directories :
>>>  spice_common_include,
>>> diff --git a/common/recorder b/common/recorder
>>> new file mode 16
>>> index 000..8f450dc
>>> --- /dev/null
>>> +++ b/common/recorder
>>> @@ -0,0 +1 @@
>>> +Subproject commit 8f450dcf0ec725a41b80c495ecdd6110dd1fcdb8
>>> diff --git a/common/recorder.h b/common/recorder.h
>>> new file mode 100644
>>> index 000..4496de9
>>> --- /dev/null
>>> +++ b/common/recorder.h
>>> @@ -0,0 +1,75 @@
>>> +/*
>>> +  Copyright (C) 2018 Red Hat, Inc.
>>> +
>>> +  This library is free software; you can redistribute it and/or
>>> +  modify it under the terms of the GNU Lesser General Public
>>> +  License as published by the Free Software Foundation; either
>>> +  version 2.1 of the License, or (at your option) any later version.
>>> +
>>> +  This library

Re: [Spice-devel] [PATCH spice-common] Integrate recorder library

2018-11-20 Thread Frediano Ziglio
> 
> Thanks Frediano,
> 
> 
> Reviewed-by: Christophe de Dinechin 
> 
> 
> > On 19 Nov 2018, at 21:46, Frediano Ziglio  wrote:
> > 
> > From: Christophe de Dinechin 
> 
> Replace with “Suggested-by: “ :-)
> 
> 
> > 
> > Allow to use recorder library. See https://github.com/c3d/recorder for
> > details.
> > The main usage will be to collect statistics while the programs will run.
> > By default the recorder will be disabled at compile time.
> > Both autoconf and Meson are supported.
> > Autoconf requires the addition of SPICE_CHECK_RECORDER call in
> > configure.ac.
> > Meson requires to add recorder option.
> > 
> > Signed-off-by: Frediano Ziglio 
> > ---
> > .gitmodules |  3 ++
> > common/Makefile.am  | 10 +
> > common/log.c|  3 ++
> > common/meson.build  | 12 +-
> > common/recorder |  1 +
> > common/recorder.h   | 75 +
> > configure.ac|  8 +++-
> > m4/spice-deps.m4| 15 
> > meson.build | 12 +-
> > meson_options.txt   |  6 +++
> > tests/Makefile.am   | 12 ++
> > tests/test-dummy-recorder.c | 53 ++
> > 12 files changed, 206 insertions(+), 4 deletions(-)
> > create mode 16 common/recorder
> > create mode 100644 common/recorder.h
> > create mode 100644 tests/test-dummy-recorder.c
> > 
> > diff --git a/.gitmodules b/.gitmodules
> > index e69de29..a7ea8b1 100644
> > --- a/.gitmodules
> > +++ b/.gitmodules
> > @@ -0,0 +1,3 @@
> > +[submodule "common/recorder"]
> > +   path = common/recorder
> > +   url = https://github.com/c3d/recorder.git
> 
> Shouldn’t you keep a clone somewhere in the SPICE repo?
> 

Sure, if no complain I would clone to https://gitlab.freedesktop.org/spice group
and change url to "../recorder.git".

> 
> > diff --git a/common/Makefile.am b/common/Makefile.am
> > index da0d941..4c0a6cd 100644
> > --- a/common/Makefile.am
> > +++ b/common/Makefile.am
> > @@ -51,8 +51,18 @@ libspice_common_la_SOURCES = \
> > snd_codec.c \
> > snd_codec.h \
> > verify.h\
> > +   recorder.h  \
> > $(NULL)
> > 
> > +if ENABLE_RECORDER
> > +libspice_common_la_SOURCES += \
> > +   recorder/recorder.c \
> > +   recorder/recorder.h \
> > +   recorder/recorder_ring.c\
> > +   recorder/recorder_ring.h\
> > +   $(NULL)
> > +endif
> 
> I see you decided not to build it as a separate DLL after all.
> 

Was already "ready", but can be changed.

> > +
> > # These 2 files are not build as part of spice-common
> > # build system, but modules using spice-common will build
> > # them with the appropriate options. We need to let automake
> > diff --git a/common/log.c b/common/log.c
> > index b59c8a8..a7806c5 100644
> > --- a/common/log.c
> > +++ b/common/log.c
> > @@ -27,6 +27,8 @@
> > #include 
> > #endif
> > 
> > +#include 
> > +
> > #include "log.h"
> > #include "backtrace.h"
> > 
> > @@ -154,6 +156,7 @@ SPICE_CONSTRUCTOR_FUNC(spice_log_init)
> > if (!g_thread_supported())
> > g_thread_init(NULL);
> > #endif
> > +recorder_dump_on_common_signals(0, 0);
> > }
> > 
> > static void spice_logv(const char *log_domain,
> > diff --git a/common/meson.build b/common/meson.build
> > index 6ac55dc..e715e31 100644
> > --- a/common/meson.build
> > +++ b/common/meson.build
> > @@ -35,9 +35,19 @@ spice_common_sources = [
> >   'rop3.h',
> >   'snd_codec.c',
> >   'snd_codec.h',
> > -  'verify.h'
> > +  'verify.h',
> > +  'recorder.h'
> > ]
> > 
> > +if get_option('recorder')
> > +  spice_common_sources += [
> > +'recorder/recorder.c',
> > +'recorder/recorder.h',
> > +'recorder/recorder_ring.c',
> > +'recorder/recorder_ring.h'
> > +  ]
> > +endif
> > +
> > spice_common_lib = static_library('spice-common', spice_common_sources,
> >   install : false,
> >   include_directories :
> >   spice_common_include,
> > diff --git a/common/recorder b/common/recorder
> > new file mode 16
> > index 000..8f450dc
> > --- /dev/null
> > +++ b/common/recorder
> > @@ -0,0 +1 @@
> > +Subproject commit 8f450dcf0ec725a41b80c495ecdd6110dd1fcdb8
> > diff --git a/common/recorder.h b/common/recorder.h
> > new file mode 100644
> > index 000..4496de9
> > --- /dev/null
> > +++ b/common/recorder.h
> > @@ -0,0 +1,75 @@
> > +/*
> > +  Copyright (C) 2018 Red Hat, Inc.
> > +
> > +  This library is free software; you can redistribute it and/or
> > +  modify it under the terms of the GNU Lesser General Public
> > +  License as published by the Free Software Foundation; either
> > +  version 2.1 of the License, or (at your option) any later version.
> > +
> > +  This library is distributed in the hope that it will be useful,
> > +  but WITHOUT ANY WARRANTY; without even the implied

Re: [Spice-devel] [PATCH vd_agent_linux v2] vdagent: Stop trying to connect to the daemon after a while

2018-11-20 Thread Lukáš Hrázký
On Mon, 2018-11-19 at 11:42 -0500, Frediano Ziglio wrote:
> > 
> > On Mon, 2018-11-19 at 10:23 +, Frediano Ziglio wrote:
> > > Do not try  indefinitely to connect to the daemon, should not
> > > take long to activate.
> > > 
> > > Signed-off-by: Frediano Ziglio 
> > > ---
> > >  src/vdagent/vdagent.c | 6 ++
> > >  1 file changed, 6 insertions(+)
> > > 
> > > Changes since v1:
> > > - log error when we reach the limit
> > > 
> > > diff --git a/src/vdagent/vdagent.c b/src/vdagent/vdagent.c
> > > index f7c8b72..7a58150 100644
> > > --- a/src/vdagent/vdagent.c
> > > +++ b/src/vdagent/vdagent.c
> > > @@ -53,6 +53,7 @@ typedef struct VDAgent {
> > >  struct vdagent_file_xfers *xfers;
> > >  struct udscs_connection *conn;
> > >  GIOChannel *x11_channel;
> > > +guint connection_attempts;
> > >  
> > >  GMainLoop *loop;
> > >  } VDAgent;
> > > @@ -370,6 +371,11 @@ static gboolean vdagent_init_async_cb(gpointer
> > > user_data)
> > >  daemon_read_complete,
> > >  daemon_disconnect_cb,
> > >  debug);
> > >  if (agent->conn == NULL) {
> > > +// limit connection attempts, this will try for 5 minutes
> > > +if (++agent->connection_attempts > 5 * 60) {
> > > +syslog(LOG_ERR, "Attempted to contact daemon for 5 minutes,
> > > giving up");
> > > +goto err_init;
> > > +}
> > 
> > (in reference to trying to keep it simple from v1 thread:)
> > I think using 60 instead of 1 in g_timeout_add_seconds() wouldn't be
> > much of a complication... unless there's something else I'm missing.
> > I'm also thinking this is being launched from a .desktop file so AFAIK
> > there is no easy way for a user to restart the service, a relogin is
> > needed?
> > 
> 
> It reconnects to the new daemon after detecting the disconnection.
> Not sure what will happen if the agent is updated... I think nothing
> or possibly the agent will end if not compatible with the new daemon.
> If you retry every minute is possible that the user has to wait 1
> minute instead of 1 second.
> Probably the optimal way would be to use inotify on the socket to
> wait for the new daemon, so something like:
> - try 10 times with 1 second delay
> - setup inotify
> - try once to avoid races
> - wait inotify
> - attempt connection again (repeat from first step)
> This way if the daemon is restored after even hours you don't have to
> keep polling.

Well, you wanted a simple solution and I agree with that. I suggested
something that keeps the agent running and doesn't really make it more
complicated. Now you're suggesting a much more complex solution... :)

> > I also just realized you should reset connection_attempts to 0 after a
> > successful connect?
> > 
> 
> The object is created again so will be 0 again (not that would hurt
> to reset on a successful connection).

Right, didn't notice that...

Cheers,
Lukas

> > Cheers,
> > Lukas
> > 
> > >  g_timeout_add_seconds(1, vdagent_init_async_cb, agent);
> > >  return G_SOURCE_REMOVE;
> > >  }
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel


Re: [Spice-devel] [PATCH spice-common] Integrate recorder library

2018-11-20 Thread Christophe de Dinechin
Thanks Frediano,


Reviewed-by: Christophe de Dinechin 


> On 19 Nov 2018, at 21:46, Frediano Ziglio  wrote:
> 
> From: Christophe de Dinechin 

Replace with “Suggested-by: “ :-)


> 
> Allow to use recorder library. See https://github.com/c3d/recorder for
> details.
> The main usage will be to collect statistics while the programs will run.
> By default the recorder will be disabled at compile time.
> Both autoconf and Meson are supported.
> Autoconf requires the addition of SPICE_CHECK_RECORDER call in configure.ac.
> Meson requires to add recorder option.
> 
> Signed-off-by: Frediano Ziglio 
> ---
> .gitmodules |  3 ++
> common/Makefile.am  | 10 +
> common/log.c|  3 ++
> common/meson.build  | 12 +-
> common/recorder |  1 +
> common/recorder.h   | 75 +
> configure.ac|  8 +++-
> m4/spice-deps.m4| 15 
> meson.build | 12 +-
> meson_options.txt   |  6 +++
> tests/Makefile.am   | 12 ++
> tests/test-dummy-recorder.c | 53 ++
> 12 files changed, 206 insertions(+), 4 deletions(-)
> create mode 16 common/recorder
> create mode 100644 common/recorder.h
> create mode 100644 tests/test-dummy-recorder.c
> 
> diff --git a/.gitmodules b/.gitmodules
> index e69de29..a7ea8b1 100644
> --- a/.gitmodules
> +++ b/.gitmodules
> @@ -0,0 +1,3 @@
> +[submodule "common/recorder"]
> + path = common/recorder
> + url = https://github.com/c3d/recorder.git

Shouldn’t you keep a clone somewhere in the SPICE repo?


> diff --git a/common/Makefile.am b/common/Makefile.am
> index da0d941..4c0a6cd 100644
> --- a/common/Makefile.am
> +++ b/common/Makefile.am
> @@ -51,8 +51,18 @@ libspice_common_la_SOURCES =   \
>   snd_codec.c \
>   snd_codec.h \
>   verify.h\
> + recorder.h  \
>   $(NULL)
> 
> +if ENABLE_RECORDER
> +libspice_common_la_SOURCES += \
> + recorder/recorder.c \
> + recorder/recorder.h \
> + recorder/recorder_ring.c\
> + recorder/recorder_ring.h\
> + $(NULL)
> +endif

I see you decided not to build it as a separate DLL after all.

> +
> # These 2 files are not build as part of spice-common
> # build system, but modules using spice-common will build
> # them with the appropriate options. We need to let automake
> diff --git a/common/log.c b/common/log.c
> index b59c8a8..a7806c5 100644
> --- a/common/log.c
> +++ b/common/log.c
> @@ -27,6 +27,8 @@
> #include 
> #endif
> 
> +#include 
> +
> #include "log.h"
> #include "backtrace.h"
> 
> @@ -154,6 +156,7 @@ SPICE_CONSTRUCTOR_FUNC(spice_log_init)
> if (!g_thread_supported())
> g_thread_init(NULL);
> #endif
> +recorder_dump_on_common_signals(0, 0);
> }
> 
> static void spice_logv(const char *log_domain,
> diff --git a/common/meson.build b/common/meson.build
> index 6ac55dc..e715e31 100644
> --- a/common/meson.build
> +++ b/common/meson.build
> @@ -35,9 +35,19 @@ spice_common_sources = [
>   'rop3.h',
>   'snd_codec.c',
>   'snd_codec.h',
> -  'verify.h'
> +  'verify.h',
> +  'recorder.h'
> ]
> 
> +if get_option('recorder')
> +  spice_common_sources += [
> +'recorder/recorder.c',
> +'recorder/recorder.h',
> +'recorder/recorder_ring.c',
> +'recorder/recorder_ring.h'
> +  ]
> +endif
> +
> spice_common_lib = static_library('spice-common', spice_common_sources,
>   install : false,
>   include_directories : spice_common_include,
> diff --git a/common/recorder b/common/recorder
> new file mode 16
> index 000..8f450dc
> --- /dev/null
> +++ b/common/recorder
> @@ -0,0 +1 @@
> +Subproject commit 8f450dcf0ec725a41b80c495ecdd6110dd1fcdb8
> diff --git a/common/recorder.h b/common/recorder.h
> new file mode 100644
> index 000..4496de9
> --- /dev/null
> +++ b/common/recorder.h
> @@ -0,0 +1,75 @@
> +/*
> +  Copyright (C) 2018 Red Hat, Inc.
> +
> +  This library is free software; you can redistribute it and/or
> +  modify it under the terms of the GNU Lesser General Public
> +  License as published by the Free Software Foundation; either
> +  version 2.1 of the License, or (at your option) any later version.
> +
> +  This library is distributed in the hope that it will be useful,
> +  but WITHOUT ANY WARRANTY; without even the implied warranty of
> +  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +  Lesser General Public License for more details.
> +
> +  You should have received a copy of the GNU Lesser General Public
> +  License along with this library; if not, see 
> .
> +*/
> +/* This file include recorder library headers or if disabled provide
> + * replacement declarations */
> +#ifndef ENABLE_RECORDER
> +
> +#include 
> +#include 
> +
> +/* Rep

Re: [Spice-devel] [PATCH spice-gtk] Use same sink settings for direct rendering

2018-11-20 Thread Snir Sheriber

Hi,


On 11/19/2018 11:21 PM, Frediano Ziglio wrote:

From: Snir Sheriber 

Set "sync" and "drop" as normal streaming.


Well, It is not the same case, with appsink we mark those two as false
since we handle the presenting of the decoded frames ourselves so frames
are still being synchronized with audio.
In the overlay case this will actually avoid synchronization (which I'm 
currently

in favor)


---
  src/channel-display-gst.c | 19 +++
  1 file changed, 19 insertions(+)

Well, I extracted this patch from a more bigger Snir sent, not much
changes so I kept the authorship.

diff --git a/src/channel-display-gst.c b/src/channel-display-gst.c
index 2c07f350..5f49c3bb 100644
--- a/src/channel-display-gst.c
+++ b/src/channel-display-gst.c
@@ -382,6 +382,24 @@ static void app_source_setup(GstElement *pipeline 
G_GNUC_UNUSED,
  }
  #endif
  
+// This function is used to set properties in dynamically added sink (if overlay is used)

+static void

/*... */ ?

+add_elem_cb(GstBin * pipeline, GstBin * bin, GstElement * element, 
SpiceGstDecoder *decoder)
+{
+char *name = gst_element_get_name(element);
+
+spice_debug("Adding element: %s", name);
+
+if (GST_IS_BASE_SINK(element)) {
+g_object_set(element,
+ "sync", FALSE,
If we doing it here this possibly can be removed from the other place we 
set it.

+ "drop", FALSE,

I'd remove "drop", i don't think it's available for most sinks.

+ NULL);
+spice_debug("SINK");

I'd remove this, (not sure about the printing the elements names).

+}
+g_free(name);
+}
+
  static gboolean create_pipeline(SpiceGstDecoder *decoder)
  {
  GstBus *bus;
@@ -442,6 +460,7 @@ static gboolean create_pipeline(SpiceGstDecoder *decoder)
  }
  }
  
+g_signal_connect(playbin, "deep-element-added", G_CALLBACK(add_elem_cb), decoder);

  g_signal_connect(playbin, "source-setup", G_CALLBACK(app_source_setup), 
decoder);
  
  g_object_set(playbin,


So if the consequences of doing it are agreed I'd do it.


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