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

[Dunrong Huang]

Hi

On Fri, Dec 7, 2012 at 7:45 PM, Marc-André Lureau
<marcandre.lureau at gmail.com> wrote:
> Hi
>
> On Wed, Dec 5, 2012 at 5:22 AM, Dunrong Huang <riegamaths at gmail.com> wrote:
>>
>> This patch is aimed to handle various file xfer messages.
>>
>> How it works:
>> 0) our main channel introduces a API spice_main_file_xfer().
>
>
> In general we prefer more human name in API, such as spice_main_file_copy()
>
> Imho, it should follow gio async model, so a client can track progress of
> each calls. (look at g_file_copy_async)
>
> I think a client API such as this one would fit better:
>
> spice_main_file_copy_async (GFile**, SpiceFileCopyFlags .., GCancellable,
> GFileProgressCallback, gpointer, GAsyncReadyCallback, gpointer)
>
Ok.
>
>>
>> 1) When user drags a file and drop to spice client, spice client will
>>    catch a signal "drag-data-received", then it should call
>>    spice_main_file_xfer() for transfering file to guest.
>>
>> 2) In main channel: when spice_main_file_xfer() get called with file
>>    list passed, the API will send a start message which includes file
>>    and other needed information for each file. Then it will create a
>>    new xfer task and insert task list for each file, and return to caller.
>
>
> We shouldn't create several tasks simultaneously for a single call, it
> should be serialized to minimize completion time of each file.
>>
It makes sense.
>>
>> 3) According to the response message sent from guest, our main channel
>>    decides whether send more data, or cancel this xfer task.
>>
>> 4) When file transfer has finished, file xfer task will be removed from
>>    task list.
>>
>> Signed-off-by: Dunrong Huang <riegamaths at gmail.com>
>> ---
>>  gtk/channel-main.c      | 202
>> ++++++++++++++++++++++++++++++++++++++++++++++++
>>  gtk/channel-main.h      |   1 +
>>  gtk/map-file            |   1 +
>>  gtk/spice-glib-sym-file |   1 +
>>  4 files changed, 205 insertions(+)
>>
>> diff --git a/gtk/channel-main.c b/gtk/channel-main.c
>> index 6b9ba8d..b2253a6 100644
>> --- a/gtk/channel-main.c
>> +++ b/gtk/channel-main.c
>> @@ -18,6 +18,7 @@
>>  #include <math.h>
>>  #include <spice/vd_agent.h>
>>  #include <common/rect.h>
>> +#include <glib/gstdio.h>
>>
>>  #include "glib-compat.h"
>>  #include "spice-client.h"
>> @@ -51,6 +52,12 @@
>>
>>  typedef struct spice_migrate spice_migrate;
>>
>> +typedef struct SpiceFileXferTask {
>> +    uint32_t                       id;
>> +    SpiceChannel                   *channel;
>> +    GFileInputStream               *file_stream;
>> +} SpiceFileXferTask;
>> +
>>  struct _SpiceMainChannelPrivate  {
>>      enum SpiceMouseMode         mouse_mode;
>>      bool                        agent_connected;
>> @@ -79,6 +86,7 @@ struct _SpiceMainChannelPrivate  {
>>      } display[MAX_DISPLAY];
>>      gint                        timer_id;
>>      GQueue                      *agent_msg_queue;
>> +    GList                       *file_xfer_task_list;
>>
>>      guint                       switch_host_delayed_id;
>>      guint                       migrate_delayed_id;
>> @@ -1384,6 +1392,100 @@ static void
>> main_handle_agent_disconnected(SpiceChannel *channel, SpiceMsgIn *in
>>      agent_stopped(SPICE_MAIN_CHANNEL(channel));
>>  }
>>
>> +static gboolean send_data(gpointer opaque)
>> +{
>> +#define FILE_XFER_CHUNK_SIZE (VD_AGENT_MAX_DATA_SIZE -
>> sizeof(VDAgentMessage))
>> +    SpiceFileXferTask *task = (SpiceFileXferTask *)opaque;
>> +
>> +    SpiceMainChannel *channel = SPICE_MAIN_CHANNEL(task->channel);
>> +    SpiceMainChannelPrivate *c = channel->priv;
>> +    gssize len;
>> +    GError *error = NULL;
>> +    VDAgentFileXferDataMessage *msg;
>> +    uint32_t msg_size;
>> +
>> +    if (!g_queue_is_empty(c->agent_msg_queue)) {
>> +        spice_channel_wakeup(SPICE_CHANNEL(channel), FALSE);
>> +        return TRUE;
>
>
> You can't use Idle functions this way, you are going to busy-loop. You need
> to handle filling the queue differently.
>
I  knew this way is not good, but I cant find a better way to handle
it. What about using g_io_scheduler_push_job() to fill data into msg
queue.
e.g.:

// gio thread context
static gboolean send_data()
{
    // code

    for (;;) {
        if (agent msg queue is not empty) {
            // wait for 1ms until msg queue become empty, so that other messages
            // can still get through.
            usleep(1000);
            continue;
        }

        // read some data from file
        // ====== code ======

        // fill data to queue in main loop.
        g_idle_add(fill_data, userdata);
    }

    // code
}

void file_xfer_send_data_msg
{
    // code

    g_io_scheduler_push_job(send_data);

    // code
}

-- 
Best Regards,

Dunrong Huang