[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 4/9] runstate_set(): Check for valid transitions
From: |
Luiz Capitulino |
Subject: |
Re: [Qemu-devel] [PATCH 4/9] runstate_set(): Check for valid transitions |
Date: |
Tue, 6 Sep 2011 13:47:18 -0300 |
On Tue, 06 Sep 2011 17:55:12 +0200
Jan Kiszka <address@hidden> 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 <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.
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
>
- [Qemu-devel] [PATCH v4 0/9]: Introduce the RunState type, Luiz Capitulino, 2011/09/06
- [Qemu-devel] [PATCH 1/9] Move vm_state_notify() prototype from cpus.h to sysemu.h, Luiz Capitulino, 2011/09/06
- [Qemu-devel] [PATCH 2/9] Replace the VMSTOP macros with a proper state type, Luiz Capitulino, 2011/09/06
- [Qemu-devel] [PATCH 7/9] Monitor/QMP: Don't allow cont on bad VM state, Luiz Capitulino, 2011/09/06
- [Qemu-devel] [PATCH 5/9] Drop the incoming_expected global variable, Luiz Capitulino, 2011/09/06
- [Qemu-devel] [PATCH 4/9] runstate_set(): Check for valid transitions, Luiz Capitulino, 2011/09/06
[Qemu-devel] [PATCH 3/9] RunState: Add additional states, Luiz Capitulino, 2011/09/06
[Qemu-devel] [PATCH 8/9] QMP: query-status: Introduce 'status' key, Luiz Capitulino, 2011/09/06
[Qemu-devel] [PATCH 6/9] Drop the vm_running global variable, Luiz Capitulino, 2011/09/06
[Qemu-devel] [PATCH 9/9] HMP: info status: Print the VM state, Luiz Capitulino, 2011/09/06