| Message ID | 20230504113841.23130-4-quintela@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | QEMU file cleanups | expand |
On Thu, May 04, 2023 at 01:38:35PM +0200, Juan Quintela wrote: > It is really size_t. Everything else uses uint64_t, so move this to > uint64_t as well. A size can't be negative anyways. > > Signed-off-by: Juan Quintela <quintela@redhat.com> > --- > migration/migration.c | 6 +++--- > migration/qemu-file.c | 8 ++++---- > migration/qemu-file.h | 4 ++-- > 3 files changed, 9 insertions(+), 9 deletions(-) Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> With regards, Daniel
Juan Quintela <quintela@redhat.com> wrote: > It is really size_t. Everything else uses uint64_t, so move this to > uint64_t as well. A size can't be negative anyways. > > Signed-off-by: Juan Quintela <quintela@redhat.com> Self-nack. > - qemu_file_set_rate_limit(ms->to_dst_file, INT64_MAX); > + qemu_file_set_rate_limit(ms->to_dst_file, UINT64_MAX); 1st: this should be zero. > } else { > qemu_file_set_rate_limit(ms->to_dst_file, bandwidth / XFER_LIMIT_RATIO); > } > @@ -2301,7 +2301,7 @@ static void migration_completion(MigrationState *s) > } > if (ret >= 0) { > s->block_inactive = !migrate_colo(); > - qemu_file_set_rate_limit(s->to_dst_file, INT64_MAX); > + qemu_file_set_rate_limit(s->to_dst_file, UINT64_MAX); Same here > ret = qemu_savevm_state_complete_precopy(s->to_dst_file, false, > s->block_inactive); > } > @@ -3049,7 +3049,7 @@ static void *bg_migration_thread(void *opaque) > rcu_register_thread(); > object_ref(OBJECT(s)); > > - qemu_file_set_rate_limit(s->to_dst_file, INT64_MAX); > + qemu_file_set_rate_limit(s->to_dst_file, UINT64_MAX); And here. > @@ -748,18 +748,18 @@ int qemu_file_rate_limit(QEMUFile *f) > if (qemu_file_get_error(f)) { > return 1; > } > - if (f->rate_limit_max > 0 && f->rate_limit_used > f->rate_limit_max) { > + if (f->rate_limit_used > f->rate_limit_max) { And this is wrong. f->rate_limit_max == 0 means that we don't do rate_limit. Will resend this one later. Sorry, Juan. PD. No, I have no clue how I have had this patch applied the whole day and no failures and now I get failures in migration-test. The number of times that I run this test on the last two days have been in the hundreds.
diff --git a/migration/migration.c b/migration/migration.c index 232e387109..ee75c6cfbd 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -2119,7 +2119,7 @@ static int postcopy_start(MigrationState *ms) */ /* 0 max-postcopy-bandwidth means unlimited */ if (!bandwidth) { - qemu_file_set_rate_limit(ms->to_dst_file, INT64_MAX); + qemu_file_set_rate_limit(ms->to_dst_file, UINT64_MAX); } else { qemu_file_set_rate_limit(ms->to_dst_file, bandwidth / XFER_LIMIT_RATIO); } @@ -2301,7 +2301,7 @@ static void migration_completion(MigrationState *s) } if (ret >= 0) { s->block_inactive = !migrate_colo(); - qemu_file_set_rate_limit(s->to_dst_file, INT64_MAX); + qemu_file_set_rate_limit(s->to_dst_file, UINT64_MAX); ret = qemu_savevm_state_complete_precopy(s->to_dst_file, false, s->block_inactive); } @@ -3049,7 +3049,7 @@ static void *bg_migration_thread(void *opaque) rcu_register_thread(); object_ref(OBJECT(s)); - qemu_file_set_rate_limit(s->to_dst_file, INT64_MAX); + qemu_file_set_rate_limit(s->to_dst_file, UINT64_MAX); setup_start = qemu_clock_get_ms(QEMU_CLOCK_HOST); /* diff --git a/migration/qemu-file.c b/migration/qemu-file.c index ee04240a21..b322b363cf 100644 --- a/migration/qemu-file.c +++ b/migration/qemu-file.c @@ -43,7 +43,7 @@ struct QEMUFile { * Maximum amount of data in bytes to transfer during one * rate limiting time window */ - int64_t rate_limit_max; + uint64_t rate_limit_max; /* * Total amount of data in bytes queued for transfer * during this rate limiting time window @@ -748,18 +748,18 @@ int qemu_file_rate_limit(QEMUFile *f) if (qemu_file_get_error(f)) { return 1; } - if (f->rate_limit_max > 0 && f->rate_limit_used > f->rate_limit_max) { + if (f->rate_limit_used > f->rate_limit_max) { return 1; } return 0; } -int64_t qemu_file_get_rate_limit(QEMUFile *f) +uint64_t qemu_file_get_rate_limit(QEMUFile *f) { return f->rate_limit_max; } -void qemu_file_set_rate_limit(QEMUFile *f, int64_t limit) +void qemu_file_set_rate_limit(QEMUFile *f, uint64_t limit) { f->rate_limit_max = limit; } diff --git a/migration/qemu-file.h b/migration/qemu-file.h index d16cd50448..9e158c00f6 100644 --- a/migration/qemu-file.h +++ b/migration/qemu-file.h @@ -138,8 +138,8 @@ void qemu_file_reset_rate_limit(QEMUFile *f); * need to be applied to the rate limiting calcuations */ void qemu_file_acct_rate_limit(QEMUFile *f, int64_t len); -void qemu_file_set_rate_limit(QEMUFile *f, int64_t new_rate); -int64_t qemu_file_get_rate_limit(QEMUFile *f); +void qemu_file_set_rate_limit(QEMUFile *f, uint64_t new_rate); +uint64_t qemu_file_get_rate_limit(QEMUFile *f); int qemu_file_get_error_obj(QEMUFile *f, Error **errp); int qemu_file_get_error_obj_any(QEMUFile *f1, QEMUFile *f2, Error **errp); void qemu_file_set_error_obj(QEMUFile *f, int ret, Error *err);
It is really size_t. Everything else uses uint64_t, so move this to uint64_t as well. A size can't be negative anyways. Signed-off-by: Juan Quintela <quintela@redhat.com> --- migration/migration.c | 6 +++--- migration/qemu-file.c | 8 ++++---- migration/qemu-file.h | 4 ++-- 3 files changed, 9 insertions(+), 9 deletions(-)