| Message ID | 1443515898-3594-39-git-send-email-dgilbert@redhat.com |
|---|---|
| State | New |
| Headers | show |
"Dr. David Alan Gilbert (git)" <dgilbert@redhat.com> wrote: > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com> > > Add MIG_RP_MSG_REQ_PAGES command on Return path for the postcopy > destination to request a page from the source. > > Two versions exist: > MIG_RP_MSG_REQ_PAGES_ID that includes a RAMBlock name and start/len > MIG_RP_MSG_REQ_PAGES that just has start/len for use with the same > RAMBlock as a previous MIG_RP_MSG_REQ_PAGES_ID > > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com> Reviewed-by: Juan Quintela <quintela@redhat.com> > diff --git a/migration/migration.c b/migration/migration.c > index 4f8ef6f..e994164 100644 > --- a/migration/migration.c > +++ b/migration/migration.c > @@ -251,6 +251,35 @@ static void deferred_incoming_migration(Error **errp) > deferred_incoming = true; > } > > +/* Request a range of pages from the source VM at the given > + * start address. > + * rbname: Name of the RAMBlock to request the page in, if NULL it's the same > + * as the last request (a name must have been given previously) > + * Start: Address offset within the RB > + * Len: Length in bytes required - must be a multiple of pagesize > + */ > +void migrate_send_rp_req_pages(MigrationIncomingState *mis, const char *rbname, > + ram_addr_t start, size_t len) > +{ > + uint8_t bufc[12 + 1 + 255]; /* start (8), len (4), rbname upto 256 */ > + size_t msglen = 12; /* start + len */ > + > + *(uint64_t *)bufc = cpu_to_be64((uint64_t)start); > + *(uint32_t *)(bufc + 8) = cpu_to_be32((uint32_t)len); struct foo { uint64_t start; uint32_t len; char msg[]; } As we are supposed to have the same qemu on both sides, this should work, no? Reviewed by anyways, because I am not sure that proposed solution is (even) better than current code. Later, Juan.
* Juan Quintela (quintela@redhat.com) wrote: > "Dr. David Alan Gilbert (git)" <dgilbert@redhat.com> wrote: > > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com> > > > > Add MIG_RP_MSG_REQ_PAGES command on Return path for the postcopy > > destination to request a page from the source. > > > > Two versions exist: > > MIG_RP_MSG_REQ_PAGES_ID that includes a RAMBlock name and start/len > > MIG_RP_MSG_REQ_PAGES that just has start/len for use with the same > > RAMBlock as a previous MIG_RP_MSG_REQ_PAGES_ID > > > > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com> > > > Reviewed-by: Juan Quintela <quintela@redhat.com> > > > > diff --git a/migration/migration.c b/migration/migration.c > > index 4f8ef6f..e994164 100644 > > --- a/migration/migration.c > > +++ b/migration/migration.c > > @@ -251,6 +251,35 @@ static void deferred_incoming_migration(Error **errp) > > deferred_incoming = true; > > } > > > > +/* Request a range of pages from the source VM at the given > > + * start address. > > + * rbname: Name of the RAMBlock to request the page in, if NULL it's the same > > + * as the last request (a name must have been given previously) > > + * Start: Address offset within the RB > > + * Len: Length in bytes required - must be a multiple of pagesize > > + */ > > +void migrate_send_rp_req_pages(MigrationIncomingState *mis, const char *rbname, > > + ram_addr_t start, size_t len) > > +{ > > + uint8_t bufc[12 + 1 + 255]; /* start (8), len (4), rbname upto 256 */ > > + size_t msglen = 12; /* start + len */ > > + > > + *(uint64_t *)bufc = cpu_to_be64((uint64_t)start); > > + *(uint32_t *)(bufc + 8) = cpu_to_be32((uint32_t)len); > > struct foo { > uint64_t start; > uint32_t len; > char msg[]; > } > > As we are supposed to have the same qemu on both sides, this should > work, no? In principal I think it should work between opposite endian hosts; if they are capable of running the same endian guest (untested); similarly there's no requirement for the two qemu's to be built with the same compiler, or I think host word size. Using structs on a wire always makes me start worrying what the compiler would do to the layout; I'd assume it would at least need a 'packed' to be sure a particularly entertaining compiler doesn't do something odd. > Reviewed by anyways, because I am not sure that proposed solution is > (even) better than current code. Dave > > Later, Juan. -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
On (Tue) 29 Sep 2015 [09:38:02], Dr. David Alan Gilbert (git) wrote: > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com> > > Add MIG_RP_MSG_REQ_PAGES command on Return path for the postcopy > destination to request a page from the source. > > Two versions exist: > MIG_RP_MSG_REQ_PAGES_ID that includes a RAMBlock name and start/len > MIG_RP_MSG_REQ_PAGES that just has start/len for use with the same > RAMBlock as a previous MIG_RP_MSG_REQ_PAGES_ID > > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com> Reviewed-by: Amit Shah <amit.shah@redhat.com> Amit
diff --git a/include/migration/migration.h b/include/migration/migration.h index 0586f8c..9be08c8 100644 --- a/include/migration/migration.h +++ b/include/migration/migration.h @@ -49,6 +49,9 @@ enum mig_rp_message_type { MIG_RP_MSG_SHUT, /* sibling will not send any more RP messages */ MIG_RP_MSG_PONG, /* Response to a PING; data (seq: be32 ) */ + MIG_RP_MSG_REQ_PAGES_ID, /* data (start: be64, len: be32, id: string) */ + MIG_RP_MSG_REQ_PAGES, /* data (start: be64, len: be32) */ + MIG_RP_MSG_MAX }; @@ -263,6 +266,8 @@ void migrate_send_rp_shut(MigrationIncomingState *mis, uint32_t value); void migrate_send_rp_pong(MigrationIncomingState *mis, uint32_t value); +void migrate_send_rp_req_pages(MigrationIncomingState *mis, const char* rbname, + ram_addr_t start, size_t len); void ram_control_before_iterate(QEMUFile *f, uint64_t flags); void ram_control_after_iterate(QEMUFile *f, uint64_t flags); diff --git a/migration/migration.c b/migration/migration.c index 4f8ef6f..e994164 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -251,6 +251,35 @@ static void deferred_incoming_migration(Error **errp) deferred_incoming = true; } +/* Request a range of pages from the source VM at the given + * start address. + * rbname: Name of the RAMBlock to request the page in, if NULL it's the same + * as the last request (a name must have been given previously) + * Start: Address offset within the RB + * Len: Length in bytes required - must be a multiple of pagesize + */ +void migrate_send_rp_req_pages(MigrationIncomingState *mis, const char *rbname, + ram_addr_t start, size_t len) +{ + uint8_t bufc[12 + 1 + 255]; /* start (8), len (4), rbname upto 256 */ + size_t msglen = 12; /* start + len */ + + *(uint64_t *)bufc = cpu_to_be64((uint64_t)start); + *(uint32_t *)(bufc + 8) = cpu_to_be32((uint32_t)len); + + if (rbname) { + int rbname_len = strlen(rbname); + assert(rbname_len < 256); + + bufc[msglen++] = rbname_len; + memcpy(bufc + msglen, rbname, rbname_len); + msglen += rbname_len; + migrate_send_rp_message(mis, MIG_RP_MSG_REQ_PAGES_ID, msglen, bufc); + } else { + migrate_send_rp_message(mis, MIG_RP_MSG_REQ_PAGES, msglen, bufc); + } +} + void qemu_start_incoming_migration(const char *uri, Error **errp) { const char *p; @@ -1092,10 +1121,23 @@ static struct rp_cmd_args { [MIG_RP_MSG_INVALID] = { .len = -1, .name = "INVALID" }, [MIG_RP_MSG_SHUT] = { .len = 4, .name = "SHUT" }, [MIG_RP_MSG_PONG] = { .len = 4, .name = "PONG" }, + [MIG_RP_MSG_REQ_PAGES] = { .len = 12, .name = "REQ_PAGES" }, + [MIG_RP_MSG_REQ_PAGES_ID] = { .len = -1, .name = "REQ_PAGES_ID" }, [MIG_RP_MSG_MAX] = { .len = -1, .name = "MAX" }, }; /* + * Process a request for pages received on the return path, + * We're allowed to send more than requested (e.g. to round to our page size) + * and we don't need to send pages that have already been sent. + */ +static void migrate_handle_rp_req_pages(MigrationState *ms, const char* rbname, + ram_addr_t start, size_t len) +{ + trace_migrate_handle_rp_req_pages(rbname, start, len); +} + +/* * Handles messages sent on the return path towards the source VM * */ @@ -1107,6 +1149,8 @@ static void *source_return_path_thread(void *opaque) const int max_len = 512; uint8_t buf[max_len]; uint32_t tmp32, sibling_error; + ram_addr_t start = 0; /* =0 to silence warning */ + size_t len = 0, expected_len; int res; trace_source_return_path_thread_entry(); @@ -1166,6 +1210,32 @@ static void *source_return_path_thread(void *opaque) trace_source_return_path_thread_pong(tmp32); break; + case MIG_RP_MSG_REQ_PAGES: + start = be64_to_cpup((uint64_t *)buf); + len = be32_to_cpup((uint32_t *)(buf + 8)); + migrate_handle_rp_req_pages(ms, NULL, start, len); + break; + + case MIG_RP_MSG_REQ_PAGES_ID: + expected_len = 12 + 1; /* header + termination */ + + if (header_len >= expected_len) { + start = be64_to_cpup((uint64_t *)buf); + len = be32_to_cpup((uint32_t *)(buf + 8)); + /* Now we expect an idstr */ + tmp32 = buf[12]; /* Length of the following idstr */ + buf[13 + tmp32] = '\0'; + expected_len += tmp32; + } + if (header_len != expected_len) { + error_report("RP: Req_Page_id with length %d expecting %zd", + header_len, expected_len); + mark_source_rp_bad(ms); + goto out; + } + migrate_handle_rp_req_pages(ms, (char *)&buf[13], start, len); + break; + default: break; } diff --git a/trace-events b/trace-events index dec2ae1..b58077f 100644 --- a/trace-events +++ b/trace-events @@ -1439,6 +1439,7 @@ migrate_set_state(int new_state) "new state %d" migrate_fd_cleanup(void) "" migrate_fd_error(void) "" migrate_fd_cancel(void) "" +migrate_handle_rp_req_pages(const char *rbname, size_t start, size_t len) "in %s at %zx len %zx" migrate_pending(uint64_t size, uint64_t max, uint64_t post, uint64_t nonpost) "pending size %" PRIu64 " max %" PRIu64 " (post=%" PRIu64 " nonpost=%" PRIu64 ")" migrate_send_rp_message(int msg_type, uint16_t len) "%d: len %d" migration_completion_file_err(void) ""