Re: [PATCH v3 0/8] Migration: Make misc.h helpers available for whole VM lifecycle

[Peter Xu <peterx@redhat.com>]

On Thu, Oct 24, 2024 at 05:30:48PM -0400, Peter Xu wrote:
> Based-on: <20241024165627.1372621-1-peterx@redhat.com>
> CI:       https://gitlab.com/peterx/qemu/-/pipelines/1511349805
> 
> This is a follow up of below patch from Avihai as a replacement:
> 
> https://lore.kernel.org/qemu-devel/20241020130108.27148-3-avihaih@nvidia.com/
> 
> This is v3 of the series, and it happened again: changelog doesn't make
> sense to compare to v2 because it's rewritten.  Meanwhile, now this series
> is based on the other TYPE_SINGLETON series I posted just now:
> 
> https://lore.kernel.org/r/20241024165627.1372621-1-peterx@redhat.com
> 
> It turns out I found more things to cleanup, as the versions spin up.
> 
> Patch 1:     I found that object_ref() in migration thread is actually also
>              racy..  this should fix it.
> 
> Patch 2-7:   It turns out I decided to clean things up first, then it'll
>              make my last patch easier on adding the mutex protection for
>              the current_migration reference
> 
> Patch 8:     The fix for NULL-dereference / race for the exported
>              functions.  VFIO can hit it only because VFIO's specialty
>              in using migration helpers in vmstate handlers, I guess.  I
>              found most functions always safe because even if they're used
>              outside migration/ they're most likely still invoked with
>              migration thread context.  So I found only two functions that
>              really need protections, exactly what VFIO is using.

I queued patch 1-5.

Ideally 6+7 can be a single patch, then we don't need to move DEVICE into
migration_is_active() at all.  Also that might break VFIO as Avihai pointed
out on VGA sync.  Avihai, I'd still appreciate if you could consider look
at vfio to behave like what kvm/vhost/.. is doing, by accepting log_sync()
before log_start().

Patch 8 still rely on singleton series, which will become rfc to be
reposted, so probably no rush on that.

> 
> Comments welcomed, thanks.
> 
> Peter Xu (8):
>   migration: Take migration object refcount earlier for threads
>   migration: Unexport dirty_bitmap_mig_init()
>   migration: Unexport ram_mig_init()
>   migration: Drop migration_is_setup_or_active()
>   migration: Drop migration_is_idle()
>   migration: Drop migration_is_device()
>   migration: Unexport migration_is_active()
>   migration: Protect updates to current_migration with a mutex
> 
>  include/migration/misc.h |  11 +---
>  migration/migration.h    |   8 +++
>  migration/ram.h          |   1 +
>  hw/vfio/common.c         |   6 +-
>  hw/virtio/virtio-mem.c   |   2 +-
>  migration/migration.c    | 118 +++++++++++++++++++--------------------
>  migration/ram.c          |   7 +--
>  net/vhost-vdpa.c         |   3 +-
>  system/dirtylimit.c      |   3 +-
>  system/qdev-monitor.c    |   4 +-
>  10 files changed, 81 insertions(+), 82 deletions(-)
> 
> -- 
> 2.45.0
> 

-- 
Peter Xu