date:20130624

On Mon, Jun 24, 2013 at 11:27:54AM +0200, Christophe Fergeau wrote:
 -Wportability used to be automatically disabled with automake = 1.12 when 
 using
 silent rules, but this is no longer the case with automake 1.13 which is what
 fedora 19 uses: 
 http://www.flameeyes.eu/autotools-mythbuster/automake/silent.html
 As of version 1.13, though, this opt-in is no longer necessary, as all the
 Makefiles are generated to support them. The silent-rules option is now a
 no-op, doing nothing at all, in particular not silencing the portability
 warnings.
 
 This commit disables these warnings in order to avoid autogen.sh breakage
 because of the use of -Werror, they can be reenabled once gtk-doc.make is
 fixed to avoid these portability warnings.
 ---
  configure.ac | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
 
 diff --git a/configure.ac b/configure.ac
 index 8ab5b6b..bd59ccf 100644
 --- a/configure.ac
 +++ b/configure.ac
 @@ -7,7 +7,7 @@ AC_CONFIG_MACRO_DIR([m4])
  AC_CONFIG_HEADER([config.h])
  AC_CONFIG_AUX_DIR([build-aux])
  
 -AM_INIT_AUTOMAKE([foreign dist-bzip2 -Wall -Werror])
 +AM_INIT_AUTOMAKE([foreign dist-bzip2 -Wall -Werror -Wno-portability])
  m4_ifdef([AM_PROG_AR], [AM_PROG_AR])
  LT_INIT
  AM_MAINTAINER_MODE

ACK

Use of -Wno-portability is pretty standard practice

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

[Spice-devel] Can we directly redir usb hub?

2013-06-24 Thread 邵鄭叡

Hi all,
maybe I should post this message to [usbredir and qemu devel list] ,
but I just didnt find it :)

By default, spice-gtk filters out 'usb hubs' from redir list, and only show
'non-hub non-interface' usb devs.

But sometime, we want to just simply pass a usb-hub to guest , and dont
care about whatever is plugged on it.

So, I tweaked spice-gtk a bit  like this:

static void spice_usb_device_manager_add_dev(SpiceUsbDeviceManager  *self,
 GUdevDevice*udev)
{
...
/* Skip hubs */
devclass = g_udev_device_get_sysfs_attr(udev, bDeviceClass);
if (!devclass || !strcmp(devclass, 09))
{
-   return;
+  printf( Yes, it's a hub, and we want it :) \n  );
...
}

Then, something interesting happened after I redired a usb-hub with a
usb-disk plugged on it.
1. The hub is successfully redired into guest.
I can see it in 'Window Device Manager' , it is correctly recognized as
'usb hub' with right usb transfer speed (in my case , 480M).

2. The usb-disk is also detected by windows, and addressed under the hub.
   But we are not lucky enough to see it as 'usb mass storage', but as
'unknown device'.

3. If I redir the usb-disk rather than the hub, that's OK.

my env:
qemu 1.3.1 ( kvm is enabled )
libusb 1.0.9
libusbredir 0.6
spice-gtk 0.19
spice   0.12.3

So my question is , can we directly redir usb hub to guest?
If not, what hinder us to accomplish this?
Maybe I can modify qemu/libusb/... to enable this ,
but now I need your hint and idea :)

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

Re: [Spice-devel] [Engine-devel] SPICE Foreign Menu Using REST

2013-06-24 Thread Christophe Fergeau

On Mon, Jun 24, 2013 at 10:02:29AM +0200, Michal Skrivanek wrote:
 I would agree as long as it is not hardcoded and can be change in some
 config file easily. We shouldn't wait for next RHEL release to do a
 little modification if possible.
 I think owning the menu in some form on RHEV-M side has an advantage of
 delivery synchronization.

This probably would also make things harder from a client documentation
point of view. With something too generic, there's also always the risk
that it becomes a way to add workaround for bugs, or that the menu starts
containing things that would be better integrated in other places in the
client, ...

Christophe


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

Re: [Spice-devel] [Users] Cannot connect to VM via browser if engine was not in /etc/hosts

2013-06-24 Thread David Jaša

Hi,

So you're connecting via User Portal but then it doesn't work? If it
doesn't, either you hit a bug or you've tweaked some value that affects
things...

In general, TLS shouldn't pose a problem because:
1) ovirt sets up its own CA that issues certificates for the hosts
2) the CA certificate and respective host certificate subject are passed to the 
client
3) the client can verify the host using these information even in cases when 
connection IP/FQDN doesn't match CN in subject of server certificate

The only condition that indeed breaks it should be display network
address override _when migrating the VM_ (because then the connection
data are passed via the host and libvirt doesn't allow to pass the
arbitrary IP/FQDN yet)

David

PS: Itamar, advice to disable SSL/TLS is IMO bad, bad thing. ;)


Itamar Heim píše v Po 24. 06. 2013 v 08:55 +0300:
 On 06/24/2013 03:10 AM, lofyer wrote:
  于 2013/6/24 1:47, Itamar Heim 写道:
  On 06/06/2013 11:51 AM, lof yer wrote:
  I connect https://192.168.1.111 and connect to the VM, then the
  remote-viewer shows up, but failed to show the VM desktop.
  Is it the https problem?
  Can I connect to the VM without modify /etc/hosts?
 
 
  ___
  Users mailing list
  us...@ovirt.org
  http://lists.ovirt.org/mailman/listinfo/users
 
 
 
  was this resolved? sounds like a certificate/dns issue?
  Yes, it's certificate/dns problem.
  But how can I connect via IP instead of FQDN without https?
 
 i guess it depends if you can tell spice client to not validate the ssl 
 certificate.
 ___
 Users mailing list
 us...@ovirt.org
 http://lists.ovirt.org/mailman/listinfo/users

-- 

David Jaša, RHCE

SPICE QE based in Brno
GPG Key: 22C33E24 
Fingerprint: 513A 060B D1B4 2A72 7F0D 0278 B125 CD00 22C3 3E24




smime.p7s
Description: S/MIME cryptographic signature
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] Questions regarding Bug 62033 - Means to detect local-only

2013-06-24 Thread David Jaša

Hi Fedor,

I'd personally prefer to:
1) monitor bandwidth and latency continuously - there's a long-standing RFE for 
it and IIRC Yonit has posted some proof-of-concept patches in recent months (as 
a part of her streamlining of video streams)
2) set the options on-the fly as the conditions allow to get at the right 
position in b/w-saving --- cpu saving scale

The reason for this preference of mine is two-fold:
* idle gigabit or 100Mbps LAN may be closer to localhost behaviour than WAN 
behaviour, especially with decreasing stream resolution
* such behaviour would be consistent with the rest of the spice protocol

I'm also not sure if I follow what you want to do with an agent:
1) the agent runs in the guest OS and communicates already with the client 
through using spice means - i.e. agent - virtio-serial/qemu - spice-server - 
spice client. You shouldn't need to invent any other client -- agent 
connection, and I can't get how such thing should help you...
2) agent has no connection of what happens with display, and it has a limited 
knowledge of network conditions as it handles just a subset of all the traffic. 
spice-server and spice client actually do have complete picture of what happens 
on the wire

David


Fedor Lyakhov píše v Ne 23. 06. 2013 v 02:26 +0400:
 Hello Zeeshan, Marc-André and other Spice developers,
 
 I think about implementing a solution for this bug 62033 (
 https://bugs.freedesktop.org/show_bug.cgi?id=62033).
 
 First of all, need to clarify what is requested and what is really needed
 in this request. Zeeshan suggests as a simple yet acceptable solution for
 vdagent to report 'local-only' connection non-dynamically.. as I understand
 this means reporting only once, at startup (?). I think it isn't enough
 because this connection state may change easily during X session lifetime -
 in general user is free to connect locally and then re-connect from remote
 host, and vise versa. So I propose more generic approach, when vdagent
 reports current connection state dynamically when state is changed. It will
 be up to gnome-settings-daemon to act on this signal and toggle animations
 accordingly.
 
 If this is acceptable, we need to decide on the interface. I think there
 are 2 appropriate technologies:
 1. raw socket [or something like this]
 2. D-Bus
 
 I'm not an expert in D-Bus and freedesktop in general (good opportunity for
 me to learn!) but looks like it is appropriate to use it here. 2 questions
 arise:
 1. vdagentd and vdagent daemons already communicate via socket using simple
 homebrew protocol (at least afaik). Probably this was done on purpose. So
 is it true vdagent wants to avoid dependency on D-Bus?
 2. It seems to better use some higher-level D-Bus bindings (GLib, Qt...)
 instead of its low-level API. So is it acceptable to add a dependency of
 GLib to vdagent? Or are you aware of some other D-Bus bindings to be used
 in vdagent to avoid this extra dependency?
 
 BTW, reporting of connection state can be implemented independently of the
 actual transport (socket/D-Bus):
 1. Client app (e.g. gnome-settings-daemon) establishes connection to
 vdagent being a server (e.g. listening at the socket, or providing D-Bus
 interface method RegisterConnectionStateObserver or w/e we name it)
 2. vdagent sends the state to registered clients (e.g. D-Bus signal); and
 continues to do so each time the state is changed.
 So we can even have 2 back-ends for this (additional work, unfortunately).
 
 And last question: while this isn't needed on Windows version of vdagent
 right now, additional use cases of this vdagent interface in guest
 userspace may arise [e.g. I'm thinking about remote media engine provided
 by spice client]. So I think we should target for similar extendable
 interface at both Windows and Linux guests (as much as possible). I haven't
 looked in vdagent for Windows code yet but guess D-Bus won't be the best
 choice there (while D-Bus reportedly works on Windows as well...).
 Something like socket and D-Bus and native DCOM would be the best and
 future-proof.. What do you think?
 
 [Note: I haven't talked about implementation of new messages between
 vdagent and vdagentd, and spice-server, and the actual local/remote
 detection code - will ask questions later]
 
 ___
 Spice-devel mailing list
 Spice-devel@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/spice-devel

-- 

David Jaša, RHCE

SPICE QE based in Brno
GPG Key: 22C33E24 
Fingerprint: 513A 060B D1B4 2A72 7F0D 0278 B125 CD00 22C3 3E24




smime.p7s
Description: S/MIME cryptographic signature
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel

[Spice-devel] [PATCH spice-gtk] channel-main: Convert text line-endings if necessary (rhbz#752350)

Signed-off-by: Hans de Goede hdego...@redhat.com
---
 gtk/channel-main.c | 73 +++---
 1 file changed, 69 insertions(+), 4 deletions(-)

diff --git a/gtk/channel-main.c b/gtk/channel-main.c
index b58af52..b9e0da2 100644
--- a/gtk/channel-main.c
+++ b/gtk/channel-main.c
@@ -1181,6 +1181,24 @@ static void agent_announce_caps(SpiceMainChannel 
*channel)
 #define HAS_CLIPBOARD_SELECTION(c) \
 VD_AGENT_HAS_CAPABILITY((c)-agent_caps, G_N_ELEMENTS((c)-agent_caps), 
VD_AGENT_CAP_CLIPBOARD_SELECTION)
 
+#define GUEST_LINEEND_LF(c) \
+VD_AGENT_HAS_CAPABILITY((c)-agent_caps, G_N_ELEMENTS((c)-agent_caps), 
VD_AGENT_CAP_GUEST_LINEEND_LF)
+
+#define GUEST_LINEEND_CRLF(c) \
+VD_AGENT_HAS_CAPABILITY((c)-agent_caps, G_N_ELEMENTS((c)-agent_caps), 
VD_AGENT_CAP_GUEST_LINEEND_CRLF)
+
+#ifdef G_OS_UNIX
+#define CLIENT_LINEEND_LF 1
+#else
+#define CLIENT_LINEEND_LF 0
+#endif
+
+#ifdef G_OS_WIN32
+#define CLIENT_LINEEND_CRLF 1
+#else
+#define CLIENT_LINEEND_CRLF 0
+#endif
+
 /* any context: the message is not flushed immediately,
you can wakeup() the channel coroutine or send_msg_queue() */
 static void agent_clipboard_grab(SpiceMainChannel *channel, guint selection,
@@ -1751,6 +1769,29 @@ static void file_xfer_handle_status(SpiceMainChannel 
*channel,
 file_xfer_completed(task, error);
 }
 
+/* any context */
+static guchar *convert_lineend(const guchar *in, gsize *size,
+   const gchar *from, const gchar *to)
+{
+gchar *nul_terminated, **split, *out;
+
+/* Nul-terminate */
+nul_terminated = g_malloc(*size + 1);
+memcpy(nul_terminated, in, *size);
+nul_terminated[*size] = 0;
+
+/* Convert */
+split = g_strsplit(nul_terminated, from, -1);
+out = g_strjoinv(to, split);
+*size = strlen(out);
+
+/* Clean-up */
+g_strfreev(split);
+g_free(nul_terminated);
+
+return (guchar *)out;
+}
+
 /* coroutine context */
 static void main_agent_handle_msg(SpiceChannel *channel,
   VDAgentMessage *msg, gpointer payload)
@@ -1809,12 +1850,22 @@ static void main_agent_handle_msg(SpiceChannel *channel,
 case VD_AGENT_CLIPBOARD:
 {
 VDAgentClipboard *cb = payload;
+guchar *data = cb-data;
+gsize size = msg-size - sizeof(VDAgentClipboard);
+if (cb-type == VD_AGENT_CLIPBOARD_UTF8_TEXT) {
+if (GUEST_LINEEND_LF(c)  CLIENT_LINEEND_CRLF)
+data = convert_lineend(data, size, \n, \r\n);
+if (GUEST_LINEEND_CRLF(c)  CLIENT_LINEEND_LF)
+data = convert_lineend(data, size, \r\n, \n);
+}
 emit_main_context(channel, SPICE_MAIN_CLIPBOARD_SELECTION, selection,
-  cb-type, cb-data, msg-size - 
sizeof(VDAgentClipboard));
+  cb-type, data, size);
 
-   if (selection == VD_AGENT_CLIPBOARD_SELECTION_CLIPBOARD)
+if (selection == VD_AGENT_CLIPBOARD_SELECTION_CLIPBOARD)
 emit_main_context(channel, SPICE_MAIN_CLIPBOARD,
-  cb-type, cb-data, msg-size - 
sizeof(VDAgentClipboard));
+  cb-type, data, size);
+if (data != cb-data)
+g_free(data);
 break;
 }
 case VD_AGENT_CLIPBOARD_GRAB:
@@ -2554,13 +2605,27 @@ void spice_main_clipboard_notify(SpiceMainChannel 
*channel,
  * Since: 0.6
  **/
 void spice_main_clipboard_selection_notify(SpiceMainChannel *channel, guint 
selection,
-   guint32 type, const guchar *data, 
size_t size)
+   guint32 type, const guchar *_data, 
size_t _size)
 {
+const guchar *data = _data;
+gsize size = _size;
+
 g_return_if_fail(channel != NULL);
 g_return_if_fail(SPICE_IS_MAIN_CHANNEL(channel));
 
+SpiceMainChannelPrivate *c = channel-priv;
+
+if (type == VD_AGENT_CLIPBOARD_UTF8_TEXT) {
+if (CLIENT_LINEEND_CRLF  GUEST_LINEEND_LF(c))
+data = convert_lineend(data, size, \r\n, \n);
+if (CLIENT_LINEEND_LF  GUEST_LINEEND_CRLF(c))
+data = convert_lineend(data, size, \n, \r\n);
+}
 agent_clipboard_notify(channel, selection, type, data, size);
 spice_channel_wakeup(SPICE_CHANNEL(channel), FALSE);
+
+if (data != _data)
+g_free((guchar *)data);
 }
 
 /**
-- 
1.8.2.1

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

Re: [Spice-devel] [PATCH spice-gtk] channel-main: Convert text line-endings if necessary (rhbz#752350)

On Mon, Jun 24, 2013 at 02:31:27PM +0200, Hans de Goede wrote:
 Signed-off-by: Hans de Goede hdego...@redhat.com
 ---
  gtk/channel-main.c | 73 
 +++---
  1 file changed, 69 insertions(+), 4 deletions(-)

IIUC, implicit in your patch here is that the communication protocol
between spice client  guest agent uses guest OS specific line ending
markers, and the spice client must do all conversions to guest OS
format. Is that really what we want ?

Personally I would have said that the line ending markers should be
invariant (always \n) in the comms protocol and that each end point
must handle conversion to/from \r\n if required.

But perhaps there are backwards compat issues forcing your choice
here ?

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

Re: [Spice-devel] Questions regarding Bug 62033 - Means to detect local-only

2013-06-24 Thread Fedor Lyakhov

Hi David,

Thanks for your answer. Please see my comments below.


On Mon, Jun 24, 2013 at 3:47 PM, David Jaša dj...@redhat.com wrote:

 Hi Fedor,

 I'd personally prefer to:
 1) monitor bandwidth and latency continuously - there's a long-standing
 RFE for it and IIRC Yonit has posted some proof-of-concept patches in
 recent months (as a part of her streamlining of video streams)
 2) set the options on-the fly as the conditions allow to get at the right
 position in b/w-saving --- cpu saving scale

 The reason for this preference of mine is two-fold:
 * idle gigabit or 100Mbps LAN may be closer to localhost behaviour than
 WAN behaviour, especially with decreasing stream resolution
 * such behaviour would be consistent with the rest of the spice protocol


[FL] I agree with your points. So implementation will be based on
continuous bandwidth and latency monitoring and reporting. This means we'd
need a heuristic algorithm to detect a threshold for 'bad' and 'good'
connection (and reporting the threshold has been crossed in one or another
direction).




 I'm also not sure if I follow what you want to do with an agent:
 1) the agent runs in the guest OS and communicates already with the client
 through using spice means - i.e. agent - virtio-serial/qemu - spice-server
 - spice client. You shouldn't need to invent any other client -- agent
 connection, and I can't get how such thing should help you...
 2) agent has no connection of what happens with display, and it has a
 limited knowledge of network conditions as it handles just a subset of all
 the traffic. spice-server and spice client actually do have complete
 picture of what happens on the wire


[FL] I do understand all this. Sorry for my previous bloated e-mail giving
you wrong picture of what I'm suggesting.
New functionality for vdagent is required: an interface for applications
running at guest OS. In particular, gnome-settings-daemon needs to know
whether the connection is good or bad, and toggle animations accordingly. I
see following options for interface vdagent-guest-apps:
1. D-Bus (low-level API, or GLib bindings)
2. Raw socket with custom protocol (similar to vdagent - vdagentd)

Personally I'd prefer D-Bus, since looks like this is the best interface
for freedesktop-based environments.

This is interface facing users (apps at guest); for internals I'm going to
reuse current vdagent-vdagentd communication mechanism, and vdagentd
-virtio-serial/qemu - spice-server (which will provide the actual state,
'good' or 'bad', and report the metrics probably).

If D-Bus is approved, I can think of enhancing vdagent-vdagentd
communication from socket to D-Bus as well.


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

Re: [Spice-devel] [PATCH spice-gtk] channel-main: Convert text line-endings if necessary (rhbz#752350)


Hi,

On 06/24/2013 02:39 PM, Daniel P. Berrange wrote:

On Mon, Jun 24, 2013 at 02:31:27PM +0200, Hans de Goede wrote:

Signed-off-by: Hans de Goede hdego...@redhat.com
---
  gtk/channel-main.c | 73 +++---
  1 file changed, 69 insertions(+), 4 deletions(-)


IIUC, implicit in your patch here is that the communication protocol
between spice client  guest agent uses guest OS specific line ending
markers, and the spice client must do all conversions to guest OS
format. Is that really what we want ?


Yes, this is by design. The reason for this is that there are no real
conventions as to what line-endings to use inside the clipboard data,
so ie the clipboard on unix (X) may contain text data with CRLF
line-endings. And some programs may depend on this.

So to minimize the change of breaking things we don't want to do any
conversion when doing copy and paste between a unix guest and unix client,
or a windows guest and windows client.


Personally I would have said that the line ending markers should be
invariant (always \n) in the comms protocol and that each end point
must handle conversion to/from \r\n if required.


That was my initial design too, but see above.


But perhaps there are backwards compat issues forcing your choice
here ?


Well currently we don't do any conversions, so keeping the wire
format in guest encoding makes things a bit easier wrt backward
compat handling, but we could have moved to always use \n on the
wire using guest capabilities bits, so that is not the main reason
for this.

Regards,

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

Re: [Spice-devel] [PATCH spice-gtk] channel-main: Convert text line-endings if necessary (rhbz#752350)

On Mon, Jun 24, 2013 at 02:45:25PM +0200, Hans de Goede wrote:
 Hi,
 
 On 06/24/2013 02:39 PM, Daniel P. Berrange wrote:
 On Mon, Jun 24, 2013 at 02:31:27PM +0200, Hans de Goede wrote:
 Signed-off-by: Hans de Goede hdego...@redhat.com
 ---
   gtk/channel-main.c | 73 
  +++---
   1 file changed, 69 insertions(+), 4 deletions(-)
 
 IIUC, implicit in your patch here is that the communication protocol
 between spice client  guest agent uses guest OS specific line ending
 markers, and the spice client must do all conversions to guest OS
 format. Is that really what we want ?
 
 Yes, this is by design. The reason for this is that there are no real
 conventions as to what line-endings to use inside the clipboard data,
 so ie the clipboard on unix (X) may contain text data with CRLF
 line-endings. And some programs may depend on this.
 
 So to minimize the change of breaking things we don't want to do any
 conversion when doing copy and paste between a unix guest and unix client,
 or a windows guest and windows client.
 
 Personally I would have said that the line ending markers should be
 invariant (always \n) in the comms protocol and that each end point
 must handle conversion to/from \r\n if required.
 
 That was my initial design too, but see above.
 
 But perhaps there are backwards compat issues forcing your choice
 here ?
 
 Well currently we don't do any conversions, so keeping the wire
 format in guest encoding makes things a bit easier wrt backward
 compat handling, but we could have moved to always use \n on the
 wire using guest capabilities bits, so that is not the main reason
 for this.

Ok, that makes sense.

Would be nice to describe this rationale in the commit message for
benefit of people looking back at this change years down the road.

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

Re: [Spice-devel] [PATCH spice-gtk] channel-main: Convert text line-endings if necessary (rhbz#752350)

Hi,

On 06/24/2013 03:04 PM, Daniel P. Berrange wrote:

On Mon, Jun 24, 2013 at 02:45:25PM +0200, Hans de Goede wrote:

Hi,

On 06/24/2013 02:39 PM, Daniel P. Berrange wrote:

On Mon, Jun 24, 2013 at 02:31:27PM +0200, Hans de Goede wrote:

Signed-off-by: Hans de Goede hdego...@redhat.com
---
gtk/channel-main.c | 73 +++---
1 file changed, 69 insertions(+), 4 deletions(-)

IIUC, implicit in your patch here is that the communication protocol
between spice client guest agent uses guest OS specific line ending
markers, and the spice client must do all conversions to guest OS
format. Is that really what we want ?

Yes, this is by design. The reason for this is that there are no real
conventions as to what line-endings to use inside the clipboard data,
so ie the clipboard on unix (X) may contain text data with CRLF
line-endings. And some programs may depend on this.

So to minimize the change of breaking things we don't want to do any
conversion when doing copy and paste between a unix guest and unix client,
or a windows guest and windows client.

Personally I would have said that the line ending markers should be
invariant (always \n) in the comms protocol and that each end point
must handle conversion to/from \r\n if required.

That was my initial design too, but see above.

But perhaps there are backwards compat issues forcing your choice
here ?

Well currently we don't do any conversions, so keeping the wire
format in guest encoding makes things a bit easier wrt backward
compat handling, but we could have moved to always use \n on the
wire using guest capabilities bits, so that is not the main reason
for this.

Ok, that makes sense.

Would be nice to describe this rationale in the commit message for
benefit of people looking back at this change years down the road.

It is already described in the commit message of the protocol update
for this, which is IMHO the proper place for this:
http://cgit.freedesktop.org/spice/spice-protocol/commit/?id=7be0e88e7e03a956b364cc847aad11b96ed47273

Regards,

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

Re: [Spice-devel] [PATCH spice-gtk] channel-main: Convert text line-endings if necessary (rhbz#752350)

On Mon, Jun 24, 2013 at 03:23:30PM +0200, Hans de Goede wrote:
 Hi,
 
 On 06/24/2013 03:04 PM, Daniel P. Berrange wrote:
 On Mon, Jun 24, 2013 at 02:45:25PM +0200, Hans de Goede wrote:
 Hi,
 
 On 06/24/2013 02:39 PM, Daniel P. Berrange wrote:
 On Mon, Jun 24, 2013 at 02:31:27PM +0200, Hans de Goede wrote:
 Signed-off-by: Hans de Goede hdego...@redhat.com
 ---
   gtk/channel-main.c | 73 
  +++---
   1 file changed, 69 insertions(+), 4 deletions(-)
 
 IIUC, implicit in your patch here is that the communication protocol
 between spice client  guest agent uses guest OS specific line ending
 markers, and the spice client must do all conversions to guest OS
 format. Is that really what we want ?
 
 Yes, this is by design. The reason for this is that there are no real
 conventions as to what line-endings to use inside the clipboard data,
 so ie the clipboard on unix (X) may contain text data with CRLF
 line-endings. And some programs may depend on this.
 
 So to minimize the change of breaking things we don't want to do any
 conversion when doing copy and paste between a unix guest and unix client,
 or a windows guest and windows client.
 
 Personally I would have said that the line ending markers should be
 invariant (always \n) in the comms protocol and that each end point
 must handle conversion to/from \r\n if required.
 
 That was my initial design too, but see above.
 
 But perhaps there are backwards compat issues forcing your choice
 here ?
 
 Well currently we don't do any conversions, so keeping the wire
 format in guest encoding makes things a bit easier wrt backward
 compat handling, but we could have moved to always use \n on the
 wire using guest capabilities bits, so that is not the main reason
 for this.
 
 Ok, that makes sense.
 
 Would be nice to describe this rationale in the commit message for
 benefit of people looking back at this change years down the road.
 
 It is already described in the commit message of the protocol update
 for this, which is IMHO the proper place for this:
 http://cgit.freedesktop.org/spice/spice-protocol/commit/?id=7be0e88e7e03a956b364cc847aad11b96ed47273

IMHO it should still be described here, or that commit could be
referenced here as the source of the policy being implemented.

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

Re: [Spice-devel] Questions regarding Bug 62033 - Means to detect local-only


Hi,

On 06/24/2013 02:44 PM, Fedor Lyakhov wrote:


Hi David,

Thanks for your answer. Please see my comments below.


On Mon, Jun 24, 2013 at 3:47 PM, David Jaša dj...@redhat.com 
mailto:dj...@redhat.com wrote:

Hi Fedor,

I'd personally prefer to:
1) monitor bandwidth and latency continuously - there's a long-standing RFE 
for it and IIRC Yonit has posted some proof-of-concept patches in recent months 
(as a part of her streamlining of video streams)
2) set the options on-the fly as the conditions allow to get at the right 
position in b/w-saving --- cpu saving scale

The reason for this preference of mine is two-fold:
* idle gigabit or 100Mbps LAN may be closer to localhost behaviour than WAN 
behaviour, especially with decreasing stream resolution
* such behaviour would be consistent with the rest of the spice protocol


[FL] I agree with your points. So implementation will be based on continuous 
bandwidth and latency monitoring and reporting. This means we'd need a 
heuristic algorithm to detect a threshold for 'bad' and 'good' connection (and 
reporting the threshold has been crossed in one or another direction).


Although I agree with doing continuous bandwidth + latency monitoring, this is 
something which
will not be trivial. I would prefer to see a simple local / non local 
detection too. Low hanging
fruit first.

And since in the future we hope to have 3d pass-through too, this will be 
useful for determine
settings there too.

Since we sometimes get passed file-descriptors rather then opening sockets 
our-selves, we cannot
simply check for localhost or some such for local detection. So we will likely 
need to fallback
to checking if the spice-server and spice-client share a filesystem. One 
possible way to do
this would be to create a shared memory segment with a unique name from one 
side and have the
other side also open this. See for example how pulseaudio does this.

This may seem like overkill for just local detection, but we will likely want 
a shared-memory
segment in the future anyways to use to speedup the local use case (by avoiding 
sending pixmaps
over a socket), so it seems like a good idea to create one now, even if just 
gets used for
local detection for now.



I'm also not sure if I follow what you want to do with an agent:
1) the agent runs in the guest OS and communicates already with the client 
through using spice means - i.e. agent - virtio-serial/qemu - spice-server - spice 
client. You shouldn't need to invent any other client -- agent connection, 
and I can't get how such thing should help you...
2) agent has no connection of what happens with display, and it has a 
limited knowledge of network conditions as it handles just a subset of all the 
traffic. spice-server and spice client actually do have complete picture of 
what happens on the wire


[FL] I do understand all this. Sorry for my previous bloated e-mail giving you 
wrong picture of what I'm suggesting.
New functionality for vdagent is required: an interface for applications running at 
guest OS. In particular, gnome-settings-daemon needs to know whether the connection 
is good or bad, and toggle animations accordingly. I see following options for 
interface vdagent-guest-apps:
1. D-Bus (low-level API, or GLib bindings)
2. Raw socket with custom protocol (similar to vdagent - vdagentd)

Personally I'd prefer D-Bus, since looks like this is the best interface for 
freedesktop-based environments.

This is interface facing users (apps at guest); for internals I'm going to reuse 
current vdagent-vdagentd communication mechanism, and vdagentd 
-virtio-serial/qemu - spice-server (which will provide the actual state, 'good' or 
'bad', and report the metrics probably).

If D-Bus is approved, I can think of enhancing vdagent-vdagentd communication 
from socket to D-Bus as well.


I don't think that any dbus api will be needed the agent can simply change the 
gsettings for these, and then
gnome-shell will notice the change and react accordingly, at least I assume 
this is how the control-center
settings work and get applied. Even if I'm wrong about the gsettings bit, there 
has to be an API between
the control-center and gnome-shell, and rather then inventing a new one it 
would be good for the agent to
re-use the existing one to talk to gnome-shell / gsd

Regards,

Hans

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

Re: [Spice-devel] Questions regarding Bug 62033 - Means to detect local-only

2013-06-24 Thread Fedor Lyakhov

Hans,

Thanks for the comments and clarifications! OK, I'll start with more simple
solution of 'local/remote' detection first.

As for gsettings - I see your point, this will solve the issue for Gnome3
as requested. But I think this is too limited - e.g. what if KDE5 wants
this in future?



On Mon, Jun 24, 2013 at 5:54 PM, Hans de Goede hdego...@redhat.com wrote:

 Hi,


 On 06/24/2013 02:44 PM, Fedor Lyakhov wrote:


 Hi David,

 Thanks for your answer. Please see my comments below.


 On Mon, Jun 24, 2013 at 3:47 PM, David Jaša dj...@redhat.com mailto:
 dj...@redhat.com wrote:

 Hi Fedor,

 I'd personally prefer to:
 1) monitor bandwidth and latency continuously - there's a
 long-standing RFE for it and IIRC Yonit has posted some proof-of-concept
 patches in recent months (as a part of her streamlining of video streams)
 2) set the options on-the fly as the conditions allow to get at the
 right position in b/w-saving --- cpu saving scale

 The reason for this preference of mine is two-fold:
 * idle gigabit or 100Mbps LAN may be closer to localhost behaviour
 than WAN behaviour, especially with decreasing stream resolution
 * such behaviour would be consistent with the rest of the spice
 protocol


 [FL] I agree with your points. So implementation will be based on
 continuous bandwidth and latency monitoring and reporting. This means we'd
 need a heuristic algorithm to detect a threshold for 'bad' and 'good'
 connection (and reporting the threshold has been crossed in one or another
 direction).


 Although I agree with doing continuous bandwidth + latency monitoring,
 this is something which
 will not be trivial. I would prefer to see a simple local / non local
 detection too. Low hanging
 fruit first.

 And since in the future we hope to have 3d pass-through too, this will be
 useful for determine
 settings there too.

 Since we sometimes get passed file-descriptors rather then opening sockets
 our-selves, we cannot
 simply check for localhost or some such for local detection. So we will
 likely need to fallback
 to checking if the spice-server and spice-client share a filesystem. One
 possible way to do
 this would be to create a shared memory segment with a unique name from
 one side and have the
 other side also open this. See for example how pulseaudio does this.

 This may seem like overkill for just local detection, but we will likely
 want a shared-memory
 segment in the future anyways to use to speedup the local use case (by
 avoiding sending pixmaps
 over a socket), so it seems like a good idea to create one now, even if
 just gets used for
 local detection for now.



  I'm also not sure if I follow what you want to do with an agent:
 1) the agent runs in the guest OS and communicates already with the
 client through using spice means - i.e. agent - virtio-serial/qemu -
 spice-server - spice client. You shouldn't need to invent any other client
 -- agent connection, and I can't get how such thing should help you...
 2) agent has no connection of what happens with display, and it has a
 limited knowledge of network conditions as it handles just a subset of all
 the traffic. spice-server and spice client actually do have complete
 picture of what happens on the wire


 [FL] I do understand all this. Sorry for my previous bloated e-mail
 giving you wrong picture of what I'm suggesting.
 New functionality for vdagent is required: an interface for applications
 running at guest OS. In particular, gnome-settings-daemon needs to know
 whether the connection is good or bad, and toggle animations accordingly. I
 see following options for interface vdagent-guest-apps:
 1. D-Bus (low-level API, or GLib bindings)
 2. Raw socket with custom protocol (similar to vdagent - vdagentd)

 Personally I'd prefer D-Bus, since looks like this is the best interface
 for freedesktop-based environments.

 This is interface facing users (apps at guest); for internals I'm going
 to reuse current vdagent-vdagentd communication mechanism, and vdagentd
 -virtio-serial/qemu - spice-server (which will provide the actual state,
 'good' or 'bad', and report the metrics probably).

 If D-Bus is approved, I can think of enhancing vdagent-vdagentd
 communication from socket to D-Bus as well.


 I don't think that any dbus api will be needed the agent can simply change
 the gsettings for these, and then
 gnome-shell will notice the change and react accordingly, at least I
 assume this is how the control-center
 settings work and get applied. Even if I'm wrong about the gsettings bit,
 there has to be an API between
 the control-center and gnome-shell, and rather then inventing a new one it
 would be good for the agent to
 re-use the existing one to talk to gnome-shell / gsd

 Regards,

 Hans




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

Re: [Spice-devel] Questions regarding Bug 62033 - Means to detect local-only


Hi,

For the purpose of disabling guest desktop effects, we already have a 
spice-gtk option which we use for Windows guests, and can be used for 
Linux guests as well. The Linux agent should support 
VD_AGENT_DISPLAY_CONFIG for this.


However, for local usage, we would also like to disable bitmap/audio 
compression, and video compression on the server side (I believe there 
is already an opened bug for decoupling lipsync from video compression). 
In addition, when using spice for local usage, you also have the 
overhead of redundant rendering on both server side (when required), and 
client side. We also need to consider rendering everything on the server 
side.
IMHO, as a first stage, we can start by adding an explicit option to 
spice-gtk for local, or more specific options for disabling 
compression (e.g., --disable-compression=audio/video/bitmaps/all). We 
can then send the settings to the server upon connection. As a second 
stage, I would continue with an automatic detection, something like hans 
suggested.


Regards,
Yonit.

On 06/24/2013 09:54 AM, Hans de Goede wrote:

Hi,

On 06/24/2013 02:44 PM, Fedor Lyakhov wrote:


Hi David,

Thanks for your answer. Please see my comments below.


On Mon, Jun 24, 2013 at 3:47 PM, David Jaša dj...@redhat.com
mailto:dj...@redhat.com wrote:

Hi Fedor,

I'd personally prefer to:
1) monitor bandwidth and latency continuously - there's a
long-standing RFE for it and IIRC Yonit has posted some
proof-of-concept patches in recent months (as a part of her
streamlining of video streams)
2) set the options on-the fly as the conditions allow to get at
the right position in b/w-saving --- cpu saving scale

The reason for this preference of mine is two-fold:
* idle gigabit or 100Mbps LAN may be closer to localhost behaviour
than WAN behaviour, especially with decreasing stream resolution
* such behaviour would be consistent with the rest of the spice
protocol


[FL] I agree with your points. So implementation will be based on
continuous bandwidth and latency monitoring and reporting. This means
we'd need a heuristic algorithm to detect a threshold for 'bad' and
'good' connection (and reporting the threshold has been crossed in one
or another direction).


Although I agree with doing continuous bandwidth + latency monitoring,
this is something which
will not be trivial. I would prefer to see a simple local / non local
detection too. Low hanging
fruit first.

And since in the future we hope to have 3d pass-through too, this will
be useful for determine
settings there too.

Since we sometimes get passed file-descriptors rather then opening
sockets our-selves, we cannot
simply check for localhost or some such for local detection. So we will
likely need to fallback
to checking if the spice-server and spice-client share a filesystem. One
possible way to do
this would be to create a shared memory segment with a unique name from
one side and have the
other side also open this. See for example how pulseaudio does this.

This may seem like overkill for just local detection, but we will
likely want a shared-memory
segment in the future anyways to use to speedup the local use case (by
avoiding sending pixmaps
over a socket), so it seems like a good idea to create one now, even if
just gets used for
local detection for now.



I'm also not sure if I follow what you want to do with an agent:
1) the agent runs in the guest OS and communicates already with
the client through using spice means - i.e. agent - virtio-serial/qemu
- spice-server - spice client. You shouldn't need to invent any other
client -- agent connection, and I can't get how such thing should
help you...
2) agent has no connection of what happens with display, and it
has a limited knowledge of network conditions as it handles just a
subset of all the traffic. spice-server and spice client actually do
have complete picture of what happens on the wire


[FL] I do understand all this. Sorry for my previous bloated e-mail
giving you wrong picture of what I'm suggesting.
New functionality for vdagent is required: an interface for
applications running at guest OS. In particular, gnome-settings-daemon
needs to know whether the connection is good or bad, and toggle
animations accordingly. I see following options for interface
vdagent-guest-apps:
1. D-Bus (low-level API, or GLib bindings)
2. Raw socket with custom protocol (similar to vdagent - vdagentd)

Personally I'd prefer D-Bus, since looks like this is the best
interface for freedesktop-based environments.

This is interface facing users (apps at guest); for internals I'm
going to reuse current vdagent-vdagentd communication mechanism, and
vdagentd -virtio-serial/qemu - spice-server (which will provide the
actual state, 'good' or 'bad', and report the metrics probably).

If D-Bus is approved, I can think of enhancing vdagent-vdagentd
communication from socket to D-Bus as well.


I don't think that any dbus api will be needed the

Re: [Spice-devel] [qxl-win PATCH 2/2] display: fix deadlock when dbg_level = 15


On 06/23/2013 06:30 AM, Uri Lublin wrote:

On 06/21/2013 04:50 PM, Yonit Halperin wrote:

DebugPrintV first locks print_sem, and then locks io_sem.
async_io, locks io_sem.
In ordr to avoid a deadlock, DebugPrintV MUSTN'T be called when
io_sem is locked.

Also notice, that locking io_sem during DebugPrintV limits our ability
to use the log_port for debugging concurrency problems related to ios.
---
  xddm/display/qxldd.h | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/xddm/display/qxldd.h b/xddm/display/qxldd.h
index 1a1b5d5..362c293 100644
--- a/xddm/display/qxldd.h
+++ b/xddm/display/qxldd.h
@@ -474,6 +474,7 @@ static _inline void async_io(PDev *pdev,
asyncable_t op, UCHAR val)
  ULONG64 millis;
  if (pdev-use_async) {
+DEBUG_PRINT((pdev, 3, %s: start async %d\n, __FUNCTION__,
(int)op));
  EngAcquireSemaphore(pdev-io_sem);
  WRITE_PORT_UCHAR(pdev-asyncable[op][ASYNC], val);
  /* Our Interrupt may be taken from us unexpectedly, by a
surprise removal.
@@ -482,7 +483,6 @@ static _inline void async_io(PDev *pdev,
asyncable_t op, UCHAR val)
   * we get reset. We use EngQueryLocalTime because there is
no way to differentiate a return on
   * timeout from a return on event set otherwise. */
  timeout.QuadPart = -INTERRUPT_NOT_PRESENT_TIMEOUT_100NS; //
negative  = relative
-DEBUG_PRINT((pdev, 15, WAIT_FOR_EVENT %d\n, (int)op));
  EngQueryLocalTime(start);
  WAIT_FOR_EVENT(pdev, pdev-io_cmd_event, timeout);
  EngQueryLocalTime(finish);
@@ -492,7 +492,7 @@ static _inline void async_io(PDev *pdev,
asyncable_t op, UCHAR val)
  DEBUG_PRINT((pdev, 0, %s: timeout reached, disabling
async io!\n, __FUNCTION__));


This DEBUG_PRINT should be deleted or marked and postpone to
after pdev-io_sem is released below.

You are right, and probably we should better lock io_sem before testing 
pdev-use_async...

  }
  EngReleaseSemaphore(pdev-io_sem);
-DEBUG_PRINT((pdev, 3, finished async %d\n, (int)op));
+DEBUG_PRINT((pdev, 3, %s: finished async %d\n,
__FUNCTION__, (int)op));
  } else {
  if (pdev-asyncable[op][SYNC] == NULL) {
  DEBUG_PRINT((pdev, 0, ERROR: trying calling sync io on
NULL port %d\n, op));




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

Re: [Spice-devel] Questions regarding Bug 62033 - Means to detect local-only


Hi,

On 06/24/2013 05:32 PM, Yonit Halperin wrote:

Hi,

For the purpose of disabling guest desktop effects, we already have a spice-gtk 
option which we use for Windows guests, and can be used for Linux guests as 
well. The Linux agent should support VD_AGENT_DISPLAY_CONFIG for this.

However, for local usage, we would also like to disable bitmap/audio 
compression, and video compression on the server side (I believe there is 
already an opened bug for decoupling lipsync from video compression). In 
addition, when using spice for local usage, you also have the overhead of 
redundant rendering on both server side (when required), and client side. We 
also need to consider rendering everything on the server side.


Yes, and when we decide to render everything on the server side, then having 
shared memory becomes really
useful...  :)


IMHO, as a first stage, we can start by adding an explicit option to spice-gtk for 
local, or more specific options for disabling compression (e.g., 
--disable-compression=audio/video/bitmaps/all). We can then send the settings to the 
server upon connection. As a second stage, I would continue with an automatic detection, 
something like hans suggested.


Regards,

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

Re: [Spice-devel] Can we directly redir usb hub?


Hi,

On 06/24/2013 11:40 AM, 邵鄭叡 wrote:

Hi all,
maybe I should post this message to [usbredir and qemu devel list] ,
but I just didnt find it :)

