[Spice-devel] [PATCH spice-gtk 2/2] file-xfer: Inform client of errors on init of xfer

[Victor Toso]

On Tue, Jul 11, 2017 at 11:39:57AM +0200, Pavel Grunt wrote:
> Hi,
>
> On Mon, 2017-07-03 at 15:28 +0200, Victor Toso wrote:
> > From: Victor Toso <me at victortoso.com>
> >
> > With SpiceFileTransferTask we suggest that Spice clients watch
> > (signal) SpiceMainChannel::new-file-transfer so it can follow the
> ... so they can..
> > transfer's progress till the moment it is finished.
> > 
> > On (signal) SpiceFileTransferTask::finished, we will issue if any
> > error has happened to the application.
> 
> imo this sentence is hard to read. What about:
>  The signal SpiceFileTransferTask::finished informs if an error happens to the
> application
> 
> 
> > 
> > This patch solves the problem of SpiceFileTransferTask::finished not
> > being emitted when the agent is not connected nor when file-transfer
> > is disabled in the host.
> > 
> > We should fist emit SpiceMainChannel::new-file-transfer followed up by
> first
> 
> > SpiceFileTransferTask::finished, which is done by the function
> > spice_file_transfer_task_completed().
> > 
> > As channel-main chains up the "finish" signal by removing the
> > SpiceFileTransferTask from its GHashTable, we have to change from
> > GHashTableIter to a simple GList of keys.
> okay
> > 
> > Related: https://bugzilla.redhat.com/show_bug.cgi?id=1373830
> > Signed-off-by: Victor Toso <victortoso at redhat.com>
> > ---
> >  src/channel-main.c | 50 ++++++++++++++++++++++----------------------------
> >  1 file changed, 22 insertions(+), 28 deletions(-)
> > 
> > diff --git a/src/channel-main.c b/src/channel-main.c
> > index 1c77c7b..0140d5b 100644
> > --- a/src/channel-main.c
> > +++ b/src/channel-main.c
> > @@ -3103,9 +3103,9 @@ void spice_main_file_copy_async(SpiceMainChannel
> > *channel,
> >                                  gpointer user_data)
> >  {
> >      SpiceMainChannelPrivate *c;
> > -    GHashTableIter iter;
> > -    gpointer key, value;
> >      FileTransferOperation *xfer_op;
> > +    GError *error = NULL;
> > +    GList *it, *keys;
> >  
> >      g_return_if_fail(channel != NULL);
> >      g_return_if_fail(SPICE_IS_MAIN_CHANNEL(channel));
> > @@ -3113,25 +3113,13 @@ void spice_main_file_copy_async(SpiceMainChannel
> > *channel,
> >  
> >      c = channel->priv;
> >      if (!c->agent_connected) {
> > -        g_task_report_new_error(channel,
> > -                                callback,
> > -                                user_data,
> > -                                spice_main_file_copy_async,
> > -                                SPICE_CLIENT_ERROR,
> > -                                SPICE_CLIENT_ERROR_FAILED,
> > -                                _("The agent is not connected"));
> > -        return;
> > -    }
> > -
> > -    if (test_agent_cap(channel, VD_AGENT_CAP_FILE_XFER_DISABLED)) {
> > -        g_task_report_new_error(channel,
> > -                                callback,
> > -                                user_data,
> > -                                spice_main_file_copy_async,
> > -                                SPICE_CLIENT_ERROR,
> > -                                SPICE_CLIENT_ERROR_FAILED,
> > -                                _("The file transfer is disabled"));
> > -        return;
> > +        error = g_error_new(SPICE_CLIENT_ERROR,
> > +                            SPICE_CLIENT_ERROR_FAILED,
> > +                            _("The agent is not connected"));
> > +    } else if (test_agent_cap(channel, VD_AGENT_CAP_FILE_XFER_DISABLED)) {
> > +        error = g_error_new(SPICE_CLIENT_ERROR,
> > +                            SPICE_CLIENT_ERROR_FAILED,
> > +                            _("The file transfer is disabled"));
> >      }
> >  
> >      xfer_op = g_new0(FileTransferOperation, 1);
> > @@ -3144,23 +3132,29 @@ void spice_main_file_copy_async(SpiceMainChannel
> > *channel,
> >                                                                 flags,
> >                                                                 cancellable);
> >      xfer_op->stats.num_files = g_hash_table_size(xfer_op->xfer_task);
> > -    g_hash_table_iter_init(&iter, xfer_op->xfer_task);
> > -    while (g_hash_table_iter_next(&iter, &key, &value)) {
> > +    keys = g_hash_table_get_keys(xfer_op->xfer_task);
> > +    for (it = keys; it != NULL; it = it->next) {
> >          guint32 task_id;
> > -        SpiceFileTransferTask *xfer_task = value;
> > +        SpiceFileTransferTask *xfer_task = g_hash_table_lookup(xfer_op-
> > >xfer_task, it->data);
> >  
> >          task_id = spice_file_transfer_task_get_id(xfer_task);
> >  
> >          SPICE_DEBUG("Insert a xfer task:%u to task list", task_id);
> >  
> > -        g_hash_table_insert(c->file_xfer_tasks, key, xfer_op);
> > +        g_hash_table_insert(c->file_xfer_tasks, it->data, xfer_op);
> >          g_signal_connect(xfer_task, "finished",
> > G_CALLBACK(file_transfer_operation_task_finished), NULL);
> >          g_signal_emit(channel, signals[SPICE_MAIN_NEW_FILE_TRANSFER], 0,
> > xfer_task);
> >  
> > -        spice_file_transfer_task_init_task_async(xfer_task,
> > -                                                 file_xfer_init_task_async_cb
> > ,
> > -                                                 xfer_op);
> > +        if (error == NULL) {
> > +            spice_file_transfer_task_init_task_async(xfer_task,
> > +                                                     file_xfer_init_task_asyn
> > c_cb,
> > +                                                     xfer_op);
> > +        } else {
> > +            spice_file_transfer_task_completed(xfer_task,
> > g_error_copy(error));
> > +        }
> >      }
> > +    g_list_free(keys);
> > +    g_clear_pointer(&error, g_error_free);
> 
> g_clear_error
> 
> Thanks,
> Pavel

Sure, Thanks for the review

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20170711/16ae9f31/attachment.sig>