[Spice-devel] [RFC] [PATCH spice-gtk 0/4] simply implement of file drag-and-drop

[Marc-André Lureau]

Hi!


On Mon, Nov 5, 2012 at 10:01 AM, Dunrong Huang <riegamaths at gmail.com> wrote:

> These patches simply implement file drag-and-drop from client to guest.
>
>
nice!


> There are some TODOs needed to be done:
>   * transfer multiple files and directories
>   * check whether guest has enough space to store file
>   * a progress bar that allows user to cancels file transfer,
>     since we always pre-cache before sending file content,
>         it has a little difficult to finish.
>
> Comments are welcome, please review!
>

Good start! I imagine there will be a lot more to do to support all the
cases. It brings lot of questions and remarks, so I will put there some of
my thoughts in random order:

We shouldn't need the SpiceDisplay to copy a file nor gtk, so we should put
it on the glib / channel level. In general, channel functions are not
mapping the protocol 1-1, they implement a fair amount of logic so instead
of having two spice_main_dnd* functions, perhaps we could have only one
spice_main_dnd_files() that would deal with the long task and protocol. The
function could take a GError, in order to report to the user like you did
in spice_dnd_file_too_large().

nautilus is an example of a generic file dnd gtk code, and it seems it uses
more than "text/plain" target list, I am not familiar with Gtk+ DnD support
yet, so more investigation needed.

I am not convinced we need to limit file size now and especially at
spice-gtk level. However, I think it would be useful to announce the file
size to the guest agent. (in the future, the guest could ask or cancel
operation, during negotiation or later), and perhaps more information about
the dnd context (although we quite clearly don't want to implement a full
dnd protocol but rather just file dnd)

I wonder if the initial request should also give the guest some more
context than just a filename. We might want to easily extend those
informations in the future, for example the file mime, a file list, date,
attributes, drop position etc.. Perhaps the dnd request should use some
metadata format where fields can easily be added. The request should
probably have an id so we can identify and control the operation during its
lifetime, and be able to process several dnd simultaneously in the future.

It's easy to use glib API for file operations from the start, it saves us
from some portability issues. Since we are in a UI context, it is a
requirement to use async IO operations too. Could you update the patches to
use those?

The DND_CHUNCK_SIZE should probably match VD_AGENT_MAX_DATA_SIZE, to avoid
some extra overhead when marshalling.

We need to throttle or prioritize the agent queue if we start to fill it
with lot of data, and we shouldn't copy the whole file there but that can
be improved a bit later too.

Later, we should consider compressing the files in tar.xz archive
(optionally). That could be part of the initial request.

Please clean up the g_print or use SPICE_DEBUG for logging.

All of this is quite some work, and I don't know how much effort you want
to put into it. It would be nice to agree on a first iteration to get it
upstream soonish. My shortlist would be:

- the initial dnd spice protocol supports single file, and initial
request/reply
- the ongoing dnd operations can be identified and cancelled, at any time
by client or agent
- the protocol should be relatively easy to extend for the future use cases
- the implementation must do async io (using gio functions in spice-gtk)
- the initial spice client API could be as simple as a single exported
function like spice_main_dnd_files() for now imho

 Thanks a lot for working on this, it's really appreciated!

-- 
Marc-André Lureau
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/spice-devel/attachments/20121112/04aaff2a/attachment.html>