From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:54799) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1R0xzk-0001fw-AC for qemu-devel@nongnu.org; Tue, 06 Sep 2011 11:55:32 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1R0xzZ-0007AD-G6 for qemu-devel@nongnu.org; Tue, 06 Sep 2011 11:55:28 -0400 Received: from goliath.siemens.de ([192.35.17.28]:20126) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1R0xzY-00079r-Vu for qemu-devel@nongnu.org; Tue, 06 Sep 2011 11:55:17 -0400 Message-ID: <4E664260.3010303@siemens.com> Date: Tue, 06 Sep 2011 17:55:12 +0200 From: Jan Kiszka MIME-Version: 1.0 References: <1315314868-24770-1-git-send-email-lcapitulino@redhat.com> <1315314868-24770-5-git-send-email-lcapitulino@redhat.com> In-Reply-To: <1315314868-24770-5-git-send-email-lcapitulino@redhat.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 4/9] runstate_set(): Check for valid transitions List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Luiz Capitulino Cc: "kwolf@redhat.com" , "amit.shah@redhat.com" , "aliguori@us.ibm.com" , "qemu-devel@nongnu.org" , "armbru@redhat.com" 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 > --- > 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