date:20140811

[Spice-devel] spice related qemu crash

2014-08-11 Thread David Mansfield


Hi All,

I have a qemu 1.6.2 (SRPM from F20 recompiled on Centos 7) which crashed 
with the following in the qemu log.


Not sure exactly what the timing of the messages are here, since the 
messages are not all timestamped:


main_channel_link: add main channel client
main_channel_handle_parsed: agent start
main_channel_handle_parsed: net test: latency 64.296000 ms, bitrate 
14767917 bps (14.083783 Mbps)

red_dispatcher_set_cursor_peer:
inputs_connect: inputs channel client create
snd_playback_send: snd_send_playback_write failed
id 0, group 0, virt start 0, virt end , generation 0, 
delta 0
id 1, group 1, virt start 7f3d9800, virt end 7f3d9fffe000, 
generation 0, delta 7f3d9800
id 2, group 1, virt start 7f3d8ee0, virt end 7f3d96e0, 
generation 0, delta 7f3d8ee0
((null):2788): Spice-CRITICAL **: red_memslots.c:123:get_virt: slot_id 
193 too big, addr=c1f9c000

2014-08-08 22:14:56.563+: shutting down

Any idea what this could be ?  Google can't come up with anything.

Are specific QEMU releases tied to specific spice-server versions in 
magic ways?  I upgraded qemu but did NOT update spice-server.  Maybe 
this is relevant.


--
Thanks,
David Mansfield
Cobite, INC.



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

Re: [Spice-devel] [PATCH] [spice-gtk] DND: fix special case that file size is 0

2014-08-11 Thread Marc-André Lureau



- Original Message -
> 
> 
> 
> On Tue, Aug 12, 2014 at 12:58 AM, Marc-André Lureau < mlur...@redhat.com >
> wrote:
> 
> 
> 
> 
> - Original Message -
> > On windows guest, after dra gging a zero-size file, the file will be
> > occupied
> > by vdagent. For spice-gtk ignores this case, and stops sending data after
> > sending
> > a start message to agent, see here for more details:
> > http://lists.freedesktop.org/archives/spice-devel/2014-August/017184.html
> > I accept the suggestion of Christophe and fix the bug in spice-gtk.
> > Pay attention to the modification of
> > 
> > agent_msg_queue_many, I think it's a small trick.
> 
> Isn't this changing the protocol?
> 
> We use to send xfer-start of data size 0 and then xfer-data of data size 0,
> and now we skip the second part.
> 
> That's an an incompatible change. I would prefer if we keep sending the
> xfer-data message, even if it is of size 0.
> 
> Or if you prefer don't initiate xfer at all if the file size is 0. But I
> don't think that's worth it, and it would be some "racy" code (check size
> before doing actual operation)
> You mean client shouldn't send a zero-size file?I think sending such file is
> meaningless, too.
> And I prefer this can be fixed through a more comfortable way.

I was talking from a protocol pov. But actually, sending 0-size file is 
meaningful in some cases. So it should keep sending it.
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [PATCH] [spice-gtk] DND: fix special case that file size is 0

2014-08-11 Thread Cody Chan

On Tue, Aug 12, 2014 at 12:58 AM, Marc-André Lureau 
wrote:

>
>
> - Original Message -
> > On windows guest, after dra gging a zero-size file, the file will be
> occupied
> > by vdagent. For spice-gtk ignores this case, and stops sending data after
> > sending
> > a start message to agent, see here for more details:
> >
> http://lists.freedesktop.org/archives/spice-devel/2014-August/017184.html
> > I accept the suggestion of Christophe and fix the bug in spice-gtk.
> > Pay attention to the modification of
> > 
> > agent_msg_queue_many, I think it's a small trick.
>
> Isn't this changing the protocol?
>
> We use to send xfer-start of data size 0 and then xfer-data of data size
> 0, and now we skip the second part.
>
> That's an an incompatible change. I would prefer if we keep sending the
> xfer-data message, even if it is of size 0.
>
> Or if you prefer don't initiate xfer at all if the file size is 0. But I
> don't think that's worth it, and it would be some "racy" code (check size
> before doing actual operation)

You mean client shouldn't send a zero-size file?I think sending such file
is meaningless, too.
And I prefer this can be fixed through a more comfortable way.

