Re: [Qemu-devel] [PATCH 13/14] savevm: New save live migration method: pending

[Paolo Bonzini]

Il 21/09/2012 16:08, Juan Quintela ha scritto:
> Code just now does (simplified for clarity)
> 
>     if (qemu_savevm_state_iterate(s->file) == 1) {
>        vm_stop_force_state(RUN_STATE_FINISH_MIGRATE);
>        qemu_savevm_state_complete(s->file);
>     }
> 
> Problem here is that qemu_savevm_state_iterate() returns 1 when it
> knows that remaining memory to sent takes less than max downtime.
> 
> But this means that we could end spending 2x max_downtime, one
> downtime in qemu_savevm_iterate, and the other in
> qemu_savevm_state_complete.

Technically, the one in qemu_savevm_iterate may not translate into
actual downtime if the guest does not need I/O, as it is "just" stolen
time.  But this is just a nit in the commit message.  Very good catch!

Another nit:

> 
> +uint64_t qemu_savevm_state_pending(QEMUFile *f, uint64_t max_size)
> +{
> +    SaveStateEntry *se;
> +    uint64_t ret = 0;
> +
> +    QTAILQ_FOREACH(se, &savevm_handlers, entry) {
> +        if (!se->ops || !se->ops->save_live_pending) {
> +            continue;
> +        }
> +        if (se->ops && se->ops->is_active) {
> +            if (!se->ops->is_active(se->opaque)) {
> +                continue;
> +            }
> +        }
> +        ret += se->ops->save_live_pending(f, se->opaque, max_size);
> +    }
> +    return ret;
> +}

Here you may want to pass max_size - ret for the last argument of
se->ops->save_live_pending.  But in practice this loop will be executed
exactly once, so:

Reviewed-by: Paolo Bonzini <address@hidden>

Paolo

> Changed code to:
> 
>     pending_size = qemu_savevm_state_pending(s->file, max_size);
>     DPRINTF("pending size %lu max %lu\n", pending_size, max_size);
>     if (pending_size >= max_size) {
>         ret = qemu_savevm_state_iterate(s->file);
>      } else {
>         vm_stop_force_state(RUN_STATE_FINISH_MIGRATE);
>         qemu_savevm_state_complete(s->file);
>      }
> 
> So what we do is: at current network speed, we calculate the maximum
> number of bytes we can sent: max_size.
> 
> Then we ask every save_live section how much they have pending.  If
> they are less than max_size, we move to complete phase, otherwise we
> do an iterate one.
> 
> This makes things much simpler, because now individual sections don't
> have to caluclate the bandwidth (it was implossible to do right from
> there).