| Message ID | 20240318154621.2361161-9-npiggin@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | replay: fixes and new test cases | expand |
Nicholas Piggin <npiggin@gmail.com> writes: > Migration reads host clocks when not holding the replay_mutex, which > asserts when recording a trace. It seems that these migration times > should be host times like other statistics in MigrationState. s/host/CLOCK_HOST/ and s/host/CLOCK_REALTIME/ but its a confusing sentence. If the MigrationState is guest visible it should be QEMU_CLOCK_VIRTUAL surely? > These > do not require the replay_mutex. > > Signed-off-by: Nicholas Piggin <npiggin@gmail.com> > --- > migration/migration.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/migration/migration.c b/migration/migration.c > index 644e073b7d..2c286ccf63 100644 > --- a/migration/migration.c > +++ b/migration/migration.c > @@ -3424,7 +3424,7 @@ static void *migration_thread(void *opaque) > { > MigrationState *s = opaque; > MigrationThread *thread = NULL; > - int64_t setup_start = qemu_clock_get_ms(QEMU_CLOCK_HOST); > + int64_t setup_start = qemu_clock_get_ms(QEMU_CLOCK_REALTIME); > MigThrError thr_error; > bool urgent = false; > > @@ -3476,7 +3476,7 @@ static void *migration_thread(void *opaque) > qemu_savevm_wait_unplug(s, MIGRATION_STATUS_SETUP, > MIGRATION_STATUS_ACTIVE); > > - s->setup_time = qemu_clock_get_ms(QEMU_CLOCK_HOST) - setup_start; > + s->setup_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME) - setup_start; > > trace_migration_thread_setup_complete(); > > @@ -3555,7 +3555,7 @@ static void *bg_migration_thread(void *opaque) > > migration_rate_set(RATE_LIMIT_DISABLED); > > - setup_start = qemu_clock_get_ms(QEMU_CLOCK_HOST); > + setup_start = qemu_clock_get_ms(QEMU_CLOCK_REALTIME); > /* > * We want to save vmstate for the moment when migration has been > * initiated but also we want to save RAM content while VM is running. > @@ -3588,7 +3588,7 @@ static void *bg_migration_thread(void *opaque) > qemu_savevm_wait_unplug(s, MIGRATION_STATUS_SETUP, > MIGRATION_STATUS_ACTIVE); > > - s->setup_time = qemu_clock_get_ms(QEMU_CLOCK_HOST) - setup_start; > + s->setup_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME) - setup_start; > > trace_migration_thread_setup_complete();
On Wed Mar 20, 2024 at 6:40 AM AEST, Alex Bennée wrote: > Nicholas Piggin <npiggin@gmail.com> writes: > > > Migration reads host clocks when not holding the replay_mutex, which > > asserts when recording a trace. It seems that these migration times > > should be host times like other statistics in MigrationState. > > s/host/CLOCK_HOST/ and s/host/CLOCK_REALTIME/ but its a confusing > sentence. Yes you're right. > If the MigrationState is guest visible it should be > QEMU_CLOCK_VIRTUAL surely? I didn't think it was visible. It was added with ed4fbd10823 and just exported to QMP. It was the first and only user of host clock in migration stats, the rest use rt clock. OTOH it does seem to have deliberately chosen host... can you see any reason for it? Thanks, Nick
diff --git a/migration/migration.c b/migration/migration.c index 644e073b7d..2c286ccf63 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -3424,7 +3424,7 @@ static void *migration_thread(void *opaque) { MigrationState *s = opaque; MigrationThread *thread = NULL; - int64_t setup_start = qemu_clock_get_ms(QEMU_CLOCK_HOST); + int64_t setup_start = qemu_clock_get_ms(QEMU_CLOCK_REALTIME); MigThrError thr_error; bool urgent = false; @@ -3476,7 +3476,7 @@ static void *migration_thread(void *opaque) qemu_savevm_wait_unplug(s, MIGRATION_STATUS_SETUP, MIGRATION_STATUS_ACTIVE); - s->setup_time = qemu_clock_get_ms(QEMU_CLOCK_HOST) - setup_start; + s->setup_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME) - setup_start; trace_migration_thread_setup_complete(); @@ -3555,7 +3555,7 @@ static void *bg_migration_thread(void *opaque) migration_rate_set(RATE_LIMIT_DISABLED); - setup_start = qemu_clock_get_ms(QEMU_CLOCK_HOST); + setup_start = qemu_clock_get_ms(QEMU_CLOCK_REALTIME); /* * We want to save vmstate for the moment when migration has been * initiated but also we want to save RAM content while VM is running. @@ -3588,7 +3588,7 @@ static void *bg_migration_thread(void *opaque) qemu_savevm_wait_unplug(s, MIGRATION_STATUS_SETUP, MIGRATION_STATUS_ACTIVE); - s->setup_time = qemu_clock_get_ms(QEMU_CLOCK_HOST) - setup_start; + s->setup_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME) - setup_start; trace_migration_thread_setup_complete();
Migration reads host clocks when not holding the replay_mutex, which asserts when recording a trace. It seems that these migration times should be host times like other statistics in MigrationState. These do not require the replay_mutex. Signed-off-by: Nicholas Piggin <npiggin@gmail.com> --- migration/migration.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)