| Message ID | 20170323204544.12015-30-quintela@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Thu, Mar 23, 2017 at 09:45:22PM +0100, Juan Quintela wrote: > It was on MigrationState when it is only used inside ram.c for > postcopy. Problem is that we need to access it without being able to > pass it RAMState directly. > > Signed-off-by: Juan Quintela <quintela@redhat.com> > --- > include/migration/migration.h | 2 -- > migration/migration.c | 1 - > migration/ram.c | 7 +++++-- > 3 files changed, 5 insertions(+), 5 deletions(-) > > diff --git a/include/migration/migration.h b/include/migration/migration.h > index 84cef4b..e032fb0 100644 > --- a/include/migration/migration.h > +++ b/include/migration/migration.h > @@ -189,8 +189,6 @@ struct MigrationState > /* Queue of outstanding page requests from the destination */ > QemuMutex src_page_req_mutex; > QSIMPLEQ_HEAD(src_page_requests, MigrationSrcPageRequest) src_page_requests; > - /* The RAMBlock used in the last src_page_request */ > - RAMBlock *last_req_rb; > /* The semaphore is used to notify COLO thread that failover is finished */ > QemuSemaphore colo_exit_sem; > > diff --git a/migration/migration.c b/migration/migration.c > index e532430..b220941 100644 > --- a/migration/migration.c > +++ b/migration/migration.c > @@ -1118,7 +1118,6 @@ MigrationState *migrate_init(const MigrationParams *params) > s->postcopy_after_devices = false; > s->postcopy_requests = 0; > s->migration_thread_running = false; > - s->last_req_rb = NULL; > error_free(s->error); > s->error = NULL; > > diff --git a/migration/ram.c b/migration/ram.c > index dd5a453..325a0f3 100644 > --- a/migration/ram.c > +++ b/migration/ram.c > @@ -203,6 +203,8 @@ struct RAMState { > QemuMutex bitmap_mutex; > /* Ram Bitmap protected by RCU */ > RAMBitmap *ram_bitmap; > + /* The RAMBlock used in the last src_page_request */ ^ "s" missing Besides: Reviewed-by: Peter Xu <peterx@redhat.com> > + RAMBlock *last_req_rb; > }; > typedef struct RAMState RAMState; -- peterx
Peter Xu <peterx@redhat.com> wrote: > On Thu, Mar 23, 2017 at 09:45:22PM +0100, Juan Quintela wrote: >> It was on MigrationState when it is only used inside ram.c for >> postcopy. Problem is that we need to access it without being able to >> pass it RAMState directly. >> >> Signed-off-by: Juan Quintela <quintela@redhat.com> >> --- >> include/migration/migration.h | 2 -- >> migration/migration.c | 1 - >> migration/ram.c | 7 +++++-- >> 3 files changed, 5 insertions(+), 5 deletions(-) >> >> diff --git a/include/migration/migration.h b/include/migration/migration.h >> index 84cef4b..e032fb0 100644 >> --- a/include/migration/migration.h >> +++ b/include/migration/migration.h >> @@ -189,8 +189,6 @@ struct MigrationState >> /* Queue of outstanding page requests from the destination */ >> QemuMutex src_page_req_mutex; >> QSIMPLEQ_HEAD(src_page_requests, MigrationSrcPageRequest) src_page_requests; >> - /* The RAMBlock used in the last src_page_request */ >> - RAMBlock *last_req_rb; >> /* The semaphore is used to notify COLO thread that failover is finished */ >> QemuSemaphore colo_exit_sem; >> >> diff --git a/migration/migration.c b/migration/migration.c >> index e532430..b220941 100644 >> --- a/migration/migration.c >> +++ b/migration/migration.c >> @@ -1118,7 +1118,6 @@ MigrationState *migrate_init(const MigrationParams *params) >> s->postcopy_after_devices = false; >> s->postcopy_requests = 0; >> s->migration_thread_running = false; >> - s->last_req_rb = NULL; >> error_free(s->error); >> s->error = NULL; >> >> diff --git a/migration/ram.c b/migration/ram.c >> index dd5a453..325a0f3 100644 >> --- a/migration/ram.c >> +++ b/migration/ram.c >> @@ -203,6 +203,8 @@ struct RAMState { >> QemuMutex bitmap_mutex; >> /* Ram Bitmap protected by RCU */ >> RAMBitmap *ram_bitmap; >> + /* The RAMBlock used in the last src_page_request */ > ^ "s" missing > > Besides: The important one is only the last one, we don't really care about the previous here, no? > > Reviewed-by: Peter Xu <peterx@redhat.com> > >> + RAMBlock *last_req_rb; >> }; >> typedef struct RAMState RAMState; > > -- peterx Thanks,
On Thu, Mar 30, 2017 at 06:08:45PM +0200, Juan Quintela wrote: > Peter Xu <peterx@redhat.com> wrote: > > On Thu, Mar 23, 2017 at 09:45:22PM +0100, Juan Quintela wrote: > >> It was on MigrationState when it is only used inside ram.c for > >> postcopy. Problem is that we need to access it without being able to > >> pass it RAMState directly. > >> > >> Signed-off-by: Juan Quintela <quintela@redhat.com> > >> --- > >> include/migration/migration.h | 2 -- > >> migration/migration.c | 1 - > >> migration/ram.c | 7 +++++-- > >> 3 files changed, 5 insertions(+), 5 deletions(-) > >> > >> diff --git a/include/migration/migration.h b/include/migration/migration.h > >> index 84cef4b..e032fb0 100644 > >> --- a/include/migration/migration.h > >> +++ b/include/migration/migration.h > >> @@ -189,8 +189,6 @@ struct MigrationState > >> /* Queue of outstanding page requests from the destination */ > >> QemuMutex src_page_req_mutex; > >> QSIMPLEQ_HEAD(src_page_requests, MigrationSrcPageRequest) src_page_requests; > >> - /* The RAMBlock used in the last src_page_request */ > >> - RAMBlock *last_req_rb; > >> /* The semaphore is used to notify COLO thread that failover is finished */ > >> QemuSemaphore colo_exit_sem; > >> > >> diff --git a/migration/migration.c b/migration/migration.c > >> index e532430..b220941 100644 > >> --- a/migration/migration.c > >> +++ b/migration/migration.c > >> @@ -1118,7 +1118,6 @@ MigrationState *migrate_init(const MigrationParams *params) > >> s->postcopy_after_devices = false; > >> s->postcopy_requests = 0; > >> s->migration_thread_running = false; > >> - s->last_req_rb = NULL; > >> error_free(s->error); > >> s->error = NULL; > >> > >> diff --git a/migration/ram.c b/migration/ram.c > >> index dd5a453..325a0f3 100644 > >> --- a/migration/ram.c > >> +++ b/migration/ram.c > >> @@ -203,6 +203,8 @@ struct RAMState { > >> QemuMutex bitmap_mutex; > >> /* Ram Bitmap protected by RCU */ > >> RAMBitmap *ram_bitmap; > >> + /* The RAMBlock used in the last src_page_request */ > > ^ "s" missing > > > > Besides: > > The important one is only the last one, we don't really care about the > previous here, no? I preferred "src_page_requests" since that's the variable name (so then people can do symbol search on that). Anyway, that's trivial, so please feel free to ignore it. :-) -- peterx
diff --git a/include/migration/migration.h b/include/migration/migration.h index 84cef4b..e032fb0 100644 --- a/include/migration/migration.h +++ b/include/migration/migration.h @@ -189,8 +189,6 @@ struct MigrationState /* Queue of outstanding page requests from the destination */ QemuMutex src_page_req_mutex; QSIMPLEQ_HEAD(src_page_requests, MigrationSrcPageRequest) src_page_requests; - /* The RAMBlock used in the last src_page_request */ - RAMBlock *last_req_rb; /* The semaphore is used to notify COLO thread that failover is finished */ QemuSemaphore colo_exit_sem; diff --git a/migration/migration.c b/migration/migration.c index e532430..b220941 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -1118,7 +1118,6 @@ MigrationState *migrate_init(const MigrationParams *params) s->postcopy_after_devices = false; s->postcopy_requests = 0; s->migration_thread_running = false; - s->last_req_rb = NULL; error_free(s->error); s->error = NULL; diff --git a/migration/ram.c b/migration/ram.c index dd5a453..325a0f3 100644 --- a/migration/ram.c +++ b/migration/ram.c @@ -203,6 +203,8 @@ struct RAMState { QemuMutex bitmap_mutex; /* Ram Bitmap protected by RCU */ RAMBitmap *ram_bitmap; + /* The RAMBlock used in the last src_page_request */ + RAMBlock *last_req_rb; }; typedef struct RAMState RAMState; @@ -1224,12 +1226,13 @@ int ram_save_queue_pages(MigrationState *ms, const char *rbname, ram_addr_t start, ram_addr_t len) { RAMBlock *ramblock; + RAMState *rs = &ram_state; ms->postcopy_requests++; rcu_read_lock(); if (!rbname) { /* Reuse last RAMBlock */ - ramblock = ms->last_req_rb; + ramblock = rs->last_req_rb; if (!ramblock) { /* @@ -1247,7 +1250,7 @@ int ram_save_queue_pages(MigrationState *ms, const char *rbname, error_report("ram_save_queue_pages no block '%s'", rbname); goto err; } - ms->last_req_rb = ramblock; + rs->last_req_rb = ramblock; } trace_ram_save_queue_pages(ramblock->idstr, start, len); if (start+len > ramblock->used_length) {
It was on MigrationState when it is only used inside ram.c for postcopy. Problem is that we need to access it without being able to pass it RAMState directly. Signed-off-by: Juan Quintela <quintela@redhat.com> --- include/migration/migration.h | 2 -- migration/migration.c | 1 - migration/ram.c | 7 +++++-- 3 files changed, 5 insertions(+), 5 deletions(-)