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

[Dunrong Huang]

Thank you for your detailed comment!

2012/11/12 Marc-André Lureau <marcandre.lureau at gmail.com>:
> 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
Agree with you.
> 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().
Yes, this makes our code more simple and easily extensible.
>
> 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.
>
In VD_AGENT_DND_START message, file name and file size had been included.
But as you said, more information is needed for future expansion. I
will design a common
struct to hold these information.
> 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?
This makes sense, I will fix it in v2.
>
> The DND_CHUNCK_SIZE should probably match VD_AGENT_MAX_DATA_SIZE, to avoid
> some extra overhead when marshalling.
Yes.
>
> 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.
very meaningful!, this option will reduce many bandwidth.
>
> Please clean up the g_print or use SPICE_DEBUG for logging.
will be fixed in v2.
>
> 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
I am very happy to implement this feature in spare time. And I have
enough time to do 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



--
Best Regards,

Dunrong Huang