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

[Dunrong Huang]

Hi,

On Thu, Jan 31, 2013 at 5:13 PM, Hans de Goede <hdegoede at redhat.com> wrote:
> 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.
>
Thank you for the explanation, it is very clear to me now.
It will be improved in next version.
> 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



-- 
Best Regards,

Dunrong Huang