diff mbox

[v10,12/31] tools/libxl: add back channel support to read stream

Message ID 1456109555-28299-13-git-send-email-wency@cn.fujitsu.com (mailing list archive)
State New, archived
Headers show

Commit Message

Wen Congyang Feb. 22, 2016, 2:52 a.m. UTC
This is used by primay to read records sent by secondary.

Signed-off-by: Yang Hongyang <hongyang.yang@easystack.cn>
Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
---
 tools/libxl/libxl_create.c      |  1 +
 tools/libxl/libxl_internal.h    |  1 +
 tools/libxl/libxl_stream_read.c | 27 +++++++++++++++++++++++----
 3 files changed, 25 insertions(+), 4 deletions(-)

Comments

Wei Liu Feb. 25, 2016, 3:54 p.m. UTC | #1
On Mon, Feb 22, 2016 at 10:52:16AM +0800, Wen Congyang wrote:
> This is used by primay to read records sent by secondary.
> 
> Signed-off-by: Yang Hongyang <hongyang.yang@easystack.cn>
> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
> ---
>  tools/libxl/libxl_create.c      |  1 +
>  tools/libxl/libxl_internal.h    |  1 +
>  tools/libxl/libxl_stream_read.c | 27 +++++++++++++++++++++++----
>  3 files changed, 25 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
> index 525bf85..fc746fb 100644
> --- a/tools/libxl/libxl_create.c
> +++ b/tools/libxl/libxl_create.c
> @@ -1030,6 +1030,7 @@ static void domcreate_bootloader_done(libxl__egc *egc,
>      dcs->srs.dcs = dcs;
>      dcs->srs.fd = restore_fd;
>      dcs->srs.legacy = (dcs->restore_params.stream_version == 1);
> +    dcs->srs.back_channel = false;
>      dcs->srs.completion_callback = domcreate_stream_done;
>  
>      if (restore_fd >= 0) {
> diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
> index cdf6751..32957ca 100644
> --- a/tools/libxl/libxl_internal.h
> +++ b/tools/libxl/libxl_internal.h
> @@ -3420,6 +3420,7 @@ struct libxl__stream_read_state {
>      libxl__domain_create_state *dcs;
>      int fd;
>      bool legacy;
> +    bool back_channel;
>      void (*completion_callback)(libxl__egc *egc,
>                                  libxl__stream_read_state *srs,
>                                  int rc);
> diff --git a/tools/libxl/libxl_stream_read.c b/tools/libxl/libxl_stream_read.c
> index f4781eb..02a2c46 100644
> --- a/tools/libxl/libxl_stream_read.c
> +++ b/tools/libxl/libxl_stream_read.c
> @@ -118,6 +118,15 @@
>   *    record, and therefore the buffered state is inconsistent. In
>   *    libxl__xc_domain_restore_done(), we just complete the stream and
>   *    stream->completion_callback() will be called to resume the guest
> + *
> + * For back channel stream:
> + * - libxl__stream_read_start()
> + *    - Set up the stream to running state
> + *
> + * - libxl__stream_read_continue()
> + *     - Set up reading the next record from a started stream.
> + *       Add some codes to process_record() to handle the record.
> + *       Then call stream->checkpoint_callback() to return.
>   */
>  
>  /* Success/error/cleanup handling. */
> @@ -221,6 +230,17 @@ void libxl__stream_read_start(libxl__egc *egc,
>      stream->running = true;
>      stream->phase   = SRS_PHASE_NORMAL;
>  
> +    dc->ao       = stream->ao;
> +    dc->copywhat = "restore v2 stream";
> +    dc->writefd  = -1;
> +
> +    if (stream->back_channel) {
> +        assert(!stream->legacy);
> +
> +        dc->readfd = stream->fd;

Why is this needed? dc->readfd is set to stream->fd no matter it is back
channel or not. This can be moved outside this if {}.

> +        return;
> +    }
> +
>      if (stream->legacy) {
>          /* Convert the legacy stream. */
>          libxl__conversion_helper_state *chs = &stream->chs;
> @@ -243,10 +263,7 @@ void libxl__stream_read_start(libxl__egc *egc,
>      }
>      /* stream->fd is now a v2 stream. */
>  
> -    dc->ao       = stream->ao;
> -    dc->copywhat = "restore v2 stream";
>      dc->readfd   = stream->fd;
> -    dc->writefd  = -1;
>  
>      /* Start reading the stream header. */
>      rc = setup_read(stream, "stream header",
> @@ -762,7 +779,9 @@ static void stream_done(libxl__egc *egc,
>      LIBXL_STAILQ_FOREACH_SAFE(rec, &stream->record_queue, entry, trec)
>          free_record(rec);
>  
> -    check_all_finished(egc, stream, rc);
> +    if (!stream->back_channel)
> +        /* back channel stream doesn't have restore helper */
> +        check_all_finished(egc, stream, rc);

Even if it doesn't have restore helper, check_all_finished also checks
if the stream and the conversion helper are till in use.  The
explanation in the comment doesn't seem to justify this change.

Wei.

>  }
>  
>  void libxl__xc_domain_restore_done(libxl__egc *egc, void *dcs_void,
> -- 
> 2.5.0
> 
> 
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
Wen Congyang Feb. 26, 2016, 2:16 a.m. UTC | #2
On 02/25/2016 11:54 PM, Wei Liu wrote:
> On Mon, Feb 22, 2016 at 10:52:16AM +0800, Wen Congyang wrote:
>> This is used by primay to read records sent by secondary.
>>
>> Signed-off-by: Yang Hongyang <hongyang.yang@easystack.cn>
>> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
>> ---
>>  tools/libxl/libxl_create.c      |  1 +
>>  tools/libxl/libxl_internal.h    |  1 +
>>  tools/libxl/libxl_stream_read.c | 27 +++++++++++++++++++++++----
>>  3 files changed, 25 insertions(+), 4 deletions(-)
>>
>> diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
>> index 525bf85..fc746fb 100644
>> --- a/tools/libxl/libxl_create.c
>> +++ b/tools/libxl/libxl_create.c
>> @@ -1030,6 +1030,7 @@ static void domcreate_bootloader_done(libxl__egc *egc,
>>      dcs->srs.dcs = dcs;
>>      dcs->srs.fd = restore_fd;
>>      dcs->srs.legacy = (dcs->restore_params.stream_version == 1);
>> +    dcs->srs.back_channel = false;
>>      dcs->srs.completion_callback = domcreate_stream_done;
>>  
>>      if (restore_fd >= 0) {
>> diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
>> index cdf6751..32957ca 100644
>> --- a/tools/libxl/libxl_internal.h
>> +++ b/tools/libxl/libxl_internal.h
>> @@ -3420,6 +3420,7 @@ struct libxl__stream_read_state {
>>      libxl__domain_create_state *dcs;
>>      int fd;
>>      bool legacy;
>> +    bool back_channel;
>>      void (*completion_callback)(libxl__egc *egc,
>>                                  libxl__stream_read_state *srs,
>>                                  int rc);
>> diff --git a/tools/libxl/libxl_stream_read.c b/tools/libxl/libxl_stream_read.c
>> index f4781eb..02a2c46 100644
>> --- a/tools/libxl/libxl_stream_read.c
>> +++ b/tools/libxl/libxl_stream_read.c
>> @@ -118,6 +118,15 @@
>>   *    record, and therefore the buffered state is inconsistent. In
>>   *    libxl__xc_domain_restore_done(), we just complete the stream and
>>   *    stream->completion_callback() will be called to resume the guest
>> + *
>> + * For back channel stream:
>> + * - libxl__stream_read_start()
>> + *    - Set up the stream to running state
>> + *
>> + * - libxl__stream_read_continue()
>> + *     - Set up reading the next record from a started stream.
>> + *       Add some codes to process_record() to handle the record.
>> + *       Then call stream->checkpoint_callback() to return.
>>   */
>>  
>>  /* Success/error/cleanup handling. */
>> @@ -221,6 +230,17 @@ void libxl__stream_read_start(libxl__egc *egc,
>>      stream->running = true;
>>      stream->phase   = SRS_PHASE_NORMAL;
>>  
>> +    dc->ao       = stream->ao;
>> +    dc->copywhat = "restore v2 stream";
>> +    dc->writefd  = -1;
>> +
>> +    if (stream->back_channel) {
>> +        assert(!stream->legacy);
>> +
>> +        dc->readfd = stream->fd;
> 
> Why is this needed? dc->readfd is set to stream->fd no matter it is back
> channel or not. This can be moved outside this if {}.

Yes, will fix it in the next version.

> 
>> +        return;
>> +    }
>> +
>>      if (stream->legacy) {
>>          /* Convert the legacy stream. */
>>          libxl__conversion_helper_state *chs = &stream->chs;
>> @@ -243,10 +263,7 @@ void libxl__stream_read_start(libxl__egc *egc,
>>      }
>>      /* stream->fd is now a v2 stream. */
>>  
>> -    dc->ao       = stream->ao;
>> -    dc->copywhat = "restore v2 stream";
>>      dc->readfd   = stream->fd;
>> -    dc->writefd  = -1;
>>  
>>      /* Start reading the stream header. */
>>      rc = setup_read(stream, "stream header",
>> @@ -762,7 +779,9 @@ static void stream_done(libxl__egc *egc,
>>      LIBXL_STAILQ_FOREACH_SAFE(rec, &stream->record_queue, entry, trec)
>>          free_record(rec);
>>  
>> -    check_all_finished(egc, stream, rc);
>> +    if (!stream->back_channel)
>> +        /* back channel stream doesn't have restore helper */
>> +        check_all_finished(egc, stream, rc);
> 
> Even if it doesn't have restore helper, check_all_finished also checks
> if the stream and the conversion helper are till in use.  The
> explanation in the comment doesn't seem to justify this change.

In stream_done(), stream->running is set to false, so libxl__stream_read_in_use()
returns false.

Back channel stream doesn't support legacy stream, so there is no conversion helper.

I will update the comments in the next version.

Thanks
Wen Congyang

> 
> Wei.
> 
>>  }
>>  
>>  void libxl__xc_domain_restore_done(libxl__egc *egc, void *dcs_void,
>> -- 
>> 2.5.0
>>
>>
>>
>>
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@lists.xen.org
>> http://lists.xen.org/xen-devel
> 
> 
> .
>
Wei Liu March 2, 2016, 3:03 p.m. UTC | #3
On Fri, Feb 26, 2016 at 10:16:43AM +0800, Wen Congyang wrote:
[...]
> > 
> > Even if it doesn't have restore helper, check_all_finished also checks
> > if the stream and the conversion helper are till in use.  The
> > explanation in the comment doesn't seem to justify this change.
> 
> In stream_done(), stream->running is set to false, so libxl__stream_read_in_use()
> returns false.
> 
> Back channel stream doesn't support legacy stream, so there is no conversion helper.
> 
> I will update the comments in the next version.
> 

Yes, please do. Say clearly why it is not needed.

Wei.
diff mbox

Patch

diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index 525bf85..fc746fb 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -1030,6 +1030,7 @@  static void domcreate_bootloader_done(libxl__egc *egc,
     dcs->srs.dcs = dcs;
     dcs->srs.fd = restore_fd;
     dcs->srs.legacy = (dcs->restore_params.stream_version == 1);
+    dcs->srs.back_channel = false;
     dcs->srs.completion_callback = domcreate_stream_done;
 
     if (restore_fd >= 0) {
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index cdf6751..32957ca 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -3420,6 +3420,7 @@  struct libxl__stream_read_state {
     libxl__domain_create_state *dcs;
     int fd;
     bool legacy;
+    bool back_channel;
     void (*completion_callback)(libxl__egc *egc,
                                 libxl__stream_read_state *srs,
                                 int rc);
diff --git a/tools/libxl/libxl_stream_read.c b/tools/libxl/libxl_stream_read.c
index f4781eb..02a2c46 100644
--- a/tools/libxl/libxl_stream_read.c
+++ b/tools/libxl/libxl_stream_read.c
@@ -118,6 +118,15 @@ 
  *    record, and therefore the buffered state is inconsistent. In
  *    libxl__xc_domain_restore_done(), we just complete the stream and
  *    stream->completion_callback() will be called to resume the guest
+ *
+ * For back channel stream:
+ * - libxl__stream_read_start()
+ *    - Set up the stream to running state
+ *
+ * - libxl__stream_read_continue()
+ *     - Set up reading the next record from a started stream.
+ *       Add some codes to process_record() to handle the record.
+ *       Then call stream->checkpoint_callback() to return.
  */
 
 /* Success/error/cleanup handling. */
@@ -221,6 +230,17 @@  void libxl__stream_read_start(libxl__egc *egc,
     stream->running = true;
     stream->phase   = SRS_PHASE_NORMAL;
 
+    dc->ao       = stream->ao;
+    dc->copywhat = "restore v2 stream";
+    dc->writefd  = -1;
+
+    if (stream->back_channel) {
+        assert(!stream->legacy);
+
+        dc->readfd = stream->fd;
+        return;
+    }
+
     if (stream->legacy) {
         /* Convert the legacy stream. */
         libxl__conversion_helper_state *chs = &stream->chs;
@@ -243,10 +263,7 @@  void libxl__stream_read_start(libxl__egc *egc,
     }
     /* stream->fd is now a v2 stream. */
 
-    dc->ao       = stream->ao;
-    dc->copywhat = "restore v2 stream";
     dc->readfd   = stream->fd;
-    dc->writefd  = -1;
 
     /* Start reading the stream header. */
     rc = setup_read(stream, "stream header",
@@ -762,7 +779,9 @@  static void stream_done(libxl__egc *egc,
     LIBXL_STAILQ_FOREACH_SAFE(rec, &stream->record_queue, entry, trec)
         free_record(rec);
 
-    check_all_finished(egc, stream, rc);
+    if (!stream->back_channel)
+        /* back channel stream doesn't have restore helper */
+        check_all_finished(egc, stream, rc);
 }
 
 void libxl__xc_domain_restore_done(libxl__egc *egc, void *dcs_void,