Re: [Qemu-devel] [RFC] New thread for the VM migration

[Avi Kivity]

On 07/14/2011 06:30 PM, Juan Quintela wrote:
> Avi Kivity<address@hidden>  wrote:
> >  On 07/14/2011 03:30 PM, Anthony Liguori wrote:
> >>>  Does this mean that the following code is sometimes executed without
> >>>  qemu_mutex? I don't think any of it is thread safe.
> >>
> >>
> >>  That was my reaction too.
> >>
> >>  I think the most rational thing to do is have a separate thread and
> >>  a pair of producer/consumer queues.
> >>
> >>  The I/O thread can push virtual addresses and sizes to the queue for
> >>  the migration thread to compress/write() to the fd.  The migration
> >>  thread can then push sent regions onto a separate queue for the I/O
> >>  thread to mark as dirty.
> >
> >  Even virtual addresses are not safe enough, because of hotunplug.
> >  Without some kind of locking, you have to copy the data.
>
> Disabling hotplug should be enough?

So is powering down the destination host.

> Notice that hotplug/unplug during
> migration don't make a lot of sense anyways.

That's completely wrong.  Hotplug is a guest/end-user operation; 
migration is a host/admin operation.  The two don't talk to each other 
at all - if the admin (usually a program) wants to migrate at the same 
time the user wants to hotplug, which one gets the bad news?  Who will 
actually test the combination?

It's true that with the current setup we can't really do migration and 
hotplug together, since we can't synchronize the hotplug on the 
destination with the migration.  But we should be able to, and we should 
design migration with that in mind.

> Not all the bitmap syncying has proper locking now (copyng towards one
> place), but rest of cade looks really thread safe to me (migration code
> is only called from this thread, so it should be safe).
>
> My understanding on how this work:
>
>   vcpu thread modifies momery
>   iothread (some times) modifies memory
>
>   migration thread: reads memory, and gets the lock before syncing its
>   bitmap with kvm one and qemu one (clearing it on the time).
>
> Assume we disable hotplug/unplug (what we have to do anyways).  What is
> the locking problem that we have?

I didn't really grok the buffering of the migration bitmap.  It does 
look correct.  Would be best in a separate patch to point out the new 
mechanism (but that doesn't really excuse the bad review).

> We do stage 3 with the iothread locked, i.e. at that point everything
> else is stopped.  Before stage 3, can kvm or qemu modify a page and
> _not_ modify the bitmap?  My understanding is not.
>
> Only real variable that we are sharing is ram_list, or I am losing
> something obvious?

You are right.  ram_list _is_ volatile though (but we can't really 
change it these days during migration).

--
error compiling committee.c: too many arguments to function