Re: [PATCH QEMU v7 4/9] migration: Introduce dirty-limit capability

[Markus Armbruster]

~hyman <hyman@git.sr.ht> writes:

> From: Hyman Huang(黄勇) <yong.huang@smartx.com>
>
> Introduce migration dirty-limit capability, which can
> be turned on before live migration and limit dirty
> page rate durty live migration.
>
> Introduce migrate_dirty_limit function to help check
> if dirty-limit capability enabled during live migration.
>
> Meanwhile, refactor vcpu_dirty_rate_stat_collect
> so that period can be configured instead of hardcoded.
>
> dirty-limit capability is kind of like auto-converge
> but using dirty limit instead of traditional cpu-throttle
> to throttle guest down. To enable this feature, turn on
> the dirty-limit capability before live migration using
> migrate-set-capabilities, and set the parameters
> "x-vcpu-dirty-limit-period", "vcpu-dirty-limit" suitably
> to speed up convergence.
>
> Signed-off-by: Hyman Huang(黄勇) <yong.huang@smartx.com>
> Acked-by: Peter Xu <peterx@redhat.com>
> Reviewed-by: Juan Quintela <quintela@redhat.com>

[...]

> diff --git a/migration/options.h b/migration/options.h
> index 9aaf363322..b5a950d4e4 100644
> --- a/migration/options.h
> +++ b/migration/options.h
> @@ -24,6 +24,7 @@ extern Property migration_properties[];
>  /* capabilities */
>  
>  bool migrate_auto_converge(void);
> +bool migrate_dirty_limit(void);
>  bool migrate_background_snapshot(void);
>  bool migrate_block(void);
>  bool migrate_colo(void);
> diff --git a/qapi/migration.json b/qapi/migration.json
> index aa590dbf0e..cc51835cdd 100644
> --- a/qapi/migration.json
> +++ b/qapi/migration.json
> @@ -497,6 +497,16 @@
>  #     are present.  'return-path' capability must be enabled to use
>  #     it.  (since 8.1)
>  #
> +# @dirty-limit: If enabled, migration will use the dirty-limit algo to
> +#               throttle down guest instead of auto-converge algo.
> +#               Throttle algo only works when vCPU's dirtyrate greater
> +#               than 'vcpu-dirty-limit', read processes in guest os
> +#               aren't penalized any more, so this algo can improve
> +#               performance of vCPU during live migration. This is an
> +#               optional performance feature and should not affect the
> +#               correctness of the existing auto-converge algo.
> +#               (since 8.1)
> +#

Please format like

   # @dirty-limit: If enabled, migration will use the dirty-limit algo to
   #     throttle down guest instead of auto-converge algo.  Throttle
   #     algo only works when vCPU's dirtyrate greater than
   #     'vcpu-dirty-limit', read processes in guest os aren't penalized
   #     any more, so this algo can improve performance of vCPU during
   #     live migration.  This is an optional performance feature and
   #     should not affect the correctness of the existing auto-converge
   #     algo.  (since 8.1)

to blend in with recent commit a937b6aa739 (qapi: Reformat doc comments
to conform to current conventions).

"Dirty rate" with a space, because that's how we spell it elsewhere.

Moreover, "algo" is not a word, "algorithm" is :)

Is "the dirty-limit algorithm" defined anywhere?  

More word-smithing is needed, but I'd like to get the reference to "the
dirty-limit algorithm" clarified first.

>  # Features:
>  #
>  # @unstable: Members @x-colo and @x-ignore-shared are experimental.
> @@ -512,7 +522,8 @@
>             'dirty-bitmaps', 'postcopy-blocktime', 'late-block-activate',
>             { 'name': 'x-ignore-shared', 'features': [ 'unstable' ] },
>             'validate-uuid', 'background-snapshot',
> -           'zero-copy-send', 'postcopy-preempt', 'switchover-ack'] }
> +           'zero-copy-send', 'postcopy-preempt', 'switchover-ack',
> +           'dirty-limit'] }
>  
>  ##
>  # @MigrationCapabilityStatus:

[...]