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

[Cody Chan]

On Mon, Aug 11, 2014 at 7:04 PM, Christophe Fergeau <cfergeau at redhat.com>
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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/spice-devel/attachments/20140812/eeaa0a34/attachment-0001.html>