Re: [PATCH 02/21] migration: Don't use INT64_MAX for unlimited rate

[Cédric Le Goater]

On 5/9/23 13:51, Juan Quintela wrote:
> Harsh Prateek Bora <harshpb@linux.ibm.com> wrote:
> > On 5/8/23 18:38, Juan Quintela wrote:
> > > Use 0 instead.
> > > Signed-off-by: Juan Quintela <quintela@redhat.com>
> > > ---
> > >    migration/migration.c | 4 ++--
> > >    migration/qemu-file.c | 3 +++
> > >    2 files changed, 5 insertions(+), 2 deletions(-)
> > > diff --git a/migration/migration.c b/migration/migration.c
> > > index 1192f1ebf1..3979a98949 100644
> > > --- a/migration/migration.c
> > > +++ b/migration/migration.c
> > > @@ -2296,7 +2296,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, 0);
> >
> > #define RATE_LIMIT_MAX 0
> >
> > How about having a macro and use that which conveys the meaning in all
> > call instances wherever it is getting passed ?
>
> I almost preffer the macro.
>
>        qemu_file_set_rate_limit(s->to_dst_file, RATE_LIMIT_MAX);
>
> seems quite explanatory?

yep. and I would drop the comment qemu_file_rate_limit().

Thanks,

C.


> Thanks, Juan.
>
> > >                    ret = qemu_savevm_state_complete_precopy(s->to_dst_file, 
> > > false,
> > >                                                             s->block_inactive);
> > >                }
> > > @@ -3044,7 +3044,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, 0);
> > >          setup_start = qemu_clock_get_ms(QEMU_CLOCK_HOST);
> > >        /*
> > > diff --git a/migration/qemu-file.c b/migration/qemu-file.c
> > > index f4cfd05c67..745361d238 100644
> > > --- a/migration/qemu-file.c
> > > +++ b/migration/qemu-file.c
> > > @@ -731,6 +731,9 @@ int qemu_file_rate_limit(QEMUFile *f)
> > >        if (qemu_file_get_error(f)) {
> > >            return 1;
> > >        }
> > > +    /*
> > > +     *  rate_limit_max == 0 means no rate_limit enfoncement.
> > > +     */
> > >        if (f->rate_limit_max > 0 && f->rate_limit_used > f->rate_limit_max) {
> > >            return 1;
> > >        }