Re: [Spice-devel] [PATCH] common: add backtrace via gstack
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
Re: [Spice-devel] merge spice-protocol master to 0.8
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
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
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
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/7/18 Alon Levy al...@redhat.com: 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
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] [PATCH 1/5] client: s/recive/receive
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;
AutoArrayuint8_t 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()
}
AutoRefCompoundInMessage message(_incomming_message);
_incomming_message = NULL;
-on_message_recived();
+
Re: [Spice-devel] [PATCH] Fix spice-server/qemu channel version checks
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) {
if (interface-major_version != SPICE_INTERFACE_CHAR_DEVICE_MAJOR ||
-interface-minor_version SPICE_INTERFACE_CHAR_DEVICE_MINOR) {
+
Re: [Spice-devel] [PATCH 5/5] client: don't crash when agent is missing WAN support
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 3/5] x11: don't return freed memory from get_clipboard
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 5/5] client: don't crash when agent is missing WAN support
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
Re: [Spice-devel] [PATCH 5/5] client: don't crash when agent is missing WAN support
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
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 v2] common: add backtrace via gstack or glibc backtrace
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 stdint.h
#include time.h
#include stdlib.h
+#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] Fix spice-server/qemu channel version checks
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
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 stdint.h
#include time.h
#include stdlib.h
+#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] Spice QXL Driver does not work for Windows 7
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
Re: [Spice-devel] [PATCH v2] common: add backtrace via gstack or glibc backtrace
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 @@
#includestdint.h
#includetime.h
#includestdlib.h
+#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
