Re: [Qemu-devel] [PATCH v5 20/45] Modify savevm handlers for postcopy

[David Gibson]

On Fri, Mar 20, 2015 at 12:37:59PM +0000, Dr. David Alan Gilbert wrote:
> * David Gibson (address@hidden) wrote:
> > On Fri, Mar 13, 2015 at 10:19:54AM +0000, Dr. David Alan Gilbert wrote:
> > > * David Gibson (address@hidden) wrote:
> > > > On Wed, Feb 25, 2015 at 04:51:43PM +0000, Dr. David Alan Gilbert (git) 
> > > > wrote:
> > > > > From: "Dr. David Alan Gilbert" <address@hidden>
> > > > > 
> > > > > Modify save_live_pending to return separate postcopiable and
> > > > > non-postcopiable counts.
> > > > > 
> > > > > Add 'can_postcopy' to allow a device to state if it can postcopy
> > > > 
> > > > What's the purpose of the can_postcopy callback?  There are no callers
> > > > in this patch - is it still necessary with the change to
> > > > save_live_pending?
> > > 
> > > The patch 'qemu_savevm_state_complete: Postcopy changes' uses
> > > it in qemu_savevm_state_postcopy_complete and qemu_savevm_state_complete
> > > to decide which devices must be completed at that point.
> > 
> > Couldn't they check for non-zero postcopiable state from
> > save_live_pending instead?
> 
> That would be a bit weird.
> 
> At the moment for each device we call the:
>        save_live_setup method (from qemu_savevm_state_begin)
> 
>    0...multiple times we call:
>        save_live_pending
>        save_live_iterate
> 
>    and then we always call
>        save_live_complete
> 
> 
> To my mind we have to call save_live_complete for any device
> that we've called save_live_setup on (maybe it allocated something
> in _setup that it clears up in _complete).
> 
> save_live_pending could perfectly well return 0 remaining at the end of
> the migrate for our device, and thus if we used that then we wouldn't
> call save_live_complete.

Um.. I don't follow.  I was suggesting that at the precopy->postcopy
transition point you call save_live_complete for everything that
reports 0 post-copiable state.


Then again, a different approach would be to split the
save_live_complete hook into (possibly NULL) "complete precopy" and
"complete postcopy" hooks.  The core would ensure that every chunk of
state has both completion hooks called (unless NULL).  That might also
address my concerns about the no longer entirely accurate
save_live_complete function name.

-- 
David Gibson                    | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
                                | _way_ _around_!
http://www.ozlabs.org/~dgibson

pgppJ3Lo0MVWy.pgp
Description: PGP signature