> > 
> > 
> > ---
> > gtk/channel-main.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/gtk/channel-main.c b/gtk/channel-main.c
> > index 7a299a4..1e8e3f5 100644
> > --- a/gtk/channel-main.c
> > +++ b/gtk/channel-main.c
> > @@ -936,7 +936,7 @@ static void
> > 
> > agent_msg_queue_many(SpiceMainChannel *channel, int type, const void
> > memcpy(payload, &msg, sizeof(VDAgentMessage));
> > payload += sizeof(VDAgentMessage);
> > paysize -= sizeof(VDAgentMessage);
> > - if (paysize == 0) {
> > + if (size != 0 && paysize == 0) {
> > g_queue_push_tail(c->agent_msg_queue, out);
> > out = NULL;
> > }
> > @@ -1637,7 +1637,7 @@ static void file_xfer_read_cb(GObject
> *source_object,
> > return;
> > }
> >
> > - if (count > 0) {
> > + if (count >= 0) {
> > task->read_bytes += count;
> > file_xfer_queue(task, count);
> > file_xfer_flush_async(channel, task->cancellable,
> > --
> > 1.9.3 
> >
> > --
> > Q S B D T 0 R F U i B G U k 9 N I F J J R V N U I E 9 G I E N U U 0V V
> >
> > ___
> > Spice-devel mailing list
> > Spice-devel@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/spice-devel
> >
>



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

Re: [Spice-devel] [PATCH] [spice-gtk] DND: fix special case that file size is 0

2014-08-11 Thread Marc-André Lureau



- Original Message -
> On windows guest, after dra gging a zero-size file, the file will be occupied
> by vdagent. For spice-gtk ignores this case, and stops sending data after
> sending
> a start message to agent, see here for more details:
> http://lists.freedesktop.org/archives/spice-devel/2014-August/017184.html
> I accept the suggestion of Christophe and fix the bug in spice-gtk.
> Pay attention to the modification of
> 
> agent_msg_queue_many, I think it's a small trick.

Isn't this changing the protocol?

We use to send xfer-start of data size 0 and then xfer-data of data size 0, and 
now we skip the second part.

That's an an incompatible change. I would prefer if we keep sending the 
xfer-data message, even if it is of size 0.

Or if you prefer don't initiate xfer at all if the file size is 0. But I don't 
think that's worth it, and it would be some "racy" code (check size before 
doing actual operation)

> 
> 
> ---
> gtk/channel-main.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/gtk/channel-main.c b/gtk/channel-main.c
> index 7a299a4..1e8e3f5 100644
> --- a/gtk/channel-main.c
> +++ b/gtk/channel-main.c
> @@ -936,7 +936,7 @@ static void
> 
> agent_msg_queue_many(SpiceMainChannel *channel, int type, const void
> memcpy(payload, &msg, sizeof(VDAgentMessage));
> payload += sizeof(VDAgentMessage);
> paysize -= sizeof(VDAgentMessage);
> - if (paysize == 0) {
> + if (size != 0 && paysize == 0) {
> g_queue_push_tail(c->agent_msg_queue, out);
> out = NULL;
> }
> @@ -1637,7 +1637,7 @@ static void file_xfer_read_cb(GObject *source_object,
> return;
> }
> 
> - if (count > 0) {
> + if (count >= 0) {
> task->read_bytes += count;
> file_xfer_queue(task, count);
> file_xfer_flush_async(channel, task->cancellable,
> --
> 1.9.3 
> 
> --
> Q S B D T 0 R F U i B G U k 9 N I F J J R V N U I E 9 G I E N U U 0V V
> 
> ___
> 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] [spice-gtk] DND: fix special case that file size is 0

2014-08-11 Thread Cody Chan

On windows guest, after dragging a zero-size file, the file will be occupied
by vdagent. For spice-gtk ignores this case, and stops sending data after
sending
a start message to agent, see here for more details:
http://lists.freedesktop.org/archives/spice-devel/2014-August/017184.html
I accept the suggestion of Christophe and fix the bug in spice-gtk.
Pay attention to the modification of

agent_msg_queue_many,  I think it's a small trick.



---
 gtk/channel-main.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/gtk/channel-main.c b/gtk/channel-main.c
index 7a299a4..1e8e3f5 100644
--- a/gtk/channel-main.c
+++ b/gtk/channel-main.c
@@ -936,7 +936,7 @@ static void

agent_msg_queue_many(SpiceMainChannel *channel, int type, const void
 memcpy(payload, &msg, sizeof(VDAgentMessage));
 payload += sizeof(VDAgentMessage);
 paysize -= sizeof(VDAgentMessage);
-if (paysize == 0) {
+if (size != 0 && paysize == 0) {
 g_queue_push_tail(c->agent_msg_queue, out);
 out = NULL;
 }
@@ -1637,7 +1637,7 @@ static void file_xfer_read_cb(GObject *source_object,
 return;
 }

-if (count > 0) {
+if (count >= 0) {
 task->read_bytes += count;
 file_xfer_queue(task, count);
 file_xfer_flush_async(channel, task->cancellable,
--
1.9.3

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

Re: [Spice-devel] [PATCH] [vd_agent] close the file handler if file_size = 0

2014-08-11 Thread Cody Chan

On Mon, Aug 11, 2014 at 7:04 PM, Christophe Fergeau 
wrote:

> On Sat, Aug 09, 2014 at 06:40:22PM +0800, Cody Chan wrote:
> > After dragging a zero-size file, then I open it in guest,
> > I get a warning message box which says:
> >  "the process cannot access the file because it is being used by another
> > process".
> > And I get to know the file is occupied by vdagent. Now we look back the
> > process:
> >
> > a) When dragging a zero-size file, spice-gtk gets the name and size, then
> > sends
> > the message to vd_agent with VD_AGENT_FILE_XFER_START
> > b) vd_agent receives and parsers the message, then gets file name and
> size,
> > creates(open)
> > the file and gets the handler, at last, sends
> > VD_AGENT_FILE_XFER_STATUS_CAN_SEND_DATA
> > c) spice-gtk receives the VD_AGENT_FILE_XFER_STATUS_CAN_SEND_DATA
> message,
> > the sends data with VD_AGENT_FILE_XFER_DATA
> > d) vd_agent receives and writes data to the file opened
> > e) After finishing the writing, vd_agent closes the handler
> >
> > But in step c, we take a look the code:
> > //spice-channel.c
> > >static void file_xfer_read_cb(...)
> > >{
> > >//...
> > >count =
> g_input_stream_read_finish(G_INPUT_STREAM(task->file_stream),
> > res, &error);
> > >if (count > 0) {
> > >task->read_bytes += count;
> > >file_xfer_queue(task, count);
> > >file_xfer_flush_async(channel, task->cancellable,
> > >  file_xfer_data_flushed_cb, task);
> > >task->pending = TRUE;
> > >} else if (error) {
> > >VDAgentFileXferStatusMessage msg = {
> > >.id = task->id,
> > >.result = VD_AGENT_FILE_XFER_STATUS_ERROR,
> > >};
> > >agent_msg_queue_many(task->channel, VD_AGENT_FILE_XFER_STATUS,
> > > &msg, sizeof(msg), NULL);
> > >spice_channel_wakeup(SPICE_CHANNEL(task->channel), FALSE);
> > >file_xfer_completed(task, error);
> > >}
> > >}
> >
> > If count == 0, then it does nothing!
> > Then vd_agent will receive nothing after opening a file, and always
> occupy
> > the file.
> > Here we close the file if file_size = 0, even though it doesn't make
> sense
> > to send
> > a zero-size file.
>
> Ah, good catch, from a quick look it seems the linux agent does not
> handle this case properly either. This can be fixed in both agents, or
> alternatively, this can also be fixed in spice-gtk in file_xfer_read_cb
> by sending a dummy empty data message when the file size is 0.
> Fixing it in the agents is probably less convoluted.
>

As you've proposed, I fix the bug in spice-gtk, and I'll send a new patch
after testing.

>
> >
> > 
> > ---
> >  vdagent/file_xfer.cpp | 4 
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/vdagent/file_xfer.cpp b/vdagent/file_xfer.cpp
> > index 34a9ee6..17d842e 100644
> > --- a/vdagent/file_xfer.cpp
> > +++ b/vdagent/file_xfer.cpp
> > @@ -89,6 +89,10 @@ void
> FileXfer::handle_start(VDAgentFileXferStartMessage*
> > start,
> >  vd_printf("failed creating %s %lu", file_path, GetLastError());
> >  return;
> >  }
> > +if (file_size == 0){
> > +CloseHandle(handle);
> > +return;
> > +}
>
> The agent should probably return VD_AGENT_FILE_XFER_STATUS_SUCCESS in
> this case to let the client know this is over.
>
> Christophe
>



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

[Spice-devel] [PATCH] [vd_agent] fix bug: g_key_get_string() is failed when keystring contains '['

2014-08-11 Thread Cody Chan

In vd_agent/file_xfer.cpp, it implemented a simple g_key_get_string,
but  when dragging a file with a name containing '[' (like te[st.txt), it
will be failed.
>From source code,
>next_group_pos = strstr(group_pos + strlen(group_pfx), "[");
>if (next_group_pos && key_pos > next_group_pos) return false;
we know that it tries to find the end of current group by '[' label,
if we drag a file like te[st.txt, the key_string many like:
[vdagent-file-xfer]
name=te[st.txt
size=10
so, it's failed when meta parsing and
returns VD_AGENT_FILE_XFER_STATUS_ERRO message.

Here's the elegant method Christophe proposed and test ok, thanks for him
again!



---
 vdagent/file_xfer.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/vdagent/file_xfer.cpp b/vdagent/file_xfer.cpp
index 1108369..cec5579 100644
--- a/vdagent/file_xfer.cpp
+++ b/vdagent/file_xfer.cpp
@@ -205,7 +205,7 @@ bool FileXfer::g_key_get_string(char* data, const char*
group, const char* key,
 snprintf(key_pfx, sizeof(key_pfx), "\n%s=", key);
 if (!(key_pos = strstr(group_pos, key_pfx))) return false;

-next_group_pos = strstr(group_pos + strlen(group_pfx), "[");
+next_group_pos = strstr(group_pos + strlen(group_pfx), "\n[");
 if (next_group_pos && key_pos > next_group_pos) return false;

 start = key_pos + strlen(key_pfx);
--
1.9.3

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

[Spice-devel] [PATCH] [vd_agent] fix bug: display error when dragging file with CJK characters

2014-08-11 Thread Cody Chan


I submitted a patch several months ago about this issue,
here
http://lists.freedesktop.org/archives/spice-devel/2014-February/016158.html

I check it again, and find that the value of
g_key_file_to_data(keyfile,...) is always utf-8,
for the value of g_uri_list_extract_uris() is utf8 urlencode.

So the display problem is caused by vd_agent, but how it displays depends
on the
language of system, the following two screenshots show the difference:
http://int64ago-tmp.qiniudn.com/guest-Chinese.png
http://int64ago-tmp.qiniudn.com/guest-English.png





---
 vdagent/file_xfer.cpp | 13 -
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/vdagent/file_xfer.cpp b/vdagent/file_xfer.cpp
index e402eb2..1108369 100644
--- a/vdagent/file_xfer.cpp
+++ b/vdagent/file_xfer.cpp
@@ -46,6 +46,7 @@ void FileXfer::handle_start(VDAgentFileXferStartMessage*
start,
 uint64_t file_size;
 HANDLE handle;
 AsUser as_user;
+int wlen;

 status->id = start->id;
 status->result = VD_AGENT_FILE_XFER_STATUS_ERROR;
@@ -81,7 +82,17 @@ void FileXfer::handle_start(VDAgentFileXferStartMessage*
start,

 strcat(file_path, "\\");
 strcat(file_path, file_name);
-handle = CreateFileA(file_path, GENERIC_WRITE, 0, NULL, CREATE_NEW, 0,
NULL);
+if((wlen = MultiByteToWideChar(CP_UTF8, 0, file_path, -1, NULL, 0)) ==
0)
+return;
+TCHAR *wfile_path = (TCHAR*)malloc(sizeof(TCHAR) * (wlen + 1));
+if (MultiByteToWideChar(CP_UTF8, 0, file_path, -1, wfile_path, wlen)
== 0){
+if(wfile_path)
+free(wfile_path);
+return;
+}
+handle = CreateFile(wfile_path, GENERIC_WRITE, 0, NULL, CREATE_NEW, 0,
NULL);
+if (wfile_path)
+free(wfile_path);
 if (handle == INVALID_HANDLE_VALUE) {
 vd_printf("failed creating %s %lu", file_path, GetLastError());
 return;
--
1.9.3

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

Re: [Spice-devel] [PATCH] [vd_agent] fix bug: display error when dragging file with CJK characters

2014-08-11 Thread Cody Chan

Sorry, memory leaks..


On Mon, Aug 11, 2014 at 7:43 PM, Cody Chan  wrote:

> 
> I submitted a patch several months ago about this issue,
> here
> http://lists.freedesktop.org/archives/spice-devel/2014-February/016158.html
>
> I check it again, and find that the value of
> g_key_file_to_data(keyfile,...) is always utf-8,
> for the value of g_uri_list_extract_uris() is utf8 urlencode.
>
> So the display problem is caused by vd_agent, but how it displays depends
> on the
> language of system, the following two screenshots show the difference:
> http://int64ago-tmp.qiniudn.com/guest-Chinese.png
> http://int64ago-tmp.qiniudn.com/guest-English.png
>
> ---
>  vdagent/file_xfer.cpp | 10 +-
>  1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/vdagent/file_xfer.cpp b/vdagent/file_xfer.cpp
> index e402eb2..6e70d63 100644
> --- a/vdagent/file_xfer.cpp
> +++ b/vdagent/file_xfer.cpp
> @@ -46,6 +46,7 @@ void FileXfer::handle_start(VDAgentFileXferStartMessage*
> start,
>  uint64_t file_size;
>  HANDLE handle;
>  AsUser as_user;
> +int wlen;
>
>  status->id = start->id;
>  status->result = VD_AGENT_FILE_XFER_STATUS_ERROR;
> @@ -81,7 +82,14 @@ void FileXfer::handle_start(VDAgentFileXferStartMessage*
> start,
>
>  strcat(file_path, "\\");
>  strcat(file_path, file_name);
> -handle = CreateFileA(file_path, GENERIC_WRITE, 0, NULL, CREATE_NEW,
> 0, NULL);
> +if((wlen = MultiByteToWideChar(CP_UTF8, 0, file_path, -1, NULL, 0))
> == 0)
> +return;
> +TCHAR *wfile_path = (TCHAR*)malloc(sizeof(TCHAR) * (wlen + 1));
> +if (MultiByteToWideChar(CP_UTF8, 0, file_path, -1, wfile_path, wlen)
> == 0)
> +return;
> +handle = CreateFile(wfile_path, GENERIC_WRITE, 0, NULL, CREATE_NEW,
> 0, NULL);
> +if (wfile_path)
> +free(wfile_path);
>  if (handle == INVALID_HANDLE_VALUE) {
>  vd_printf("failed creating %s %lu", file_path, GetLastError());
>  return;
> --
> 1.9.3
>
> --
> QSBDT0RFUiBGUk9NIFJJRVNUIE9GIENUU0VV
>



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

[Spice-devel] [PATCH] [vd_agent] fix bug: display error when dragging file with CJK characters

2014-08-11 Thread Cody Chan


I submitted a patch several months ago about this issue,
here
http://lists.freedesktop.org/archives/spice-devel/2014-February/016158.html

I check it again, and find that the value of
g_key_file_to_data(keyfile,...) is always utf-8,
for the value of g_uri_list_extract_uris() is utf8 urlencode.

So the display problem is caused by vd_agent, but how it displays depends
on the
language of system, the following two screenshots show the difference:
http://int64ago-tmp.qiniudn.com/guest-Chinese.png
http://int64ago-tmp.qiniudn.com/guest-English.png

---
 vdagent/file_xfer.cpp | 10 +-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/vdagent/file_xfer.cpp b/vdagent/file_xfer.cpp
index e402eb2..6e70d63 100644
--- a/vdagent/file_xfer.cpp
+++ b/vdagent/file_xfer.cpp
@@ -46,6 +46,7 @@ void FileXfer::handle_start(VDAgentFileXferStartMessage*
start,
 uint64_t file_size;
 HANDLE handle;
 AsUser as_user;
+int wlen;

 status->id = start->id;
 status->result = VD_AGENT_FILE_XFER_STATUS_ERROR;
@@ -81,7 +82,14 @@ void FileXfer::handle_start(VDAgentFileXferStartMessage*
start,

 strcat(file_path, "\\");
 strcat(file_path, file_name);
-handle = CreateFileA(file_path, GENERIC_WRITE, 0, NULL, CREATE_NEW, 0,
NULL);
+if((wlen = MultiByteToWideChar(CP_UTF8, 0, file_path, -1, NULL, 0)) ==
0)
+return;
+TCHAR *wfile_path = (TCHAR*)malloc(sizeof(TCHAR) * (wlen + 1));
+if (MultiByteToWideChar(CP_UTF8, 0, file_path, -1, wfile_path, wlen)
== 0)
+return;
+handle = CreateFile(wfile_path, GENERIC_WRITE, 0, NULL, CREATE_NEW, 0,
NULL);
+if (wfile_path)
+free(wfile_path);
 if (handle == INVALID_HANDLE_VALUE) {
 vd_printf("failed creating %s %lu", file_path, GetLastError());
 return;
--
1.9.3

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

Re: [Spice-devel] [PATCH] [vd_agent] close the file handler if file_size = 0

2014-08-11 Thread Christophe Fergeau

On Sat, Aug 09, 2014 at 06:40:22PM +0800, Cody Chan wrote:
> After dragging a zero-size file, then I open it in guest,
> I get a warning message box which says:
>  "the process cannot access the file because it is being used by another
> process".
> And I get to know the file is occupied by vdagent. Now we look back the
> process:
> 
> a) When dragging a zero-size file, spice-gtk gets the name and size, then
> sends
> the message to vd_agent with VD_AGENT_FILE_XFER_START
> b) vd_agent receives and parsers the message, then gets file name and size,
> creates(open)
> the file and gets the handler, at last, sends
> VD_AGENT_FILE_XFER_STATUS_CAN_SEND_DATA
> c) spice-gtk receives the VD_AGENT_FILE_XFER_STATUS_CAN_SEND_DATA message,
> the sends data with VD_AGENT_FILE_XFER_DATA
> d) vd_agent receives and writes data to the file opened
> e) After finishing the writing, vd_agent closes the handler
> 
> But in step c, we take a look the code:
> //spice-channel.c
> >static void file_xfer_read_cb(...)
> >{
> >//...
> >count = g_input_stream_read_finish(G_INPUT_STREAM(task->file_stream),
> res, &error);
> >if (count > 0) {
> >task->read_bytes += count;
> >file_xfer_queue(task, count);
> >file_xfer_flush_async(channel, task->cancellable,
> >  file_xfer_data_flushed_cb, task);
> >task->pending = TRUE;
> >} else if (error) {
> >VDAgentFileXferStatusMessage msg = {
> >.id = task->id,
> >.result = VD_AGENT_FILE_XFER_STATUS_ERROR,
> >};
> >agent_msg_queue_many(task->channel, VD_AGENT_FILE_XFER_STATUS,
> > &msg, sizeof(msg), NULL);
> >spice_channel_wakeup(SPICE_CHANNEL(task->channel), FALSE);
> >file_xfer_completed(task, error);
> >}
> >}
> 
> If count == 0, then it does nothing!
> Then vd_agent will receive nothing after opening a file, and always occupy
> the file.
> Here we close the file if file_size = 0, even though it doesn't make sense
> to send
> a zero-size file.

Ah, good catch, from a quick look it seems the linux agent does not
handle this case properly either. This can be fixed in both agents, or
alternatively, this can also be fixed in spice-gtk in file_xfer_read_cb
by sending a dummy empty data message when the file size is 0.
Fixing it in the agents is probably less convoluted.

> 
> 
> ---
>  vdagent/file_xfer.cpp | 4 
>  1 file changed, 4 insertions(+)
> 
> diff --git a/vdagent/file_xfer.cpp b/vdagent/file_xfer.cpp
> index 34a9ee6..17d842e 100644
> --- a/vdagent/file_xfer.cpp
> +++ b/vdagent/file_xfer.cpp
> @@ -89,6 +89,10 @@ void FileXfer::handle_start(VDAgentFileXferStartMessage*
> start,
>  vd_printf("failed creating %s %lu", file_path, GetLastError());
>  return;
>  }
> +if (file_size == 0){
> +CloseHandle(handle);
> +return;
> +}

The agent should probably return VD_AGENT_FILE_XFER_STATUS_SUCCESS in
this case to let the client know this is over.

Christophe


pgpJFyzAGCE8Y.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] [vd_agent] fix bug: g_key_get_string() of vd_agent failed when keystring contains '['

2014-08-11 Thread Cody Chan

Yeah!I think yours is elegant and simple!


On Mon, Aug 11, 2014 at 6:33 PM, Christophe Fergeau 
wrote:

> Hey,
>
> On Fri, Aug 08, 2014 at 08:33:59PM +0800, Cody Chan wrote:
> > Hi, as you can see, in vd_agent/file_xfer.cpp, it implemented
> > the g_key_get_string,
> > but I find a problem, when dragging a file with a name containing '['
> (like
> > te[st.txt), it will be failed.
> > From source code,
> > >next_group_pos = strstr(group_pos + strlen(group_pfx), "[");
> > >if (next_group_pos && key_pos > next_group_pos) return false;
> > we know that it tries to find the end of current group by '[' label,
> > if we drag a file like te[st.txt, the key_string many like:
> > [vdagent-file-xfer]
> > name=te[st.txt
> > size=10
> > so, it's failed when meta parsing and
> > returns VD_AGENT_FILE_XFER_STATUS_ERRO message.
> >
> > I find a way to fix it, just replace '[' ']' with '<' '>' between group
> > name,
> > for it's illegal for windows to create a file with a name containing
> > '<'/'>'.
>
> Unfortunately, I don't think we can change the file format at this
> point, especially as this format is based on the somehow standard 'ini'
> file format, and apart from the windows agent, we use glib
> GKeyFile parser to handle this format.
> From a quick look at the code, wouldn't the patch below avoid the issue
> you described?
>
> diff --git a/vdagent/file_xfer.cpp b/vdagent/file_xfer.cpp
> index c258926..17cdb3c 100644
> --- a/vdagent/file_xfer.cpp
> +++ b/vdagent/file_xfer.cpp
> @@ -203,7 +203,7 @@ bool FileXfer::g_key_get_string(char* data, const
> char* grou
>  sprintf_s(key_pfx, sizeof(key_pfx), "\n%s=", key);
>  if (!(key_pos = strstr(group_pos, key_pfx))) return false;
>
> -next_group_pos = strstr(group_pos + strlen(group_pfx), "[");
> +next_group_pos = strstr(group_pos + strlen(group_pfx), "\n[");
>  if (next_group_pos && key_pos > next_group_pos) return false;
>
>  start = key_pos + strlen(key_pfx);
>
> Christophe
>



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

Re: [Spice-devel] [PATCH] [vd_agent] fix bug: g_key_get_string() of vd_agent failed when keystring contains '['

2014-08-11 Thread Christophe Fergeau

Hey,

On Fri, Aug 08, 2014 at 08:33:59PM +0800, Cody Chan wrote:
> Hi, as you can see, in vd_agent/file_xfer.cpp, it implemented
> the g_key_get_string,
> but I find a problem, when dragging a file with a name containing '[' (like
> te[st.txt), it will be failed.
> From source code,
> >next_group_pos = strstr(group_pos + strlen(group_pfx), "[");
> >if (next_group_pos && key_pos > next_group_pos) return false;
> we know that it tries to find the end of current group by '[' label,
> if we drag a file like te[st.txt, the key_string many like:
> [vdagent-file-xfer]
> name=te[st.txt
> size=10
> so, it's failed when meta parsing and
> returns VD_AGENT_FILE_XFER_STATUS_ERRO message.
> 
> I find a way to fix it, just replace '[' ']' with '<' '>' between group
> name,
> for it's illegal for windows to create a file with a name containing
> '<'/'>'.

Unfortunately, I don't think we can change the file format at this
point, especially as this format is based on the somehow standard 'ini'
file format, and apart from the windows agent, we use glib
GKeyFile parser to handle this format.
From a quick look at the code, wouldn't the patch below avoid the issue
you described?

diff --git a/vdagent/file_xfer.cpp b/vdagent/file_xfer.cpp
index c258926..17cdb3c 100644
--- a/vdagent/file_xfer.cpp
+++ b/vdagent/file_xfer.cpp
@@ -203,7 +203,7 @@ bool FileXfer::g_key_get_string(char* data, const
char* grou
 sprintf_s(key_pfx, sizeof(key_pfx), "\n%s=", key);
 if (!(key_pos = strstr(group_pos, key_pfx))) return false;
 
-next_group_pos = strstr(group_pos + strlen(group_pfx), "[");
+next_group_pos = strstr(group_pos + strlen(group_pfx), "\n[");
 if (next_group_pos && key_pos > next_group_pos) return false;
 
 start = key_pos + strlen(key_pfx);

Christophe


pgpt2HwjNaiV6.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 bug: display error when dragging file with CJK characters

2014-08-11 Thread Christophe Fergeau

Hey,

On Fri, Aug 08, 2014 at 02:22:30AM +0800, Cody Chan wrote:
> I submitted a patch several months ago about this issue,
> here
> http://lists.freedesktop.org/archives/spice-devel/2014-February/016158.html
> 
> I check it again, and find that the value of
> g_key_file_to_data(keyfile,...) is always utf-8,
> for the value of g_uri_list_extract_uris() is utf8 urlencode.
> 
> So the display problem is caused by vd_agent, but how it displays depends
> on the
> language of system, the following two screenshots show the difference:
> http://int64ago-tmp.qiniudn.com/guest-Chinese.png
> http://int64ago-tmp.qiniudn.com/guest-English.png
> 
> ---
>  vdagent/file_xfer.cpp | 5 -
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/vdagent/file_xfer.cpp b/vdagent/file_xfer.cpp
> index e402eb2..553a3f3 100644
> --- a/vdagent/file_xfer.cpp
> +++ b/vdagent/file_xfer.cpp
> @@ -81,7 +81,10 @@ void FileXfer::handle_start(VDAgentFileXferStartMessage*
> start,
> 
>  strcat(file_path, "\\");
>  strcat(file_path, file_name);
> -handle = CreateFileA(file_path, GENERIC_WRITE, 0, NULL, CREATE_NEW, 0,
> NULL);
> +int len = MultiByteToWideChar(CP_UTF8, 0, file_path, -1, NULL, 0);

We need to check if len is 0 as this indicates a MultiByteToWideChar
failure.

> +wchar_t wfile_path[MAX_PATH];
> +MultiByteToWideChar(CP_UTF8, 0, file_path, -1, wfile_path, len);

Shouldn't we allocate a wfile_path so that it can contain 'len'
characters? What happens if 'len' is bigger than MAX_PATH?

> +handle = CreateFileW(wfile_path, GENERIC_WRITE, 0, NULL, CREATE_NEW,
> 0, NULL);

I think CreateFile could be used here instead of CreateFileW as we are
building with -DUNICODE/-D_UNICODE ?

Christophe


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

Re: [Spice-devel] Patches welcome??

2014-08-11 Thread Christophe Fergeau

Hey Cody,

On Sun, Aug 10, 2014 at 09:01:02PM +0800, Cody Chan wrote:
> Here're four patches I've contributed, but no ACK && no challenge, any
> reason?

Things tend to be slow in August, especially during the week-end, so
don't worry, your patches will be looked at soon hopefully :)

Thanks a lot for these patches and your interest in spice!

Christophe


pgpWCjKT9ckWo.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 3/3] Fix "NULL_RETURNS" caught by coverity

2014-08-11 Thread Christophe Fergeau

On Wed, Aug 06, 2014 at 12:08:52PM +0200, Fabiano Fidêncio wrote:
> On Mon, Aug 4, 2014 at 3:39 PM, Christophe Fergeau 
> wrote:
> 
> > On Mon, Jul 14, 2014 at 01:44:45PM +0200, Fabiano Fidêncio wrote:
> > > On Mon, Jul 14, 2014 at 1:30 PM, Marc-André Lureau 
> > > wrote:
> > > > Could malloc & free do not have annotations like the glib ones.
> > > >
> > > > What happens for example if you replace malloc() by g_malloc(), and
> > free()
> > > > by g_free() in the function below?
> > > >
> > >
> > > I don't think that would help as one of the Coverity's complaints came
> > > from: "data = g_malloc(d->area.width * d->area.height * 3);"
> >
> > Did it complain about an unchecked return value?
> 
> 
> This is the message:
> Error: NULL_RETURNS (CWE-476): [#def51]
> spice-gtk-0.25.28-591b-dirty/spice-gtk-0.25.28/gtk/spice-widget.c:2564:
> returned_null: Function "g_malloc0(gsize)" returns null.
> spice-gtk-0.25.28-591b-dirty/spice-gtk-0.25.28/gtk/spice-widget.c:2564:
> var_assigned: Assigning: "data" = null return value from "g_malloc0(gsize)".
> spice-gtk-0.25.28-591b-dirty/spice-gtk-0.25.28/gtk/spice-widget.c:2566:
> alias: Assigning: "dest" = "data".  Both pointers are now null.
> spice-gtk-0.25.28-591b-dirty/spice-gtk-0.25.28/gtk/spice-widget.c:2571:
> dereference: Dereferencing a null pointer "dest".
> 
> 
> It could be complaining about a potential integer overflow when computing
> > how much to alloc.
> >
> 
> Yeah, actually it does make sense, but still weird. I don't remember a
> place where we could set, for instance, width or height with a really big
> value.

This seems to come more or less directly from the remote side, though
maybe there is a place where we validate these values are small enough.

> Anyway, what is your suggestion? Check if d->area.width * d->area.height *
> 3 > 0 before alloc?

You can add a g_return_if_fail(d->area.width != 0); and
g_return_if_fail(d->area.height != 0); as this is unexpected and
wouldn't work right now anyway.


Christophe


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

[Spice-devel] spice related qemu crash

Re: [Spice-devel] [PATCH] [spice-gtk] DND: fix special case that file size is 0

Re: [Spice-devel] [PATCH] [spice-gtk] DND: fix special case that file size is 0

Re: [Spice-devel] [PATCH] [spice-gtk] DND: fix special case that file size is 0

[Spice-devel] [PATCH] [spice-gtk] DND: fix special case that file size is 0

Re: [Spice-devel] [PATCH] [vd_agent] close the file handler if file_size = 0

[Spice-devel] [PATCH] [vd_agent] fix bug: g_key_get_string() is failed when keystring contains '['

[Spice-devel] [PATCH] [vd_agent] fix bug: display error when dragging file with CJK characters

Re: [Spice-devel] [PATCH] [vd_agent] fix bug: display error when dragging file with CJK characters

[Spice-devel] [PATCH] [vd_agent] fix bug: display error when dragging file with CJK characters

Re: [Spice-devel] [PATCH] [vd_agent] close the file handler if file_size = 0

Re: [Spice-devel] [PATCH] [vd_agent] fix bug: g_key_get_string() of vd_agent failed when keystring contains '['

Re: [Spice-devel] [PATCH] [vd_agent] fix bug: g_key_get_string() of vd_agent failed when keystring contains '['

Re: [Spice-devel] [PATCH] fix bug: display error when dragging file with CJK characters

Re: [Spice-devel] Patches welcome??

Re: [Spice-devel] [PATCH 3/3] Fix "NULL_RETURNS" caught by coverity

16 matches

Site Navigation

Mail list logo

Footer information