File xen.sr-restore-read_record.patch of Package xen

Overview Repositories Revisions Requests Users Attributes Meta

File xen.sr-restore-read_record.patch of Package xen

From: Olaf Hering <olaf@aepfle.de>
Date: Mon, 26 Oct 2020 12:19:17 +0100
Subject: sr restore read_record

tools: restore: split record processing

handle_page_data must be able to read directly into mapped guest memory.
This will avoid unneccesary memcpy calls for data which can be consumed verbatim.

Rearrange the code to allow decisions based on the incoming record.

This change is preparation for future changes in handle_page_data,
no change in behavior is intended.

Signed-off-by: Olaf Hering <olaf@aepfle.de>
Reviewed-by: Juergen Gross <jgross@suse.com>
---
 tools/libs/guest/xg_sr_common.c  | 33 +++---
 tools/libs/guest/xg_sr_common.h  |  4 +-
 tools/libs/guest/xg_sr_restore.c | 49 ++++++---
 tools/libs/guest/xg_sr_save.c    |  7 +-
 4 files changed, 63 insertions(+), 30 deletions(-)

--- a/tools/libs/guest/xg_sr_common.c
+++ b/tools/libs/guest/xg_sr_common.c
@@ -82,73 +82,80 @@ int write_split_record(struct xc_sr_context *ctx, struct xc_sr_record *rec,
         assert(buf);
 
     if ( writev_exact(ctx->fd, parts, ARRAY_SIZE(parts)) )
         goto err;
 
     return 0;
 
  err:
     PERROR("Unable to write record to stream");
     return -1;
 }
 
-int read_record(struct xc_sr_context *ctx, int fd, struct xc_sr_record *rec)
+int read_record_header(struct xc_sr_context *ctx, int fd, struct xc_sr_rhdr *rhdr)
 {
     xc_interface *xch = ctx->xch;
-    struct xc_sr_rhdr rhdr;
-    size_t datasz;
 
-    if ( read_exact(fd, &rhdr, sizeof(rhdr)) )
+    if ( read_exact(fd, rhdr, sizeof(*rhdr)) )
     {
         PERROR("Failed to read Record Header from stream");
         return -1;
     }
 
-    if ( rhdr.length > REC_LENGTH_MAX )
+    if ( rhdr->length > REC_LENGTH_MAX )
     {
-        ERROR("Record (0x%08x, %s) length %#x exceeds max (%#x)", rhdr.type,
-              rec_type_to_str(rhdr.type), rhdr.length, REC_LENGTH_MAX);
+        ERROR("Record (0x%08x, %s) length %#x exceeds max (%#x)", rhdr->type,
+              rec_type_to_str(rhdr->type), rhdr->length, REC_LENGTH_MAX);
         return -1;
     }
 
-    datasz = ROUNDUP(rhdr.length, REC_ALIGN_ORDER);
+    return 0;
+}
+
+int read_record_data(struct xc_sr_context *ctx, int fd, struct xc_sr_rhdr *rhdr,
+                     struct xc_sr_record *rec)
+{
+    xc_interface *xch = ctx->xch;
+    size_t datasz;
+
+    datasz = ROUNDUP(rhdr->length, REC_ALIGN_ORDER);
 
     if ( datasz )
     {
         rec->data = malloc(datasz);
 
         if ( !rec->data )
         {
             ERROR("Unable to allocate %zu bytes for record data (0x%08x, %s)",
-                  datasz, rhdr.type, rec_type_to_str(rhdr.type));
+                  datasz, rhdr->type, rec_type_to_str(rhdr->type));
             return -1;
         }
 
         if ( read_exact(fd, rec->data, datasz) )
         {
             free(rec->data);
             rec->data = NULL;
             PERROR("Failed to read %zu bytes of data for record (0x%08x, %s)",
-                   datasz, rhdr.type, rec_type_to_str(rhdr.type));
+                   datasz, rhdr->type, rec_type_to_str(rhdr->type));
             return -1;
         }
     }
     else
         rec->data = NULL;
 
-    rec->type   = rhdr.type;
-    rec->length = rhdr.length;
+    rec->type   = rhdr->type;
+    rec->length = rhdr->length;
 
     return 0;
