From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45942) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XgAZR-0000ed-1Q for qemu-devel@nongnu.org; Mon, 20 Oct 2014 06:52:17 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XgAZM-0001H8-5e for qemu-devel@nongnu.org; Mon, 20 Oct 2014 06:52:12 -0400 Received: from mx1.redhat.com ([209.132.183.28]:32385) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XgAZL-0001Gw-Un for qemu-devel@nongnu.org; Mon, 20 Oct 2014 06:52:08 -0400 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s9KAq7w3011626 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL) for ; Mon, 20 Oct 2014 06:52:07 -0400 Date: Mon, 20 Oct 2014 11:52:03 +0100 From: "Dr. David Alan Gilbert" Message-ID: <20141020105200.GG2517@work-vm> References: <1413359710-2799-1-git-send-email-quintela@redhat.com> <1413359710-2799-6-git-send-email-quintela@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1413359710-2799-6-git-send-email-quintela@redhat.com> Subject: Re: [Qemu-devel] [PATCH 5/7] migration: create now section to store global state List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Juan Quintela Cc: kwolf@redhat.com, qemu-devel@nongnu.org, laine@redhat.com, lcapitulino@redhat.com * Juan Quintela (quintela@redhat.com) wrote: > This includes a new section that for now just stores the current qemu state. > > Right now, there are only one way to control what is the state of the > target after migration. > > - If you run the target qemu with -S, it would start stopped. > - If you run the target qemu without -S, it would run just after migration finishes. > > The problem here is what happens if we start the target without -S and > there happens one error during migration that puts current state as > -EIO. Migration would ends (notice that the error happend doing block > IO, network IO, i.e. nothing related with migration), and when > migration finish, we would just "continue" running on destination, > probably hanging the guest/corruption data, whatever. A couple of questions: 1) Does the ordering of loading this state matter - lets say that the source was in an error state, then all the other device states get loaded and then it loads your global_state which tells the destination is in error - is that too late ? Could the device emulation have already started doing some IO or something to the devices which it wouldn't have done if it knew there was already a problem? 2) What's the advantage of the optional section over using the 'command' sections I use in postcopy; http://lists.gnu.org/archive/html/qemu-devel/2014-10/msg00337.html ? Dave > Signed-off-by: Juan Quintela > --- > include/migration/migration.h | 4 ++ > migration.c | 88 +++++++++++++++++++++++++++++++++++++++++-- > vl.c | 1 + > 3 files changed, 90 insertions(+), 3 deletions(-) > > diff --git a/include/migration/migration.h b/include/migration/migration.h > index 3cb5ba8..bc1069b 100644 > --- a/include/migration/migration.h > +++ b/include/migration/migration.h > @@ -174,4 +174,8 @@ size_t ram_control_save_page(QEMUFile *f, ram_addr_t block_offset, > ram_addr_t offset, size_t size, > int *bytes_sent); > > +void register_global_state(void); > +void global_state_store(void); > +char *global_state_get_runstate(void); > + > #endif > diff --git a/migration.c b/migration.c > index 8d675b3..6f7e50e 100644 > --- a/migration.c > +++ b/migration.c > @@ -112,10 +112,20 @@ static void process_incoming_migration_co(void *opaque) > exit(EXIT_FAILURE); > } > > - if (autostart) { > + /* runstate == "" means that we haven't received it through the > + * wire, so we obey autostart. runstate == runing means that we > + * need to run it, we need to make sure that we do it after > + * everything else has finished. Every other state change is done > + * at the post_load function */ > + > + if (strcmp(global_state_get_runstate(), "running") == 0 ) { > vm_start(); > - } else { > - runstate_set(RUN_STATE_PAUSED); > + } else if (strcmp(global_state_get_runstate(), "") == 0 ) { > + if (autostart) { > + vm_start(); > + } else { > + runstate_set(RUN_STATE_PAUSED); > + } > } > } > > @@ -608,6 +618,7 @@ static void *migration_thread(void *opaque) > qemu_system_wakeup_request(QEMU_WAKEUP_REASON_OTHER); > old_vm_running = runstate_is_running(); > > + global_state_store(); > ret = vm_stop_force_state(RUN_STATE_FINISH_MIGRATE); > if (ret >= 0) { > qemu_file_set_rate_limit(s->file, INT64_MAX); > @@ -699,3 +710,74 @@ void migrate_fd_connect(MigrationState *s) > qemu_thread_create(&s->thread, "migration", migration_thread, s, > QEMU_THREAD_JOINABLE); > } > + > +typedef struct { > + int32_t size; > + uint8_t runstate[100]; > +} GlobalState; > + > +static GlobalState global_state; > + > +void global_state_store(void) > +{ > + if (runstate_store((char*)global_state.runstate, > + sizeof(global_state.runstate)) == -1) { > + printf("Runstate is too big\n"); > + exit(-1); > + } > +} > + > +char *global_state_get_runstate(void) > +{ > + return (char *)global_state.runstate; > +} > + > +static int global_state_post_load(void *opaque, int version_id) > +{ > + GlobalState *s = opaque; > + int ret = 0; > + char *runstate = (char*)s->runstate; > + > + printf("loaded state: %s\n", runstate); > + > + if (strcmp(runstate, "running") != 0) { > + > + RunState r = runstate_index(runstate); > + > + if (r == -1) { > + printf("Unknown received state %s\n", runstate); > + return -1; > + } > + ret = vm_stop_force_state(r); > + } > + > + return ret; > +} > + > +static void global_state_pre_save(void *opaque) > +{ > + GlobalState *s = opaque; > + > + s->size = strlen((char*)s->runstate) + 1; > + printf("saved state: %s\n", s->runstate); > +} > + > +static const VMStateDescription vmstate_globalstate = { > + .name = "globalstate", > + .version_id = 1, > + .minimum_version_id = 1, > + .post_load = global_state_post_load, > + .pre_save = global_state_pre_save, > + .fields = (VMStateField[]) { > + VMSTATE_INT32(size, GlobalState), > + VMSTATE_BUFFER(runstate, GlobalState), > + VMSTATE_END_OF_LIST() > + }, > +}; > + > +void register_global_state(void) > +{ > + /* We would use it independently that we receive it */ > + strcpy((char*)&global_state.runstate, ""); > + vmstate_register(NULL, 0, &vmstate_globalstate, &global_state); > +} > diff --git a/vl.c b/vl.c > index 1788b6a..75e855e 100644 > --- a/vl.c > +++ b/vl.c > @@ -4511,6 +4511,7 @@ int main(int argc, char **argv, char **envp) > return 0; > } > > + register_global_state(); > if (incoming) { > Error *local_err = NULL; > qemu_start_incoming_migration(incoming, &local_err); > -- > 2.1.0 > > -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK