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

2018-11-29 Thread Wei Wang
On 11/30/2018 01:57 PM, Peter Xu wrote: Or you can introduce migration_bitmap_sync_precopy() and let precopy code call that one instead. Agree, thanks. PS. I'm a bit unsure on why we need to sync bitmap in ram_init_bitmaps. I feel like it can be removed... Seems it was added in early

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

2018-11-29 Thread Peter Xu
On Fri, Nov 30, 2018 at 01:05:51PM +0800, Wei Wang wrote: > On 11/29/2018 01:10 PM, Peter Xu wrote: > > On Thu, Nov 29, 2018 at 11:40:57AM +0800, Wei Wang wrote: > > I think this precopy notifier callchain is expected to be used only for > > the precopy mode. Postcopy has its dedicated notifier

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

2018-11-29 Thread Wei Wang
On 11/29/2018 01:10 PM, Peter Xu wrote: On Thu, Nov 29, 2018 at 11:40:57AM +0800, Wei Wang wrote: 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

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

2018-11-28 Thread Wei Wang
On 11/29/2018 01:10 PM, Peter Xu wrote: 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

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

2018-11-28 Thread Peter Xu
On Thu, Nov 29, 2018 at 01:10:14PM +0800, Peter Xu wrote: > On Thu, Nov 29, 2018 at 11:40:57AM +0800, Wei Wang wrote: > > 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

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

2018-11-28 Thread Peter Xu
On Thu, Nov 29, 2018 at 11:40:57AM +0800, Wei Wang wrote: > 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) > >

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

2018-11-28 Thread Wei Wang
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) START_MIGRATION BEFORE_SYNC AFTER_SYNC ... BEFORE_SYNC AFTER_SYNC (some

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

2018-11-28 Thread Peter Xu
On Wed, Nov 28, 2018 at 05:01:31PM +0800, Wei Wang wrote: > On 11/28/2018 01:26 PM, Peter Xu wrote: > > > > Ok thanks. Please just make sure you will capture all the error > > cases, e.g., I also see path like this (a few lines below): > > > > if (pages < 0) { > >

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

2018-11-28 Thread Wei Wang
On 11/28/2018 01:26 PM, Peter Xu wrote: Ok thanks. Please just make sure you will capture all the error cases, e.g., I also see path like this (a few lines below): if (pages < 0) { qemu_file_set_error(f, pages); break; } It seems that you missed

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

2018-11-27 Thread Peter Xu
On Tue, Nov 27, 2018 at 06:25:40PM +0800, Wei Wang wrote: > On 11/27/2018 03:38 PM, Peter Xu wrote: > > On Thu, Nov 15, 2018 at 06:08:01PM +0800, Wei Wang wrote: > > > +typedef enum PrecopyNotifyReason { > > > +PRECOPY_NOTIFY_ERR = 0, > > > +PRECOPY_NOTIFY_START_ITERATION = 1, > > > +

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

2018-11-27 Thread Wei Wang
On 11/27/2018 03:38 PM, Peter Xu wrote: On Thu, Nov 15, 2018 at 06:08:01PM +0800, Wei Wang wrote: +typedef enum PrecopyNotifyReason { +PRECOPY_NOTIFY_ERR = 0, +PRECOPY_NOTIFY_START_ITERATION = 1, +PRECOPY_NOTIFY_BEFORE_SYNC_BITMAP = 2, +PRECOPY_NOTIFY_AFTER_SYNC_BITMAP = 3, +

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

2018-11-26 Thread Peter Xu
On Thu, Nov 15, 2018 at 06:08:01PM +0800, Wei Wang wrote: > This patch adds a notifier chain for the memory precopy. This enables various > precopy optimizations to be invoked at specific places. > > Signed-off-by: Wei Wang > CC: Dr. David Alan Gilbert > CC: Juan Quintela > CC: Michael S.

[Qemu-devel] [PATCH v9 5/8] migration/ram.c: add a notifier chain for precopy

2018-11-15 Thread Wei Wang
This patch adds a notifier chain for the memory precopy. This enables various precopy optimizations to be invoked at specific places. Signed-off-by: Wei Wang CC: Dr. David Alan Gilbert CC: Juan Quintela CC: Michael S. Tsirkin CC: Peter Xu --- include/migration/misc.h | 12