By default, spice-gtk filters out 'usb hubs' from redir list, and only show 
'non-hub non-interface' usb devs.

But sometime, we want to just simply pass a usb-hub to guest , and dont care 
about whatever is plugged on it.

So, I tweaked spice-gtk a bit  like this:

static void spice_usb_device_manager_add_dev(SpiceUsbDeviceManager  *self,
  GUdevDevice*udev)
{
...
 /* Skip hubs */
 devclass = g_udev_device_get_sysfs_attr(udev, bDeviceClass);
 if (!devclass || !strcmp(devclass, 09))
 {
-   return;
+  printf( Yes, it's a hub, and we want it :) \n  );
...
 }

Then, something interesting happened after I redired a usb-hub with a usb-disk 
plugged on it.
1. The hub is successfully redired into guest.
 I can see it in 'Window Device Manager' , it is correctly recognized as 
'usb hub' with right usb transfer speed (in my case , 480M).

2. The usb-disk is also detected by windows, and addressed under the hub.
But we are not lucky enough to see it as 'usb mass storage', but as 
'unknown device'.

3. If I redir the usb-disk rather than the hub, that's OK.


USB-redirection works on a usb-device bases, if you redirect a hub, you will 
only be redirecting
the hub and not any devices attached to it, which is why your test fails and 
why you cannot
redirect hubs (as you've found out). This is also the reason why the check is 
there (and should stay
there) in spice_usb_device_manager.

Regards,

Hans


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

[Spice-devel] [PATCH server 1/3] spice: silencing most of the ping/pong logging

Those messages are too frequent and don't contribute much
---
 server/red_channel.c | 13 ++---
 1 file changed, 2 insertions(+), 11 deletions(-)

diff --git a/server/red_channel.c b/server/red_channel.c
index 5662041..c0b1781 100644
--- a/server/red_channel.c
+++ b/server/red_channel.c
@@ -518,7 +518,6 @@ static void red_channel_client_send_ping(RedChannelClient 
*rcc)
 }  else {
 rcc-latency_monitor.tcp_nodelay = delay_val;
 if (!delay_val) {
-spice_debug(switching to TCP_NODELAY);
 delay_val = 1;
 if (setsockopt(rcc-stream-socket, IPPROTO_TCP, TCP_NODELAY, 
delay_val,
sizeof(delay_val)) == -1) {
@@ -535,7 +534,6 @@ static void red_channel_client_send_ping(RedChannelClient 
*rcc)
 clock_gettime(CLOCK_MONOTONIC, ts);
 ping.timestamp = ts.tv_sec * 10LL + ts.tv_nsec;
 spice_marshall_msg_ping(rcc-send_data.marshaller, ping);
-spice_debug(time %lu, ping.timestamp);
 red_channel_client_begin_send_message(rcc);
 }
 
@@ -709,7 +707,6 @@ static int 
red_channel_client_pre_create_validate(RedChannel *channel, RedClient
 
 static void red_channel_client_push_ping(RedChannelClient *rcc)
 {
-spice_debug(NULL);
 spice_assert(rcc-latency_monitor.state == PING_STATE_NONE);
 rcc-latency_monitor.state = PING_STATE_WARMUP;
 rcc-latency_monitor.warmup_was_sent = FALSE;
@@ -730,7 +727,7 @@ static void red_channel_client_ping_timer(void *opaque)
 spice_printerr(ioctl(TIOCOUTQ) failed, %s, strerror(errno));
 }
 if (so_unsent_size  0) {
-spice_debug(tcp snd buffer is still occupied. rescheduling ping);
+/* tcp snd buffer is still occupied. rescheduling ping */
 red_channel_client_start_ping_timer(rcc, 
PING_TEST_IDLE_NET_TIMEOUT_MS);
 } else {
 red_channel_client_push_ping(rcc);
@@ -1352,10 +1349,8 @@ static void 
red_channel_client_handle_pong(RedChannelClient *rcc, SpiceMsgPing *
 clock_gettime(CLOCK_MONOTONIC, ts);
 now =  ts.tv_sec * 10LL + ts.tv_nsec;
 
-spice_debug(now %lu, now);
 if (rcc-latency_monitor.state == PING_STATE_WARMUP) {
 rcc-latency_monitor.state = PING_STATE_LATENCY;
-spice_debug(warmup roundtrip  %.2f (ms), (now - 
ping-timestamp)/1000.0/1000.0);
 return;
 } else if (rcc-latency_monitor.state != PING_STATE_LATENCY) {
 spice_warning(unexpected);
@@ -1366,7 +1361,6 @@ static void 
red_channel_client_handle_pong(RedChannelClient *rcc, SpiceMsgPing *
 if (!rcc-latency_monitor.tcp_nodelay) {
 int delay_val = 0;
 
-spice_debug(switching to back TCP_NODELAY=0);
 if (setsockopt(rcc-stream-socket, IPPROTO_TCP, TCP_NODELAY, 
delay_val,
sizeof(delay_val)) == -1) {
 if (errno != ENOTSUP) {
@@ -1384,10 +1378,7 @@ static void 
red_channel_client_handle_pong(RedChannelClient *rcc, SpiceMsgPing *
 if (rcc-latency_monitor.roundtrip  0 ||
 now - ping-timestamp  rcc-latency_monitor.roundtrip) {
 rcc-latency_monitor.roundtrip = now - ping-timestamp;
-spice_debug(roundtrip ms %.2f (ms), 
rcc-latency_monitor.roundtrip/1000.0/1000.0);
-} else {
-spice_debug(not updating roundtrip. The latest latency measured was 
bigger (%.2f),
-(now - ping-timestamp)/1000.0/1000.0);
+spice_debug(update roundtrip %.2f(ms), 
rcc-latency_monitor.roundtrip/1000.0/1000.0);
 }
 
 rcc-latency_monitor.last_pong_time = now;
-- 
1.8.1.4

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

[Spice-devel] [PATCH server 3/3] red_worker: improve stream stats readability and ease of parsing

also added start/end-bit-rate and avg-quality to the final stream stats.
---
 server/mjpeg_encoder.c |  1 -
 server/red_worker.c| 22 +++---
 2 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/server/mjpeg_encoder.c b/server/mjpeg_encoder.c
index 92aef27..04b244e 100644
--- a/server/mjpeg_encoder.c
+++ b/server/mjpeg_encoder.c
@@ -221,7 +221,6 @@ MJpegEncoder *mjpeg_encoder_new(int bit_rate_control, 
uint64_t starting_bit_rate
 
 void mjpeg_encoder_destroy(MJpegEncoder *encoder)
 {
-spice_debug(avg-quality %.2f, (double)encoder-avg_quality / 
encoder-num_frames);
 jpeg_destroy_compress(encoder-cinfo);
 free(encoder-row);
 free(encoder);
diff --git a/server/red_worker.c b/server/red_worker.c
index f047d29..5b9f387 100644
--- a/server/red_worker.c
+++ b/server/red_worker.c
@@ -2633,26 +2633,34 @@ static void red_print_stream_stats(DisplayChannelClient 
*dcc, StreamAgent *agent
 #ifdef STREAM_STATS
 StreamStats *stats = agent-stats;
 double passed_mm_time = (stats-end - stats-start) / 1000.0;
+MJpegEncoderStats encoder_stats = {0};
 
-spice_debug(stream %ld (%dx%d): #frames-in %lu, #in-avg-fps %.2f, 
#frames-sent %lu, 
-#drops %lu (pipe %lu, fps %lu), avg_fps %.2f, 
-ratio(#frames-out/#frames-in) %.2f, 
-passed-mm-time %.2f (sec), size-total %.2f (MB), size-per-sec 
%.2f (Mbps), 
-size-per-frame %.2f (KBpf),
+if (agent-mjpeg_encoder) {
+mjpeg_encoder_get_stats(agent-mjpeg_encoder, encoder_stats);
+}
+
+spice_debug(stream=%ld dim=(%dx%d) #in-frames=%lu #in-avg-fps=%.2f 
#out-frames=%lu 
+out/in=%.2f #drops=%lu (#pipe=%lu #fps=%lu) out-avg-fps=%.2f 
+passed-mm-time(sec)=%.2f size-total(MB)=%.2f 
size-per-sec(Mbps)=%.2f 
+size-per-frame(KBpf)=%.2f avg-quality=%.2f 
+start-bit-rate(Mbps)=%.2f end-bit-rate(Mbps)=%.2f,
 agent - dcc-stream_agents, agent-stream-width, 
agent-stream-height,
 stats-num_input_frames,
 stats-num_input_frames / passed_mm_time,
 stats-num_frames_sent,
+(stats-num_frames_sent + 0.0) / stats-num_input_frames,
 stats-num_drops_pipe +
 stats-num_drops_fps,
 stats-num_drops_pipe,
 stats-num_drops_fps,
 stats-num_frames_sent / passed_mm_time,
-(stats-num_frames_sent + 0.0) / stats-num_input_frames,
 passed_mm_time,
 stats-size_sent / 1024.0 / 1024.0,
 ((stats-size_sent * 8.0) / (1024.0 * 1024)) / passed_mm_time,
-stats-size_sent / 1000.0 / stats-num_frames_sent);
+stats-size_sent / 1000.0 / stats-num_frames_sent,
+encoder_stats.avg_quality,
+encoder_stats.starting_bit_rate / (1024.0 * 1024),
+encoder_stats.cur_bit_rate / (1024.0 * 1024));
 #endif
 }
 
-- 
1.8.1.4

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

[Spice-devel] [PATCH spice-gtk 1/2] display: video stats logging - improve readability and ease of parsing

---
 gtk/channel-display.c | 19 +++
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/gtk/channel-display.c b/gtk/channel-display.c
index bc6fc08..07f6c1e 100644
--- a/gtk/channel-display.c
+++ b/gtk/channel-display.c
@@ -1511,6 +1511,7 @@ static void destroy_stream(SpiceChannel *channel, int id)
 SpiceDisplayChannelPrivate *c = SPICE_DISPLAY_CHANNEL(channel)-priv;
 display_stream *st;
 guint64 drops_duration_total = 0;
+guint32 num_out_frames;
 int i;
 
 g_return_if_fail(c != NULL);
@@ -1521,29 +1522,31 @@ static void destroy_stream(SpiceChannel *channel, int 
id)
 if (!st)
 return;
 
-CHANNEL_DEBUG(channel, %s: id %d #frames-in %d #drops-on-arive %d (%.2f) 
avg-late-time %.2f
- #drops-on-playback %d (%.2f), __FUNCTION__,
+num_out_frames = st-num_input_frames - st-num_drops_on_arive - 
st-num_drops_on_playback;
+CHANNEL_DEBUG(channel, %s: id=%d #in-frames=%d out/in=%.2f 
+#drops-on-arrive=%d avg-late-time(ms)=%.2f 
+#drops-on-playback=%d, __FUNCTION__,
 id,
 st-num_input_frames,
-st-num_drops_on_arive, st-num_drops_on_arive / 
((double)st-num_input_frames),
+num_out_frames / (double)st-num_input_frames,
+st-num_drops_on_arive,
 st-num_drops_on_arive ? st-arrive_late_time / 
((double)st-num_drops_on_arive): 0,
-st-num_drops_on_playback,
-st-num_drops_on_playback / ((double)st-num_input_frames));
+st-num_drops_on_playback);
 if (st-num_drops_seqs) {
-CHANNEL_DEBUG(channel, %s: #drops sequences %u ==, __FUNCTION__, 
st-num_drops_seqs);
+CHANNEL_DEBUG(channel, %s: #drops-sequences=%u ==, __FUNCTION__, 
st-num_drops_seqs);
 }
 for (i = 0; i  st-num_drops_seqs; i++) {
 drops_sequence_stats *stats = 
g_array_index(st-drops_seqs_stats_arr,
  drops_sequence_stats,
  i);
 drops_duration_total += stats-duration;
-CHANNEL_DEBUG(channel, %s: \t len %u start-ms %u duration-ms %u, 
__FUNCTION__,
+CHANNEL_DEBUG(channel, %s: \t len=%u start-ms=%u duration-ms=%u, 
__FUNCTION__,
stats-len,
stats-start_mm_time - 
st-first_frame_mm_time,
stats-duration);
 }
 if (st-num_drops_seqs) {
-CHANNEL_DEBUG(channel, %s: drops total duration %G_GUINT64_FORMAT 
==, __FUNCTION__, drops_duration_total);
+CHANNEL_DEBUG(channel, %s: drops-total-duration=%G_GUINT64_FORMAT 
==, __FUNCTION__, drops_duration_total);
 }
 
 g_array_free(st-drops_seqs_stats_arr, TRUE);
-- 
1.8.1.4

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

[Spice-devel] [PATCH server 2/3] mjpeg_encoder: add mjpeg_encoder_get_stats

---
 server/mjpeg_encoder.c | 11 +++
 server/mjpeg_encoder.h |  7 +++
 2 files changed, 18 insertions(+)

diff --git a/server/mjpeg_encoder.c b/server/mjpeg_encoder.c
index 4460322..92aef27 100644
--- a/server/mjpeg_encoder.c
+++ b/server/mjpeg_encoder.c
@@ -169,6 +169,7 @@ struct MJpegEncoder {
 void *cbs_opaque;
 
 /* stats */
+uint64_t starting_bit_rate;
 uint64_t avg_quality;
 uint32_t num_frames;
 };
@@ -195,6 +196,8 @@ MJpegEncoder *mjpeg_encoder_new(int bit_rate_control, 
uint64_t starting_bit_rate
 enc-first_frame = TRUE;
 enc-rate_control_is_active = bit_rate_control;
 enc-rate_control.byte_rate = starting_bit_rate / 8;
+enc-starting_bit_rate = starting_bit_rate;
+
 if (bit_rate_control) {
 struct timespec time;
 
@@ -1259,3 +1262,11 @@ uint64_t mjpeg_encoder_get_bit_rate(MJpegEncoder 
*encoder)
 {
 return encoder-rate_control.byte_rate * 8;
 }
+
+void mjpeg_encoder_get_stats(MJpegEncoder *encoder, MJpegEncoderStats *stats)
+{
+spice_assert(encoder != NULL  stats != NULL);
+stats-starting_bit_rate = encoder-starting_bit_rate;
+stats-cur_bit_rate = mjpeg_encoder_get_bit_rate(encoder);
+stats-avg_quality = (double)encoder-avg_quality / encoder-num_frames;
+}
diff --git a/server/mjpeg_encoder.h b/server/mjpeg_encoder.h
index 310d289..741ea1c 100644
--- a/server/mjpeg_encoder.h
+++ b/server/mjpeg_encoder.h
@@ -43,6 +43,12 @@ typedef struct MJpegEncoderRateControlCbs {
 void (*update_client_playback_delay)(void *opaque, uint32_t delay_ms);
 } MJpegEncoderRateControlCbs;
 
+typedef struct MJpegEncoderStats {
+uint64_t starting_bit_rate;
+uint64_t cur_bit_rate;
+double avg_quality;
+} MJpegEncoderStats;
+
 MJpegEncoder *mjpeg_encoder_new(int bit_rate_control, uint64_t 
starting_bit_rate,
 MJpegEncoderRateControlCbs *cbs, void *opaque);
 void mjpeg_encoder_destroy(MJpegEncoder *encoder);
@@ -103,5 +109,6 @@ void mjpeg_encoder_client_stream_report(MJpegEncoder 
*encoder,
 void mjpeg_encoder_notify_server_frame_drop(MJpegEncoder *encoder);
 
 uint64_t mjpeg_encoder_get_bit_rate(MJpegEncoder *encoder);
+void mjpeg_encoder_get_stats(MJpegEncoder *encoder, MJpegEncoderStats *stats);
 
 #endif
-- 
1.8.1.4

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

[Spice-devel] [PATCH spice-gtk 2/2] display: disabling adaptive video streaming via env var SPICE_DISABLE_ADAPTIVE_STREAMING

spice-server supports adaptive video streaming only if the client
publishes SPICE_DISPLAY_CAP_STREAM_REPORT.

Disabling this feature is useful for debugging and performance comparison.
---
 gtk/channel-display.c | 24 ++--
 1 file changed, 18 insertions(+), 6 deletions(-)

diff --git a/gtk/channel-display.c b/gtk/channel-display.c
index 07f6c1e..14c80d2 100644
--- a/gtk/channel-display.c
+++ b/gtk/channel-display.c
@@ -79,6 +79,7 @@ struct _SpiceDisplayChannelPrivate {
 guint   mark_false_event_id;
 GArray  *monitors;
 guint   monitors_max;
+gbooleanenable_adaptive_streaming;
 #ifdef WIN32
 HDC dc;
 #endif
@@ -693,7 +694,9 @@ static void 
spice_display_channel_reset_capabilities(SpiceChannel *channel)
 spice_channel_set_capability(SPICE_CHANNEL(channel), 
SPICE_DISPLAY_CAP_MONITORS_CONFIG);
 spice_channel_set_capability(SPICE_CHANNEL(channel), 
SPICE_DISPLAY_CAP_COMPOSITE);
 spice_channel_set_capability(SPICE_CHANNEL(channel), 
SPICE_DISPLAY_CAP_A8_SURFACE);
-spice_channel_set_capability(SPICE_CHANNEL(channel), 
SPICE_DISPLAY_CAP_STREAM_REPORT);
+if (SPICE_DISPLAY_CHANNEL(channel)-priv-enable_adaptive_streaming) {
+spice_channel_set_capability(SPICE_CHANNEL(channel), 
SPICE_DISPLAY_CAP_STREAM_REPORT);
+}
 }
 
 static void spice_display_channel_init(SpiceDisplayChannel *channel)
@@ -710,6 +713,13 @@ static void spice_display_channel_init(SpiceDisplayChannel 
*channel)
 c-dc = create_compatible_dc();
 #endif
 c-monitors_max = 1;
+
+if (g_getenv(SPICE_DISABLE_ADAPTIVE_STREAMING)) {
+SPICE_DEBUG(adaptive video disabled);
+c-enable_adaptive_streaming = FALSE;
+} else {
+c-enable_adaptive_streaming = TRUE;
+}
 spice_display_channel_reset_capabilities(SPICE_CHANNEL(channel));
 }
 
@@ -1471,11 +1481,13 @@ static void display_handle_stream_data(SpiceChannel 
*channel, SpiceMsgIn *in)
 }
 st-playback_sync_drops_seq_len = 0;
 }
-display_update_stream_report(SPICE_DISPLAY_CHANNEL(channel), op-id,
- op-multi_media_time, latency);
-if (st-playback_sync_drops_seq_len = 
STREAM_PLAYBACK_SYNC_DROP_SEQ_LEN_LIMIT) {
-
spice_session_sync_playback_latency(spice_channel_get_session(channel));
-st-playback_sync_drops_seq_len = 0;
+if (c-enable_adaptive_streaming) {
+display_update_stream_report(SPICE_DISPLAY_CHANNEL(channel), op-id,
+ op-multi_media_time, latency);
+if (st-playback_sync_drops_seq_len = 
STREAM_PLAYBACK_SYNC_DROP_SEQ_LEN_LIMIT) {
+
spice_session_sync_playback_latency(spice_channel_get_session(channel));
+st-playback_sync_drops_seq_len = 0;
+}
 }
 }
 
-- 
1.8.1.4

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

Re: [Spice-devel] [PATCH server 1/3] spice: silencing most of the ping/pong logging


Looks good, ACK-series

On 06/24/2013 09:38 PM, Yonit Halperin wrote:

Those messages are too frequent and don't contribute much
---
  server/red_channel.c | 13 ++---
  1 file changed, 2 insertions(+), 11 deletions(-)

diff --git a/server/red_channel.c b/server/red_channel.c
index 5662041..c0b1781 100644
--- a/server/red_channel.c
+++ b/server/red_channel.c
@@ -518,7 +518,6 @@ static void red_channel_client_send_ping(RedChannelClient 
*rcc)
  }  else {
  rcc-latency_monitor.tcp_nodelay = delay_val;
  if (!delay_val) {
-spice_debug(switching to TCP_NODELAY);
  delay_val = 1;
  if (setsockopt(rcc-stream-socket, IPPROTO_TCP, TCP_NODELAY, 
delay_val,
 sizeof(delay_val)) == -1) {
@@ -535,7 +534,6 @@ static void red_channel_client_send_ping(RedChannelClient 
*rcc)
  clock_gettime(CLOCK_MONOTONIC, ts);
  ping.timestamp = ts.tv_sec * 10LL + ts.tv_nsec;
  spice_marshall_msg_ping(rcc-send_data.marshaller, ping);
-spice_debug(time %lu, ping.timestamp);
  red_channel_client_begin_send_message(rcc);
  }

@@ -709,7 +707,6 @@ static int 
red_channel_client_pre_create_validate(RedChannel *channel, RedClient

  static void red_channel_client_push_ping(RedChannelClient *rcc)
  {
-spice_debug(NULL);
  spice_assert(rcc-latency_monitor.state == PING_STATE_NONE);
  rcc-latency_monitor.state = PING_STATE_WARMUP;
  rcc-latency_monitor.warmup_was_sent = FALSE;
@@ -730,7 +727,7 @@ static void red_channel_client_ping_timer(void *opaque)
  spice_printerr(ioctl(TIOCOUTQ) failed, %s, strerror(errno));
  }
  if (so_unsent_size  0) {
-spice_debug(tcp snd buffer is still occupied. rescheduling ping);
+/* tcp snd buffer is still occupied. rescheduling ping */
  red_channel_client_start_ping_timer(rcc, 
PING_TEST_IDLE_NET_TIMEOUT_MS);
  } else {
  red_channel_client_push_ping(rcc);
@@ -1352,10 +1349,8 @@ static void 
red_channel_client_handle_pong(RedChannelClient *rcc, SpiceMsgPing *
  clock_gettime(CLOCK_MONOTONIC, ts);
  now =  ts.tv_sec * 10LL + ts.tv_nsec;

-spice_debug(now %lu, now);
  if (rcc-latency_monitor.state == PING_STATE_WARMUP) {
  rcc-latency_monitor.state = PING_STATE_LATENCY;
-spice_debug(warmup roundtrip  %.2f (ms), (now - 
ping-timestamp)/1000.0/1000.0);
  return;
  } else if (rcc-latency_monitor.state != PING_STATE_LATENCY) {
  spice_warning(unexpected);
@@ -1366,7 +1361,6 @@ static void 
red_channel_client_handle_pong(RedChannelClient *rcc, SpiceMsgPing *
  if (!rcc-latency_monitor.tcp_nodelay) {
  int delay_val = 0;

-spice_debug(switching to back TCP_NODELAY=0);
  if (setsockopt(rcc-stream-socket, IPPROTO_TCP, TCP_NODELAY, 
delay_val,
 sizeof(delay_val)) == -1) {
  if (errno != ENOTSUP) {
@@ -1384,10 +1378,7 @@ static void 
red_channel_client_handle_pong(RedChannelClient *rcc, SpiceMsgPing *
  if (rcc-latency_monitor.roundtrip  0 ||
  now - ping-timestamp  rcc-latency_monitor.roundtrip) {
  rcc-latency_monitor.roundtrip = now - ping-timestamp;
-spice_debug(roundtrip ms %.2f (ms), 
rcc-latency_monitor.roundtrip/1000.0/1000.0);
-} else {
-spice_debug(not updating roundtrip. The latest latency measured was bigger 
(%.2f),
-(now - ping-timestamp)/1000.0/1000.0);
+spice_debug(update roundtrip %.2f(ms), 
rcc-latency_monitor.roundtrip/1000.0/1000.0);
  }

  rcc-latency_monitor.last_pong_time = now;


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

Re: [Spice-devel] [PATCH spice-gtk 1/2] display: video stats logging - improve readability and ease of parsing