| Message ID | 1452235131-1861-3-git-send-email-wency@cn.fujitsu.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 08/01/16 06:38, 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. 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> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
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)? > 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? Or does the called infer this has happened because otherwise libxl_domain_remus_start is not supposed to return? > + else > + write_emulator_xenstore_record(egc, stream); > + } > } > > static void write_emulator_xenstore_record(libxl__egc *egc,
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. > > 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. Thanks Wen Congyang > >> + else >> + write_emulator_xenstore_record(egc, stream); >> + } >> } >> >> static void write_emulator_xenstore_record(libxl__egc *egc, > > > . >
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.
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 > > > > > . >
On Fri, 2016-01-15 at 13:44 +0800, Wen Congyang wrote: > 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; This is (obviously) not documentation.
On 01/15/2016 05:48 PM, Ian Campbell wrote: > On Fri, 2016-01-15 at 13:44 +0800, Wen Congyang wrote: >> 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; > > This is (obviously) not documentation. OK, I will update the comment. Thanks Wen Congyang > > > > . >
On 01/08/2016 02:38 PM, 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. 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, xc_domain_save() completes. > + * there was an error sending data to the secondary. > + * Resume the primary ASAP. > + */ > + stream_complete(egc, stream, 0); > + else > + write_emulator_xenstore_record(egc, stream); > + } > } > > static void write_emulator_xenstore_record(libxl__egc *egc, >
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); + else + write_emulator_xenstore_record(egc, stream); + } } static void write_emulator_xenstore_record(libxl__egc *egc,
For example: if the secondary host is down, and we fail to send the data to the secondary host. xc_domain_save() returns 0. 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(-)