Re: [PATCH v2 16/27] tools/libxl: Infrastructure for reading a libxl migration v2 stream

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 10/07/15 12:25, Ian Campbell wrote:
>>>> +
>>>> +    stream->completion_callback(egc, stream, stream->rc);
>>>> +}
>>>> +
>>>> +static void stream_continue(libxl__egc *egc,
>>>> +                            libxl__stream_read_state *stream)
>>>> +{
>>>> +    STATE_AO_GC(stream->ao);
>>>> +
>>>> +    /* Must not mutually recurse with process_record() */
>>>> +    assert(stream->recursion_guard == false);
>>>> +    stream->recursion_guard = true;
>>> This smells a bit like it ought to be a SRS_PHASE_PROCESSING or some
>>> such, but lets leave that alone...
>> This check is pre-emptively avoid the naive bug which would occur if
>> process_record() called back into stream_continue() and there were many
>> TOOLSTACK records back to back in the processing queue.
>>
>> In that case (and potentially future records as well), the two functions
>> would mutually recurse based on the contents of the stream.
> Do you mean they would do so legitimately in that case, or in error?

It is wrong in all cases to mutually recurse like this.  The data
controlling the degree of mutual recursion is read from a pipe.

The issue with the TOOLSTACK record is that it a synchronous library
call, not an aync one.  This is not a problem pe say, but it means that
process_record() must queue something further to do.

In a checkpoint it is possible (although very unlikely) to have $N
thousand TOOLSTACK records back to back.

The guards are in place to prevent introducing a codepath which does end
up in mutual recursion.  Such a calltree would function for any
reasonable input, but would fall off the stack given a certain sequence
of records.

~Andrew