Re: [Qemu-devel] [PATCH 4/9] runstate_set(): Check for valid transitions

[Jan Kiszka]

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 <address@hidden>
> ---
>  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.

    case RSTATE_IO_ERROR:
    case RSTATE_PAUSED:
        switch (new_state) {
        case RSTATE_RUNNING:
            goto transition_ok;
        default:
            /* invalid transition */
            abort();
        }
        abort();

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux