date:20170220

Re: [Spice-devel] Spice protocol behind a Firewall

2017-02-20 Thread Victor Toso

Hi,

On Sun, Feb 19, 2017 at 11:48:44AM +0200, Uri Lublin wrote:
> On 02/19/2017 08:07 AM, Oscar Segarra wrote:
> > Hi,
> > 
> > First of all, I'd like to say that I'm not sure enough I'm writing to
> > the correct mailing list, I have not been able to find a common users
> > mailing list.
> > 
> > I'm planning to deploy a VDI solution based on SPICE. I'd like to grant
> > access through the Internet to the VDI desktops but I don't want to
> > expose the hypervisors to the Internet.
> > 
> > Using virt-viewer or remote-viewer (not the html5 feature as I want USB
> > redirection), is there any trick to make this scenario work:
> > 
> > /Internet --> FW --> Kind of spice reverse proxy --> FW --> Hypervisors
> > (more than one)./
>
> Hi,
>
> If you have an http/https proxy server, please try:
>   SPICE_PROXY=proxy_ip:proxy_port  remote-viewer host_ip:host_port

I guess we can add this to documentation. I've filed an issue upstream
https://gitlab.com/spice/spice-space-pages/issues/2

toso

>
> Hope that helps,
> Uri.
>
> ___
> 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] gtk-session: Set value directly

2017-02-20 Thread Victor Toso

Hi,

On Fri, Feb 17, 2017 at 01:44:55PM +0100, Pavel Grunt wrote:
> Spotted by coverity

Including the coverity message instead would be more interesting IMHO.
Acked-by: Victor Toso 

> ---
>  src/spice-gtk-session.c | 6 ++
>  1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/src/spice-gtk-session.c b/src/spice-gtk-session.c
> index a3a2e90..5688cba 100644
> --- a/src/spice-gtk-session.c
> +++ b/src/spice-gtk-session.c
> @@ -916,25 +916,23 @@ static gboolean 
> check_clipboard_size_limits(SpiceGtkSession *session,
>   */
>  static char *fixup_clipboard_text(SpiceGtkSession *self, const char *text, 
> int *len)
>  {
>  char *conv = NULL;
> -int new_len = *len;
>  
>  if (spice_main_agent_test_capability(self->priv->main, 
> VD_AGENT_CAP_GUEST_LINEEND_CRLF)) {
>  conv = spice_unix2dos(text, *len);
> -new_len = strlen(conv);
> +*len = strlen(conv);
>  } else {
>  /* On Windows, with some versions of gtk+, GtkSelectionData::length
>   * will include the final '\0'. When a string with this trailing '\0'
>   * is pasted in some linux applications, it will be pasted as  
> or
>   * as an invisible character, which is unwanted. Ensure the length we
>   * send to the agent does not include any trailing '\0'
>   * This is gtk+ bug https://bugzilla.gnome.org/show_bug.cgi?id=734670
>   */
> -new_len = strlen(text);
> +*len = strlen(text);
>  }
>  
> -*len = new_len;
>  return conv;
>  }
>  
>  static void clipboard_received_text_cb(GtkClipboard *clipboard,
> -- 
> 2.11.1
> 
> ___
> 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] [spice v8] dcc: handle preferred video codec message

2017-02-20 Thread Victor Toso

Hi,

On Fri, Feb 10, 2017 at 02:14:11PM +0100, Victor Toso wrote:
> Hi,
> 
> On Fri, Feb 10, 2017 at 07:51:05AM -0500, Frediano Ziglio wrote:
> > > [0] SPICE_MSGC_DISPLAY_PREFERRED_VIDEO_CODEC_TYPE
> > > 
> > > This message provides a list of video codecs based on client's order
> > > of preference.
> > > 
> > > We duplicate the video codecs array from reds.c and sort it using the
> > > order of codecs as reference.
> > > 
> > > This message will not change an ongoing streaming but it could change
> > > newly created streams depending the rank value of each video codec
> > > that can be set by spice_server_set_video_codecs()
> > > 
> > > Signed-off-by: Victor Toso 
> > > ---
> > >  server/dcc-private.h |   5 ++
> > >  server/dcc.c | 126
> > >  +++
> > >  server/dcc.h |   1 +
> > >  server/display-channel.c |   2 +
> > >  server/stream.c  |   5 +-
> > >  5 files changed, 138 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/server/dcc-private.h b/server/dcc-private.h
> > > index 64b32a7..dd54c70 100644
> > > --- a/server/dcc-private.h
> > > +++ b/server/dcc-private.h
> > > @@ -51,6 +51,11 @@ struct DisplayChannelClientPrivate
> > >  int num_pixmap_cache_items;
> > >  } send_data;
> > >  
> > > +/* Host prefererred video-codec order sorted with client preferred */
> > > +GArray *preferred_video_codecs;
> >
> > I know this patch is important but every time I arrive here I ask
> > "array of what?" and I close the email.
> 
> Ah :(
> 
> >
> > C has type[] or type*... GArray is an array of everything you want...
> > I don't like it!
> 
> I used GArray here to keep consistency as we already use GArray for this
> RedVideoCodec struct in other places and this code actually interact
> with it.
> 
> I don't mind moving it to GPtrArray, at least you know that it will be
> an array of pointers.
> 
> Let me know what you think as while I was discussion with Pavel this
> morning, he find out another issue in the patch, which I plan to fix it
> in a followup patch later on.

Ping

> 
> PS: You do read the spice code tons of time more then I do, so I might
> not agree but I would do the change for what you think is more legible
> here (unless others disagree, of course)
> 
> Cheers,
> toso
> 
> >
> > Frediano
> > ___
> > 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] bool or gboolean

2017-02-20 Thread Victor Toso

Hi,

On Thu, Feb 16, 2017 at 01:24:04PM +0100, Christophe de Dinechin wrote:
> 
> > On 16 Feb 2017, at 13:17, Pavel Grunt  wrote:
> >
> > On Wed, 2017-02-15 at 11:39 -0500, Frediano Ziglio wrote:
> >> Hi,
> >>   question was raised recently on the ML and IRC.
> >>
> >> Some time ago we decided to use gboolean but some of us would like
> >> to discuss again.
> >>
> >> As any style changes there's no right or wrong, mainly personal
> >> opinions but I think consistency is quite important.
> >>
> >> Some consideration (feel free to add/remove/comment)
> >> - gboolean is more used in the code (about 76%)
> >> - TRUE/FALSE are more used (96%)
> >> - bool is C99 convention, defined in stdbool.h
> >> - using gcc the bool type is a bit different from gboolean
> >>   (which basically is an int) catching some problems as
> >>   warnings (like cast between different function pointers
> >>   using bool instead of gboolean/int)
> > this is very important point in my opinion
> >
> >> - bool is easier to write (OT: and my vim is more happy too)
> >>
> >> There are 2 kind of decision:
> >> - prefer bool or gboolean
> >> - stay consistent with constants (bool -> true/false,
> >>   gboolean -> TRUE/FALSE), continue to use TRUE/FALSE.
> >>
> >> I think as usual new code should follow style while old
> >> for "blame" purposes should stay as is (unless code stop
> >> compiling for instance).
> > I agree, no mass rename

IMHO, moving from int to bool where it fits (teuf's patch) would be
great too.

> >
> >> 
> >> For opinions
> >> 
> >> - bool/gboolean
> >>   - bool
> >>> +1
> > +1
> +1
+1

> >>
> >>   - gboolean
> >>
> >> - consistent with type
> >>   - yes
> >>> +1
> > +1
> +1
+1

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


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] [spice v8] dcc: handle preferred video codec message

2017-02-20 Thread Christophe de Dinechin


