Re: [PATCH] vfio/migrate: Move switch of dirty tracking into vfio_memory_listener

["Dr. David Alan Gilbert" <dgilbert@redhat.com>]

* Paolo Bonzini (pbonzini@redhat.com) wrote:
> On 11/01/21 08:34, Keqian Zhu wrote:
> > +static void vfio_listener_log_start(MemoryListener *listener,
> > +                                    MemoryRegionSection *section,
> > +                                    int old, int new)
> > +{
> > +    VFIOContainer *container = container_of(listener, VFIOContainer, listener);
> > +
> > +    vfio_set_dirty_page_tracking(container, true);
> > +}
> > +
> > +static void vfio_listener_log_stop(MemoryListener *listener,
> > +                                   MemoryRegionSection *section,
> > +                                   int old, int new)
> > +{
> > +    VFIOContainer *container = container_of(listener, VFIOContainer, listener);
> > +
> > +    vfio_set_dirty_page_tracking(container, false);
> > +}
> > +
> 
> This would enable dirty page tracking also just for having a framebuffer
> (DIRTY_MEMORY_VGA).  Technically it would be correct, but it would also be
> more heavyweight than expected.

Wouldn't that only happen on emulated video devices?

> In order to only cover live migration, you can use the log_global_start and
> log_global_stop callbacks instead.
> 
> If you want to use log_start and log_stop, you need to add respectively
> 
>     if (old != 0) {
>         return;
>     }
> 
> and
> 
>     if (new != 0) {
>         return;
>     }

Why 0, wouldn't you be checking for DIRTY_LOG_MIGRATION somewhere?

Dave

> before the calls to vfio_set_dirty_page_tracking.  But I think it's more
> appropriate for VFIO to use log_global_*.
> 
> Thanks,
> 
> Paolo
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK