Hi! <div class="gmail_extra"> <div class="gmail_quote">On Mon, Nov 5, 2012 at 10:01 AM, Dunrong Huang <<a href="mailto:riegamaths@gmail.com" target="_blank">riegamaths@gmail.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">These patches simply implement file drag-and-drop from client to guest. </blockquote><div> nice! </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> 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! </blockquote><div> 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! </div></div> -- Marc-André Lureau </div>