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

[Dr. David Alan Gilbert]

* Paolo Bonzini (address@hidden) wrote:
> Il 03/10/2014 19:47, Dr. David Alan Gilbert (git) ha scritto:
> >  
> > +/* These are ORable flags */
> 
> ... make them an "enum".

OK, will do - I'd generally tended to avoid using enum for things
that were ORable where the combinations weren't themselves members
of the enum; but I can do that.

> > +const int LOADVM_EXITCODE_QUITLOOP     =  1;
> > +const int LOADVM_EXITCODE_QUITPARENT   =  2;
> 
> LOADVM_QUIT_ALL, LOADVM_QUIT respectively?


> > +const int LOADVM_EXITCODE_KEEPHANDLERS =  4;
> > +
> 
> Is it more common to drop or keep handlers?

I'ts more common to drop them.

> In either case, please add a comment to the three constants that details
> how to use them.  In particular, please document why you should drop
> (resp. keep) handlers...

Does this make it clearer:

/* ORable flags that control the (potentially nested) loadvm_state loops */
enum LoadVMExitCodes {
    /* Quit the loop level that received this command */
    LOADVM_QUIT_LOOP     =  1,
    /* Quit this loop and our parent */
    LOADVM_QUIT_PARENT   =  2,
    /*
     * Keep the LoadStateEntry handler list after the loop exits,
     * because they're being used in another thread.
     */
    LOADVM_KEEP_HANDLERS =  4,
};

> Is it by chance that they are only used in savevm.c?  Should they be
> moved to a header file?

They're local.

> > +    if (exitcode & LOADVM_EXITCODE_QUITPARENT) {
> > +        DPRINTF("loadvm_handlers_state_main: End of loop with QUITPARENT");
> > +        exitcode &= ~LOADVM_EXITCODE_QUITPARENT;
> > +        exitcode &= LOADVM_EXITCODE_QUITLOOP;
> 
> Either you want |=, or the first &= is useless.

Ooh nicely spotted; yes that should be |=  - now I need to figure out why this
didn't break things.

The idea is we have:
 1   outer loadvm_state loop
 2      receives packaged command
 3        inner_loadvm_state loop
 4          receives handle_listen
 5          < QUITPARENT
 6        < QUITLOOP
 7       < QUITLOOP
 8   exits

so QUITPARENT causes it's parent to exit, and to do that
the inner loop transforms QUITPARENT into QUITLOOP as it's
exit.

Dave

> 
> Paolo
--
Dr. David Alan Gilbert / address@hidden / Manchester, UK