* David Gibson (da...@gibson.dropbear.id.au) wrote:
> On Fri, Oct 03, 2014 at 06:47:34PM +0100, Dr. David Alan Gilbert (git) wrote:
> > From: "Dr. David Alan Gilbert" <dgilb...@redhat.com>
> >
> > When postcopy calls qemu_savevm_state_complete it's not really
> > the end of migration, so skip:
> > a) Finishing postcopiable iterative devices - they'll carry on
> > b) The termination byte on the end of the stream.
> >
> > We then also add:
> > qemu_savevm_state_postcopy_complete
> > which is called at the end of a postcopy migration to call the
> > complete methods on devices skipped in the _complete call.
>
> So, we should probably rename qemu_savevm_state_complete() to reflect
> the fact that it's no longer actually a completion, but just the
> transition from pre-copy to post-copy phases. A good, brief name
> doesn't immediately occur to me, unfortunately.
Well it's still completion in the non-postcopy case; if you do think
of a good obvious name then I'd be happy to change it.
(Another way would be to add aparameter to qemu_savevm_state_complete
to make it do one or the other, but some of the conditions in it were
already a bit hairy).
> > Signed-off-by: Dr. David Alan Gilbert <dgilb...@redhat.com>
> > ---
> > include/sysemu/sysemu.h | 1 +
> > savevm.c | 52
> > ++++++++++++++++++++++++++++++++++++++++++++++++-
> > 2 files changed, 52 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
> > index e7ff3d0..46665ce 100644
> > --- a/include/sysemu/sysemu.h
> > +++ b/include/sysemu/sysemu.h
> > @@ -113,6 +113,7 @@ void qemu_savevm_state_cancel(void);
> > void qemu_savevm_state_pending(QEMUFile *f, uint64_t max_size,
> > uint64_t *res_non_postcopiable,
> > uint64_t *res_postcopiable);
> > +void qemu_savevm_state_postcopy_complete(QEMUFile *f);
> > void qemu_savevm_command_send(QEMUFile *f, enum qemu_vm_cmd command,
> > uint16_t len, uint8_t *data);
> > void qemu_savevm_send_reqack(QEMUFile *f, uint32_t value);
> > diff --git a/savevm.c b/savevm.c
> > index a0cb88b..7c4541d 100644
> > --- a/savevm.c
> > +++ b/savevm.c
> > @@ -854,10 +854,51 @@ int qemu_savevm_state_iterate(QEMUFile *f)
> > return ret;
> > }
> >
> > +/*
> > + * Calls the complete routines just for those devices that are
> > postcopiable;
> > + * causing the last few pages to be sent immediately and doing any
> > associated
> > + * cleanup.
> > + * Note postcopy also calls the plain qemu_savevm_state_complete to
> > complete
> > + * all the other devices, but that happens at the point we switch to
> > postcopy.
> > + */
> > +void qemu_savevm_state_postcopy_complete(QEMUFile *f)
> > +{
> > + SaveStateEntry *se;
> > + int ret;
> > +
> > + QTAILQ_FOREACH(se, &savevm_handlers, entry) {
> > + if (!se->ops || !se->ops->save_live_complete ||
> > + !se->ops->can_postcopy) {
>
> So, you check for the presence of a can_postcopy callback, but you
> don't ever actually invoke it.
Thanks, fixed.
> > + continue;
> > + }
> > + if (se->ops && se->ops->is_active) {
> > + if (!se->ops->is_active(se->opaque)) {
> > + continue;
> > + }
> > + }
> > + trace_savevm_section_start(se->idstr, se->section_id);
> > + /* Section type */
> > + qemu_put_byte(f, QEMU_VM_SECTION_END);
> > + qemu_put_be32(f, se->section_id);
> > +
> > + ret = se->ops->save_live_complete(f, se->opaque);
>
> I'm wondering if it might be clearer not to overload the
> save_live_complete hook, but instead allow both "execution transition"
> (old complete) and "final complete" (postcopy complete) hooks
> (expecting only one to be non-NULL in most cases).
Note that I only call save_live_complete once for any one device.
Non-postcopied devices get done in the original loop, postcopied
devices in the new ones, so from the point of view of a device
it's still a complete.
Dave
>
> --
> 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
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
