[Spice-devel] [PATCH spice-gtk 11/15] channel-main: Make SpiceFileXferTask-s ref the channel

[Hans de Goede]

Hi,

On 03/10/2013 03:44 PM, Hans de Goede wrote:
> Hi,
>
> First of all thanks for the review.
>
> On 03/10/2013 03:28 PM, Marc-André Lureau wrote:
>> On Sat, Mar 9, 2013 at 12:06 PM, Hans de Goede <hdegoede at redhat.com> wrote:
>>> So that the channel sticks around while their callbacks are completing.
>>
>> I am not sure this is a good idea. Keeping channel references around
>> will break migration code.
>>
>> Have you checked with seamless & non-seamless migrations?
>
> I don't expect much of a problem with non-seamless since the reset code
> I added will cancel all pending xfers and thus free all references
> (although with a slight delay if any operation is pending).
>
> And the channel is not replaced in the seamless case, so the
> really troublesome scenario is the non-seamless, where the reset_agent
> code will cancel the pending xfers (eventually) freeing the refs.

As they say the proof is in the pudding.

I finally have the agent "channel" data corruption issue I was seeing
while testing seamless migration fixed. It turned out to not be migration
related, but a tricky spicevmc bug in qemu, see:
http://lists.gnu.org/archive/html/qemu-devel/2013-03/msg05209.html

So with this fixed I've now been able to finally test seamless and
semi-seamless migration with active file-xfers.

I've run various tests repeatedly and things work fine, with
semi-seamless the file-xfer gets cancelled, but that is to be expected
as the agent "channel" gets broken.

So as I already expected (see my previous reply above), the (temporary)
taking of a ref on the channel does not seem to cause any problems.

In the mean time I've reworked the patch series a bit, to fix some
scenarios where the file-xfer-task would be destroyed while callbacks
referencing it where still pending.

I'll send a new version of the series after this mail.

Regards,

Hans