> On 10 Feb 2017, at 14:14, Victor Toso  wrote:
> 
> Hi,
> 
> On Fri, Feb 10, 2017 at 07:51:05AM -0500, Frediano Ziglio wrote:
>>> [0] SPICE_MSGC_DISPLAY_PREFERRED_VIDEO_CODEC_TYPE
>>> 
>>> This message provides a list of video codecs based on client's order
>>> of preference.
>>> 
>>> We duplicate the video codecs array from reds.c and sort it using the
>>> order of codecs as reference.
>>> 
>>> This message will not change an ongoing streaming but it could change
>>> newly created streams depending the rank value of each video codec
>>> that can be set by spice_server_set_video_codecs()
>>> 
>>> Signed-off-by: Victor Toso 
>>> ---
>>> server/dcc-private.h |   5 ++
>>> server/dcc.c | 126
>>> +++
>>> server/dcc.h |   1 +
>>> server/display-channel.c |   2 +
>>> server/stream.c  |   5 +-
>>> 5 files changed, 138 insertions(+), 1 deletion(-)
>>> 
>>> diff --git a/server/dcc-private.h b/server/dcc-private.h
>>> index 64b32a7..dd54c70 100644
>>> --- a/server/dcc-private.h
>>> +++ b/server/dcc-private.h
>>> @@ -51,6 +51,11 @@ struct DisplayChannelClientPrivate
>>> int num_pixmap_cache_items;
>>> } send_data;
>>> 
>>> +/* Host prefererred video-codec order sorted with client preferred */
>>> +GArray *preferred_video_codecs;
>> 
>> I know this patch is important but every time I arrive here I ask
>> "array of what?" and I close the email.
> 
> Ah :(
> 
>> 
>> C has type[] or type*... GArray is an array of everything you want...
>> I don't like it!
> 
> I used GArray here to keep consistency as we already use GArray for this
> RedVideoCodec struct in other places and this code actually interact
> with it.

GArray and type[] are not identical. GArray is dynamically allocated and can 
grow/shrink. It is closer to calloc/realloc than to type[].

In that case, I think that the number of codecs is fixed within the lifetime of 
the array. However, GArray naturally includes a size (the len field), whereas 
we don’t have the information for a plain array or pointer. So in that specific 
case, as we pass arrays around, even if they have a fixed size, I think GArray 
is the right choice.



> 
> I don't mind moving it to GPtrArray, at least you know that it will be
> an array of pointers.
> 
> Let me know what you think as while I was discussion with Pavel this
> morning, he find out another issue in the patch, which I plan to fix it
> in a followup patch later on.
> 
> PS: You do read the spice code tons of time more then I do, so I might
> not agree but I would do the change for what you think is more legible
> here (unless others disagree, of course)
> 
> Cheers,
>toso
> 
>> 
>> Frediano
>> ___
>> Spice-devel mailing list
>> Spice-devel@lists.freedesktop.org 
>> https://lists.freedesktop.org/mailman/listinfo/spice-devel 
>> 
> ___
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org 
> https://lists.freedesktop.org/mailman/listinfo/spice-devel 
> 
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [vdagent-linux v8 0/4]

2017-02-20 Thread Victor Toso

Hi,

This is now pushed as:

commit 99d9d3583143aef7143ec986cebe2980fdeeb776
vdagentd: Do endian swapping.

commit 3903ebd5935014cb13bfe697bc8c717ac737455d
vdagentd: Adjust size checks

commit 72acb448f76d07c1d3b74538bdedcde3d8f0d0f4
vdagentd: Add missing size checks

commit bf7cb2353455e1e7f289dc559393bab80cafa3fe
vdagentd: early return on bad message size

Cheers,
toso

On Wed, Feb 15, 2017 at 10:48:03AM +0100, Victor Toso wrote:
> From: Victor Toso 
> 
> Hi there!
> 
> This patches were also pending for some time and the patches were
> distributed in two mail threads and for that reason I'm puting them
> here in (another) thread :)
> 
> This first three patches were attached to Christophe's email [0], the
> last patch is the v7 from Michal [1]. I did not change them in any way
> at this moment.
> 
> [0] 
> https://lists.freedesktop.org/archives/spice-devel/2017-January/035323.html
> [1] 
> https://lists.freedesktop.org/archives/spice-devel/2017-January/035303.html
> 
> I've only tested them in LE Fedora 25. I've only small comments to the
> patches but IMHO these can be addressed before pushing if nobody
> complains.
> 
> Cheers,
> toso
> 
> Christophe Fergeau (2):
>   Add missing size checks
>   Adjust size checks
> 
> Michal Suchanek (1):
>   vdagentd: Do endian swapping.
> 
> Victor Toso (1):
>   vdagentd: early return on bad message size
> 
>  src/vdagentd/vdagentd.c| 236 
> +++--
>  src/vdagentd/virtio-port.c |  36 ---
>  2 files changed, 207 insertions(+), 65 deletions(-)
> 
> -- 
> 2.9.3
> 
> ___
> 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] [spice v8] dcc: handle preferred video codec message

2017-02-20 Thread Christophe de Dinechin


