On Thu, Nov 29, 2018 at 11:40:57AM +0800, Wei Wang wrote:
On 11/28/2018 05:32 PM, Peter Xu wrote:
I'm not sure we can use start_postcopy. It's a variable being set in
the QMP handler but it does not mean postcopy has started. I'm afraid
there can be race where it's still precopy but the variable is set so
event could be missed...
IMHO the problem is not that complicated. How about this proposal:
[1]
typedef enum PrecopyNotifyReason {
PRECOPY_NOTIFY_RAM_START,
PRECOPY_NOTIFY_RAM_BEFORE_SYNC,
PRECOPY_NOTIFY_RAM_AFTER_SYNC,
PRECOPY_NOTIFY_COMPLETE,
PRECOPY_NOTIFY_RAM_CLEANUP,
};
The first three keep the same as your old ones. Notify RAM_CLEANUP in
ram_save_cleanup() to make sure it'll always be cleaned up (the same
as PRECOPY_END, just another name). Notify COMPLETE in
qemu_savevm_state_complete_precopy() to show that precopy is
completed. Meanwhile on balloon side you should stop the hinting for
either RAM_CLEANUP or COMPLETE event. Then either:
- precopy is switching to postcopy, or
- precopy completed, or
- precopy failed/cancelled
You should always get at least a notification to stop the balloon.
Though you could also get one RAM_CLEANUP after one COMPLETE, but
the balloon should easily handle it (stop the hinting twice).
Here maybe you can even remove the "RAM_" in both RAM_START and
RAM_CLEANUP if we're going to have COMPLETE since after all it'll be
not only limited to RAM.
Another suggestion is that you can add an Error into the notify hooks,
please refer to the postcopy one:
int postcopy_notify(enum PostcopyNotifyReason reason, Error **errp);
So the hook functions have a way to even stop the migration (though
for balloon hinting it'll be always optional so no error should be
reported...), then the two interfaces are matched.