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

[Alon Levy]

----- Original Message -----
> 
> 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)

The guest must get the file size so the agent can check that there is space available in the guest (It will then need to create a dummy file with that size in order to ensure the space is not siphoned away during the file transfer).

> 
> 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
> 
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel
>