* Paolo Bonzini (pbonz...@redhat.com) 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 / dgilb...@redhat.com / Manchester, UK
