On Mon, Jun 08, 2026 at 03:07:32PM +0300, Avihai Horon wrote:
> > > 2. It calls migration_bitmap_sync and not migration_bitmap_sync_precopy,
> > > i.e., without precopy_notify calls.
> > This is another thing I feel like got overlooked in the free page hint
> > feature.  The only notifiers registered is that balloon device, logically I
> > think we need these notifiers in postcopy too to make sure we properly stop
> > the free page hints seeing BEFORE_BITMAP_SYNC, then don't start it
> > (vm_running=false) in AFTER_BITMAP_SYNC:
> > 
> > virtio_balloon_free_page_hint_notify():
> > 
> >      case PRECOPY_NOTIFY_BEFORE_BITMAP_SYNC:
> >          virtio_balloon_free_page_stop(dev);
> >          break;
> >      case PRECOPY_NOTIFY_AFTER_BITMAP_SYNC:
> >          if (vdev->vm_running) {
> >              virtio_balloon_free_page_start(dev);
> >              break;
> >          }
> 
> Actually, looking more closely in the code, I see that it's not used if
> postcopy is enabled [1].
> So calling the precopy notifiers in postcopy switchover is basically a
> no-op.
> 
> Then it seems fine to call migration_bitmap_sync_precopy from postcopy
> switchover flow.
> 
> [1] See commit fd51e54fa102 ("virtio-balloon: don't start free page hinting
> if postcopy is possible")

Yes, thanks for the pointer.  So it relies on the DONE event rather than
stop() to really stop the reporting.. good to know it was already bypassed
for postcopy, I definitely forgot that change.  For now, we can add another
trivial comment if we want.  So what we really need is final=true for
postcopy.

>From a notifier semantics POV, IIUC it's also correct to notify here in
postcopy_start(), because at this stage it is still precopy.  Source QEMU
will move to postcopy stage (starting from POSTCOPY_DEVICE state) only if
postcopy_start() succeeded.  So a precopy notifier, no matter the analysis
of balloon use case, should theoretically apply here as well.

Thanks,

-- 
Peter Xu


Reply via email to