Re: [Spice-devel] [PATCH v2] common: add backtrace via gstack or glibc backtrace

2011-07-18 Thread Uri Lublin 

On 07/18/2011 07:21 PM, Christophe Fergeau wrote:

On Mon, Jul 18, 2011 at 06:42:14PM +0300, Alon Levy wrote:

diff --git a/common/spice_common.h b/common/spice_common.h
index bc74486..6c5154c 100644
--- a/common/spice_common.h
+++ b/common/spice_common.h
@@ -22,9 +22,11 @@
  #include
  #include
  #include
+#include "backtrace.h"

  #define ASSERT(x) if (!(x)) {   \
  printf("%s: ASSERT %s failed\n", __FUNCTION__, #x); \
+spice_backtrace();  \
  abort();\
  }


Given that it's the only change in existing code, and that it's only
trigger right before an abort(), no need to go over this series for days
and days imo ;)


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

Re: [Spice-devel] Spice QXL Driver does not work for Windows 7

2011-07-18 Thread Alon Levy 
On Mon, Jul 18, 2011 at 07:34:03PM +0200, Mario wrote:
> Hi,
> 
> I was just trying to build the latest qxl windows drivers with the
> 0.8.0 protocol headers. I followed this guide exactly:
> http://spice-space.org/page/WinQXL
> 
> I built the drivers for both Windows XP/x86 and Windows 7/x86. XP is
> working well but my Windows 7 box tells me that the driver cannot
> start (code 10) in the Windows device manager (see attached
> picture).
> 
> I checked the dependencies for both - qxl.sys and qxldd.dll with
> Depends 2.2 but all dependencies are valid.
> 
> Is there anything I can do to debug the reason why the driver cannot
> start? Any Idea?
> 
> Can someone provide me Windows 7 x86 QXL drivers known to work?
> 

Looks like it isn't signed and windows doesn't like that. Try signing it.
Also, not sure how you installed it, but maybe try choosing all the manual
paths in the device-manager upate driver wizard.

> Thanks a lot for you help.
> 
> Cheers,
> Mario


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

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

[Spice-devel] [Announce] spice-gtk 0.7 "So you do have a plan?! Yeah, Mr. White! Yeah, science!"

2011-07-18 Thread Marc-André Lureau 
Hey!

We released spice-gtk 0.7 today, and it is now available for
download at its usual place:

http://spice-space.org/download/gtk/spice-gtk-0.7.tar.bz2

a47ca51ef4bb27cab35ddbfb9e6c0fff  spice-gtk-0.7.tar.bz2

Changes since 0.6:

- smartcard support
- better video playback performance (jpeg-turbo & audio improvements)
- support for audio volume (needs qemu support)
- controller support for Windows (NamedPipe)
- make perl-Text-CSV optional for tarball builds
- new spice_get_option_group()/spice_set_session_option()
- keyboard improvements, grab-sequence can be configured, various windows fixes
- new tool spicy-stats, to collect informations during a session
- bugfixes: memleak fixes, SASL fixes, crash with virt-manager
- various build fixes, should build on MacOS as well now

Contributors to this release:

Alon Levy, Attila Sukosd, Christophe Fergeau, Daniel P. Berrange,
Marc-André Lureau, Zeeshan Ali (Khattak).

Thanks!

Now let me enjoy the new season of Breaking Bad!

cheers

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

[Spice-devel] Issues while compiling QXL driver for x64 env....

2011-07-18 Thread Naga Mohan Pothula 
Hi,
I have seen issues while building QXL driver in x64 free env. I'm not seeeing 
the same with x86 free env.error C2373: 'DrvEnableDriver' : redefinition; 
different type modifierserror C2373: 'DrvDisableDriver' : redefinition; 
different type modifiers
...
like this getting for all Drv* functions. I have noticed these while checking 
Auto code review tool OACR. 
But binaries get build with "build -czg". I added /P switch after /WX option 
but I'm seeing this issue.
Could anyone tell where exactly i'm missing? I just want to eliminate all 
errors and warnings reported by OACR.

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

Re: [Spice-devel] [PATCH v2] common: add backtrace via gstack or glibc backtrace

2011-07-18 Thread Alon Levy 
On Mon, Jul 18, 2011 at 06:21:43PM +0200, Christophe Fergeau wrote:
> On Mon, Jul 18, 2011 at 06:42:14PM +0300, Alon Levy wrote:
> > diff --git a/common/spice_common.h b/common/spice_common.h
> > index bc74486..6c5154c 100644
> > --- a/common/spice_common.h
> > +++ b/common/spice_common.h
> > @@ -22,9 +22,11 @@
> >  #include 
> >  #include 
> >  #include 
> > +#include "backtrace.h"
> >  
> >  #define ASSERT(x) if (!(x)) {   \
> >  printf("%s: ASSERT %s failed\n", __FUNCTION__, #x); \
> > +spice_backtrace();  \
> >  abort();\
> >  }
> 
> Given that it's the only change in existing code, and that it's only
> trigger right before an abort(), no need to go over this series for days
> and days imo ;) What happens on Windows though? Shouldn't we have an empty
> stub for spice_backtrace() there?

I think it would work as is because both the define won't be there and the
access call will fail. And if you build with mingw then maybe backtrace is
there (should be I guess).

> 
> Christophe


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

Re: [Spice-devel] [PATCH] Fix spice-server/qemu channel version checks

2011-07-18 Thread Christophe Fergeau 
On Mon, Jul 18, 2011 at 05:47:11PM +0300, Alon Levy wrote:
> On Mon, Jul 18, 2011 at 04:16:59PM +0200, Christophe Fergeau wrote:
> > Ping?
> > 
> ACK.

Pushed, thanks.

Christophe


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

Re: [Spice-devel] [PATCH v2] common: add backtrace via gstack or glibc backtrace

2011-07-18 Thread Christophe Fergeau 
On Mon, Jul 18, 2011 at 06:42:14PM +0300, Alon Levy wrote:
> diff --git a/common/spice_common.h b/common/spice_common.h
> index bc74486..6c5154c 100644
> --- a/common/spice_common.h
> +++ b/common/spice_common.h
> @@ -22,9 +22,11 @@
>  #include 
>  #include 
>  #include 
> +#include "backtrace.h"
>  
>  #define ASSERT(x) if (!(x)) {   \
>  printf("%s: ASSERT %s failed\n", __FUNCTION__, #x); \
> +spice_backtrace();  \
>  abort();\
>  }

Given that it's the only change in existing code, and that it's only
trigger right before an abort(), no need to go over this series for days
and days imo ;) What happens on Windows though? Shouldn't we have an empty
stub for spice_backtrace() there?

Christophe


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

Re: [Spice-devel] [PATCH 1/5] client: s/recive/receive

2011-07-18 Thread Christophe Fergeau 
On Mon, Jul 18, 2011 at 05:52:02PM +0300, Alon Levy wrote:
> On Fri, Jul 08, 2011 at 12:17:28PM +0200, Christophe Fergeau wrote:
> > ---
> >  client/red_channel.cpp |   32 
> >  client/red_channel.h   |6 +++---
> >  client/red_peer.cpp|8 
> >  client/red_peer.h  |4 ++--
> >  4 files changed, 25 insertions(+), 25 deletions(-)
> > 
> 
> ACK.

Pushed all 5 patches with the change you asked in 4/5

Christophe


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

Re: [Spice-devel] [PATCH 5/5] client: don't crash when agent is missing WAN support

