OASIS Mailing List ArchivesView the OASIS mailing list archive below
or browse/search using MarkMail.


Help: OASIS Mailing Lists Help | MarkMail Help

virtio-dev message

[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]

Subject: Re: [PATCH v9 5/8] migration/ram.c: add a notifier chain for precopy

On 11/28/2018 05:32 PM, Peter Xu wrote:

So what I am worrying here are corner cases where we might forget to
stop the hinting.  I'm fabricating one example sequence of events:

   (start migration)
   (some SaveStateEntry failed rather than RAM, then
    migration_detect_error returned MIG_THR_ERR_FATAL so we need to
    fail the migration, however when running the previous
    ram_save_iterate for RAM's specific SaveStateEntry we didn't see
    any error so no ERROR event detected)

Then it seems the hinting will last forever.  Considering that now I'm
not sure whether this can be done ram-only, since even if you capture
ram_save_complete() and at the same time you introduce PRECOPY_END you
may still miss the PRECOPY_END event since AFAIU ram_save_complete()
won't be called at all in this case.

Could this happen?

Thanks, indeed this case could happen if we add PRECOPY_END in

How about putting PRECOPY_END in ram_save_cleanup?
I think it would be called in any case.

I'm also thinking probably we don't need PRECOPY_ERR when we have PRECOPY_END,
and what do you think of the notifier names below:

+typedef enum PrecopyNotifyReason {
+} PrecopyNotifyReason;


Another thing to mention about the "reasons" (though I see it more
like "events"): have you thought about adding a PRECOPY_NOTIFY_END?
It might help in some cases:

     - then you don't need to trickily export the migrate_postcopy()
       since you'll notify that before postcopy starts
I'm thinking probably we don't need to export migrate_postcopy even now.
It's more like a sanity check, and not needed because now we have the
notifier registered to the precopy specific callchain, which has ensured
it is invoked via precopy.
But postcopy will always start with precopy, no?
Yes, but I think we could add the check in precopy_notify()
I'm not sure that's good.  If the notifier could potentially have
other user, they might still work with postcopy, and they might expect
e.g. BEFORE_SYNC to be called for every sync, even if it's at the
precopy stage of a postcopy.

I think this precopy notifier callchain is expected to be used only for
the precopy mode. Postcopy has its dedicated notifier callchain that
users could use.

How about changing the migrate_postcopy() check to "ms->start_postcopy":

bool migration_postcopy_start(void)
    MigrationState *s;

    s = migrate_get_current();

    return atomic_read(&s->start_postcopy);

static void precopy_notify(PrecopyNotifyReason reason)
    if (migration_postcopy_start())

    notifier_list_notify(&precopy_notifier_list, &reason);

If postcopy started with precopy, the precopy optimization feature
could still be used until it switches to the postcopy mode.

In that sense I still feel the
PRECOPY_END is better (so contantly call it at the end of precopy, no
matter whether there's another postcopy afterwards).  It sounds like a
cleaner interface.

Probably I still haven't got the point how PRECOPY_END could help above yet.


[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]