[Spice-devel] [PATCH spice-gtk V4 1/3] file-xfer: handling various transfer messages in main channel

[Dunrong Huang]

Hi,

On Fri, Jan 4, 2013 at 9:43 PM, Marc-André Lureau
<marcandre.lureau at gmail.com> wrote:
> Hi
>
> On Fri, Jan 4, 2013 at 9:34 AM, Dunrong Huang <riegamaths at gmail.com> wrote:
>> spice_main_file_copy_async() was designed to copy multi-files at a
>> time, which I think is a little unreasonable:
>>
>> 1) If something error happens when opening or reading a file. should
>> we report error immediately(using callback), or wait for all tasks to
>> complete?
>
> Report error immediately.
>
>> 2) We have to set group_id to identify tasks created at the same
>> time(e.g. for report_progress, report_finish, ...)
>

> With a single request, we wouldn't need the group id.
Since all copy tasks are inserted into a list, and one request may
create many tasks, in order to
distinguish between different requests. I have to set same gorup id
for tasks which belong to the same request,
because there are more than one requests exist at the same time.

 For example, if caller passes more than
one files as arguments, these files belong to the same request, when a
copy task has been finished,
I should check whether other tasks which belong to same request have
been finished, then decide whether to report finished.

Maybe it is a bad design, I will impove it later.
>
>> 3) For reporting progress, we have to calculate all tasks' remain
>> bytes(see report_progress()), this is a bit weird. It should be done
>> by caller of this API(just as nautilus does).
>
> I don't understand what is the problem, I mean each file has a size we
> can query when starting the copy. What else would you expect the
> caller to do?
>
When reporting progress, I  have to calculate total sizes of all
files, and total size copied,
by using the group id. I mean if there is only one file, the code will
be more simple.

>> 4) After a task is finished, we have to check whether other tasks with
>> same group_id have been finished(see report_finish()). I think it's a
>> bit weird.
>
> Indeed, we have to wait for all files to be copied before reporting
> success for the whole operation. Progress can be reported regularly
> via callback.
>
>> If spice_main_file_copy_async() only copys one file at a time, our
>> code will be more simple and clean. The caller of this API can
>> implement
>> coping multi-files, just as nautilus does.
>
> But that would be inefficient and cumbersome if the caller has to
> request each and every file. (especially for recursive copy, where
> each caller implementation would have to implement diving through
> subdirs). I don't think there are good technical reasons to split each
> file in a seperate request. However, you can implement the copy ()
> method to be limited to a single file atm, with a g_return
> pre-condition. Support for many files and dirs can be added later.
>

Ok, thank you for your suggestion, it makes sense. I will limit the
copy() to a single
file this moment, other feature can be adder later.

Should go to bed, it's kind of late here. Patch V5 will be sent tomorrow.

-- 
Best Regards,

Dunrong Huang