> On 9 Feb 2017, at 09:21, Victor Toso  wrote:
> 
> [0] SPICE_MSGC_DISPLAY_PREFERRED_VIDEO_CODEC_TYPE
> 
> This message provides a list of video codecs based on client's order
> of preference.
> 
> We duplicate the video codecs array from reds.c and sort it using the
> order of codecs as reference.
> 
> This message will not change an ongoing streaming but it could change
> newly created streams depending the rank value of each video codec
> that can be set by spice_server_set_video_codecs()
> 
> Signed-off-by: Victor Toso 
> ---
> server/dcc-private.h |   5 ++
> server/dcc.c | 126 +++
> server/dcc.h |   1 +
> server/display-channel.c |   2 +
> server/stream.c  |   5 +-
> 5 files changed, 138 insertions(+), 1 deletion(-)
> 
> diff --git a/server/dcc-private.h b/server/dcc-private.h
> index 64b32a7..dd54c70 100644
> --- a/server/dcc-private.h
> +++ b/server/dcc-private.h
> @@ -51,6 +51,11 @@ struct DisplayChannelClientPrivate
> int num_pixmap_cache_items;
> } send_data;
> 
> +/* Host prefererred video-codec order sorted with client preferred */
> +GArray *preferred_video_codecs;
> +/* Last client preferred video-codec order */
> +GArray *client_preferred_video_codecs;
> +
> uint8_t surface_client_created[NUM_SURFACES];
> QRegion surface_client_lossy_region[NUM_SURFACES];
> 
> diff --git a/server/dcc.c b/server/dcc.c
> index afe37b1..9271ea5 100644
> --- a/server/dcc.c
> +++ b/server/dcc.c
> @@ -39,6 +39,8 @@ enum
> PROP_ZLIB_GLZ_STATE
> };
> 
> +static void on_display_video_codecs_update(GObject *gobject, GParamSpec 
> *pspec, gpointer user_data);
> +
> static void
> display_channel_client_get_property(GObject *object,
> guint property_id,
> @@ -99,12 +101,19 @@ display_channel_client_constructed(GObject *object)
> dcc_init_stream_agents(self);
> 
> image_encoders_init(&self->priv->encoders, 
> &DCC_TO_DC(self)->priv->encoder_shared_data);
> +
> +g_signal_connect(DCC_TO_DC(self), "notify::video-codecs",
> + G_CALLBACK(on_display_video_codecs_update), self);
> }
> 
> static void
> display_channel_client_finalize(GObject *object)
> {
> DisplayChannelClient *self = DISPLAY_CHANNEL_CLIENT(object);
> +
> +g_signal_handlers_disconnect_by_func(DCC_TO_DC(self), 
> on_display_video_codecs_update, self);
> +g_clear_pointer(&self->priv->preferred_video_codecs, g_array_unref);
> +g_clear_pointer(&self->priv->client_preferred_video_codecs, 
> g_array_unref);
> g_free(self->priv);
> 
> G_OBJECT_CLASS(display_channel_client_parent_class)->finalize(object);
> @@ -1105,6 +1114,120 @@ static int 
> dcc_handle_preferred_compression(DisplayChannelClient *dcc,
> return TRUE;
> }
> 
> +
> +/* TODO: Client preference should only be considered when host has 
> video-codecs
> + * with the same priority value. At the moment, the video-codec GArray will 
> be
> + * sorted following only the client's preference (@user_data)
> + *
> + * example:
> + * host encoding preference: gstreamer:mjpeg;gstreamer:vp8;gstreamer:h264
> + * client decoding preference: h264, vp9, mjpeg
> + * result: gstreamer:h264;gstreamer:mjpeg;gstreamer:vp8
> + */
> +static gint sort_video_codecs_by_client_preference(gconstpointer a_pointer,
> +   gconstpointer b_pointer,
> +   gpointer user_data)
> +{
> +guint i;
> +const RedVideoCodec *a = a_pointer;
> +const RedVideoCodec *b = b_pointer;
> +GArray *client_pref = user_data;
> +
> +for (i = 0; i < client_pref->len; i++) {
> +if (a->type == g_array_index(client_pref, gint, i))
> +return -1;
> +
> +if (b->type == g_array_index(client_pref, gint, i))
> +return 1;
> +}

The order as you wrote it is not good, because it is possible to have both a < 
b and b < a, e.g. if client_pref[0] matches both a and b, then we will return 
-1 if we pass (a,b) and also -1 if we pass (b, a). That is likely to break the 
sorting algorithm in some subtle cases. My recommendation is something like:

+int client = g_array_index(client_pref, gint, i);
+if (a->type == client && b->type != client))
+return -1;
+
+if (b->type == client && a->type != client)
+return 1;

this will ensure that you get an “equal” and not “less than” result if both a 
and b match.

Alternatively, add a comment explaining why the situation may never arise 
(which I don’t think is true).

> +
> +return 0;
> +}
> +
> +static void dcc_update_preferred_video_codecs(DisplayChannelClient *dcc)
> +{
> +guint i;
> +RedsState *reds;
> +GArray *video_codecs, *server_codecs;
> +GString *msg;
> +
> +reds = 
> red_channel_get_server(red_channel_client_get_channel(&dcc->parent));
> +server_codecs = reds_get_video_codecs(reds);
> +

Re: [Spice-devel] [PATCH spice-gtk v5 6/6] usb-device-widget: Migrate to GtkGrid from GtkBox

2017-02-20 Thread Victor Toso

Hi,

On Fri, Feb 17, 2017 at 11:24:55AM +0100, Pavel Grunt wrote:
> GtkVBox is deprecated since Gtk 3.2, GtkBox is going to be
> deprecated. Just use the GtkGrid and GtkContainer api.

Sure

> ---
> GtkContainer is an abstract class, so let's use the Grid with the
> vertical orientation

At first I thought this was a ABI break but it was questioned in the
public IRC and the SpiceUsbDeviceWidget should not be subclassed, so
this should be fine. The chat is below for reference.

pgrunt | i have this patch https://paste.fedoraproject.org/.. is it abi
 break ? SpiceUsbDeviceWidget is publically a GtkWidget
 
https://www.spice-space.org/api/spice-gtk/spice-gtk-SpiceUsbDeviceWidget.html,
 but internally I am changing it from GtkBox to GtkContainer, so
 binary is different api is same
teuf   | yeah, depends if we consider it a valid/likely use case to
 derive from SpiceUsbDeviceWidget or not
teuf   | (ie whether G_DECLARE_FINAL_TYPE could be used in the header or
 not)
pgrunt | hm, it should be final
teuf   | note that it's glib 2.44+ only
pgrunt | well i can keep the type GtkBox and use the gtk_container api
 anyway

> ---
>  src/usb-device-widget.c | 25 ++---
>  1 file changed, 14 insertions(+), 11 deletions(-)
>
> diff --git a/src/usb-device-widget.c b/src/usb-device-widget.c
> index b394499..a7bbe8f 100644
> --- a/src/usb-device-widget.c
> +++ b/src/usb-device-widget.c
> @@ -40,14 +40,14 @@
>  
>  struct _SpiceUsbDeviceWidget
>  {
> -GtkVBox parent;
> +GtkGrid parent;
>  
>  SpiceUsbDeviceWidgetPrivate *priv;
>  };
>  
>  struct _SpiceUsbDeviceWidgetClass
>  {
> -GtkVBoxClass parent_class;
> +GtkGridClass parent_class;
>  
>  /* signals */
>  void (*connect_failed) (SpiceUsbDeviceWidget *widget,
> @@ -94,7 +94,7 @@ struct _SpiceUsbDeviceWidgetPrivate {
>  
>  static guint signals[LAST_SIGNAL] = { 0, };
>  
> -G_DEFINE_TYPE(SpiceUsbDeviceWidget, spice_usb_device_widget, GTK_TYPE_BOX);
> +G_DEFINE_TYPE(SpiceUsbDeviceWidget, spice_usb_device_widget, GTK_TYPE_GRID);
>  
>  static void spice_usb_device_widget_get_property(GObject *gobject,
>   guintprop_id,
> @@ -163,20 +163,22 @@ 
> spice_usb_device_widget_show_info_bar(SpiceUsbDeviceWidget *self,
>  gtk_info_bar_set_message_type(GTK_INFO_BAR(info_bar), message_type);
>  
>  content_area = gtk_info_bar_get_content_area(GTK_INFO_BAR(info_bar));
> -hbox = gtk_box_new(GTK_ORIENTATION_HORIZONTAL, 12);
> +hbox = gtk_grid_new();
> +gtk_grid_set_column_spacing(GTK_GRID(hbox), 12);

It is horizontal grid but you are set column space. I guess it should be
_row_spacing() - although I can't see any problem in my test.

>  gtk_container_add(GTK_CONTAINER(content_area), hbox);
>  
>  widget = gtk_image_new_from_icon_name(stock_icon_id,
>GTK_ICON_SIZE_SMALL_TOOLBAR);
> -gtk_box_pack_start(GTK_BOX(hbox), widget, FALSE, FALSE, 0);
> +gtk_container_add(GTK_CONTAINER(hbox), widget);
>  
>  widget = gtk_label_new(message);
> -gtk_box_pack_start(GTK_BOX(hbox), widget, TRUE, TRUE, 0);
> +g_object_set(G_OBJECT(widget), "expand", TRUE, NULL);
> +gtk_container_add(GTK_CONTAINER(hbox), widget);
>  
>  priv->info_bar = gtk_alignment_new(0.0, 0.0, 1.0, 0.0);
>  gtk_alignment_set_padding(GTK_ALIGNMENT(priv->info_bar), 0, 0, 12, 0);
>  gtk_container_add(GTK_CONTAINER(priv->info_bar), info_bar);
> -gtk_box_pack_start(GTK_BOX(self), priv->info_bar, FALSE, FALSE, 0);
> +gtk_container_add(GTK_CONTAINER(self), priv->info_bar);
>  gtk_widget_show_all(priv->info_bar);
>  }
>  
> @@ -203,12 +205,14 @@ static GObject *spice_usb_device_widget_constructor(
>  if (!priv->session)
>  g_error("SpiceUsbDeviceWidget constructed without a session");
>  
> +gtk_orientable_set_orientation(GTK_ORIENTABLE(self), 
> GTK_ORIENTATION_VERTICAL);
> +
>  priv->label = gtk_label_new(NULL);
>  str = g_strdup_printf("%s", _("Select USB devices to redirect"));
>  gtk_label_set_markup(GTK_LABEL (priv->label), str);
>  g_free(str);
>  gtk_misc_set_alignment(GTK_MISC(priv->label), 0.0, 0.5);
> -gtk_box_pack_start(GTK_BOX(self), priv->label, FALSE, FALSE, 0);
> +gtk_container_add(GTK_CONTAINER(self), priv->label);
>  
>  priv->manager = spice_usb_device_manager_get(priv->session, &err);
>  if (err) {
> @@ -348,10 +352,9 @@ GtkWidget *spice_usb_device_widget_new(SpiceSession
> *session,
> const gchar *device_format_string)
>  {
>  return g_object_new(SPICE_TYPE_USB_DEVICE_WIDGET,
> -"orientation", GTK_ORIENTATION_VERTICAL,
>  "session", session,
>  "device-format-string", device_format_string,
> -"spacing", 6,
> +"row-spacing",

Re: [Spice-devel] [spice v8] dcc: handle preferred video codec message

2017-02-20 Thread Frediano Ziglio

> 
> [0] SPICE_MSGC_DISPLAY_PREFERRED_VIDEO_CODEC_TYPE
> 
> This message provides a list of video codecs based on client's order
> of preference.
> 
> We duplicate the video codecs array from reds.c and sort it using the
> order of codecs as reference.
> 
> This message will not change an ongoing streaming but it could change
> newly created streams depending the rank value of each video codec
> that can be set by spice_server_set_video_codecs()
> 
> Signed-off-by: Victor Toso 
> ---
>  server/dcc-private.h |   5 ++
>  server/dcc.c | 126
>  +++
>  server/dcc.h |   1 +
>  server/display-channel.c |   2 +
>  server/stream.c  |   5 +-
>  5 files changed, 138 insertions(+), 1 deletion(-)
> 
> diff --git a/server/dcc-private.h b/server/dcc-private.h
> index 64b32a7..dd54c70 100644
> --- a/server/dcc-private.h
> +++ b/server/dcc-private.h
> @@ -51,6 +51,11 @@ struct DisplayChannelClientPrivate
>  int num_pixmap_cache_items;
>  } send_data;
>  
> +/* Host prefererred video-codec order sorted with client preferred */

prefererred -> preferred

> +GArray *preferred_video_codecs;
> +/* Last client preferred video-codec order */
> +GArray *client_preferred_video_codecs;
> +
>  uint8_t surface_client_created[NUM_SURFACES];
>  QRegion surface_client_lossy_region[NUM_SURFACES];
>  
> diff --git a/server/dcc.c b/server/dcc.c
> index afe37b1..9271ea5 100644
> --- a/server/dcc.c
> +++ b/server/dcc.c
> @@ -39,6 +39,8 @@ enum
>  PROP_ZLIB_GLZ_STATE
>  };
>  
> +static void on_display_video_codecs_update(GObject *gobject, GParamSpec
> *pspec, gpointer user_data);
> +
>  static void
>  display_channel_client_get_property(GObject *object,
>  guint property_id,
> @@ -99,12 +101,19 @@ display_channel_client_constructed(GObject *object)
>  dcc_init_stream_agents(self);
>  
>  image_encoders_init(&self->priv->encoders,
>  &DCC_TO_DC(self)->priv->encoder_shared_data);
> +
> +g_signal_connect(DCC_TO_DC(self), "notify::video-codecs",
> + G_CALLBACK(on_display_video_codecs_update), self);
>  }
>  
>  static void
>  display_channel_client_finalize(GObject *object)
>  {
>  DisplayChannelClient *self = DISPLAY_CHANNEL_CLIENT(object);
> +
> +g_signal_handlers_disconnect_by_func(DCC_TO_DC(self),
> on_display_video_codecs_update, self);
> +g_clear_pointer(&self->priv->preferred_video_codecs, g_array_unref);
> +g_clear_pointer(&self->priv->client_preferred_video_codecs,
> g_array_unref);
>  g_free(self->priv);
>  
>  G_OBJECT_CLASS(display_channel_client_parent_class)->finalize(object);
> @@ -1105,6 +1114,120 @@ static int
> dcc_handle_preferred_compression(DisplayChannelClient *dcc,
>  return TRUE;
>  }
>  
> +
> +/* TODO: Client preference should only be considered when host has
> video-codecs
> + * with the same priority value. At the moment, the video-codec GArray will
> be
> + * sorted following only the client's preference (@user_data)
> + *
> + * example:
> + * host encoding preference: gstreamer:mjpeg;gstreamer:vp8;gstreamer:h264
> + * client decoding preference: h264, vp9, mjpeg
> + * result: gstreamer:h264;gstreamer:mjpeg;gstreamer:vp8
> + */
> +static gint sort_video_codecs_by_client_preference(gconstpointer a_pointer,
> +   gconstpointer b_pointer,
> +   gpointer user_data)
> +{
> +guint i;
> +const RedVideoCodec *a = a_pointer;
> +const RedVideoCodec *b = b_pointer;
> +GArray *client_pref = user_data;
> +
> +for (i = 0; i < client_pref->len; i++) {
> +if (a->type == g_array_index(client_pref, gint, i))
> +return -1;

style, always use brackets

> +
> +if (b->type == g_array_index(client_pref, gint, i))
> +return 1;
> +}
> +
> +return 0;
> +}
> +
> +static void dcc_update_preferred_video_codecs(DisplayChannelClient *dcc)
> +{
> +guint i;
> +RedsState *reds;
> +GArray *video_codecs, *server_codecs;
> +GString *msg;
> +
> +reds =
> red_channel_get_server(red_channel_client_get_channel(&dcc->parent));
> +server_codecs = reds_get_video_codecs(reds);

This is not thread safe, don't call reds_get_video_codecs from here,
use display_channel_get_video_codecs.

> +spice_return_if_fail(server_codecs != NULL);
> +
> +/* Copy current host preference */
> +video_codecs = g_array_new(FALSE, FALSE, sizeof(RedVideoCodec));
> +for (i = 0; i < server_codecs->len; i++) {
> +RedVideoCodec codec = g_array_index(server_codecs, RedVideoCodec,
> i);
> +g_array_append_val(video_codecs, codec);
> +}
> +
> +/* Sort the copy of current host preference based on client's preference
> */
> +g_array_sort_with_data(video_codecs,
> sort_video_codecs_by_client_preference,
> +   dcc->priv->client_pr

Re: [Spice-devel] [PATCH spice-gtk v5 6/6] usb-device-widget: Migrate to GtkGrid from GtkBox

2017-02-20 Thread Pavel Grunt

Hi,

On Mon, 2017-02-20 at 12:44 +0100, Victor Toso wrote:
> Hi,
> 
> On Fri, Feb 17, 2017 at 11:24:55AM +0100, Pavel Grunt wrote:
> > GtkVBox is deprecated since Gtk 3.2, GtkBox is going to be
> > deprecated. Just use the GtkGrid and GtkContainer api.
> 
> Sure
> 
> > ---
> > GtkContainer is an abstract class, so let's use the Grid with the
> > vertical orientation
> 
> At first I thought this was a ABI break but it was questioned in the
> public IRC and the SpiceUsbDeviceWidget should not be subclassed, so
> this should be fine. The chat is below for reference.
> 
> pgrunt | i have this patch https://paste.fedoraproject.org/.. is it
> abi
>  break ? SpiceUsbDeviceWidget is publically a GtkWidget
>  https://www.spice-space.org/api/spice-gtk/spice-gtk-SpiceUs
> bDeviceWidget.html,
>  but internally I am changing it from GtkBox to
> GtkContainer, so
>  binary is different api is same
> teuf   | yeah, depends if we consider it a valid/likely use case to
>  derive from SpiceUsbDeviceWidget or not
> teuf   | (ie whether G_DECLARE_FINAL_TYPE could be used in the
> header or
>  not)
> pgrunt | hm, it should be final
> teuf   | note that it's glib 2.44+ only
> pgrunt | well i can keep the type GtkBox and use the gtk_container
> api
>  anyway
> 
> > ---
> >  src/usb-device-widget.c | 25 ++---
> >  1 file changed, 14 insertions(+), 11 deletions(-)
> > 
> > diff --git a/src/usb-device-widget.c b/src/usb-device-widget.c
> > index b394499..a7bbe8f 100644
> > --- a/src/usb-device-widget.c
> > +++ b/src/usb-device-widget.c
> > @@ -40,14 +40,14 @@
> >  
> >  struct _SpiceUsbDeviceWidget
> >  {
> > -GtkVBox parent;
> > +GtkGrid parent;
> >  
> >  SpiceUsbDeviceWidgetPrivate *priv;
> >  };
> >  
> >  struct _SpiceUsbDeviceWidgetClass
> >  {
> > -GtkVBoxClass parent_class;
> > +GtkGridClass parent_class;
> >  
> >  /* signals */
> >  void (*connect_failed) (SpiceUsbDeviceWidget *widget,
> > @@ -94,7 +94,7 @@ struct _SpiceUsbDeviceWidgetPrivate {
> >  
> >  static guint signals[LAST_SIGNAL] = { 0, };
> >  
> > -G_DEFINE_TYPE(SpiceUsbDeviceWidget, spice_usb_device_widget,
> > GTK_TYPE_BOX);
> > +G_DEFINE_TYPE(SpiceUsbDeviceWidget, spice_usb_device_widget,
> > GTK_TYPE_GRID);
> >  
> >  static void
> > spice_usb_device_widget_get_property(GObject *gobject,
> >   guintpro
> > p_id,
> > @@ -163,20 +163,22 @@
> > spice_usb_device_widget_show_info_bar(SpiceUsbDeviceWidget *self,
> >  gtk_info_bar_set_message_type(GTK_INFO_BAR(info_bar),
> > message_type);
> >  
> >  content_area =
> > gtk_info_bar_get_content_area(GTK_INFO_BAR(info_bar));
> > -hbox = gtk_box_new(GTK_ORIENTATION_HORIZONTAL, 12);
> > +hbox = gtk_grid_new();
> > +gtk_grid_set_column_spacing(GTK_GRID(hbox), 12);

this hbox widget is a child of SpiceUsbDeviceWidget (GtkGrid). The
hbox contains an icon and label.

The g_object_new below is related to the SpiceUsbDeviceWidget itself
which is vertically oriented -> row spacing.

> 
> It is horizontal grid but you are set column space. I guess it
> should be
> _row_spacing() - although I can't see any problem in my test.
> 
> >  gtk_container_add(GTK_CONTAINER(content_area), hbox);
> >  
> >  widget = gtk_image_new_from_icon_name(stock_icon_id,
> >    GTK_ICON_SIZE_SMALL_TOO
> > LBAR);
> > -gtk_box_pack_start(GTK_BOX(hbox), widget, FALSE, FALSE, 0);
> > +gtk_container_add(GTK_CONTAINER(hbox), widget);
> >  
> >  widget = gtk_label_new(message);
> > -gtk_box_pack_start(GTK_BOX(hbox), widget, TRUE, TRUE, 0);
> > +g_object_set(G_OBJECT(widget), "expand", TRUE, NULL);
> > +gtk_container_add(GTK_CONTAINER(hbox), widget);
> >  
> >  priv->info_bar = gtk_alignment_new(0.0, 0.0, 1.0, 0.0);
> >  gtk_alignment_set_padding(GTK_ALIGNMENT(priv->info_bar), 0,
> > 0, 12, 0);
> >  gtk_container_add(GTK_CONTAINER(priv->info_bar), info_bar);
> > -gtk_box_pack_start(GTK_BOX(self), priv->info_bar, FALSE,
> > FALSE, 0);
> > +gtk_container_add(GTK_CONTAINER(self), priv->info_bar);
> >  gtk_widget_show_all(priv->info_bar);
> >  }
> >  
> > @@ -203,12 +205,14 @@ static GObject
> > *spice_usb_device_widget_constructor(
> >  if (!priv->session)
> >  g_error("SpiceUsbDeviceWidget constructed without a
> > session");
> >  
> > +gtk_orientable_set_orientation(GTK_ORIENTABLE(self),
> > GTK_ORIENTATION_VERTICAL);
> > +
> >  priv->label = gtk_label_new(NULL);
> >  str = g_strdup_printf("%s", _("Select USB devices to
> > redirect"));
> >  gtk_label_set_markup(GTK_LABEL (priv->label), str);
> >  g_free(str);
> >  gtk_misc_set_alignment(GTK_MISC(priv->label), 0.0, 0.5);
> > -gtk_box_pack_start(GTK_BOX(self), priv->label, FALSE, FALSE,
> > 0);
> > +gtk_container_add(GTK_CONTAINER(self), priv->label);
> >  
> >  priv->manager = spice_usb

Re: [Spice-devel] [PATCH spice-gtk v5 6/6] usb-device-widget: Migrate to GtkGrid from GtkBox

2017-02-20 Thread Daniel P. Berrange

On Mon, Feb 20, 2017 at 12:44:52PM +0100, Victor Toso wrote:
> Hi,
> 
> On Fri, Feb 17, 2017 at 11:24:55AM +0100, Pavel Grunt wrote:
> > GtkVBox is deprecated since Gtk 3.2, GtkBox is going to be
> > deprecated. Just use the GtkGrid and GtkContainer api.
> 
> Sure
> 
> > ---
> > GtkContainer is an abstract class, so let's use the Grid with the
> > vertical orientation
> 
> At first I thought this was a ABI break but it was questioned in the
> public IRC and the SpiceUsbDeviceWidget should not be subclassed, so
> this should be fine. The chat is below for reference.

Saying on IRC that it shouldn't be subclassed doesn't mean that it hasn't
been sub-classed, particularly if this rule about not-subclassing has not
been documented anywhere for app developers to learn about it...

> pgrunt | i have this patch https://paste.fedoraproject.org/.. is it abi
>  break ? SpiceUsbDeviceWidget is publically a GtkWidget
>  
> https://www.spice-space.org/api/spice-gtk/spice-gtk-SpiceUsbDeviceWidget.html,
>  but internally I am changing it from GtkBox to GtkContainer, so
>  binary is different api is same
> teuf   | yeah, depends if we consider it a valid/likely use case to
>  derive from SpiceUsbDeviceWidget or not
> teuf   | (ie whether G_DECLARE_FINAL_TYPE could be used in the header or
>  not)
> pgrunt | hm, it should be final
> teuf   | note that it's glib 2.44+ only
> pgrunt | well i can keep the type GtkBox and use the gtk_container api
>  anyway

Even if you keep the type GtkBox in the struct, but change the
G_DEFINE_TYPE to use GTK_CONTAINER, instad of GTK_TYPE_BOX, that is
still technically an API break as you've changed the semantics of
the widget. ie if a subclass exists, and they are calling any of
the gtk_box_* APIs those will break, even though the struct still
contains the GtkBox content. ie while ABI refers to compiled type
sizes, API covers semantics of the types too, and you need to
preserve both ABI + API if you are not changing the soname.

Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://entangle-photo.org   -o-http://search.cpan.org/~danberr/ :|
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [spice v8] dcc: handle preferred video codec message

2017-02-20 Thread Victor Toso

Hi,

On Mon, Feb 20, 2017 at 12:16:33PM +0100, Christophe de Dinechin wrote:
>
> > On 9 Feb 2017, at 09:21, Victor Toso  wrote:
> > 
> > [0] SPICE_MSGC_DISPLAY_PREFERRED_VIDEO_CODEC_TYPE
> > 
> > This message provides a list of video codecs based on client's order
> > of preference.
> > 
> > We duplicate the video codecs array from reds.c and sort it using the
> > order of codecs as reference.
> > 
> > This message will not change an ongoing streaming but it could change
> > newly created streams depending the rank value of each video codec
> > that can be set by spice_server_set_video_codecs()
> > 
> > Signed-off-by: Victor Toso 
> > ---
> > server/dcc-private.h |   5 ++
> > server/dcc.c | 126 
> > +++
> > server/dcc.h |   1 +
> > server/display-channel.c |   2 +
> > server/stream.c  |   5 +-
> > 5 files changed, 138 insertions(+), 1 deletion(-)
> > 
> > diff --git a/server/dcc-private.h b/server/dcc-private.h
> > index 64b32a7..dd54c70 100644
> > --- a/server/dcc-private.h
> > +++ b/server/dcc-private.h
> > @@ -51,6 +51,11 @@ struct DisplayChannelClientPrivate
> > int num_pixmap_cache_items;
> > } send_data;
> > 
> > +/* Host prefererred video-codec order sorted with client preferred */
> > +GArray *preferred_video_codecs;
> > +/* Last client preferred video-codec order */
> > +GArray *client_preferred_video_codecs;
> > +
> > uint8_t surface_client_created[NUM_SURFACES];
> > QRegion surface_client_lossy_region[NUM_SURFACES];
> > 
> > diff --git a/server/dcc.c b/server/dcc.c
> > index afe37b1..9271ea5 100644
> > --- a/server/dcc.c
> > +++ b/server/dcc.c
> > @@ -39,6 +39,8 @@ enum
> > PROP_ZLIB_GLZ_STATE
> > };
> > 
> > +static void on_display_video_codecs_update(GObject *gobject, GParamSpec 
> > *pspec, gpointer user_data);
> > +
> > static void
> > display_channel_client_get_property(GObject *object,
> > guint property_id,
> > @@ -99,12 +101,19 @@ display_channel_client_constructed(GObject *object)
> > dcc_init_stream_agents(self);
> > 
> > image_encoders_init(&self->priv->encoders, 
> > &DCC_TO_DC(self)->priv->encoder_shared_data);
> > +
> > +g_signal_connect(DCC_TO_DC(self), "notify::video-codecs",
> > + G_CALLBACK(on_display_video_codecs_update), self);
> > }
> > 
> > static void
> > display_channel_client_finalize(GObject *object)
> > {
> > DisplayChannelClient *self = DISPLAY_CHANNEL_CLIENT(object);
> > +
> > +g_signal_handlers_disconnect_by_func(DCC_TO_DC(self), 
> > on_display_video_codecs_update, self);
> > +g_clear_pointer(&self->priv->preferred_video_codecs, g_array_unref);
> > +g_clear_pointer(&self->priv->client_preferred_video_codecs, 
> > g_array_unref);
> > g_free(self->priv);
> > 
> > G_OBJECT_CLASS(display_channel_client_parent_class)->finalize(object);
> > @@ -1105,6 +1114,120 @@ static int 
> > dcc_handle_preferred_compression(DisplayChannelClient *dcc,
> > return TRUE;
> > }
> > 
> > +
> > +/* TODO: Client preference should only be considered when host has 
> > video-codecs
> > + * with the same priority value. At the moment, the video-codec GArray 
> > will be
> > + * sorted following only the client's preference (@user_data)
> > + *
> > + * example:
> > + * host encoding preference: gstreamer:mjpeg;gstreamer:vp8;gstreamer:h264
> > + * client decoding preference: h264, vp9, mjpeg
> > + * result: gstreamer:h264;gstreamer:mjpeg;gstreamer:vp8
> > + */
> > +static gint sort_video_codecs_by_client_preference(gconstpointer a_pointer,
> > +   gconstpointer b_pointer,
> > +   gpointer user_data)
> > +{
> > +guint i;
> > +const RedVideoCodec *a = a_pointer;
> > +const RedVideoCodec *b = b_pointer;
> > +GArray *client_pref = user_data;
> > +
> > +for (i = 0; i < client_pref->len; i++) {
> > +if (a->type == g_array_index(client_pref, gint, i))
> > +return -1;
> > +
> > +if (b->type == g_array_index(client_pref, gint, i))
> > +return 1;
> > +}
>
> The order as you wrote it is not good, because it is possible to have
> both a < b and b < a, e.g. if client_pref[0] matches both a and b,
> then we will return -1 if we pass (a,b) and also -1 if we pass (b, a).
> That is likely to break the sorting algorithm in some subtle cases. My
> recommendation is something like:
>
> +int client = g_array_index(client_pref, gint, i);
> +if (a->type == client && b->type != client))
> +return -1;
> +
> +if (b->type == client && a->type != client)
> +return 1;
>
> this will ensure that you get an “equal” and not “less than” result if
> both a and b match.
>
> Alternatively, add a comment explaining why the situation may never
> arise (which I don’t think is true).

You are saying that I should be mor

Re: [Spice-devel] [PATCH spice-gtk v5 6/6] usb-device-widget: Migrate to GtkGrid from GtkBox

2017-02-20 Thread Pavel Grunt

Hi Daniel,

On Mon, 2017-02-20 at 13:04 +, Daniel P. Berrange wrote:
> On Mon, Feb 20, 2017 at 12:44:52PM +0100, Victor Toso wrote:
> > Hi,
> > 
> > On Fri, Feb 17, 2017 at 11:24:55AM +0100, Pavel Grunt wrote:
> > > GtkVBox is deprecated since Gtk 3.2, GtkBox is going to be
> > > deprecated. Just use the GtkGrid and GtkContainer api.
> > 
> > Sure
> > 
> > > ---
> > > GtkContainer is an abstract class, so let's use the Grid with
> > > the
> > > vertical orientation
> > 
> > At first I thought this was a ABI break but it was questioned in
> > the
> > public IRC and the SpiceUsbDeviceWidget should not be subclassed,
> > so
> > this should be fine. The chat is below for reference.
> 
> Saying on IRC that it shouldn't be subclassed doesn't mean that it
> hasn't
> been sub-classed, particularly if this rule about not-subclassing
> has not
> been documented anywhere for app developers to learn about it...
> 
> > pgrunt | i have this patch https://paste.fedoraproject.org/.. is
> > it abi
> >  break ? SpiceUsbDeviceWidget is publically a GtkWidget
> >  https://www.spice-space.org/api/spice-gtk/spice-gtk-Spice
> > UsbDeviceWidget.html,
> >  but internally I am changing it from GtkBox to
> > GtkContainer, so
> >  binary is different api is same
> > teuf   | yeah, depends if we consider it a valid/likely use case
> > to
> >  derive from SpiceUsbDeviceWidget or not
> > teuf   | (ie whether G_DECLARE_FINAL_TYPE could be used in the
> > header or
> >  not)
> > pgrunt | hm, it should be final
> > teuf   | note that it's glib 2.44+ only
> > pgrunt | well i can keep the type GtkBox and use the gtk_container
> > api
> >  anyway
> 
> Even if you keep the type GtkBox in the struct, but change the
> G_DEFINE_TYPE to use GTK_CONTAINER, 

by "i can keep the type GtkBox" i meant to keep GTK_TYPE_BOX

> instad of GTK_TYPE_BOX, that is
> still technically an API break as you've changed the semantics of
> the widget. ie if a subclass exists,

SpiceUsbDeviceWidget is considered a final class
https://cgit.freedesktop.org/spice/spice-gtk/commit/?id=bc3d12efb20423
d5b1ebd490658f687c4bd323fd
It is not intended to be subclassed.

>  and they are calling any of
> the gtk_box_* APIs those will break, 

It is Publicly declared as a GtkWidget, not GtkBox/Container/Grid

Pavel

> even though the struct still
> contains the GtkBox content. ie while ABI refers to compiled type
> sizes, API covers semantics of the types too, and you need to
> preserve both ABI + API if you are not changing the soname.
> 
> Regards,
> Daniel
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [PATCH spice-server 3/3] Add message counters to statistics

2017-02-20 Thread Frediano Ziglio

Didn't forget to merge this.

It's just that not every time you call send_item a message is sent
(just remembered before merging).

Frediano

> 
> Acked-by: Jonathon Jongsma 
> 
> 
> On Tue, 2017-02-14 at 15:51 +, Frediano Ziglio wrote:
> > Signed-off-by: Frediano Ziglio 
> > ---
> >  server/red-channel.c | 4 
> >  1 file changed, 4 insertions(+)
> > 
> > Ping.
> > 
> > diff --git a/server/red-channel.c b/server/red-channel.c
> > index e70c46b..3599c3b 100644
> > --- a/server/red-channel.c
> > +++ b/server/red-channel.c
> > @@ -103,6 +103,7 @@ struct RedChannelPrivate
> >  RedsState *reds;
> >  RedStatNode stat;
> >  RedStatCounter out_bytes_counter;
> > +RedStatCounter out_messages;
> >  };
> >  
> >  enum {
> > @@ -414,6 +415,8 @@ void red_channel_init_stat_node(RedChannel
> > *channel, const RedStatNode *parent,
> >  stat_init_node(&channel->priv->stat, channel->priv->reds,
> > parent, name, TRUE);
> >  stat_init_counter(&channel->priv->out_bytes_counter,
> >    channel->priv->reds, &channel->priv->stat,
> > "out_bytes", TRUE);
> > +stat_init_counter(&channel->priv->out_messages,
> > +  channel->priv->reds, &channel->priv->stat,
> > "out_messages", TRUE);
> >  }
> >  
> >  const RedStatNode *red_channel_get_stat_node(RedChannel *channel)
> > @@ -782,6 +785,7 @@ void red_channel_send_item(RedChannel *self,
> > RedChannelClient *rcc, RedPipeItem
> >  RedChannelClass *klass = RED_CHANNEL_GET_CLASS(self);
> >  g_return_if_fail(klass->send_item);
> >  
> > +stat_inc_counter(self->priv->out_messages, 1);
> >  klass->send_item(rcc, item);
> >  }
> >  
> 
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

[Spice-devel] [spice-gtk v1] gtk-session: Use GWeakRef

2017-02-20 Thread Victor Toso

From: Victor Toso 

The custom WeakRef structure was introduced in 9baba9fd89 (2012) to
fix rhbz#743773. Since glib 2.32, GWeakRef was introduced and it
behaves similarly to our WeakRef.

Moving to GWeakRef to remove some code which exists in glib.

Note that I'm keeping to utility functions:
- get_weak_ref(gpointer object): which returns a newly allocated
  GWeakRef, initialized with @object;
- free_weak_ref(gpointer pdata): which frees the GWeakRef and returns
  the original object or NULL. It also takes care of an extra
  reference to the object.

Signed-off-by: Victor Toso 
---
 src/spice-gtk-session.c | 52 +++--
 1 file changed, 20 insertions(+), 32 deletions(-)

diff --git a/src/spice-gtk-session.c b/src/spice-gtk-session.c
index 5688cba..88f4488 100644
--- a/src/spice-gtk-session.c
+++ b/src/spice-gtk-session.c
@@ -575,31 +575,26 @@ static const struct {
 }
 };
 
-typedef struct _WeakRef {
-GObject *object;
-} WeakRef;
-
-static void weak_notify_cb(WeakRef *weakref, GObject *object)
-{
-weakref->object = NULL;
-}
-
-static WeakRef* weak_ref(GObject *object)
+static GWeakRef* get_weak_ref(gpointer object)
 {
-WeakRef *weakref = g_new(WeakRef, 1);
-
-g_object_weak_ref(object, (GWeakNotify)weak_notify_cb, weakref);
-weakref->object = object;
-
+GWeakRef *weakref = g_new(GWeakRef, 1);
+g_weak_ref_init(weakref, object);
 return weakref;
 }
 
-static void weak_unref(WeakRef* weakref)
+static gpointer free_weak_ref(gpointer data)
 {
-if (weakref->object)
-g_object_weak_unref(weakref->object, (GWeakNotify)weak_notify_cb, 
weakref);
+GWeakRef *weakref = data;
+gpointer object = g_weak_ref_get(weakref);
 
+g_weak_ref_clear(weakref);
 g_free(weakref);
+if (object != NULL) {
+/* The main referece still exists as object is not NULL, so we can
+ * remove the strong reference given by g_weak_ref_get */
+g_object_unref(object);
+}
+return object;
 }
 
 static void clipboard_get_targets(GtkClipboard *clipboard,
@@ -607,9 +602,7 @@ static void clipboard_get_targets(GtkClipboard *clipboard,
   gint n_atoms,
   gpointer user_data)
 {
-WeakRef *weakref = user_data;
-SpiceGtkSession *self = (SpiceGtkSession*)weakref->object;
-weak_unref(weakref);
+SpiceGtkSession *self = free_weak_ref(user_data);
 
 if (self == NULL)
 return;
@@ -706,7 +699,7 @@ static void clipboard_owner_change(GtkClipboard
*clipboard,
 s->clip_hasdata[selection] = TRUE;
 if (s->auto_clipboard_enable && !read_only(self))
 gtk_clipboard_request_targets(clipboard, clipboard_get_targets,
-  weak_ref(G_OBJECT(self)));
+  get_weak_ref(self));
 break;
 default:
 s->clip_hasdata[selection] = FALSE;
@@ -939,14 +932,11 @@ static void clipboard_received_text_cb(GtkClipboard 
*clipboard,
const gchar *text,
gpointer user_data)
 {
-WeakRef *weakref = user_data;
-SpiceGtkSession *self = (SpiceGtkSession*)weakref->object;
+SpiceGtkSession *self = free_weak_ref(user_data);
 char *conv = NULL;
 int len = 0;
 int selection;
 
-weak_unref(weakref);
-
 if (self == NULL)
 return;
 
@@ -982,9 +972,7 @@ static void clipboard_received_cb(GtkClipboard *clipboard,
   GtkSelectionData *selection_data,
   gpointer user_data)
 {
-WeakRef *weakref = user_data;
-SpiceGtkSession *self = (SpiceGtkSession*)weakref->object;
-weak_unref(weakref);
+SpiceGtkSession *self = free_weak_ref(user_data);
 
 if (self == NULL)
 return;
@@ -1055,7 +1043,7 @@ static gboolean clipboard_request(SpiceMainChannel *main, 
guint selection,
 
 if (type == VD_AGENT_CLIPBOARD_UTF8_TEXT) {
 gtk_clipboard_request_text(cb, clipboard_received_text_cb,
-   weak_ref(G_OBJECT(self)));
+   get_weak_ref(self));
 } else {
 for (m = 0; m < SPICE_N_ELEMENTS(atom2agent); m++) {
 if (atom2agent[m].vdagent == type)
@@ -1066,7 +1054,7 @@ static gboolean clipboard_request(SpiceMainChannel *main, 
guint selection,
 
 atom = gdk_atom_intern_static_string(atom2agent[m].xatom);
 gtk_clipboard_request_contents(cb, atom, clipboard_received_cb,
-   weak_ref(G_OBJECT(self)));
+   get_weak_ref(self));
 }
 
 return TRUE;
@@ -1239,7 +1227,7 @@ void spice_gtk_session_copy_to_guest(SpiceGtkSession 
*self)
 
 if (s->clip_hasdata[selection] && !s->clip_grabbed[selection]) {
 gtk_clipboard_request_targets(s->clipboard, clipboard_get_targets,
-

Re: [Spice-devel] [PATCH] Don't close all but one display during reboot.

2017-02-20 Thread Jonathon Jongsma

On Thu, 2017-02-02 at 10:30 -0600, Jonathon Jongsma wrote:
> On Thu, 2017-02-02 at 05:29 -0500, Frediano Ziglio wrote:
> > > 
> > > When a guest is rebooted, the QXL driver gets unloaded at some
> > > point in
> > > the reboot process. When the driver is unloaded, the spice server
> > > sets a
> > > single flag to FALSE: RedWorker::driver_cap_monitors_config. This
> > > flag
> > > indicates whether the driver is capable of sending its own
> > > monitors
> > > config
> > > messages to the client.
> > > 
> > > The only place this flag is used is when a new primary surface is
> > > created. If
> > > this flag is true, the server assumes that the driver will send
> > > its
> > > own
> > > monitors config very soon after the surface is created. If it's
> > > false, the
> > > server directly sends its own temporary monitors config message
> > > to
> > > the client
> > > since the driver cannot.  This temporary monitors config message
> > > is
> > > based on
> > > the size of the just-created primary surface and always has a
> > > maximum monitor
> > > count of 1.
> > > 
> > > This flag is initialized to false at startup so that in early
> > > boot
> > > (e.g. vga
> > > text mode), the server will send out these 'temporary' monitor
> > > config
> > > messages
> > > to the client whenever the primary surface is destroyed and re-
> > > created. This
> > > causes the client to resize its window to match the size of the
> > > guest. When
> > > the
> > > QXL driver is eventually loaded and starts taking over the
> > > monitors
> > > config
> > > responsibilities, we set this flag to true and the server stops
> > > sending
> > > monitors config messages out on its own.
> > > 
> > > If we reboot and set this flag to false, it will result in the
> > > server sending
> > > a
> > > monitors config message to the client indicating that the guest
> > > now
> > > supports
> > > a
> > > maximum of 1 monitor. If the guest happens to have more than one
> > > display
> > > window
> > > open, it will destroy those extra windows because they exceed the
> > > maximum
> > > allowed number of monitors. This means that if you reboot a guest
> > > with 4
> > > monitors, after reboot it will only have 1 monitor.
> > > 
> > 
> > After reading this paragraph and the bug I don't see the issue.
> > I mean, if on my laptop I reboot when I reboot I get a single
> > monitor
> > till the OS and other stuff kick in. After a reboot the firmware
> > use
> > one monitor or if capable do the mirroring but always the same way.
> > 
> > I think the issue is that on first boot the guest activate the
> > additional monitors and the client do the same while on second
> > boot (reboot) not so to me looks like more an issue of the client
> > instead of the server.
> > 
> 
> Well, it could be considered a client issue to some extent, but not
> an
> easy one to fix. 
> 
> > I would try to get a network capture to look at DisplayChannel
> > messages if they are more or less the same for first boot and
> > reboot. If they are the same I would look at the client state
> > to check that while booting it's the same.
> 
> The initial boot and the reboot are the same, and that's basically
> why
> the problem exists. At initial boot, it brings up a single display.
> And
> on reboot, it also brings up a single display. The issue is that we
> want the reboot to behave differently.
> 
> Initial boot:
> -
>  - In early boot, the server sends monitors config message when
> primary
> surface is created. monitors=1, max-monitors=1
>  - client only shows a single window, disables menu items for
> enabling
> additional displays because the guest doesn't support more than 1
>  - When QXL driver is loaded and takes over, it takes over sending
> monitors config messages. monitors=1, max-monitors=4
>  - client still shows a single window, but now the menu items for
> enabling additional menus are enabled
>  - client enables a second monitor. Client sends monitors-config
> request to server 
>  - QXL driver enables the second monitor and sends a new monitors-
> config response to the client. monitors = 2, max-monitors=4
> 
> Reboot:
> ---
>  - At some point in the reboot process while the QXL driver is still
> loaded, it disables all but the first monitor, but still claims to
> support more than one. monitors=1, max-monitors=4
>  - client keeps both display windows open, but the second one just
> says
> "Waiting for display 2..."
>  - eventually the QXL driver gets unloaded, and the
> RedWorker::driver_cap_monitors_config flag gets set to false
>  - The next time a primary surface is created (during early boot?)
> the
> server sends out a new monitors config message to match the size of
> the
> primary surface. monitors=1, max-monitors=1
>  - the client sees that the guest only supports a single monitor, so
> it
> closes that inactive second display window
>  - when the qxl driver is loaded and takes over, it takes over
> sending
> monitors-config messages. monitor

Re: [Spice-devel] [PATCH] Don't close all but one display during reboot.

2017-02-20 Thread Christophe de Dinechin

Is it possible to make the max number of monitors something persistent (e.g. 
set / get that using libvirt), so that the behavior would be consistent between 
a first boot and a reboot?

Christophe

> On 20 Feb 2017, at 15:49, Jonathon Jongsma  wrote:
> 
> On Thu, 2017-02-02 at 10:30 -0600, Jonathon Jongsma wrote:
>> On Thu, 2017-02-02 at 05:29 -0500, Frediano Ziglio wrote:

 When a guest is rebooted, the QXL driver gets unloaded at some
 point in
 the reboot process. When the driver is unloaded, the spice server
 sets a
 single flag to FALSE: RedWorker::driver_cap_monitors_config. This
 flag
 indicates whether the driver is capable of sending its own
 monitors
 config
 messages to the client.

 The only place this flag is used is when a new primary surface is
 created. If
 this flag is true, the server assumes that the driver will send
 its
 own
 monitors config very soon after the surface is created. If it's
 false, the
 server directly sends its own temporary monitors config message
 to
 the client
 since the driver cannot.  This temporary monitors config message
 is
 based on
 the size of the just-created primary surface and always has a
 maximum monitor
 count of 1.

 This flag is initialized to false at startup so that in early
 boot
 (e.g. vga
 text mode), the server will send out these 'temporary' monitor
 config
 messages
 to the client whenever the primary surface is destroyed and re-
 created. This
 causes the client to resize its window to match the size of the
 guest. When
 the
 QXL driver is eventually loaded and starts taking over the
 monitors
 config
 responsibilities, we set this flag to true and the server stops
 sending
 monitors config messages out on its own.

 If we reboot and set this flag to false, it will result in the
 server sending
 a
 monitors config message to the client indicating that the guest
 now
 supports
 a
 maximum of 1 monitor. If the guest happens to have more than one
 display
 window
 open, it will destroy those extra windows because they exceed the
 maximum
 allowed number of monitors. This means that if you reboot a guest
 with 4
 monitors, after reboot it will only have 1 monitor.

>>> 
>>> After reading this paragraph and the bug I don't see the issue.
>>> I mean, if on my laptop I reboot when I reboot I get a single
>>> monitor
>>> till the OS and other stuff kick in. After a reboot the firmware
>>> use
>>> one monitor or if capable do the mirroring but always the same way.
>>> 
>>> I think the issue is that on first boot the guest activate the
>>> additional monitors and the client do the same while on second
>>> boot (reboot) not so to me looks like more an issue of the client
>>> instead of the server.
>>> 
>> 
>> Well, it could be considered a client issue to some extent, but not
>> an
>> easy one to fix. 
>> 
>>> I would try to get a network capture to look at DisplayChannel
>>> messages if they are more or less the same for first boot and
>>> reboot. If they are the same I would look at the client state
>>> to check that while booting it's the same.
>> 
>> The initial boot and the reboot are the same, and that's basically
>> why
>> the problem exists. At initial boot, it brings up a single display.
>> And
>> on reboot, it also brings up a single display. The issue is that we
>> want the reboot to behave differently.
>> 
>> Initial boot:
>> -
>>  - In early boot, the server sends monitors config message when
>> primary
>> surface is created. monitors=1, max-monitors=1
>>  - client only shows a single window, disables menu items for
>> enabling
>> additional displays because the guest doesn't support more than 1
>>  - When QXL driver is loaded and takes over, it takes over sending
>> monitors config messages. monitors=1, max-monitors=4
>>  - client still shows a single window, but now the menu items for
>> enabling additional menus are enabled
>>  - client enables a second monitor. Client sends monitors-config
>> request to server 
>>  - QXL driver enables the second monitor and sends a new monitors-
>> config response to the client. monitors = 2, max-monitors=4
>> 
>> Reboot:
>> ---
>>  - At some point in the reboot process while the QXL driver is still
>> loaded, it disables all but the first monitor, but still claims to
>> support more than one. monitors=1, max-monitors=4
>>  - client keeps both display windows open, but the second one just
>> says
>> "Waiting for display 2..."
>>  - eventually the QXL driver gets unloaded, and the
>> RedWorker::driver_cap_monitors_config flag gets set to false
>>  - The next time a primary surface is created (during early boot?)
>> the
>> server sends out a new monitors config message to match the size of
>> the
>> primary surface. monitors=1, max-mo

Re: [Spice-devel] [PATCH v2 spice-gtk 1/2] authentication: Handle failed SASL authentication separately

2017-02-20 Thread Christophe de Dinechin


> On 19 Feb 2017, at 15:47, Snir Sheriber  wrote:
> 
> Remove handling with failures in the SASL authentication
> process to separate function
> ---
> src/spice-channel.c | 44 +++-
> 1 file changed, 27 insertions(+), 17 deletions(-)
> 
> diff --git a/src/spice-channel.c b/src/spice-channel.c
> index af67931..cbf1291 100644
> --- a/src/spice-channel.c
> +++ b/src/spice-channel.c
> @@ -1113,28 +1113,38 @@ static int spice_channel_read(SpiceChannel *channel, 
> void *data, size_t length)
> return length;
> }
> 
> +#if HAVE_SASL
> /* coroutine context */
> -static void spice_channel_failed_authentication(SpiceChannel *channel,
> -gboolean invalidPassword)
> +static void spice_channel_failed_sasl_authentication(SpiceChannel *channel)
> {
> SpiceChannelPrivate *c = channel->priv;
> +gint err_code; /* Affects the authentication window activated fileds */
> 
> if (c->auth_needs_username && c->auth_needs_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"));
> +err_code = SPICE_CLIENT_ERROR_AUTH_NEEDS_PASSWORD_AND_USERNAME;
> else if (c->auth_needs_username)
> -g_set_error_literal(&c->error,
> -SPICE_CLIENT_ERROR,
> -SPICE_CLIENT_ERROR_AUTH_NEEDS_USERNAME,
> -_("Authentication failed: username is 
> required"));
> -else if (c->auth_needs_password)
> -g_set_error_literal(&c->error,
> -SPICE_CLIENT_ERROR,
> -SPICE_CLIENT_ERROR_AUTH_NEEDS_PASSWORD,
> -_("Authentication failed: password is 
> required"));
> -else if (invalidPassword)
> +err_code = SPICE_CLIENT_ERROR_AUTH_NEEDS_USERNAME;
> +else
> +err_code = SPICE_CLIENT_ERROR_AUTH_NEEDS_PASSWORD;
> +
> +g_set_error_literal(&c->error,
> +SPICE_CLIENT_ERROR,
> +err_code,
> +_("SASL authentication failed"));

Per the recent discussion (Feb 14 with Christophe F), can’t we map common SASL 
errors to Spice messages? To me, it’s different if the problem is that I used a 
wrong password or if the server is down. The message as is seems quite terse.

Errors that seem be reportable (although not all of them seem relevant to 
Spice):

SASL_BADAUTHAuthentication failure.
SASL_NOAUTHZAuthorization failure.
SASL_EXPIREDThe passphrase expired and must be reset.
SASL_DISABLED   Account disabled.
SASL_NOUSER User not found.
SASL_BADVERSVersion mismatch with plug-in.
SASL_NOVERIFY   The user exists, but there is no verifier for the user.
SASL_WEAKPASS   The passphrase is too weak for security policy.
SASL_NOUSERPASS User supplied passwords are not permitted.


Some that may need to be “translated” in Spicese if they ever get back to us:

SASL_TOOWEAKThe mechanism is too weak for this user.
SASL_ENCRYPTEncryption is needed to use this mechanism.
SASL_TRANS  One time use of a plaintext password will enable 
requested mechanism for user.

Others should probably collected into a “default” in a switch statement, 
something like “Unexpected SASL error code ”.


> +
> +c->event = SPICE_CHANNEL_ERROR_AUTH;
> +
> +c->has_error = TRUE; /* force disconnect */
> +}
> +#endif
> +
> +/* coroutine context */
> +static void spice_channel_failed_authentication(SpiceChannel *channel,
> +gboolean invalidPassword)
> +{
> +SpiceChannelPrivate *c = channel->priv;
> +
> +if (invalidPassword)
> g_set_error_literal(&c->error,
> SPICE_CLIENT_ERROR,
> SPICE_CLIENT_ERROR_AUTH_NEEDS_PASSWORD,
> @@ -1808,7 +1818,7 @@ error:
> if (saslconn)
> sasl_dispose(&saslconn);
> 
> -spice_channel_failed_authentication(channel, FALSE);
> +spice_channel_failed_sasl_authentication(channel);
> ret = FALSE;
> 
> cleanup:
> -- 
> 2.9.3
> 
> ___
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel

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

[Spice-devel] Remote control delay when I access to windows7 kvm virtual machine with spice

2017-02-20 Thread John Y.

I had a kvm virtual machine with windows 7 which is created by qemu2.5 and
spice-server-1.8.0.
I used spice to remote access the vm, but I felled lag sometime.
I opened the notepad and typed some words, but words displayed after 0.5-1
seconds. I can fell the delay obviously.

The vm is on a centos server and ip is 192.168.11.10.
My client is on ubuntu and the statistics of ping:

--— 192.168.11.10 ping statistics ---
213 packets transmitted, 213 received, 0% packet loss, time 212345ms
rtt min/avg/max/mdev = 0.294/6.786/17.672/5.125 ms


How can I get more information to solve this problem ?


Here are the information:

qemu version:
QEMU emulator version 2.5.1.1, Copyright (c) 2003-2008 Fabrice Bellard

uname -a
Linux server01 4.2.0-1.el7.elrepo.x86_64 #1 SMP Sun Aug 30 21:25:29 EDT
2015 x86_64 x86_64 x86_64 GNU/Linux

libvirt.xml:
http://libvirt.org/schemas/domain/qemu/1.0"; type="kvm">

  win7_001
  
hvm
  
  



  
  
  destroy
  restart
  destroy
  2
  

  
  4194304
  4194304
  
/root/qemu25/x86_64-softmmu/qemu-system-x86_64

  
  


  
  


  
  




  


  
  


  
  


  
  


  


  


  


  




  


  
  


  
  
  

  
  


  



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

Re: [Spice-devel] Remote control delay when I access to windows7 kvm virtual machine with spice

2017-02-20 Thread Pavel Grunt

Hello John,

On Tue, 2017-02-21 at 10:28 +0800, John Y. wrote:
> I had a kvm virtual machine with windows 7 which is created by
> qemu2.5 and spice-server-1.8.0.
do you mean spice-server 0.12.8 ?

> I used spice to remote access the vm, but I felled lag sometime.
> I opened the notepad and typed some words, but words displayed after
> 0.5-1 seconds. I can fell the delay obviously.
> 
> The vm is on a centos server and ip is 192.168.11.10.
> My client is on ubuntu and the statistics of ping:
> 
> --— 192.168.11.10 ping statistics ---
> 213 packets transmitted, 213 received, 0% packet loss, time 212345ms
> rtt min/avg/max/mdev = 0.294/6.786/17.672/5.125 ms 
> 
> 
> How can I get more information to solve this problem ?
> 
Let us know if adding the spice agent
https://www.spice-space.org/spice-user-manual.html#agent

and installing spice guest tools 
https://www.spice-space.org/download.html#windows-binaries

changes anything.

Thanks,
Pavel

> 
> Here are the information:
> 
> qemu version:
> QEMU emulator version 2.5.1.1, Copyright (c) 2003-2008 Fabrice
> Bellard
> 
> uname -a
> Linux server01 4.2.0-1.el7.elrepo.x86_64 #1 SMP Sun Aug 30 21:25:29
> EDT 2015 x86_64 x86_64 x86_64 GNU/Linux
> 
> libvirt.xml:
> http://libvirt.org/schemas/domain/qemu/1.0";
> type="kvm">  
>   win7_001     
>    
>     hvm 
>     
>    
>       
>       
>      
>    
>     
>   destroy  
>   restart  
>   destroy  
>   2  
>    
>      
>   
>   4194304  
>   4194304  
>    
>     /root/qemu25/x86_64-softmmu/qemu-system-
> x86_64  
>      
>         
>        
>       
>      
>         
>        
>       
>      
>         
>        
>       
>       
>       
>      
>        
>       
>      
>         
>        
>       
>      
>         
>        
>       
>      
>         
>        
>       
>      
>        
>       
>      
>        
>       
>      
>        
>       
>      
>        
>       
>      keymap='en-us'/>
>       
>      
>        
>       
>      
>         
>        
>       
>     
>       
>       
>       
>     
>     
>    
>       
>      
>    
> 
> 
> 
> Regards
> John.
> 
> ___
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] Remote control delay when I access to windows7 kvm virtual machine with spice

2017-02-20 Thread John Y.

spice-server is 0.12.4-20

I did not add the spice agent.
Can spice agent provide better VGA performance?
I though it just provide data transferring.

I had install spice guest tools.

Regards,
John


2017-02-21 14:29 GMT+08:00 Pavel Grunt :

> Hello John,
>
> On Tue, 2017-02-21 at 10:28 +0800, John Y. wrote:
> > I had a kvm virtual machine with windows 7 which is created by
> > qemu2.5 and spice-server-1.8.0.
> do you mean spice-server 0.12.8 ?
>
> > I used spice to remote access the vm, but I felled lag sometime.
> > I opened the notepad and typed some words, but words displayed after
> > 0.5-1 seconds. I can fell the delay obviously.
> >
> > The vm is on a centos server and ip is 192.168.11.10.
> > My client is on ubuntu and the statistics of ping:
> >
> > --— 192.168.11.10 ping statistics ---
> > 213 packets transmitted, 213 received, 0% packet loss, time 212345ms
> > rtt min/avg/max/mdev = 0.294/6.786/17.672/5.125 ms
> >
> >
> > How can I get more information to solve this problem ?
> >
> Let us know if adding the spice agent
> https://www.spice-space.org/spice-user-manual.html#agent
>
> and installing spice guest tools
> https://www.spice-space.org/download.html#windows-binaries
>
> changes anything.
>
> Thanks,
> Pavel
>
> >
> > Here are the information:
> >
> > qemu version:
> > QEMU emulator version 2.5.1.1, Copyright (c) 2003-2008 Fabrice
> > Bellard
> >
> > uname -a
> > Linux server01 4.2.0-1.el7.elrepo.x86_64 #1 SMP Sun Aug 30 21:25:29
> > EDT 2015 x86_64 x86_64 x86_64 GNU/Linux
> >
> > libvirt.xml:
> > http://libvirt.org/schemas/domain/qemu/1.0";
> > type="kvm">
> >   win7_001
> >   
> > hvm
> >   
> >   
> > 
> > 
> > 
> >   
> >   
> >   destroy
> >   restart
> >   destroy
> >   2
> >   
> > 
> >   
> >   4194304
> >   4194304
> >   
> > /root/qemu25/x86_64-softmmu/qemu-system-
> > x86_64
> > 
> >   
> >   
> > 
> > 
> >   
> >   
> > 
> > 
> >   
> >   
> > 
> > 
> > 
> > 
> >   
> > 
> > 
> >   
> >   
> > 
> > 
> >   
> >   
> > 
> > 
> >   
> >   
> > 
> > 
> >   
> > 
> > 
> >   
> > 
> > 
> >   
> > 
> > 
> >   
> > 
> >  > keymap='en-us'/>
> > 
> > 
> >   
> > 
> > 
> >   
> >   
> > 
> > 
> >   
> >   
> >   
> > 
> >   
> >   
> > 
> > 
> >   
> > 
> >
> >
> > Regards
> > John.
> >
> > ___
> > Spice-devel mailing list
> > Spice-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/spice-devel
>
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] Remote control delay when I access to windows7 kvm virtual machine with spice

2017-02-20 Thread Pavel Grunt

On Tue, 2017-02-21 at 15:10 +0800, John Y. wrote:
> spice-server is 0.12.4-20
> 
> I did not add the spice agent.
> Can spice agent provide better VGA performance? 

It will give you the arbitrary resolution feature (guest will change
its resolution to match the remote-viewer's window size)

> I though it just provide data transferring. 
> 
Also copy & paste between the client and the guest, it allows to
enable the multimonitor support

> I had install spice guest tools.

This may have an effect on performance since it installs some drivers

Pavel

> 
> Regards,
> John
> 
> 
> 2017-02-21 14:29 GMT+08:00 Pavel Grunt :
> > Hello John,
> > 
> > On Tue, 2017-02-21 at 10:28 +0800, John Y. wrote:
> > > I had a kvm virtual machine with windows 7 which is created by
> > > qemu2.5 and spice-server-1.8.0.
> > do you mean spice-server 0.12.8 ?
> > 
> > > I used spice to remote access the vm, but I felled lag sometime.
> > > I opened the notepad and typed some words, but words displayed
> > after
> > > 0.5-1 seconds. I can fell the delay obviously.
> > >
> > > The vm is on a centos server and ip is 192.168.11.10.
> > > My client is on ubuntu and the statistics of ping:
> > >
> > > --— 192.168.11.10 ping statistics ---
> > > 213 packets transmitted, 213 received, 0% packet loss, time
> > 212345ms
> > > rtt min/avg/max/mdev = 0.294/6.786/17.672/5.125 ms 
> > >
> > >
> > > How can I get more information to solve this problem ?
> > >
> > Let us know if adding the spice agent
> > https://www.spice-space.org/spice-user-manual.html#agent
> > 
> > and installing spice guest tools
> > https://www.spice-space.org/download.html#windows-binaries
> > 
> > changes anything.
> > 
> > Thanks,
> > Pavel
> > 
> > >
> > > Here are the information:
> > >
> > > qemu version:
> > > QEMU emulator version 2.5.1.1, Copyright (c) 2003-2008 Fabrice
> > > Bellard
> > >
> > > uname -a
> > > Linux server01 4.2.0-1.el7.elrepo.x86_64 #1 SMP Sun Aug 30
> > 21:25:29
> > > EDT 2015 x86_64 x86_64 x86_64 GNU/Linux
> > >
> > > libvirt.xml:
> > > http://libvirt.org/schemas/domain/qemu/1.0";
> > > type="kvm">  
> > >   win7_001     
> > >    
> > >     hvm 
> > >     
> > >    
> > >       
> > >       
> > >      
> > >    
> > >     
> > >   destroy  
> > >   restart  
> > >   destroy  
> > >   2  
> > >    
> > >      
> > >   
> > >   4194304  
> > >   4194304  
> > >    
> > >     /root/qemu25/x86_64-softmmu/qemu-system-
> > > x86_64  
> > >      
> > >         
> > >        
> > >       
> > >      
> > >         
> > >        
> > >       
> > >      
> > >         
> > >        
> > >       
> > >       
> > >       
> > >      
> > >        
> > >       
> > >      
> > >         
> > >        
> > >       
> > >      
> > >         
> > >        
> > >       
> > >      
> > >         
> > >        
> > >       
> > >      
> > >        
> > >       
> > >      
> > >        
> > >       
> > >      
> > >        
> > >       
> > >      
> > >        
> > >       
> > >      > > keymap='en-us'/>
> > >       
> > >      
> > >        
> > >       
> > >      
> > >         
> > >        
> > >       
> > >     
> > >       
> > >       
> > >       
> > >     
> > >     
> > >    
> > >       
> > >      
> > >    
> > > 
> > >
> > >
> > > Regards
> > > John.
> > >
> > > ___
> > > Spice-devel mailing list
> > > Spice-devel@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/spice-devel
> > 
> 
> 
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] Spice protocol behind a Firewall

Re: [Spice-devel] [PATCH spice-gtk] gtk-session: Set value directly

Re: [Spice-devel] [spice v8] dcc: handle preferred video codec message

Re: [Spice-devel] bool or gboolean

Re: [Spice-devel] [spice v8] dcc: handle preferred video codec message

Re: [Spice-devel] [vdagent-linux v8 0/4]

Re: [Spice-devel] [spice v8] dcc: handle preferred video codec message

Re: [Spice-devel] [PATCH spice-gtk v5 6/6] usb-device-widget: Migrate to GtkGrid from GtkBox

Re: [Spice-devel] [spice v8] dcc: handle preferred video codec message

Re: [Spice-devel] [PATCH spice-gtk v5 6/6] usb-device-widget: Migrate to GtkGrid from GtkBox

Re: [Spice-devel] [PATCH spice-gtk v5 6/6] usb-device-widget: Migrate to GtkGrid from GtkBox

Re: [Spice-devel] [spice v8] dcc: handle preferred video codec message

Re: [Spice-devel] [PATCH spice-gtk v5 6/6] usb-device-widget: Migrate to GtkGrid from GtkBox

Re: [Spice-devel] [PATCH spice-server 3/3] Add message counters to statistics

[Spice-devel] [spice-gtk v1] gtk-session: Use GWeakRef

Re: [Spice-devel] [PATCH] Don't close all but one display during reboot.

Re: [Spice-devel] [PATCH] Don't close all but one display during reboot.

Re: [Spice-devel] [PATCH v2 spice-gtk 1/2] authentication: Handle failed SASL authentication separately

[Spice-devel] Remote control delay when I access to windows7 kvm virtual machine with spice

Re: [Spice-devel] Remote control delay when I access to windows7 kvm virtual machine with spice

Re: [Spice-devel] Remote control delay when I access to windows7 kvm virtual machine with spice

Re: [Spice-devel] Remote control delay when I access to windows7 kvm virtual machine with spice

22 matches

Site Navigation

Mail list logo

Footer information