Re: [PATCH 03/21] migration: We set the rate_limit by a second

[Cédric Le Goater]

On 5/8/23 15:08, Juan Quintela wrote:
> That the implementation does the check every 100 milliseconds is an
> implementation detail that shouldn't be seen on the interfaz.

Si. Pero, "interface" es mejor aqui.

> Notice that all callers of qemu_file_set_rate_limit() used the
> division or pass 0, so this change is a NOP.
>
> Signed-off-by: Juan Quintela <quintela@redhat.com>


Reviewed-by: Cédric Le Goater <clg@kaod.org>

C.


> ---
>   migration/migration.c | 7 +++----
>   migration/options.c   | 4 ++--
>   migration/qemu-file.c | 6 +++++-
>   3 files changed, 10 insertions(+), 7 deletions(-)
>
> diff --git a/migration/migration.c b/migration/migration.c
> index 3979a98949..e17a6538b4 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -2117,7 +2117,7 @@ static int postcopy_start(MigrationState *ms)
>        * will notice we're in POSTCOPY_ACTIVE and not actually
>        * wrap their state up here
>        */
> -    qemu_file_set_rate_limit(ms->to_dst_file, bandwidth / XFER_LIMIT_RATIO);
> +    qemu_file_set_rate_limit(ms->to_dst_file, bandwidth);
>       if (migrate_postcopy_ram()) {
>           /* Ping just for debugging, helps line traces up */
>           qemu_savevm_send_ping(ms->to_dst_file, 2);
> @@ -3207,11 +3207,10 @@ void migrate_fd_connect(MigrationState *s, Error 
> *error_in)
>   
>       if (resume) {
>           /* This is a resumed migration */
> -        rate_limit = migrate_max_postcopy_bandwidth() /
> -            XFER_LIMIT_RATIO;
> +        rate_limit = migrate_max_postcopy_bandwidth();
>       } else {
>           /* This is a fresh new migration */
> -        rate_limit = migrate_max_bandwidth() / XFER_LIMIT_RATIO;
> +        rate_limit = migrate_max_bandwidth();
>   
>           /* Notify before starting migration thread */
>           notifier_list_notify(&migration_state_notifiers, s);
> diff --git a/migration/options.c b/migration/options.c
> index 2e759cc306..d04b5fbc3a 100644
> --- a/migration/options.c
> +++ b/migration/options.c
> @@ -1243,7 +1243,7 @@ static void migrate_params_apply(MigrateSetParameters 
> *params, Error **errp)
>           s->parameters.max_bandwidth = params->max_bandwidth;
>           if (s->to_dst_file && !migration_in_postcopy()) {
>               qemu_file_set_rate_limit(s->to_dst_file,
> -                                s->parameters.max_bandwidth / 
> XFER_LIMIT_RATIO);
> +                                s->parameters.max_bandwidth);
>           }
>       }
>   
> @@ -1275,7 +1275,7 @@ static void migrate_params_apply(MigrateSetParameters *params, Error **errp)
>           s->parameters.max_postcopy_bandwidth = params->max_postcopy_bandwidth;
>           if (s->to_dst_file && migration_in_postcopy()) {
>               qemu_file_set_rate_limit(s->to_dst_file,
> -                    s->parameters.max_postcopy_bandwidth / XFER_LIMIT_RATIO);
> +                    s->parameters.max_postcopy_bandwidth);
>           }
>       }
>       if (params->has_max_cpu_throttle) {
> diff --git a/migration/qemu-file.c b/migration/qemu-file.c
> index 745361d238..12cf7fb04e 100644
> --- a/migration/qemu-file.c
> +++ b/migration/qemu-file.c
> @@ -29,6 +29,7 @@
>   #include "migration.h"
>   #include "qemu-file.h"
>   #include "trace.h"
> +#include "options.h"
>   #include "qapi/error.h"
>   
>   #define IO_BUF_SIZE 32768
> @@ -747,7 +748,10 @@ int64_t qemu_file_get_rate_limit(QEMUFile *f)
>   
>   void qemu_file_set_rate_limit(QEMUFile *f, int64_t limit)
>   {
> -    f->rate_limit_max = limit;
> +    /*
> +     * 'limit' is per second.  But we check it each 100 miliseconds.
> +     */
> +    f->rate_limit_max = limit / XFER_LIMIT_RATIO;
>   }
>   
>   void qemu_file_reset_rate_limit(QEMUFile *f)