-};
+}
 
 static void __attribute__((unused)) build_assertions(void)
 {
     BUILD_BUG_ON(sizeof(struct xc_sr_ihdr) != 24);
     BUILD_BUG_ON(sizeof(struct xc_sr_dhdr) != 16);
     BUILD_BUG_ON(sizeof(struct xc_sr_rhdr) != 8);
 
     BUILD_BUG_ON(sizeof(struct xc_sr_rec_page_data_header)  != 8);
     BUILD_BUG_ON(sizeof(struct xc_sr_rec_x86_pv_info)       != 8);
     BUILD_BUG_ON(sizeof(struct xc_sr_rec_x86_pv_p2m_frames) != 8);
     BUILD_BUG_ON(sizeof(struct xc_sr_rec_x86_pv_vcpu_hdr)   != 8);
     BUILD_BUG_ON(sizeof(struct xc_sr_rec_x86_tsc_info)      != 24);
--- a/tools/libs/guest/xg_sr_common.h
+++ b/tools/libs/guest/xg_sr_common.h
@@ -449,25 +449,27 @@ static inline int write_record(struct xc_sr_context *ctx,
 /*
  * Reads a record from the stream, and fills in the record structure.
  *
  * Returns 0 on success and non-0 on failure.
  *
  * On success, the records type and size shall be valid.
  * - If size is 0, data shall be NULL.
  * - If size is non-0, data shall be a buffer allocated by malloc() which must
  *   be passed to free() by the caller.
  *
  * On failure, the contents of the record structure are undefined.
  */
-int read_record(struct xc_sr_context *ctx, int fd, struct xc_sr_record *rec);
+int read_record_header(struct xc_sr_context *ctx, int fd, struct xc_sr_rhdr *rhdr);
+int read_record_data(struct xc_sr_context *ctx, int fd, struct xc_sr_rhdr *rhdr,
+                     struct xc_sr_record *rec);
 
 /*
  * This would ideally be private in restore.c, but is needed by
  * x86_pv_localise_page() if we receive pagetables frames ahead of the
  * contents of the frames they point at.
  */
 int populate_pfns(struct xc_sr_context *ctx, unsigned int count,
                   const xen_pfn_t *original_pfns, const uint32_t *types);
 
 /* Handle a STATIC_DATA_END record. */
 int handle_static_data_end(struct xc_sr_context *ctx);
 
--- a/tools/libs/guest/xg_sr_restore.c
+++ b/tools/libs/guest/xg_sr_restore.c
@@ -444,25 +444,25 @@ static int send_checkpoint_dirty_pfn_list(struct xc_sr_context *ctx)
     if ( writev_exact(ctx->restore.send_back_fd, iov, ARRAY_SIZE(iov)) )
     {
         PERROR("Failed to write dirty bitmap to stream");
         goto err;
     }
 
     rc = 0;
  err:
     free(pfns);
     return rc;
 }
 
-static int process_record(struct xc_sr_context *ctx, struct xc_sr_record *rec);
+static int process_buffered_record(struct xc_sr_context *ctx, struct xc_sr_record *rec);
 static int handle_checkpoint(struct xc_sr_context *ctx)
 {
     xc_interface *xch = ctx->xch;
     int rc = 0, ret;
     unsigned int i;
 
     if ( ctx->stream_type == XC_STREAM_PLAIN )
     {
         ERROR("Found checkpoint in non-checkpointed stream");
         rc = -1;
         goto err;
     }
@@ -483,25 +483,25 @@ static int handle_checkpoint(struct xc_sr_context *ctx)
 
     default: /* Other fatal error */
         rc = -1;
         goto err;
     }
 
     if ( ctx->restore.buffer_all_records )
     {
         IPRINTF("All records buffered");
 
         for ( i = 0; i < ctx->restore.buffered_rec_num; i++ )
         {
-            rc = process_record(ctx, &ctx->restore.buffered_records[i]);
+            rc = process_buffered_record(ctx, &ctx->restore.buffered_records[i]);
             if ( rc )
                 goto err;
         }
         ctx->restore.buffered_rec_num = 0;
         IPRINTF("All records processed");
     }
     else
         ctx->restore.buffer_all_records = true;
 
     if ( ctx->stream_type == XC_STREAM_COLO )
     {
 #define HANDLE_CALLBACK_RETURN_VALUE(ret)                   \
@@ -544,47 +544,53 @@ static int handle_checkpoint(struct xc_sr_context *ctx)
 
 #undef HANDLE_CALLBACK_RETURN_VALUE
 
         rc = send_checkpoint_dirty_pfn_list(ctx);
         if ( rc )
             goto err;
     }
 
  err:
     return rc;
 }
 
