Re: [Qemu-devel] [PATCH v4 19/47] Rework loadvm path for subloops

[David Gibson]

On Wed, Nov 19, 2014 at 05:50:11PM +0000, Dr. David Alan Gilbert wrote:
> * David Gibson (address@hidden) wrote:
> > On Fri, Oct 03, 2014 at 06:47:25PM +0100, Dr. David Alan Gilbert (git) 
> > wrote:
> > > From: "Dr. David Alan Gilbert" <address@hidden>
> > > 
> > > Postcopy needs to have two migration streams loading concurrently;
> > > one from memory (with the device state) and the other from the fd
> > > with the memory transactions.
> > > 
> > > Split the core of qemu_loadvm_state out so we can use it for both.
> > > 
> > > Allow the inner loadvm loop to quit and signal whether the parent
> > > should.
> > > 
> > > loadvm_handlers is made static since it's lifetime is greater
> > > than the outer qemu_loadvm_state.
> > 
> > Maybe it's just me, but "made static" to me indicates either a change
> > from fully-global to module-global, or (function) local automatic to
> > local static, not a change from function local-automatic to
> > module-global as here.
> > 
> > It's also not clear from this patch alone why the lifetime of
> > loadvm_handlers now needs to exceed that of qemu_loadvm_state().
> 
> OK, how about if I reworked that last sentence to be:
> 
>    loadvm_handlers is made module-global to survive beyond the lifetime
>    of the outer qemu_loadvm_state since it may still be in use by
>    a subloop in the postcopy listen thread.

Yeah, that's better.  A global seems ugly though.  Would it be better
to dynamically allocate the list head and pass a pointer into the
listen thread, or even to pass the list head by value into the listen
thread.

The individual list elements need to be cleaned up at some point
anyway, so I don't think that introduces any lifetime questions that
weren't already there.

-- 
David Gibson                    | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
                                | _way_ _around_!
http://www.ozlabs.org/~dgibson

pgpOr1MK7_4nf.pgp
Description: PGP signature