2011-07-18 Thread Christophe Fergeau 
On Mon, Jul 18, 2011 at 06:43:31PM +0300, Alon Levy wrote:
> On Mon, Jul 18, 2011 at 05:28:26PM +0200, Christophe Fergeau wrote:
> > On Mon, Jul 18, 2011 at 05:51:30PM +0300, Alon Levy wrote:
> > > > @@ -1046,6 +1047,17 @@ void RedClient::on_agent_announce_capabilities(
> > > >  // not sending the color depth through 
> > > > send_agent_monitors_config, since
> > > >  // it applies only for attached screens.
> > > >  send_agent_display_config();
> > > > +} else if (!_auto_display_res) {
> > > 
> > > Who sets _auto_display_res? does this affect windows guest agents? the 
> > > comment
> > > below says linux but the test above seems to be not linux specific.
> > 
> > No idea, I added this test to mirror the
> > if (_auto_display_res) {
> >send_agent_monitors_config();
> > }
> > in handle_init. This avoids changing behaviour when we decided to send the
> > monitors config in handle_init. If the comment is misleading, I can change
> > it, for example "Some agents don't support monitors/displays ..."
> 
> I didn't care about the comment, but about the behavior. I want to be sure
> this doesn't change the behavior for the windows agent.

In the windows case, we'll go through the 1st block:

if (VD_AGENT_HAS_CAPABILITY(caps->caps, caps_size,
VD_AGENT_CAP_DISPLAY_CONFIG) && !_agent_disp_config_sent) {
// not sending the color depth through send_agent_monitors_config,
// sinc
// it applies only for attached screens.
send_agent_display_config();
}
The if (!_auto_display_res) test was added to avoid as much as possible
changing the old behaviour, we used to be waiting in this case, since we
sent a message in handle_init, it makes sense to still be waiting in this
case.


Christophe


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

Re: [Spice-devel] [PATCH 5/5] client: don't crash when agent is missing WAN support

2011-07-18 Thread Alon Levy 
On Fri, Jul 08, 2011 at 12:17:32PM +0200, Christophe Fergeau wrote:
> If you try to connect to a linux guest with WAN options, SPICE window opens up
> and is blank - it then fails with vdagent timeout message.  It should give a
> warning that this is only applicable for windows guest and still connect to
> guest.
> 
> It all starts in RedClient::handle_init
> This function checks whether we have an agent or not, because if we have an
> agent, there will be some kind of handshake to check both sides capabilities
> before all the spice channels are created.
> 
> When there is no agent running, the startup process goes on with
> SPICE_MSGC_MAIN_ATTACH_CHANNELS
> 
> When there is a windows agent running, VD_AGENT_ANNOUNCE_CAPABILITIES and
> VD_AGENT_DISPLAY_CONFIG messages are sent to the agent, and when processing 
> the
> agent answer to the VD_AGENT_DISPLAY_CONFIG message,
> SPICE_MSGC_MAIN_ATTACH_CHANNELS will be sent and the startup process will go
> on.
> 
> However, when there is no agent running but --color-depth was used, 
> handle_init
> won't send the SPICE_MSGC_MAIN_ATTACH_CHANNELS message but will wait for the
> agent handshake to proceed to its end, which won't happen, so it will timeout
> waiting for agent answers.
> 
> Similarly, the linux agent handles VD_AGENT_ANNOUNCE_CAPABILITIES messages, 
> but
> it doesn't handle VD_AGENT_DISPLAY_CONFIG messages, so we'll never reach the
> point where a SPICE_MSGC_MAIN_ATTACH_CHANNELS will be sent.
> 
> This commit fixes this in 2 places:
> - unconditionnally send SPICE_MSGC_ATTACH_CHANNELS when no agent is running in
> handle_init
> - send SPICE_MSGC_MAIN_ATTACH_CHANNELS in
> RedClient::on_agent_announce_capabilities if the agent doesn't have the
> VD_AGENT_CAP_DISPLAY_CONFIG capability
> 
> This fixes RH bug #712938

ACK, with minor comment below.

> ---
>  client/red_client.cpp |   28 
>  1 files changed, 20 insertions(+), 8 deletions(-)
> 
> diff --git a/client/red_client.cpp b/client/red_client.cpp
> index 8918e4f..edcdb02 100644
> --- a/client/red_client.cpp
> +++ b/client/red_client.cpp
> @@ -966,19 +966,20 @@ void RedClient::handle_init(RedPeer::InMessage* message)
>  agent_start.num_tokens = ~0;
>  _marshallers->msgc_main_agent_start(msg->marshaller(), &agent_start);
>  post_message(msg);
> -}
> -
> -if (_agent_connected) {
>  send_agent_announce_capabilities(true);
>  if (_auto_display_res) {
> send_agent_monitors_config();
>  }
> -}
> -
> -if (!_auto_display_res && _display_setting.is_empty()) {
> -post_message(new Message(SPICE_MSGC_MAIN_ATTACH_CHANNELS));
> +if (_auto_display_res || !_display_setting.is_empty()) {
> +_application.activate_interval_timer(*_agent_timer, 
> AGENT_TIMEOUT);
> +} else {
> +post_message(new Message(SPICE_MSGC_MAIN_ATTACH_CHANNELS));
> +}
>  } else {
> -_application.activate_interval_timer(*_agent_timer, AGENT_TIMEOUT);
> +if (_auto_display_res || !_display_setting.is_empty()) {
> +LOG_WARN("no agent running, display options have been ignored");
> +}
> +post_message(new Message(SPICE_MSGC_MAIN_ATTACH_CHANNELS));
>  }
>  }
>  
> @@ -1046,6 +1047,17 @@ void RedClient::on_agent_announce_capabilities(
>  // not sending the color depth through send_agent_monitors_config, 
> since
>  // it applies only for attached screens.
>  send_agent_display_config();
> +} else if (!_auto_display_res) {

(*)

> +/* linux agent doesn't support monitors/displays agent messages, so
> + * we'll never reach on_agent_reply which sends this
> + * ATTACH_CHANNELS message which is needed for client startup to go
> + * on.
> + */
> +if (_auto_display_res || !_display_setting.is_empty()) {

You just verified that !_auto_display_res above (*), so no need to check it 
again.

> +LOG_WARN("display options have been requested, but the agent 
> doesn't support these options");
> +}
> +post_message(new Message(SPICE_MSGC_MAIN_ATTACH_CHANNELS));
> +_application.deactivate_interval_timer(*_agent_timer);
>  }
>  }
>  
> -- 
> 1.7.6
> 
> ___
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [PATCH 5/5] client: don't crash when agent is missing WAN support

2011-07-18 Thread Alon Levy 
On Mon, Jul 18, 2011 at 05:28:26PM +0200, Christophe Fergeau wrote:
> On Mon, Jul 18, 2011 at 05:51:30PM +0300, Alon Levy wrote:
> > > ---
> > >  client/red_client.cpp |   28 
> > >  1 files changed, 20 insertions(+), 8 deletions(-)
> > > 
> > > diff --git a/client/red_client.cpp b/client/red_client.cpp
> > > index 8918e4f..edcdb02 100644
> > > --- a/client/red_client.cpp
> > > +++ b/client/red_client.cpp
> > > @@ -966,19 +966,20 @@ void RedClient::handle_init(RedPeer::InMessage* 
> > > message)
> > >  agent_start.num_tokens = ~0;
> > >  _marshallers->msgc_main_agent_start(msg->marshaller(), 
> > > &agent_start);
> > >  post_message(msg);
> > > -}
> > > -
> > > -if (_agent_connected) {
> > 
> > Why do we want to send agent_announce_capabilities if !_agent_connected?
> 
> we don't, the old code was doing
> 
> if (_agent_connected) {
> agent_start.num_tokens = ~0;
> _marshallers->msgc_main_agent_start(msg->marshaller(), &agent_start);
> post_message(msg);
> }
> 
> if (_agent_connected) {
> send_agent_announce_capabilities(true);
> if (_auto_display_res) {
>send_agent_monitors_config();
> }
> }
> 
> I merged the 2 blocks, unless I missed something there's no change as to
> when the send_announce_capabilities() is called.
> 
> > > @@ -1046,6 +1047,17 @@ void RedClient::on_agent_announce_capabilities(
> > >  // not sending the color depth through 
> > > send_agent_monitors_config, since
> > >  // it applies only for attached screens.
> > >  send_agent_display_config();
> > > +} else if (!_auto_display_res) {
> > 
> > Who sets _auto_display_res? does this affect windows guest agents? the 
> > comment
> > below says linux but the test above seems to be not linux specific.
> 
> No idea, I added this test to mirror the
> if (_auto_display_res) {
>send_agent_monitors_config();
> }
> in handle_init. This avoids changing behaviour when we decided to send the
> monitors config in handle_init. If the comment is misleading, I can change
> it, for example "Some agents don't support monitors/displays ..."

I didn't care about the comment, but about the behavior. I want to be sure
this doesn't change the behavior for the windows agent.

> 
> Christophe



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

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

[Spice-devel] [PATCH v2] common: add backtrace via gstack or glibc backtrace

2011-07-18 Thread Alon Levy 
Add a backtrace printing function copied from xserver os/backtrace.c
that uses gstack, and if that isn't found then glibc's backtrace.
Used in ASSERT, tested on F15.
---
 common/Makefile.am|2 +
 common/backtrace.c|  114 +
 common/backtrace.h|   24 ++
 common/spice_common.h |2 +
 configure.ac  |1 +
 5 files changed, 143 insertions(+), 0 deletions(-)
 create mode 100644 common/backtrace.c
 create mode 100644 common/backtrace.h

diff --git a/common/Makefile.am b/common/Makefile.am
index e0f4d49..f07f948 100644
--- a/common/Makefile.am
+++ b/common/Makefile.am
@@ -36,6 +36,8 @@ libspice_common_la_SOURCES =  \
spice_common.h  \
ssl_verify.c\
ssl_verify.h\
+   backtrace.c \
+   backtrace.h \
$(NULL)
 
 if SUPPORT_GL
diff --git a/common/backtrace.c b/common/backtrace.c
new file mode 100644
index 000..d606dd9
--- /dev/null
+++ b/common/backtrace.c
@@ -0,0 +1,114 @@
+/* -*- Mode: C; c-basic-offset: 4; indent-tabs-mode: nil -*- */
+/*
+   Copyright (C) 2011 Red Hat, Inc.
+
+   This library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   This library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with this library; if not, see .
+*/
+
+/*
+ * Taken from xserver os/backtrace.c:
+ * Copyright 2008 Red Hat, Inc.
+ */
+
+#include "config.h"
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include "common/spice_common.h"
+
+#define GSTACK_PATH "/usr/bin/gstack"
+
+#if HAVE_EXECINFO_H
+#include 
+
+static void spice_backtrace_backtrace(void)
+{
+void *array[100];
+int size;
+
+size = backtrace(array, sizeof(array)/sizeof(array[0]));
+backtrace_symbols_fd(array, size, STDERR_FILENO);
+}
+#else
+static void spice_backtrace_backtrace(void)
+{
+}
+#endif
+
+static int spice_backtrace_gstack(void)
+{
+pid_t kidpid;
+int pipefd[2];
+
+if (pipe(pipefd) != 0) {
+return -1;
+}
+
+kidpid = fork();
+
+if (kidpid == -1) {
+/* ERROR */
+return -1;
+} else if (kidpid == 0) {
+/* CHILD */
+char parent[16];
+
+seteuid(0);
+close(STDIN_FILENO);
+close(STDOUT_FILENO);
+dup2(pipefd[1],STDOUT_FILENO);
+close(STDERR_FILENO);
+
+snprintf(parent, sizeof(parent), "%d", getppid());
+execle(GSTACK_PATH, "gstack", parent, NULL, NULL);
+exit(1);
+} else {
+/* PARENT */
+char btline[256];
+int kidstat;
+int bytesread;
+int done = 0;
+
+close(pipefd[1]);
+
+while (!done) {
+bytesread = read(pipefd[0], btline, sizeof(btline) - 1);
+
+if (bytesread > 0) {
+btline[bytesread] = 0;
+fprintf(stderr, "%s", btline);
+}
+else if ((bytesread == 0) ||
+ ((errno != EINTR) && (errno != EAGAIN))) {
+done = 1;
+}
+}
+close(pipefd[0]);
+waitpid(kidpid, &kidstat, 0);
+if (kidstat != 0)
+return -1;
+}
+return 0;
+}
+
+void spice_backtrace() {
+if (!access(GSTACK_PATH, X_OK)) {
+spice_backtrace_gstack();
+} else {
+spice_backtrace_backtrace();
+}
+}
diff --git a/common/backtrace.h b/common/backtrace.h
new file mode 100644
index 000..3b0c132
--- /dev/null
+++ b/common/backtrace.h
@@ -0,0 +1,24 @@
+/* -*- Mode: C; c-basic-offset: 4; indent-tabs-mode: nil -*- */
+/*
+   Copyright (C) 2011 Red Hat, Inc.
+
+   This library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   This library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with this library; if not, see .
+*/
+
+#ifndef BACKTRACE_H
+#define BACKTRACE_H
+
+void spice_backtrace(void);
+
+#endif // BACKTRACE_H
diff --git a/common/spice_common.h

Re: [Spice-devel] [PATCH 5/5] client: don't crash when agent is missing WAN support

2011-07-18 Thread Christophe Fergeau 
On Mon, Jul 18, 2011 at 05:51:30PM +0300, Alon Levy wrote:
> > ---
> >  client/red_client.cpp |   28 
> >  1 files changed, 20 insertions(+), 8 deletions(-)
> > 
> > diff --git a/client/red_client.cpp b/client/red_client.cpp
> > index 8918e4f..edcdb02 100644
> > --- a/client/red_client.cpp
> > +++ b/client/red_client.cpp
> > @@ -966,19 +966,20 @@ void RedClient::handle_init(RedPeer::InMessage* 
> > message)
> >  agent_start.num_tokens = ~0;
> >  _marshallers->msgc_main_agent_start(msg->marshaller(), 
> > &agent_start);
> >  post_message(msg);
> > -}
> > -
> > -if (_agent_connected) {
> 
> Why do we want to send agent_announce_capabilities if !_agent_connected?

we don't, the old code was doing

if (_agent_connected) {
agent_start.num_tokens = ~0;
_marshallers->msgc_main_agent_start(msg->marshaller(), &agent_start);
post_message(msg);
}

if (_agent_connected) {
send_agent_announce_capabilities(true);
if (_auto_display_res) {
   send_agent_monitors_config();
}
}

I merged the 2 blocks, unless I missed something there's no change as to
when the send_announce_capabilities() is called.

> > @@ -1046,6 +1047,17 @@ void RedClient::on_agent_announce_capabilities(
> >  // not sending the color depth through send_agent_monitors_config, 
> > since
> >  // it applies only for attached screens.
> >  send_agent_display_config();
> > +} else if (!_auto_display_res) {
> 
> Who sets _auto_display_res? does this affect windows guest agents? the comment
> below says linux but the test above seems to be not linux specific.

No idea, I added this test to mirror the
if (_auto_display_res) {
   send_agent_monitors_config();
}
in handle_init. This avoids changing behaviour when we decided to send the
monitors config in handle_init. If the comment is misleading, I can change
it, for example "Some agents don't support monitors/displays ..."

Christophe


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

Re: [Spice-devel] [PATCH 4/5] client: improve WAN option description

2011-07-18 Thread Alon Levy 
On Fri, Jul 08, 2011 at 12:17:31PM +0200, Christophe Fergeau wrote:
> The WAN options (--color-depth and --disable-effects) need
> support from the guest agent to be working. Currently they are
> only supported on Windows. While I don't want to explicitly
> mention Windows in --help output, we can hint that it won't
> work with all guests in --help. This fixes RH bug #712941

ACK

> ---
>  client/application.cpp |4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/client/application.cpp b/client/application.cpp
> index 8e9fd8a..18101a5 100644
> --- a/client/application.cpp
> +++ b/client/application.cpp
> @@ -2276,11 +2276,11 @@ bool Application::process_cmd_line(int argc, char** 
> argv, bool &full_screen)
>  parser.add(SPICE_OPT_CANVAS_TYPE, "canvas-type", "set rendering canvas", 
> "canvas_type", true);
>  parser.set_multi(SPICE_OPT_CANVAS_TYPE, ',');
>  
> -parser.add(SPICE_OPT_DISPLAY_COLOR_DEPTH, "color-depth", "guest display 
> color depth",
> +parser.add(SPICE_OPT_DISPLAY_COLOR_DEPTH, "color-depth", "guest display 
> color depth (if supported by the guest vdagent)"
> "16/32", true);
>  
>  parser.add(SPICE_OPT_DISABLE_DISPLAY_EFFECTS, "disable-effects",
> -   "disable guest display effects", 
> "wallpaper/font-smooth/animation/all", true);
> +   "disable guest display effects (if supported by the guest 
> vdagent)", "wallpaper/font-smooth/animation/all", true);
>  parser.set_multi(SPICE_OPT_DISABLE_DISPLAY_EFFECTS, ',');
>  
>  parser.add(SPICE_OPT_CONTROLLER, "controller", "enable external 
> controller");
> -- 
> 1.7.6
> 
> ___
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [PATCH 3/5] x11: don't return freed memory from get_clipboard

2011-07-18 Thread Alon Levy 
On Fri, Jul 08, 2011 at 12:17:30PM +0200, Christophe Fergeau wrote:
> There is a double free in client/x11/platform.cpp.
> In get_selection(), in the exit: case with ret_val == -1 and data != NULL,
> *data_ret (which is returned to the caller) has already been
> assigned "data", so it will be pointing to freed memory when "data" is
> XFree'd'. Then in handle_selection_notify, get_selection_free is called on
> this pointer, which causes a double free.
> When the length of the read data = 0, set the returned value to NULL,
> this way subsequent free attempts will be a noop.
> Fixes RH bug #710461

ACK.

> ---
>  client/x11/platform.cpp |8 ++--
>  1 files changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/client/x11/platform.cpp b/client/x11/platform.cpp
> index 910d61e..fe98eae 100644
> --- a/client/x11/platform.cpp
> +++ b/client/x11/platform.cpp
> @@ -2575,8 +2575,12 @@ static int get_selection(XEvent &event, Atom type, 
> Atom prop, int format,
>  }
>  len = clipboard_data_size;
>  *data_ret = clipboard_data;
> -} else
> -*data_ret = data;
> +} else {
> +if (len > 0)
> +*data_ret = data;
> +else
> +*data_ret = NULL;
> +}
>  
>  if (len > 0)
>  ret_val = len;
> -- 
> 1.7.6
> 
> ___
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [PATCH 2/5] client: match delete[] with new[]

2011-07-18 Thread Alon Levy 
On Fri, Jul 08, 2011 at 12:17:29PM +0200, Christophe Fergeau wrote:
> vinfo in x11/platform.cpp is allocated using new[] so it needs to
> be freed with delete[]

ACK.

> ---
>  client/x11/platform.cpp |2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/client/x11/platform.cpp b/client/x11/platform.cpp
> index 7c31058..910d61e 100644
> --- a/client/x11/platform.cpp
> +++ b/client/x11/platform.cpp
> @@ -2881,7 +2881,7 @@ static void cleanup(void)
>  for (i = 0; i < ScreenCount(x_display); ++i) {
>  XFree(vinfo[i]);
>  }
> -delete vinfo;
> +delete[] vinfo;
>  vinfo = NULL;
>  }
>  #ifdef USE_OPENGL
> -- 
> 1.7.6
> 
> ___
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [PATCH 1/5] client: s/recive/receive

2011-07-18 Thread Alon Levy 
On Fri, Jul 08, 2011 at 12:17:28PM +0200, Christophe Fergeau wrote:
> ---
>  client/red_channel.cpp |   32 
>  client/red_channel.h   |6 +++---
>  client/red_peer.cpp|8 
>  client/red_peer.h  |4 ++--
>  4 files changed, 25 insertions(+), 25 deletions(-)
> 

ACK.

> diff --git a/client/red_channel.cpp b/client/red_channel.cpp
> index d8dcc42..5f8bd25 100644
> --- a/client/red_channel.cpp
> +++ b/client/red_channel.cpp
> @@ -122,7 +122,7 @@ void RedChannelBase::link(uint32_t connection_id, const 
> std::string& password,
>  send(buffer, p - buffer);
>  delete [] buffer;
>  
> -recive((uint8_t*)&header, sizeof(header));
> +receive((uint8_t*)&header, sizeof(header));
>  
>  if (header.magic != SPICE_MAGIC) {
>  THROW_ERR(SPICEC_ERROR_CODE_CONNECT_FAILED, "bad magic");
> @@ -139,7 +139,7 @@ void RedChannelBase::link(uint32_t connection_id, const 
> std::string& password,
>  _remote_minor = header.minor_version;
>  
>  AutoArray reply_buf(new uint8_t[header.size]);
> -recive(reply_buf.get(), header.size);
> +receive(reply_buf.get(), header.size);
>  
>  reply = (SpiceLinkReply *)reply_buf.get();
>  
> @@ -196,7 +196,7 @@ void RedChannelBase::link(uint32_t connection_id, const 
> std::string& password,
>  
>  BIO_free(bioKey);
>  
> -recive((uint8_t*)&link_res, sizeof(link_res));
> +receive((uint8_t*)&link_res, sizeof(link_res));
>  if (link_res != SPICE_LINK_ERR_OK) {
>  int error_code = (link_res == SPICE_LINK_ERR_PERMISSION_DENIED) ?
>  SPICEC_ERROR_CODE_CONNECT_FAILED : 
> SPICEC_ERROR_CODE_CONNECT_FAILED;
> @@ -359,10 +359,10 @@ void RedChannel::post_message(RedChannel::OutMessage* 
> message)
>  _send_trigger.trigger();
>  }
>  
> -RedPeer::CompoundInMessage *RedChannel::recive()
> +RedPeer::CompoundInMessage *RedChannel::receive()
>  {
> -CompoundInMessage *message = RedChannelBase::recive();
> -on_message_recived();
> +CompoundInMessage *message = RedChannelBase::receive();
> +on_message_received();
>  return message;
>  }
>  
> @@ -589,7 +589,7 @@ void RedChannel::on_send_trigger()
>  send_messages();
>  }
>  
> -void RedChannel::on_message_recived()
> +void RedChannel::on_message_received()
>  {
>  if (_message_ack_count && !--_message_ack_count) {
>  post_message(new Message(SPICE_MSGC_ACK));
> @@ -604,10 +604,10 @@ void RedChannel::on_message_complition(uint64_t serial)
>  _sync_info.condition->notify_all();
>  }
>  
> -void RedChannel::recive_messages()
> +void RedChannel::receive_messages()
>  {
>  for (;;) {
> -uint32_t n = RedPeer::recive((uint8_t*)&_incomming_header, 
> sizeof(SpiceDataHeader));
> +uint32_t n = RedPeer::receive((uint8_t*)&_incomming_header, 
> sizeof(SpiceDataHeader));
>  if (n != sizeof(SpiceDataHeader)) {
>  _incomming_header_pos = n;
>  return;
> @@ -616,13 +616,13 @@ void RedChannel::recive_messages()
>   
> _incomming_header.type,
>   
> _incomming_header.size,
>   
> _incomming_header.sub_list));
> -n = RedPeer::recive((*message)->data(), (*message)->compound_size());
> +n = RedPeer::receive((*message)->data(), 
> (*message)->compound_size());
>  if (n != (*message)->compound_size()) {
>  _incomming_message = message.release();
>  _incomming_message_pos = n;
>  return;
>  }
> -on_message_recived();
> +on_message_received();
>  _message_handler->handle_message(*(*message));
>  on_message_complition((*message)->serial());
>  }
> @@ -642,7 +642,7 @@ void RedChannel::on_event()
>  send_messages();
>  
>  if (_incomming_header_pos) {
> -_incomming_header_pos += 
> RedPeer::recive(((uint8_t*)&_incomming_header) +
> +_incomming_header_pos += 
> RedPeer::receive(((uint8_t*)&_incomming_header) +
>   _incomming_header_pos,
>   sizeof(SpiceDataHeader) - 
> _incomming_header_pos);
>  if (_incomming_header_pos != sizeof(SpiceDataHeader)) {
> @@ -657,7 +657,7 @@ void RedChannel::on_event()
>  }
>  
>  if (_incomming_message) {
> -_incomming_message_pos += RedPeer::recive(_incomming_message->data() 
> +
> +_incomming_message_pos += 
> RedPeer::receive(_incomming_message->data() +
>_incomming_message_pos,
>
> _incomming_message->compound_size() -
>_incomming_message_pos);
> @@ -666,11 +666,11 @@ void RedChannel::on_event()
>  }
>

Re: [Spice-devel] [PATCH 5/5] client: don't crash when agent is missing WAN support

2011-07-18 Thread Alon Levy 
On Fri, Jul 08, 2011 at 12:17:32PM +0200, Christophe Fergeau wrote:
> If you try to connect to a linux guest with WAN options, SPICE window opens up
> and is blank - it then fails with vdagent timeout message.  It should give a
> warning that this is only applicable for windows guest and still connect to
> guest.
> 
> It all starts in RedClient::handle_init
> This function checks whether we have an agent or not, because if we have an
> agent, there will be some kind of handshake to check both sides capabilities
> before all the spice channels are created.
> 
> When there is no agent running, the startup process goes on with
> SPICE_MSGC_MAIN_ATTACH_CHANNELS
> 
> When there is a windows agent running, VD_AGENT_ANNOUNCE_CAPABILITIES and
> VD_AGENT_DISPLAY_CONFIG messages are sent to the agent, and when processing 
> the
> agent answer to the VD_AGENT_DISPLAY_CONFIG message,
> SPICE_MSGC_MAIN_ATTACH_CHANNELS will be sent and the startup process will go
> on.
> 
> However, when there is no agent running but --color-depth was used, 
> handle_init
> won't send the SPICE_MSGC_MAIN_ATTACH_CHANNELS message but will wait for the
> agent handshake to proceed to its end, which won't happen, so it will timeout
> waiting for agent answers.
> 
> Similarly, the linux agent handles VD_AGENT_ANNOUNCE_CAPABILITIES messages, 
> but
> it doesn't handle VD_AGENT_DISPLAY_CONFIG messages, so we'll never reach the
> point where a SPICE_MSGC_MAIN_ATTACH_CHANNELS will be sent.
> 
> This commit fixes this in 2 places:
> - unconditionnally send SPICE_MSGC_ATTACH_CHANNELS when no agent is running in
> handle_init
> - send SPICE_MSGC_MAIN_ATTACH_CHANNELS in
> RedClient::on_agent_announce_capabilities if the agent doesn't have the
> VD_AGENT_CAP_DISPLAY_CONFIG capability
> 
> This fixes RH bug #712938
> ---
>  client/red_client.cpp |   28 
>  1 files changed, 20 insertions(+), 8 deletions(-)
> 
> diff --git a/client/red_client.cpp b/client/red_client.cpp
> index 8918e4f..edcdb02 100644
> --- a/client/red_client.cpp
> +++ b/client/red_client.cpp
> @@ -966,19 +966,20 @@ void RedClient::handle_init(RedPeer::InMessage* message)
>  agent_start.num_tokens = ~0;
>  _marshallers->msgc_main_agent_start(msg->marshaller(), &agent_start);
>  post_message(msg);
> -}
> -
> -if (_agent_connected) {

Why do we want to send agent_announce_capabilities if !_agent_connected?

>  send_agent_announce_capabilities(true);
>  if (_auto_display_res) {
> send_agent_monitors_config();
>  }
> -}
> -
> -if (!_auto_display_res && _display_setting.is_empty()) {
> -post_message(new Message(SPICE_MSGC_MAIN_ATTACH_CHANNELS));
> +if (_auto_display_res || !_display_setting.is_empty()) {
> +_application.activate_interval_timer(*_agent_timer, 
> AGENT_TIMEOUT);
> +} else {
> +post_message(new Message(SPICE_MSGC_MAIN_ATTACH_CHANNELS));
> +}
>  } else {
> -_application.activate_interval_timer(*_agent_timer, AGENT_TIMEOUT);
> +if (_auto_display_res || !_display_setting.is_empty()) {
> +LOG_WARN("no agent running, display options have been ignored");
> +}
> +post_message(new Message(SPICE_MSGC_MAIN_ATTACH_CHANNELS));
>  }
>  }
>  
> @@ -1046,6 +1047,17 @@ void RedClient::on_agent_announce_capabilities(
>  // not sending the color depth through send_agent_monitors_config, 
> since
>  // it applies only for attached screens.
>  send_agent_display_config();
> +} else if (!_auto_display_res) {

Who sets _auto_display_res? does this affect windows guest agents? the comment
below says linux but the test above seems to be not linux specific.

> +/* linux agent doesn't support monitors/displays agent messages, so
> + * we'll never reach on_agent_reply which sends this
> + * ATTACH_CHANNELS message which is needed for client startup to go
> + * on.
> + */
> +if (_auto_display_res || !_display_setting.is_empty()) {
> +LOG_WARN("display options have been requested, but the agent 
> doesn't support these options");
> +}
> +post_message(new Message(SPICE_MSGC_MAIN_ATTACH_CHANNELS));
> +_application.deactivate_interval_timer(*_agent_timer);
>  }
>  }
>  
> -- 
> 1.7.6
> 
> ___
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [PATCH] Fix spice-server/qemu channel version checks

2011-07-18 Thread Alon Levy 
On Mon, Jul 18, 2011 at 04:16:59PM +0200, Christophe Fergeau wrote:
> Ping?
> 
ACK.

> On Fri, Jul 08, 2011 at 03:58:28PM +0200, Christophe Fergeau wrote:
> > When qemu creates a channel, reds.c contains code to check the
> > minor/major channel versions known to QEMU (ie the ones that were
> > current in spice-server when QEMU was compiled) and to compare these
> > versions against the current ones the currently installed spice-server
> > version.
> > 
> > According to kraxel [1], the rules for these interface numbers are:
> > 
> > "The purpose of the versions is exactly to avoid the need for a new
> > soname.  The rules are basically:
> > 
> >(1) You add stuff to the interface, strictly append-only to not break
> >binary compatibility.
> >(2) You bump the minor version of the interface.
> >(3) You check the minor version at runtime to figure whenever the
> >added fields contain valid stuff or not.
> > 
> > An example is here (core interface, minor goes from 2 to 3, new
> > channel_event callback):
> > 
> > http://cgit.freedesktop.org/spice/spice/commit/?id=97f33fa86aa6edd25111b173dc0d9599ac29f879
> > "
> > 
> > The code currently refuses to create a channel if QEMU minor version is
> > less than the current spice-server version. This does not correspond
> > to the intended behaviour, this patch changes to fail is qemu was compiled
> > with a spice-server that is *newer* than the one currently installed. This
> > case is something we cannot support nicely.
> > 
> > [1] http://lists.freedesktop.org/archives/spice-devel/2011-July/004440.html
> > ---
> >  server/reds.c |   16 
> >  1 files changed, 8 insertions(+), 8 deletions(-)
> > 
> > diff --git a/server/reds.c b/server/reds.c
> > index ca6cf4d..ee24e87 100644
> > --- a/server/reds.c
> > +++ b/server/reds.c
> > @@ -3270,7 +3270,7 @@ SPICE_GNUC_VISIBLE int 
> > spice_server_add_interface(SpiceServer *s,
> >  if (strcmp(interface->type, SPICE_INTERFACE_KEYBOARD) == 0) {
> >  red_printf("SPICE_INTERFACE_KEYBOARD");
> >  if (interface->major_version != SPICE_INTERFACE_KEYBOARD_MAJOR ||
> > -interface->minor_version < SPICE_INTERFACE_KEYBOARD_MINOR) {
> > +interface->minor_version > SPICE_INTERFACE_KEYBOARD_MINOR) {
> >  red_printf("unsupported keyboard interface");
> >  return -1;
> >  }
> > @@ -3280,7 +3280,7 @@ SPICE_GNUC_VISIBLE int 
> > spice_server_add_interface(SpiceServer *s,
> >  } else if (strcmp(interface->type, SPICE_INTERFACE_MOUSE) == 0) {
> >  red_printf("SPICE_INTERFACE_MOUSE");
> >  if (interface->major_version != SPICE_INTERFACE_MOUSE_MAJOR ||
> > -interface->minor_version < SPICE_INTERFACE_MOUSE_MINOR) {
> > +interface->minor_version > SPICE_INTERFACE_MOUSE_MINOR) {
> >  red_printf("unsupported mouse interface");
> >  return -1;
> >  }
> > @@ -3292,7 +3292,7 @@ SPICE_GNUC_VISIBLE int 
> > spice_server_add_interface(SpiceServer *s,
> >  
> >  red_printf("SPICE_INTERFACE_QXL");
> >  if (interface->major_version != SPICE_INTERFACE_QXL_MAJOR ||
> > -interface->minor_version < SPICE_INTERFACE_QXL_MINOR) {
> > +interface->minor_version > SPICE_INTERFACE_QXL_MINOR) {
> >  red_printf("unsupported qxl interface");
> >  return -1;
> >  }
> > @@ -3305,7 +3305,7 @@ SPICE_GNUC_VISIBLE int 
> > spice_server_add_interface(SpiceServer *s,
> >  } else if (strcmp(interface->type, SPICE_INTERFACE_TABLET) == 0) {
> >  red_printf("SPICE_INTERFACE_TABLET");
> >  if (interface->major_version != SPICE_INTERFACE_TABLET_MAJOR ||
> > -interface->minor_version < SPICE_INTERFACE_TABLET_MINOR) {
> > +interface->minor_version > SPICE_INTERFACE_TABLET_MINOR) {
> >  red_printf("unsupported tablet interface");
> >  return -1;
> >  }
> > @@ -3320,7 +3320,7 @@ SPICE_GNUC_VISIBLE int 
> > spice_server_add_interface(SpiceServer *s,
> >  } else if (strcmp(interface->type, SPICE_INTERFACE_PLAYBACK) == 0) {
> >  red_printf("SPICE_INTERFACE_PLAYBACK");
> >  if (interface->major_version != SPICE_INTERFACE_PLAYBACK_MAJOR ||
> > -interface->minor_version < SPICE_INTERFACE_PLAYBACK_MINOR) {
> > +interface->minor_version > SPICE_INTERFACE_PLAYBACK_MINOR) {
> >  red_printf("unsupported playback interface");
> >  return -1;
> >  }
> > @@ -3329,7 +3329,7 @@ SPICE_GNUC_VISIBLE int 
> > spice_server_add_interface(SpiceServer *s,
> >  } else if (strcmp(interface->type, SPICE_INTERFACE_RECORD) == 0) {
> >  red_printf("SPICE_INTERFACE_RECORD");
> >  if (interface->major_version != SPICE_INTERFACE_RECORD_MAJOR ||
> > -interface->minor_version < SPICE_INTERFACE_RECORD_MINOR) {
> > +interface->minor_version > SPICE_INTERFACE_RECORD_MINOR) {
>

Re: [Spice-devel] [PATCH] Fix spice-server/qemu channel version checks

2011-07-18 Thread Christophe Fergeau 
Ping?

On Fri, Jul 08, 2011 at 03:58:28PM +0200, Christophe Fergeau wrote:
> When qemu creates a channel, reds.c contains code to check the
> minor/major channel versions known to QEMU (ie the ones that were
> current in spice-server when QEMU was compiled) and to compare these
> versions against the current ones the currently installed spice-server
> version.
> 
> According to kraxel [1], the rules for these interface numbers are:
> 
> "The purpose of the versions is exactly to avoid the need for a new
> soname.  The rules are basically:
> 
>(1) You add stuff to the interface, strictly append-only to not break
>binary compatibility.
>(2) You bump the minor version of the interface.
>(3) You check the minor version at runtime to figure whenever the
>added fields contain valid stuff or not.
> 
> An example is here (core interface, minor goes from 2 to 3, new
> channel_event callback):
> 
> http://cgit.freedesktop.org/spice/spice/commit/?id=97f33fa86aa6edd25111b173dc0d9599ac29f879
> "
> 
> The code currently refuses to create a channel if QEMU minor version is
> less than the current spice-server version. This does not correspond
> to the intended behaviour, this patch changes to fail is qemu was compiled
> with a spice-server that is *newer* than the one currently installed. This
> case is something we cannot support nicely.
> 
> [1] http://lists.freedesktop.org/archives/spice-devel/2011-July/004440.html
> ---
>  server/reds.c |   16 
>  1 files changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/server/reds.c b/server/reds.c
> index ca6cf4d..ee24e87 100644
> --- a/server/reds.c
> +++ b/server/reds.c
> @@ -3270,7 +3270,7 @@ SPICE_GNUC_VISIBLE int 
> spice_server_add_interface(SpiceServer *s,
>  if (strcmp(interface->type, SPICE_INTERFACE_KEYBOARD) == 0) {
>  red_printf("SPICE_INTERFACE_KEYBOARD");
>  if (interface->major_version != SPICE_INTERFACE_KEYBOARD_MAJOR ||
> -interface->minor_version < SPICE_INTERFACE_KEYBOARD_MINOR) {
> +interface->minor_version > SPICE_INTERFACE_KEYBOARD_MINOR) {
>  red_printf("unsupported keyboard interface");
>  return -1;
>  }
> @@ -3280,7 +3280,7 @@ SPICE_GNUC_VISIBLE int 
> spice_server_add_interface(SpiceServer *s,
>  } else if (strcmp(interface->type, SPICE_INTERFACE_MOUSE) == 0) {
>  red_printf("SPICE_INTERFACE_MOUSE");
>  if (interface->major_version != SPICE_INTERFACE_MOUSE_MAJOR ||
> -interface->minor_version < SPICE_INTERFACE_MOUSE_MINOR) {
> +interface->minor_version > SPICE_INTERFACE_MOUSE_MINOR) {
>  red_printf("unsupported mouse interface");
>  return -1;
>  }
> @@ -3292,7 +3292,7 @@ SPICE_GNUC_VISIBLE int 
> spice_server_add_interface(SpiceServer *s,
>  
>  red_printf("SPICE_INTERFACE_QXL");
>  if (interface->major_version != SPICE_INTERFACE_QXL_MAJOR ||
> -interface->minor_version < SPICE_INTERFACE_QXL_MINOR) {
> +interface->minor_version > SPICE_INTERFACE_QXL_MINOR) {
>  red_printf("unsupported qxl interface");
>  return -1;
>  }
> @@ -3305,7 +3305,7 @@ SPICE_GNUC_VISIBLE int 
> spice_server_add_interface(SpiceServer *s,
>  } else if (strcmp(interface->type, SPICE_INTERFACE_TABLET) == 0) {
>  red_printf("SPICE_INTERFACE_TABLET");
>  if (interface->major_version != SPICE_INTERFACE_TABLET_MAJOR ||
> -interface->minor_version < SPICE_INTERFACE_TABLET_MINOR) {
> +interface->minor_version > SPICE_INTERFACE_TABLET_MINOR) {
>  red_printf("unsupported tablet interface");
>  return -1;
>  }
> @@ -3320,7 +3320,7 @@ SPICE_GNUC_VISIBLE int 
> spice_server_add_interface(SpiceServer *s,
>  } else if (strcmp(interface->type, SPICE_INTERFACE_PLAYBACK) == 0) {
>  red_printf("SPICE_INTERFACE_PLAYBACK");
>  if (interface->major_version != SPICE_INTERFACE_PLAYBACK_MAJOR ||
> -interface->minor_version < SPICE_INTERFACE_PLAYBACK_MINOR) {
> +interface->minor_version > SPICE_INTERFACE_PLAYBACK_MINOR) {
>  red_printf("unsupported playback interface");
>  return -1;
>  }
> @@ -3329,7 +3329,7 @@ SPICE_GNUC_VISIBLE int 
> spice_server_add_interface(SpiceServer *s,
>  } else if (strcmp(interface->type, SPICE_INTERFACE_RECORD) == 0) {
>  red_printf("SPICE_INTERFACE_RECORD");
>  if (interface->major_version != SPICE_INTERFACE_RECORD_MAJOR ||
> -interface->minor_version < SPICE_INTERFACE_RECORD_MINOR) {
> +interface->minor_version > SPICE_INTERFACE_RECORD_MINOR) {
>  red_printf("unsupported record interface");
>  return -1;
>  }
> @@ -3337,7 +3337,7 @@ SPICE_GNUC_VISIBLE int 
> spice_server_add_interface(SpiceServer *s,
>  
>  } else if (strcmp(interface->type, SPICE_INTERFACE_CHAR_DEVICE) == 0)

Re: [Spice-devel] [PATCH 1/5] client: s/recive/receive

2011-07-18 Thread Christophe Fergeau 
Ping for this series?

On Fri, Jul 08, 2011 at 12:17:28PM +0200, Christophe Fergeau wrote:
> ---
>  client/red_channel.cpp |   32 
>  client/red_channel.h   |6 +++---
>  client/red_peer.cpp|8 
>  client/red_peer.h  |4 ++--
>  4 files changed, 25 insertions(+), 25 deletions(-)
> 
> diff --git a/client/red_channel.cpp b/client/red_channel.cpp
> index d8dcc42..5f8bd25 100644
> --- a/client/red_channel.cpp
> +++ b/client/red_channel.cpp
> @@ -122,7 +122,7 @@ void RedChannelBase::link(uint32_t connection_id, const 
> std::string& password,
>  send(buffer, p - buffer);
>  delete [] buffer;
>  
> -recive((uint8_t*)&header, sizeof(header));
> +receive((uint8_t*)&header, sizeof(header));
>  
>  if (header.magic != SPICE_MAGIC) {
>  THROW_ERR(SPICEC_ERROR_CODE_CONNECT_FAILED, "bad magic");
> @@ -139,7 +139,7 @@ void RedChannelBase::link(uint32_t connection_id, const 
> std::string& password,
>  _remote_minor = header.minor_version;
>  
>  AutoArray reply_buf(new uint8_t[header.size]);
> -recive(reply_buf.get(), header.size);
> +receive(reply_buf.get(), header.size);
>  
>  reply = (SpiceLinkReply *)reply_buf.get();
>  
> @@ -196,7 +196,7 @@ void RedChannelBase::link(uint32_t connection_id, const 
> std::string& password,
>  
>  BIO_free(bioKey);
>  
> -recive((uint8_t*)&link_res, sizeof(link_res));
> +receive((uint8_t*)&link_res, sizeof(link_res));
>  if (link_res != SPICE_LINK_ERR_OK) {
>  int error_code = (link_res == SPICE_LINK_ERR_PERMISSION_DENIED) ?
>  SPICEC_ERROR_CODE_CONNECT_FAILED : 
> SPICEC_ERROR_CODE_CONNECT_FAILED;
> @@ -359,10 +359,10 @@ void RedChannel::post_message(RedChannel::OutMessage* 
> message)
>  _send_trigger.trigger();
>  }
>  
> -RedPeer::CompoundInMessage *RedChannel::recive()
> +RedPeer::CompoundInMessage *RedChannel::receive()
>  {
> -CompoundInMessage *message = RedChannelBase::recive();
> -on_message_recived();
> +CompoundInMessage *message = RedChannelBase::receive();
> +on_message_received();
>  return message;
>  }
>  
> @@ -589,7 +589,7 @@ void RedChannel::on_send_trigger()
>  send_messages();
>  }
>  
> -void RedChannel::on_message_recived()
> +void RedChannel::on_message_received()
>  {
>  if (_message_ack_count && !--_message_ack_count) {
>  post_message(new Message(SPICE_MSGC_ACK));
> @@ -604,10 +604,10 @@ void RedChannel::on_message_complition(uint64_t serial)
>  _sync_info.condition->notify_all();
>  }
>  
> -void RedChannel::recive_messages()
> +void RedChannel::receive_messages()
>  {
>  for (;;) {
> -uint32_t n = RedPeer::recive((uint8_t*)&_incomming_header, 
> sizeof(SpiceDataHeader));
> +uint32_t n = RedPeer::receive((uint8_t*)&_incomming_header, 
> sizeof(SpiceDataHeader));
>  if (n != sizeof(SpiceDataHeader)) {
>  _incomming_header_pos = n;
>  return;
> @@ -616,13 +616,13 @@ void RedChannel::recive_messages()
>   
> _incomming_header.type,
>   
> _incomming_header.size,
>   
> _incomming_header.sub_list));
> -n = RedPeer::recive((*message)->data(), (*message)->compound_size());
> +n = RedPeer::receive((*message)->data(), 
> (*message)->compound_size());
>  if (n != (*message)->compound_size()) {
>  _incomming_message = message.release();
>  _incomming_message_pos = n;
>  return;
>  }
> -on_message_recived();
> +on_message_received();
>  _message_handler->handle_message(*(*message));
>  on_message_complition((*message)->serial());
>  }
> @@ -642,7 +642,7 @@ void RedChannel::on_event()
>  send_messages();
>  
>  if (_incomming_header_pos) {
> -_incomming_header_pos += 
> RedPeer::recive(((uint8_t*)&_incomming_header) +
> +_incomming_header_pos += 
> RedPeer::receive(((uint8_t*)&_incomming_header) +
>   _incomming_header_pos,
>   sizeof(SpiceDataHeader) - 
> _incomming_header_pos);
>  if (_incomming_header_pos != sizeof(SpiceDataHeader)) {
> @@ -657,7 +657,7 @@ void RedChannel::on_event()
>  }
>  
>  if (_incomming_message) {
> -_incomming_message_pos += RedPeer::recive(_incomming_message->data() 
> +
> +_incomming_message_pos += 
> RedPeer::receive(_incomming_message->data() +
>_incomming_message_pos,
>
> _incomming_message->compound_size() -
>_incomming_message_pos);
> @@ -666,11 +666,11 @@ void RedChannel::on_event()
>

Re: [Spice-devel] [PATCH] common: add backtrace via gstack

2011-07-18 Thread Christophe Fergeau 
On Mon, Jul 18, 2011 at 01:19:05PM +0300, Uri Lublin wrote:
> >On Mon, Jul 18, 2011 at 11:13:50AM +0200, Christophe Fergeau wrote:
> >>Or this can be checked for in configure.ac
> 
> I'm not sure configure.ac is the place to check it.

Why not? If you want to check if a function is available or not on the
system you're building on before using it, configure.ac is a good place for
that.

Christophe


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

Re: [Spice-devel] merge spice-protocol master to 0.8

2011-07-18 Thread Arnon Gilboa 

Christophe Fergeau wrote:

On Mon, Jul 18, 2011 at 12:06:35PM +0300, Alon Levy wrote:
  

Ok, I've pushed everything through except the INLINE defining patch since
that breaks windows build right now (because it treats warnings as errors
and INLINE is defined twice in common/lz_config.h, easy to fix with
cherry-pick-ing the patch from master, but I wanted to avoid that).



I thought that spice-gtk needed this INLINE, but I just tried building
spice-gtk against the 0.8 branch and things are fine.

Christophe

  
The INLINE patches to spice & spice-protocol were needed just for fixing 
the broken windows builds of the client and driver, which was not an 
issue in 0.8 but only on master.


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

Re: [Spice-devel] [PATCH] common: add backtrace via gstack

2011-07-18 Thread Uri Lublin 

On 07/18/2011 12:47 PM, Alon Levy wrote:

On Mon, Jul 18, 2011 at 11:13:50AM +0200, Christophe Fergeau wrote:

On Mon, Jul 18, 2011 at 11:57:47AM +0300, Alon Levy wrote:

On Mon, Jul 18, 2011 at 10:13:27AM +0200, Christophe Fergeau wrote:

What use case do you have in mind for this? Is it meant for use during
development when something asserts but we forgot to run it in gdb? Or do
you want to get more backtraces from crashes in bugzilla? I'm asking
because /usr/bin/gstack comes in the gdb package here, which is not likely
to be installed on "users" boxes. glibc also has backtrace(3) and
backtrace_symbols(3), but they are explicitly documented as GNU extensions.


Both I guess. Ok, so this fails the second use case. I can do both -
compile backtrack support in if it is available (there must be some
define I can use)


Or this can be checked for in configure.ac


I'm not sure configure.ac is the place to check it.





and do gstack first if available, otherwise (if
compiled in) glibc's backtrace.


Yep, though for now we can just go with the gstack based trace, and add the
backtrace() code later if it's really needed.

Christophe


I'll take that as an ack.


Maybe just check (stat) that /usr/bin/gstack exists before fork+exec (although 
it's already safe as in such a scenario exec fails and child exits)


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

Re: [Spice-devel] merge spice-protocol master to 0.8

2011-07-18 Thread Christophe Fergeau 
On Mon, Jul 18, 2011 at 12:06:35PM +0300, Alon Levy wrote:
> Ok, I've pushed everything through except the INLINE defining patch since
> that breaks windows build right now (because it treats warnings as errors
> and INLINE is defined twice in common/lz_config.h, easy to fix with
> cherry-pick-ing the patch from master, but I wanted to avoid that).

I thought that spice-gtk needed this INLINE, but I just tried building
spice-gtk against the 0.8 branch and things are fine.

Christophe



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

Re: [Spice-devel] [PATCH] common: add backtrace via gstack

2011-07-18 Thread Alon Levy 
On Mon, Jul 18, 2011 at 11:13:50AM +0200, Christophe Fergeau wrote:
> On Mon, Jul 18, 2011 at 11:57:47AM +0300, Alon Levy wrote:
> > On Mon, Jul 18, 2011 at 10:13:27AM +0200, Christophe Fergeau wrote:
> > > What use case do you have in mind for this? Is it meant for use during
> > > development when something asserts but we forgot to run it in gdb? Or do
> > > you want to get more backtraces from crashes in bugzilla? I'm asking
> > > because /usr/bin/gstack comes in the gdb package here, which is not likely
> > > to be installed on "users" boxes. glibc also has backtrace(3) and
> > > backtrace_symbols(3), but they are explicitly documented as GNU 
> > > extensions.
> > 
> > Both I guess. Ok, so this fails the second use case. I can do both -
> > compile backtrack support in if it is available (there must be some
> > define I can use)
> 
> Or this can be checked for in configure.ac
> 
> 
> > and do gstack first if available, otherwise (if
> > compiled in) glibc's backtrace.
> 
> Yep, though for now we can just go with the gstack based trace, and add the
> backtrace() code later if it's really needed.
> 
> Christophe

I'll take that as an ack.

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

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

Re: [Spice-devel] merge spice-protocol master to 0.8

2011-07-18 Thread Liang Guo 
2011/7/18 Alon Levy :
> On Mon, Jul 18, 2011 at 10:28:13AM +0200, Christophe Fergeau wrote:
>> Hi,
>>
>> On Sun, Jul 17, 2011 at 06:12:57PM +0300, Alon Levy wrote:
>> >  Going over all the changes in master from 0.8 branch, there is nothing
>> >  that breaks an old client or server afaict. Can anyone give a good
>> >  reason not to git merge master 0.8?
>>
>> Yes, I went through these commits too and didn't spot anything bad.
>>
>> Christophe
>
> Ok, I've pushed everything through except the INLINE defining patch since 
> that breaks
> windows build right now (because it treats warnings as errors and INLINE is 
> defined twice
> in common/lz_config.h, easy to fix with cherry-pick-ing the patch from 
> master, but I wanted
> to avoid that). I've put an updated dist tarballs on spice-space (0.8.1), 
> pushed to spice-space
> and I'm trying to do a fedora package and later a rhel package.
>

Hi, It looks like spice-protocol can be downloaded from spice-space,
but the download link in

http://www.spice-space.org/download.html

have not updated, would you like update it ?

I'll update spice-protocol to 0.8.1 in debian/sid soon.
-- 
Liang GUO
http://bluestone.cublog.cn
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [PATCH] common: add backtrace via gstack

2011-07-18 Thread Christophe Fergeau 
On Mon, Jul 18, 2011 at 11:57:47AM +0300, Alon Levy wrote:
> On Mon, Jul 18, 2011 at 10:13:27AM +0200, Christophe Fergeau wrote:
> > What use case do you have in mind for this? Is it meant for use during
> > development when something asserts but we forgot to run it in gdb? Or do
> > you want to get more backtraces from crashes in bugzilla? I'm asking
> > because /usr/bin/gstack comes in the gdb package here, which is not likely
> > to be installed on "users" boxes. glibc also has backtrace(3) and
> > backtrace_symbols(3), but they are explicitly documented as GNU extensions.
> 
> Both I guess. Ok, so this fails the second use case. I can do both -
> compile backtrack support in if it is available (there must be some
> define I can use)

Or this can be checked for in configure.ac


> and do gstack first if available, otherwise (if
> compiled in) glibc's backtrace.

Yep, though for now we can just go with the gstack based trace, and add the
backtrace() code later if it's really needed.

Christophe


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

Re: [Spice-devel] merge spice-protocol master to 0.8

2011-07-18 Thread Alon Levy 
On Mon, Jul 18, 2011 at 10:28:13AM +0200, Christophe Fergeau wrote:
> Hi,
> 
> On Sun, Jul 17, 2011 at 06:12:57PM +0300, Alon Levy wrote:
> >  Going over all the changes in master from 0.8 branch, there is nothing
> >  that breaks an old client or server afaict. Can anyone give a good
> >  reason not to git merge master 0.8?
> 
> Yes, I went through these commits too and didn't spot anything bad.
> 
> Christophe

Ok, I've pushed everything through except the INLINE defining patch since that 
breaks
windows build right now (because it treats warnings as errors and INLINE is 
defined twice
in common/lz_config.h, easy to fix with cherry-pick-ing the patch from master, 
but I wanted
to avoid that). I've put an updated dist tarballs on spice-space (0.8.1), 
pushed to spice-space
and I'm trying to do a fedora package and later a rhel package.


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

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

Re: [Spice-devel] [PATCH] common: add backtrace via gstack

2011-07-18 Thread Alon Levy 
On Mon, Jul 18, 2011 at 10:13:27AM +0200, Christophe Fergeau wrote:
> On Thu, Jul 14, 2011 at 10:50:49AM +0300, Alon Levy wrote:
> > Add a backtrace printing function copied from xserver os/backtrace.c
> > that uses gstack which seems to be available enough that xserver uses it :)
> > Used in ASSERT, tested on F15.
> 
> What use case do you have in mind for this? Is it meant for use during
> development when something asserts but we forgot to run it in gdb? Or do
> you want to get more backtraces from crashes in bugzilla? I'm asking
> because /usr/bin/gstack comes in the gdb package here, which is not likely
> to be installed on "users" boxes. glibc also has backtrace(3) and
> backtrace_symbols(3), but they are explicitly documented as GNU extensions.

Both I guess. Ok, so this fails the second use case. I can do both - compile 
backtrack
support in if it is available (there must be some define I can use) and do 
gstack first
if available, otherwise (if compiled in) glibc's backtrace.

> 
> Apart from this, this is a good idea to have this.
> 
> Christophe


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

Re: [Spice-devel] merge spice-protocol master to 0.8

2011-07-18 Thread Christophe Fergeau 
Hi,

On Sun, Jul 17, 2011 at 06:12:57PM +0300, Alon Levy wrote:
>  Going over all the changes in master from 0.8 branch, there is nothing
>  that breaks an old client or server afaict. Can anyone give a good
>  reason not to git merge master 0.8?

Yes, I went through these commits too and didn't spot anything bad.

Christophe


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

Re: [Spice-devel] [PATCH] common: add backtrace via gstack

2011-07-18 Thread Christophe Fergeau 
On Thu, Jul 14, 2011 at 10:50:49AM +0300, Alon Levy wrote:
> Add a backtrace printing function copied from xserver os/backtrace.c
> that uses gstack which seems to be available enough that xserver uses it :)
> Used in ASSERT, tested on F15.

What use case do you have in mind for this? Is it meant for use during
development when something asserts but we forgot to run it in gdb? Or do
you want to get more backtraces from crashes in bugzilla? I'm asking
because /usr/bin/gstack comes in the gdb package here, which is not likely
to be installed on "users" boxes. glibc also has backtrace(3) and
backtrace_symbols(3), but they are explicitly documented as GNU extensions.

Apart from this, this is a good idea to have this.

Christophe


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