Re: [PULL 03/30] migration: Split save_live_pending() into state_pending_*

[Avihai Horon]

On 07/02/2023 2:56, Juan Quintela wrote:
> External email: Use caution opening links or attachments
>
>
> We split the function into to:
>
> - state_pending_estimate: We estimate the remaining state size without
>    stopping the machine.
>
> - state pending_exact: We calculate the exact amount of remaining
>    state.
>
> The only "device" that implements different functions for _estimate()
> and _exact() is ram.
>
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---
>   docs/devel/migration.rst       | 18 ++++++++-------
>   docs/devel/vfio-migration.rst  |  4 ++--
>   include/migration/register.h   | 19 +++++++++------
>   migration/savevm.h             | 12 ++++++----
>   hw/s390x/s390-stattrib.c       | 11 +++++----
>   hw/vfio/migration.c            | 21 +++++++++--------
>   migration/block-dirty-bitmap.c | 15 ++++++------
>   migration/block.c              | 13 ++++++-----
>   migration/migration.c          | 20 +++++++++++-----
>   migration/ram.c                | 35 ++++++++++++++++++++--------
>   migration/savevm.c             | 42 +++++++++++++++++++++++++++-------
>   hw/vfio/trace-events           |  2 +-
>   migration/trace-events         |  7 +++---
>   13 files changed, 143 insertions(+), 76 deletions(-)

[snip]

> diff --git a/migration/savevm.c b/migration/savevm.c
> index 5e4bccb966..7f9f770c1e 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -1472,10 +1472,10 @@ flush:
>    * the result is split into the amount for units that can and
>    * for units that can't do postcopy.
>    */
> -void qemu_savevm_state_pending(uint64_t threshold_size,
> -                               uint64_t *res_precopy_only,
> -                               uint64_t *res_compatible,
> -                               uint64_t *res_postcopy_only)
> +void qemu_savevm_state_pending_estimate(uint64_t threshold_size,
> +                                        uint64_t *res_precopy_only,
> +                                        uint64_t *res_compatible,
> +                                        uint64_t *res_postcopy_only)
>   {
>       SaveStateEntry *se;
>
> @@ -1485,7 +1485,7 @@ void qemu_savevm_state_pending(uint64_t threshold_size,
>
>
>       QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
> -        if (!se->ops || !se->ops->save_live_pending) {
> +        if (!se->ops || !se->ops->state_pending_exact) {
>               continue;
>           }
>           if (se->ops->is_active) {
> @@ -1493,9 +1493,35 @@ void qemu_savevm_state_pending(uint64_t threshold_size,
>                   continue;
>               }
>           }
> -        se->ops->save_live_pending(se->opaque, threshold_size,
> -                                   res_precopy_only, res_compatible,
> -                                   res_postcopy_only);
> +        se->ops->state_pending_exact(se->opaque, threshold_size,
> +                                     res_precopy_only, res_compatible,
> +                                     res_postcopy_only);
> +    }
> +}
> +
> +void qemu_savevm_state_pending_exact(uint64_t threshold_size,
> +                                     uint64_t *res_precopy_only,
> +                                     uint64_t *res_compatible,
> +                                     uint64_t *res_postcopy_only)
> +{
> +    SaveStateEntry *se;
> +
> +    *res_precopy_only = 0;
> +    *res_compatible = 0;
> +    *res_postcopy_only = 0;
> +
> +    QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
> +        if (!se->ops || !se->ops->state_pending_estimate) {
> +            continue;
> +        }
> +        if (se->ops->is_active) {
> +            if (!se->ops->is_active(se->opaque)) {
> +                continue;
> +            }
> +        }
> +        se->ops->state_pending_estimate(se->opaque, threshold_size,
> +                                        res_precopy_only, res_compatible,
> +                                        res_postcopy_only);
>       }
>   }

Hi Juan,

I only noticed it now while rebasing my series on top of yours.

I think the exact and estimate callbacks got mixed up here: we call 
.state_pending_estimate() in qemu_savevm_state_pending_exact() and 
.state_pending_exact() in qemu_savevm_state_pending_estimate().
Also need to switch the !se->ops->state_pending_exact/estimate checks.

Thanks.