-static int buffer_record(struct xc_sr_context *ctx, struct xc_sr_record *rec)
+static int buffer_record(struct xc_sr_context *ctx, struct xc_sr_rhdr *rhdr)
 {
     xc_interface *xch = ctx->xch;
     unsigned int new_alloc_num;
+    struct xc_sr_record rec;
     struct xc_sr_record *p;
 
     if ( ctx->restore.buffered_rec_num >= ctx->restore.allocated_rec_num )
     {
         new_alloc_num = ctx->restore.allocated_rec_num + DEFAULT_BUF_RECORDS;
         p = realloc(ctx->restore.buffered_records,
                     new_alloc_num * sizeof(struct xc_sr_record));
         if ( !p )
         {
             ERROR("Failed to realloc memory for buffered records");
             return -1;
         }
 
         ctx->restore.buffered_records = p;
         ctx->restore.allocated_rec_num = new_alloc_num;
     }
 
+    if ( read_record_data(ctx, ctx->fd, rhdr, &rec) )
+    {
+        return -1;
+    }
+
     memcpy(&ctx->restore.buffered_records[ctx->restore.buffered_rec_num++],
-           rec, sizeof(*rec));
+           &rec, sizeof(rec));
 
     return 0;
 }
 
 int handle_static_data_end(struct xc_sr_context *ctx)
 {
     xc_interface *xch = ctx->xch;
     unsigned int missing = 0;
     int rc = 0;
 
     if ( ctx->restore.seen_static_data_end )
     {
@@ -597,25 +603,25 @@ int handle_static_data_end(struct xc_sr_context *ctx)
     rc = ctx->restore.ops.static_data_complete(ctx, &missing);
     if ( rc )
         return rc;
 
     if ( ctx->restore.callbacks->static_data_done &&
          (rc = ctx->restore.callbacks->static_data_done(
              missing, ctx->restore.callbacks->data) != 0) )
         ERROR("static_data_done() callback failed: %d\n", rc);
 
     return rc;
 }
 
-static int process_record(struct xc_sr_context *ctx, struct xc_sr_record *rec)
+static int process_buffered_record(struct xc_sr_context *ctx, struct xc_sr_record *rec)
 {
     xc_interface *xch = ctx->xch;
     int rc = 0;
 
     switch ( rec->type )
     {
     case REC_TYPE_END:
         break;
 
     case REC_TYPE_PAGE_DATA:
         rc = handle_page_data(ctx, rec);
         break;
@@ -635,24 +641,37 @@ static int process_record(struct xc_sr_context *ctx, struct xc_sr_record *rec)
 
     default:
         rc = ctx->restore.ops.process_record(ctx, rec);
         break;
     }
 
     free(rec->data);
     rec->data = NULL;
 
     return rc;
 }
 
+static int process_incoming_record_header(struct xc_sr_context *ctx, struct xc_sr_rhdr *rhdr)
+{
+    struct xc_sr_record rec;
+    int rc;
+
+    rc = read_record_data(ctx, ctx->fd, rhdr, &rec);
+    if ( rc )
+        return rc;
+
+    return process_buffered_record(ctx, &rec);
+}
+
+
 static int setup(struct xc_sr_context *ctx)
 {
     xc_interface *xch = ctx->xch;
     int rc;
     DECLARE_HYPERCALL_BUFFER_SHADOW(unsigned long, dirty_bitmap,
                                     &ctx->restore.dirty_bitmap_hbuf);
 
     if ( ctx->stream_type == XC_STREAM_COLO )
     {
         dirty_bitmap = xc_hypercall_buffer_alloc_pages(
             xch, dirty_bitmap, NRPAGES(bitmap_size(ctx->restore.p2m_size)));
 
@@ -731,75 +750,75 @@ static void cleanup(struct xc_sr_context *ctx)
     free(ctx->restore.pfns);
 
     if ( ctx->restore.ops.cleanup(ctx) )
         PERROR("Failed to clean up");
 }
 
 /*
  * Restore a domain.
  */
 static int restore(struct xc_sr_context *ctx)
 {
     xc_interface *xch = ctx->xch;
-    struct xc_sr_record rec;
+    struct xc_sr_rhdr rhdr;
     int rc, saved_rc = 0, saved_errno = 0;
 
     IPRINTF("Restoring domain");
 
     rc = setup(ctx);
     if ( rc )
         goto err;
 
     do
     {
-        rc = read_record(ctx, ctx->fd, &rec);
+        rc = read_record_header(ctx, ctx->fd, &rhdr);
         if ( rc )
         {
             if ( ctx->restore.buffer_all_records )
                 goto remus_failover;
             else
                 goto err;
         }
 
         if ( ctx->restore.buffer_all_records &&
-             rec.type != REC_TYPE_END &&
-             rec.type != REC_TYPE_CHECKPOINT )
+             rhdr.type != REC_TYPE_END &&
+             rhdr.type != REC_TYPE_CHECKPOINT )
         {
-            rc = buffer_record(ctx, &rec);
+            rc = buffer_record(ctx, &rhdr);
             if ( rc )
                 goto err;
         }
         else
         {
-            rc = process_record(ctx, &rec);
+            rc = process_incoming_record_header(ctx, &rhdr);
             if ( rc == RECORD_NOT_PROCESSED )
             {
-                if ( rec.type & REC_TYPE_OPTIONAL )
+                if ( rhdr.type & REC_TYPE_OPTIONAL )
                     DPRINTF("Ignoring optional record %#x (%s)",
-                            rec.type, rec_type_to_str(rec.type));
+                            rhdr.type, rec_type_to_str(rhdr.type));
                 else
                 {
                     ERROR("Mandatory record %#x (%s) not handled",
-                          rec.type, rec_type_to_str(rec.type));
+                          rhdr.type, rec_type_to_str(rhdr.type));
                     rc = -1;
                     goto err;
                 }
             }
             else if ( rc == BROKEN_CHANNEL )
                 goto remus_failover;
             else if ( rc )
                 goto err;
         }
 
-    } while ( rec.type != REC_TYPE_END );
+    } while ( rhdr.type != REC_TYPE_END );
 
  remus_failover:
     if ( ctx->stream_type == XC_STREAM_COLO )
     {
         /* With COLO, we have already called stream_complete */
         rc = 0;
         IPRINTF("COLO Failover");
         goto done;
     }
 
     /*
      * With Remus, if we reach here, there must be some error on primary,
--- a/tools/libs/guest/xg_sr_save.c
+++ b/tools/libs/guest/xg_sr_save.c
@@ -582,33 +582,38 @@ static int send_memory_live(struct xc_sr_context *ctx)
         goto out;
     }
 
  out:
     xc_set_progress_prefix(xch, NULL);
     free(progress_str);
     return rc;
 }
 
 static int colo_merge_secondary_dirty_bitmap(struct xc_sr_context *ctx)
 {
     xc_interface *xch = ctx->xch;
+    struct xc_sr_rhdr rhdr;
     struct xc_sr_record rec;
     uint64_t *pfns = NULL;
     uint64_t pfn;
     unsigned int count, i;
     int rc;
     DECLARE_HYPERCALL_BUFFER_SHADOW(unsigned long, dirty_bitmap,
                                     &ctx->save.dirty_bitmap_hbuf);
 
-    rc = read_record(ctx, ctx->save.recv_fd, &rec);
+    rc = read_record_header(ctx, ctx->save.recv_fd, &rhdr);
+    if ( rc )
+        goto err;
+
+    rc = read_record_data(ctx, ctx->save.recv_fd, &rhdr, &rec);
     if ( rc )
         goto err;
 
     if ( rec.type != REC_TYPE_CHECKPOINT_DIRTY_PFN_LIST )
     {
         PERROR("Expect dirty bitmap record, but received %u", rec.type);
         rc = -1;
         goto err;
     }
 
     if ( rec.length % sizeof(*pfns) )
     {

Places

File xen.sr-restore-read_record.patch of Package xen

Places