On Tue, 06 Sep 2011 17:55:12 +0200
Jan Kiszka <jan.kis...@siemens.com> wrote:
> On 2011-09-06 15:14, Luiz Capitulino wrote:
> > This commit could have been folded with the previous one, however
> > doing it separately will allow for easy bisect and revert if needed.
> >
> > Checking and testing all valid transitions wasn't trivial, chances
> > are this will need broader testing to become more stable.
> >
> > Signed-off-by: Luiz Capitulino <lcapitul...@redhat.com>
> > ---
> > vl.c | 149
> > +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
> > 1 files changed, 148 insertions(+), 1 deletions(-)
> >
> > diff --git a/vl.c b/vl.c
> > index 9926d2a..fe3628a 100644
> > --- a/vl.c
> > +++ b/vl.c
> > @@ -332,9 +332,156 @@ bool runstate_check(RunState state)
> > return current_run_state == state;
> > }
> >
> > +/* This function will abort() on invalid state transitions */
> > void runstate_set(RunState new_state)
> > {
> > - assert(new_state < RSTATE_MAX);
> > + switch (current_run_state) {
> > + case RSTATE_NO_STATE:
> > + switch (new_state) {
> > + case RSTATE_RUNNING:
> > + case RSTATE_IN_MIGRATE:
> > + case RSTATE_PRE_LAUNCH:
> > + goto transition_ok;
> > + default:
> > + /* invalid transition */
> > + abort();
> > + }
> > + abort();
> > + case RSTATE_DEBUG:
> > + switch (new_state) {
> > + case RSTATE_RUNNING:
> > + goto transition_ok;
> > + default:
> > + /* invalid transition */
> > + abort();
> > + }
> > + abort();
> > + case RSTATE_IN_MIGRATE:
> > + switch (new_state) {
> > + case RSTATE_RUNNING:
> > + case RSTATE_PRE_LAUNCH:
> > + goto transition_ok;
> > + default:
> > + /* invalid transition */
> > + abort();
> > + }
> > + abort();
> > + case RSTATE_PANICKED:
> > + switch (new_state) {
> > + case RSTATE_PAUSED:
> > + goto transition_ok;
> > + default:
> > + /* invalid transition */
> > + abort();
> > + }
> > + abort();
> > + case RSTATE_IO_ERROR:
> > + switch (new_state) {
> > + case RSTATE_RUNNING:
> > + goto transition_ok;
> > + default:
> > + /* invalid transition */
> > + abort();
> > + }
> > + abort();
> > + case RSTATE_PAUSED:
> > + switch (new_state) {
> > + case RSTATE_RUNNING:
> > + goto transition_ok;
> > + default:
> > + /* invalid transition */
> > + abort();
> > + }
> > + abort();
> > + case RSTATE_POST_MIGRATE:
> > + switch (new_state) {
> > + case RSTATE_RUNNING:
> > + goto transition_ok;
> > + default:
> > + /* invalid transition */
> > + abort();
> > + }
> > + abort();
> > + case RSTATE_PRE_LAUNCH:
> > + switch (new_state) {
> > + case RSTATE_RUNNING:
> > + case RSTATE_POST_MIGRATE:
> > + goto transition_ok;
> > + default:
> > + /* invalid transition */
> > + abort();
> > + }
> > + abort();
> > + case RSTATE_PRE_MIGRATE:
> > + switch (new_state) {
> > + case RSTATE_RUNNING:
> > + case RSTATE_POST_MIGRATE:
> > + goto transition_ok;
> > + default:
> > + /* invalid transition */
> > + abort();
> > + }
> > + abort();
> > + case RSTATE_RESTORE:
> > + switch (new_state) {
> > + case RSTATE_RUNNING:
> > + goto transition_ok;
> > + default:
> > + /* invalid transition */
> > + abort();
> > + }
> > + abort();
> > + case RSTATE_RUNNING:
> > + switch (new_state) {
> > + case RSTATE_DEBUG:
> > + case RSTATE_PANICKED:
> > + case RSTATE_IO_ERROR:
> > + case RSTATE_PAUSED:
> > + case RSTATE_PRE_MIGRATE:
> > + case RSTATE_RESTORE:
> > + case RSTATE_SAVEVM:
> > + case RSTATE_SHUTDOWN:
> > + case RSTATE_WATCHDOG:
> > + goto transition_ok;
> > + default:
> > + /* invalid transition */
> > + abort();
> > + }
> > + abort();
> > + case RSTATE_SAVEVM:
> > + switch (new_state) {
> > + case RSTATE_RUNNING:
> > + goto transition_ok;
> > + default:
> > + /* invalid transition */
> > + abort();
> > + }
> > + abort();
> > + case RSTATE_SHUTDOWN:
> > + switch (new_state) {
> > + case RSTATE_PAUSED:
> > + goto transition_ok;
> > + default:
> > + /* invalid transition */
> > + abort();
> > + }
> > + abort();
> > + case RSTATE_WATCHDOG:
> > + switch (new_state) {
> > + case RSTATE_RUNNING:
> > + goto transition_ok;
> > + default:
> > + /* invalid transition */
> > + abort();
> > + }
> > + abort();
> > + default:
> > + fprintf(stderr, "current run state is invalid: %s\n",
> > + runstate_as_string());
> > + abort();
> > + }
> > +
> > +transition_ok:
> > current_run_state = new_state;
> > }
> >
>
> I haven't looked at the transitions yet, but just to make the function
> smaller: you could fold identical blocks together, e.g.
I thought about doing that but I fear it's error-prone: you extend
RSTATE_PAUSED and forget about RSTATE_IO_ERROR.
I think it's better to have different things separated, that's, each state
has its own switch statement.
>
> case RSTATE_IO_ERROR:
> case RSTATE_PAUSED:
> switch (new_state) {
> case RSTATE_RUNNING:
> goto transition_ok;
> default:
> /* invalid transition */
> abort();
> }
> abort();
>
> Jan
>
