| Message ID | 20200903152646.93336-2-peterx@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | migration/postcopy: Sync faulted addresses after network recovered | expand |
* Peter Xu (peterx@redhat.com) wrote: > We duplicated the logic of maintaining the last_rb variable at both callers of > this function. Pass *rb pointer into the function so that we can avoid > duplicating the logic. Also, when we have the rb pointer, it's also easier to > remove the original 2nd & 4th parameters, because both of them (name of the > ramblock when needed, or the page size) can be fetched from the ramblock > pointer too. > > Signed-off-by: Peter Xu <peterx@redhat.com> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> > --- > migration/migration.c | 26 ++++++++++++++++++-------- > migration/migration.h | 4 ++-- > migration/postcopy-ram.c | 24 ++---------------------- > 3 files changed, 22 insertions(+), 32 deletions(-) > > diff --git a/migration/migration.c b/migration/migration.c > index 58a5452471..6761e3f233 100644 > --- a/migration/migration.c > +++ b/migration/migration.c > @@ -311,25 +311,35 @@ error: > return ret; > } > > -/* 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) > +/* Request one page from the source VM at the given start address. > + * rb: the RAMBlock to request the page in > * Start: Address offset within the RB > * Len: Length in bytes required - must be a multiple of pagesize > */ > -int migrate_send_rp_req_pages(MigrationIncomingState *mis, const char *rbname, > - ram_addr_t start, size_t len) > +int migrate_send_rp_req_pages(MigrationIncomingState *mis, RAMBlock *rb, > + ram_addr_t start) > { > uint8_t bufc[12 + 1 + 255]; /* start (8), len (4), rbname up to 256 */ > size_t msglen = 12; /* start + len */ > + size_t len = qemu_ram_pagesize(rb); > enum mig_rp_message_type msg_type; > + const char *rbname; > + int rbname_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); > + /* > + * We maintain the last ramblock that we requested for page. Note that we > + * don't need locking because this function will only be called within the > + * postcopy ram fault thread. > + */ > + if (rb != mis->last_rb) { > + mis->last_rb = rb; > + > + rbname = qemu_ram_get_idstr(rb); > + rbname_len = strlen(rbname); > + > assert(rbname_len < 256); > > bufc[msglen++] = rbname_len; > diff --git a/migration/migration.h b/migration/migration.h > index ae497bd45a..ca8dc4c773 100644 > --- a/migration/migration.h > +++ b/migration/migration.h > @@ -328,8 +328,8 @@ void migrate_send_rp_shut(MigrationIncomingState *mis, > uint32_t value); > void migrate_send_rp_pong(MigrationIncomingState *mis, > uint32_t value); > -int migrate_send_rp_req_pages(MigrationIncomingState *mis, const char* rbname, > - ram_addr_t start, size_t len); > +int migrate_send_rp_req_pages(MigrationIncomingState *mis, RAMBlock *rb, > + ram_addr_t start); > void migrate_send_rp_recv_bitmap(MigrationIncomingState *mis, > char *block_name); > void migrate_send_rp_resume_ack(MigrationIncomingState *mis, uint32_t value); > diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c > index 1bb22f2b6c..11a70441a6 100644 > --- a/migration/postcopy-ram.c > +++ b/migration/postcopy-ram.c > @@ -684,14 +684,7 @@ int postcopy_request_shared_page(struct PostCopyFD *pcfd, RAMBlock *rb, > qemu_ram_get_idstr(rb), rb_offset); > return postcopy_wake_shared(pcfd, client_addr, rb); > } > - if (rb != mis->last_rb) { > - mis->last_rb = rb; > - migrate_send_rp_req_pages(mis, qemu_ram_get_idstr(rb), > - aligned_rbo, pagesize); > - } else { > - /* Save some space */ > - migrate_send_rp_req_pages(mis, NULL, aligned_rbo, pagesize); > - } > + migrate_send_rp_req_pages(mis, rb, aligned_rbo); > return 0; > } > > @@ -986,20 +979,7 @@ retry: > * Send the request to the source - we want to request one > * of our host page sizes (which is >= TPS) > */ > - if (rb != mis->last_rb) { > - mis->last_rb = rb; > - ret = migrate_send_rp_req_pages(mis, > - qemu_ram_get_idstr(rb), > - rb_offset, > - qemu_ram_pagesize(rb)); > - } else { > - /* Save some space */ > - ret = migrate_send_rp_req_pages(mis, > - NULL, > - rb_offset, > - qemu_ram_pagesize(rb)); > - } > - > + ret = migrate_send_rp_req_pages(mis, rb, rb_offset); > if (ret) { > /* May be network failure, try to wait for recovery */ > if (ret == -EIO && postcopy_pause_fault_thread(mis)) { > -- > 2.26.2 >
diff --git a/migration/migration.c b/migration/migration.c index 58a5452471..6761e3f233 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -311,25 +311,35 @@ error: return ret; } -/* 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) +/* Request one page from the source VM at the given start address. + * rb: the RAMBlock to request the page in * Start: Address offset within the RB * Len: Length in bytes required - must be a multiple of pagesize */ -int migrate_send_rp_req_pages(MigrationIncomingState *mis, const char *rbname, - ram_addr_t start, size_t len) +int migrate_send_rp_req_pages(MigrationIncomingState *mis, RAMBlock *rb, + ram_addr_t start) { uint8_t bufc[12 + 1 + 255]; /* start (8), len (4), rbname up to 256 */ size_t msglen = 12; /* start + len */ + size_t len = qemu_ram_pagesize(rb); enum mig_rp_message_type msg_type; + const char *rbname; + int rbname_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); + /* + * We maintain the last ramblock that we requested for page. Note that we + * don't need locking because this function will only be called within the + * postcopy ram fault thread. + */ + if (rb != mis->last_rb) { + mis->last_rb = rb; + + rbname = qemu_ram_get_idstr(rb); + rbname_len = strlen(rbname); + assert(rbname_len < 256); bufc[msglen++] = rbname_len; diff --git a/migration/migration.h b/migration/migration.h index ae497bd45a..ca8dc4c773 100644 --- a/migration/migration.h +++ b/migration/migration.h @@ -328,8 +328,8 @@ void migrate_send_rp_shut(MigrationIncomingState *mis, uint32_t value); void migrate_send_rp_pong(MigrationIncomingState *mis, uint32_t value); -int migrate_send_rp_req_pages(MigrationIncomingState *mis, const char* rbname, - ram_addr_t start, size_t len); +int migrate_send_rp_req_pages(MigrationIncomingState *mis, RAMBlock *rb, + ram_addr_t start); void migrate_send_rp_recv_bitmap(MigrationIncomingState *mis, char *block_name); void migrate_send_rp_resume_ack(MigrationIncomingState *mis, uint32_t value); diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c index 1bb22f2b6c..11a70441a6 100644 --- a/migration/postcopy-ram.c +++ b/migration/postcopy-ram.c @@ -684,14 +684,7 @@ int postcopy_request_shared_page(struct PostCopyFD *pcfd, RAMBlock *rb, qemu_ram_get_idstr(rb), rb_offset); return postcopy_wake_shared(pcfd, client_addr, rb); } - if (rb != mis->last_rb) { - mis->last_rb = rb; - migrate_send_rp_req_pages(mis, qemu_ram_get_idstr(rb), - aligned_rbo, pagesize); - } else { - /* Save some space */ - migrate_send_rp_req_pages(mis, NULL, aligned_rbo, pagesize); - } + migrate_send_rp_req_pages(mis, rb, aligned_rbo); return 0; } @@ -986,20 +979,7 @@ retry: * Send the request to the source - we want to request one * of our host page sizes (which is >= TPS) */ - if (rb != mis->last_rb) { - mis->last_rb = rb; - ret = migrate_send_rp_req_pages(mis, - qemu_ram_get_idstr(rb), - rb_offset, - qemu_ram_pagesize(rb)); - } else { - /* Save some space */ - ret = migrate_send_rp_req_pages(mis, - NULL, - rb_offset, - qemu_ram_pagesize(rb)); - } - + ret = migrate_send_rp_req_pages(mis, rb, rb_offset); if (ret) { /* May be network failure, try to wait for recovery */ if (ret == -EIO && postcopy_pause_fault_thread(mis)) {
We duplicated the logic of maintaining the last_rb variable at both callers of this function. Pass *rb pointer into the function so that we can avoid duplicating the logic. Also, when we have the rb pointer, it's also easier to remove the original 2nd & 4th parameters, because both of them (name of the ramblock when needed, or the page size) can be fetched from the ramblock pointer too. Signed-off-by: Peter Xu <peterx@redhat.com> --- migration/migration.c | 26 ++++++++++++++++++-------- migration/migration.h | 4 ++-- migration/postcopy-ram.c | 24 ++---------------------- 3 files changed, 22 insertions(+), 32 deletions(-)