Steve Sistare <steven.sist...@oracle.com> writes:
> If the outgoing machine was previously suspended, propagate that to the
> incoming side via global_state, so a subsequent vm_start restores the
> suspended state. To maintain backward and forward compatibility, reclaim
> some space from the runstate member.
>
> Signed-off-by: Steve Sistare <steven.sist...@oracle.com>
> ---
> migration/global_state.c | 35 +++++++++++++++++++++++++++++++++--
> 1 file changed, 33 insertions(+), 2 deletions(-)
>
> diff --git a/migration/global_state.c b/migration/global_state.c
> index 4e2a9d8..d4f61a1 100644
> --- a/migration/global_state.c
> +++ b/migration/global_state.c
> @@ -22,7 +22,16 @@
>
> typedef struct {
> uint32_t size;
> - uint8_t runstate[100];
> +
> + /*
> + * runstate was 100 bytes, zero padded, but we trimmed it to add a
> + * few fields and maintain backwards compatibility.
> + */
> + uint8_t runstate[32];
> + uint8_t has_vm_was_suspended;
> + uint8_t vm_was_suspended;
> + uint8_t unused[66];
> +
> RunState state;
> bool received;
> } GlobalState;
> @@ -35,6 +44,10 @@ static void global_state_do_store(RunState state)
> assert(strlen(state_str) < sizeof(global_state.runstate));
> strpadcpy((char *)global_state.runstate, sizeof(global_state.runstate),
> state_str, '\0');
> + global_state.has_vm_was_suspended = true;
> + global_state.vm_was_suspended = vm_get_suspended();
> +
> + memset(global_state.unused, 0, sizeof(global_state.unused));
> }
>
> void global_state_store(void)
> @@ -68,6 +81,12 @@ static bool global_state_needed(void *opaque)
> return true;
> }
>
> + /* If the suspended state must be remembered, it is needed */
> +
> + if (vm_get_suspended()) {
> + return true;
> + }
> +
> /* If state is running or paused, it is not needed */
>
> if (strcmp(runstate, "running") == 0 ||
> @@ -85,6 +104,7 @@ static int global_state_post_load(void *opaque, int
> version_id)
> Error *local_err = NULL;
> int r;
> char *runstate = (char *)s->runstate;
> + bool vm_was_suspended = s->has_vm_was_suspended && s->vm_was_suspended;
Why do you need has_vm_was_suspended? If they're always read like this,
then one flag should do, no?
>
> s->received = true;
> trace_migrate_global_state_post_load(runstate);
> @@ -93,7 +113,7 @@ static int global_state_post_load(void *opaque, int
> version_id)
> sizeof(s->runstate)) == sizeof(s->runstate)) {
> /*
> * This condition should never happen during migration, because
> - * all runstate names are shorter than 100 bytes (the size of
> + * all runstate names are shorter than 32 bytes (the size of
> * s->runstate). However, a malicious stream could overflow
> * the qapi_enum_parse() call, so we force the last character
> * to a NUL byte.
> @@ -110,6 +130,14 @@ static int global_state_post_load(void *opaque, int
> version_id)
> }
> s->state = r;
>
> + /*
> + * global_state is saved on the outgoing side before forcing a stopped
> + * state, so it may have saved state=suspended and vm_was_suspended=0.
> + * Now we are in a paused state, and when we later call vm_start, it must
> + * restore the suspended state, so we must set vm_was_suspended=1 here.
> + */
> + vm_set_suspended(vm_was_suspended || r == RUN_STATE_SUSPENDED);
> +
> return 0;
> }
>
> @@ -134,6 +162,9 @@ static const VMStateDescription vmstate_globalstate = {
> .fields = (VMStateField[]) {
> VMSTATE_UINT32(size, GlobalState),
> VMSTATE_BUFFER(runstate, GlobalState),
> + VMSTATE_UINT8(has_vm_was_suspended, GlobalState),
> + VMSTATE_UINT8(vm_was_suspended, GlobalState),
> + VMSTATE_BUFFER(unused, GlobalState),
> VMSTATE_END_OF_LIST()
> },
> };
