[Spice-devel] [PATCH vd_agent V4 4/4] file-xfer: Add file-xfer support for linux agent

[Hans de Goede]

Hi,

On 01/30/2013 03:30 PM, Marc-André Lureau wrote:
> Hi,
>
> It looks pretty ready to me, just a few things:
>
> On Wed, Jan 30, 2013 at 12:51 PM, Dunrong Huang <riegamaths at gmail.com> wrote:
>> The patch makes linux agent support file-xfer feature.
>>
>> Signed-off-by: Dunrong Huang <riegamaths at gmail.com>

<snip>

>> +typedef struct AgentFileXferTask {
>> +    uint32_t                       id;
>> +    int                            file_fd;
>> +    uint64_t                       read_bytes;
>> +    char                           *file_name;
>> +    uint64_t                       file_size;
>> +} AgentFileXferTask;
>> +
>> +GHashTable *agent_file_xfer_tasks = NULL;
>
> it should be static.
>
> Since the agent gets more stateful, we need a way to clean up when the
> client is disconnected. Otherwise, it is very easy to leak in the
> agent. If it's not that simple to know when the client is disconnected
> (Hans?), I think we should consider for the moment limiting the number
> of concurrent xfer tasks.

We don't get any message when the client disconnects, but we do always get
a VD_AGENT_ANNOUNCE_CAPABILITIES message with the request flag set to 1
when a new client connects!

So we could send a message from spice-vdagentd to spice-vdagent when that
happens telling the spice-vdagent to cancel any pending file-transfers.

Actually we need to do the same when the active-session is changed
midway through a file transfer.

So I would like to suggest to add a new
VDAGENTD_CONNECTION_RESET message to src/vdagentd-proto.h and
src/vdagentd-proto-strings.h, and to have
spice-vdagentd send that to the active_session_conn on
active_session_conn change (sending it to the previous / old
active_session_conn, and when receiving a VD_AGENT_ANNOUNCE_CAPABILITIES
message with the request flag set to 1

And then the spice-vdagent can cleanup all pending file transfers when
this happens.

This is probably best done in a follow-up patch.

Regards,

Hans