Re: [PATCH v3 2/5] remus: resume immediately if libxl__xc_domain_save_done() completes

[Wen Congyang <wency@cn.fujitsu.com>]

On 01/14/2016 06:21 PM, Ian Campbell wrote:
> On Tue, 2016-01-12 at 09:40 +0800, Wen Congyang wrote:
>> On 01/09/2016 12:27 AM, Ian Campbell wrote:
>>> On Fri, 2016-01-08 at 14:38 +0800, Wen Congyang wrote:
>>>> For example: if the secondary host is down, and we fail to send the
>>>> data
>>>> to
>>>> the secondary host. xc_domain_save() returns 0.
>>>
>>> Just to be check: On failure in this way xc_domain_save() returns 0
>>> (i.e.
>>> success)?
>>
>> Yes, it returns 0. I am not sure the return value is right.
>>
>>>
>>>>  So in the function
>>>> libxl__xc_domain_save_done(), rc is 0(the helper program exits
>>>> normally),
>>>> and retval is 0(it is xc_domain_save()'s return value). In such case,
>>>> we
>>>> just need to complete the stream.
>>>>
>>>> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
>>>> ---
>>>>  tools/libxl/libxl_stream_write.c | 13 +++++++++++--
>>>>  1 file changed, 11 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/tools/libxl/libxl_stream_write.c
>>>> b/tools/libxl/libxl_stream_write.c
>>>> index 80d9208..82e7719 100644
>>>> --- a/tools/libxl/libxl_stream_write.c
>>>> +++ b/tools/libxl/libxl_stream_write.c
>>>> @@ -354,8 +354,17 @@ void libxl__xc_domain_save_done(libxl__egc *egc,
>>>> void *dss_void,
>>>>       * alive, and check_all_finished() may have torn it down around
>>>> us.
>>>>       * If the stream is not still alive, we must not continue any
>>>> work.
>>>>       */
>>>> -    if (libxl__stream_write_inuse(stream))
>>>> -        write_emulator_xenstore_record(egc, stream);
>>>> +    if (libxl__stream_write_inuse(stream)) {
>>>> +        if (dss->remus)
>>>> +            /*
>>>> +             * For remus, if libxl__xc_domain_save_done() completes,
>>>> +             * there was an error sending data to the secondary.
>>>> +             * Resume the primary ASAP.
>>>> +             */
>>>> +            stream_complete(egc, stream, 0);
>>>
>>> Is there an indication to the caller that things have failed in this
>>> way?
>>> Would that information be of use to the caller?
>>
>> For remus, when we come here, something is wrong regardless of the return
>> value.
> 
> But does the caller know this? Can it tell.
> 
>>
>>>
>>> Or does the called infer this has happened because
>>> otherwise libxl_domain_remus_start is not supposed to return?
>>
>> Yes, libxl_domain_remus_start() should not return unless somethins is
>> wrong.
> 
> This really ought to be documented somewhere.

libxl_domain_remus_start():
    /* Point of no return */
    libxl__remus_setup(egc, dss);
    return AO_INPROGRESS;

Thanks
Wen Congyang

> 
> 
> 
> 
> .
>