Juan Quintela <quint...@redhat.com> writes:
> David Edmondson <david.edmond...@oracle.com> wrote:
>> Juan Quintela <quint...@redhat.com> writes:
>>
>>> Define and use RATE_LIMIT_MAX instead.
>>
>> Suggest "RATE_LIMIT_MAX_NONE".
>
> Then even better
>
> RATE_LIMIT_DISABLED?
> RATE_LIMIT_NONE?
RATE_LIMIT_NONE sounds good to me.
>
> Using MAX and NONE at the same time looks strange.
>
>>> Signed-off-by: Juan Quintela <quint...@redhat.com>
>>> ---
>>> migration/migration-stats.h | 6 ++++++
>>> migration/migration.c | 4 ++--
>>> migration/qemu-file.c | 6 +++++-
>>> 3 files changed, 13 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/migration/migration-stats.h b/migration/migration-stats.h
>>> index cf8a4f0410..e782f1b0df 100644
>>> --- a/migration/migration-stats.h
>>> +++ b/migration/migration-stats.h
>>> @@ -15,6 +15,12 @@
>>>
>>> #include "qemu/stats64.h"
>>>
>>> +/*
>>> + * If rate_limit_max is 0, there is special code to remove the rate
>>> + * limit.
>>> + */
>>> +#define RATE_LIMIT_MAX 0
>>> +
>>> /*
>>> * These are the ram migration statistic counters. It is loosely
>>> * based on MigrationStats. We change to Stat64 any counter that
>>> diff --git a/migration/migration.c b/migration/migration.c
>>> index 039bba4804..c41c7491bb 100644
>>> --- a/migration/migration.c
>>> +++ b/migration/migration.c
>>> @@ -2304,7 +2304,7 @@ static void migration_completion(MigrationState *s)
>>> * them if migration fails or is cancelled.
>>> */
>>> 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, RATE_LIMIT_MAX);
>>> ret = qemu_savevm_state_complete_precopy(s->to_dst_file,
>>> false,
>>>
>>> s->block_inactive);
>>> }
>>> @@ -3048,7 +3048,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, RATE_LIMIT_MAX);
>>>
>>> setup_start = qemu_clock_get_ms(QEMU_CLOCK_HOST);
>>> /*
>>> diff --git a/migration/qemu-file.c b/migration/qemu-file.c
>>> index 597054759d..4bc875b452 100644
>>> --- a/migration/qemu-file.c
>>> +++ b/migration/qemu-file.c
>>> @@ -27,6 +27,7 @@
>>> #include "qemu/error-report.h"
>>> #include "qemu/iov.h"
>>> #include "migration.h"
>>> +#include "migration-stats.h"
>>> #include "qemu-file.h"
>>> #include "trace.h"
>>> #include "options.h"
>>> @@ -732,7 +733,10 @@ int qemu_file_rate_limit(QEMUFile *f)
>>
>> Given that qemu_file_rate_limit() is really a boolean, could it be
>> declared as such?
>
> I wanted to do on this patch justn $Subject.
>
> You can see that when I move this function to
> migration/migration-stats.c already do the type change.
Thank you đź‘Ť
> That is patch:
>
> [PATCH v2 05/16] migration: Move rate_limit_max and rate_limit_used to
> migration_stats
>
> diff --git a/migration/migration-stats.c b/migration/migration-stats.c
> index 3431453c90..1b16edae7d 100644
> --- a/migration/migration-stats.c
> +++ b/migration/migration-stats.c
> @@ -22,3 +23,46 @@ void migration_time_since(MigrationAtomicStats *stats,
> int64_t since)
> int64_t now = qemu_clock_get_ms(QEMU_CLOCK_HOST);
> stat64_set(&stats->setup_time, now - since);
> }
> +
> +bool migration_rate_exceeded(QEMUFile *f)
> +{
> + if (qemu_file_get_error(f)) {
> + return true;
> + }
> +
> + uint64_t rate_limit_used = stat64_get(&mig_stats.rate_limit_used);
> + uint64_t rate_limit_max = stat64_get(&mig_stats.rate_limit_max);
> +
> + if (rate_limit_max == RATE_LIMIT_MAX) {
> + return false;
> + }
> + if (rate_limit_max > 0 && rate_limit_used > rate_limit_max) {
> + return true;
> + }
> + return false;
> +}
>
> Thanks, Juan.
--
I was better off when I was on your side, and I was holding on.
