Re: [Qemu-devel] [PATCH 2/9] block-migration: fix pending() return value

[John Snow]

On 12/11/2014 09:17 AM, Vladimir Sementsov-Ogievskiy wrote:
> Because of wrong return value of .save_live_pending() in
> block-migration, migration finishes before the whole disk
> is transferred. Such situation occures when the migration
(occurs)
> process is fast enouth, for example when source and dest
(enough)
> are on the same host.
>
> If in the bulk phase we return something < max_size, we will skip
> transferring the tail of the device. Currently we have "set pending to
> BLOCK_SIZE if it is zero" for bulk phase, but there no guarantee, that
> it will be < max_size.
>
> True approach is to return, for example, max_size+1 when we are in the
> bulk phase.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
> ---
>   block-migration.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/block-migration.c b/block-migration.c
> index 6f3aa18..8df30d9 100644
> --- a/block-migration.c
> +++ b/block-migration.c
> @@ -757,8 +757,8 @@ static uint64_t block_save_pending(QEMUFile *f, void 
> *opaque, uint64_t max_size)
>                          block_mig_state.read_done * BLOCK_SIZE;
>
>       /* Report at least one block pending during bulk phase */
> -    if (pending == 0 && !block_mig_state.bulk_completed) {
> -        pending = BLOCK_SIZE;
> +    if (pending <= max_size && !block_mig_state.bulk_completed) {
> +        pending = max_size + BLOCK_SIZE;
>       }
>       blk_mig_unlock();
>       qemu_mutex_unlock_iothread();

This looks sane to me, but I am CC'ing the Migration maintainers to give 
it a proper look.

Looks to me like this is to prevent this from happening, in 
migration/migration.c:

            pending_size = qemu_savevm_state_pending(s->file, max_size);
            trace_migrate_pending(pending_size, max_size);
            if (pending_size && pending_size >= max_size) {

If we fail that condition, we omit data because we do not call 
qemu_savevm_state_iterate.