[Spice-devel] [PATCH spice-gtk v2 2/2] New file transfer API

[Jonathon Jongsma]

On Tue, 2015-10-06 at 15:33 +0200, Pavel Grunt wrote:
> Hi Jonathon,
> 
> thanks for the work, the user should have a way to cancel & monitor the file
> transfer progress.
> 
> Ack from me, just a few comments below.
> 
> On Mon, 2015-10-05 at 13:39 -0500, Jonathon Jongsma wrote:
> > There were several shortcomings to the existing file transfer API,
> > particularly in terms of monitoring ongoing file transfers. The major
> > issue is that spice_main_file_copy_async() allows you to pass an array
> > of files, but the progress callback does not provide a way to
> > identify which file the callback is associated with. This makes it
> > nearly impossible for an application to monitor file transfers.
> > 
> > In addition, the SpiceDisplay widget automatically handles drag-and-drop
> > actions on the widget, and initiates file transfers without allowing the
> > application to specify a progress callback. So there's no way for an app
> > to monitor file transfers that are initiated via drag and drop.
> > 
> > http://lists.freedesktop.org/archives/spice-devel/2015-September/021931.html
> > has a more detailed explanation of the issues.
> > 
> > This change doesn't break the existing API, but adds some new API that
> > will allow an application to monitor file transfer progress, even for
> > transfers that are initiated within spice-gtk itself.
> > 
> > - A new public SpiceFileTransferTask object is added.
> > - The SpiceMainChannel object gains a "new-file-transfer" signal that is
> >   emitted whenever a new file transfer is initiated. The
> >   SpiceFileTransferTask object is passed to the signal handler.
> > - The application can retain this object and monitor its 'progress'
> >   property to be notified when the progress of the file transfer
> >   changes. The SpiceFileTransferTask::finished signal indicates when the
> >   given file transfer has completed. The application can also cancel the
> >   file transfer by calling the _cancel() method.
> > 
> > The 'spicy' test application has been updated to use this new API and
> > display a simple dialog showing the progress of individual files.
> 
> In general I don't like pop up dialogs, but they are ok for spicy.

I basically agree. I was thinking about doing the same for virt-viewer
as a temporary solution until we can come up with a better design
though. Or would you rather wait to implement it in virt-viewer until we
have a better design?


> > diff --git a/src/channel-main.h b/src/channel-main.h
> > index f4fc005..6fb8395 100644
> > --- a/src/channel-main.h
> > +++ b/src/channel-main.h
> > @@ -23,6 +23,7 @@
> >  #endif
> >  
> >  #include "spice-channel.h"
> > +#include "spice-file-transfer-task.h"
> >  
> this should be in spice-client.h, I also could not apply the patch because of
> it.

Oops, I had my header include changes on this branch. Sorry about that.


> > +
> > +#ifndef __SPICE_FILE_TRANSFER_TASK_H__
> > +#define __SPICE_FILE_TRANSFER_TASK_H__
> > +
> > +#include <gio/gio.h>
> > +#include <spice/vd_agent.h>
> Why these includes? Just <glib-object.h> should be enough, no?

Not really sure where those came from... Must have been a previous
iteration or something. Yes, glib-object.h should be enough.


> 
> There are some symbols undocumented. Please, take a look at
> /doc/reference/spice-gtk-undocumented.txt to make gtk-doc happy.
> 
> Thanks,
> Pavel
> 
> 

Ah, thanks. Will send a new version.