[Qemu-devel] [PULL 0/6] Migration pull request

[Qemu-devel] [PULL 0/6] Migration pull request

[Juan Quintela]

Hi

This pull requset:
- small optimization by Li
- fixes global state with savevm (reported by John Snow)
- get migration traces for migration state changes even if migration
  events are not enabled
- Change state after migration has finished (reported by Christian)
- add documentation for events

Please, Apply.

PD.  Yes, it had to pass:
   make check
   virt-test
   savevm by hand


The following changes since commit 661725da09f47eb92d356fac10a4cf3b7ad1f61d:

  Merge remote-tracking branch 'remotes/cohuck/tags/s390x-20150714' into staging (2015-07-14 18:50:17 +0100)

are available in the git repository at:

  git://github.com/juanquintela/qemu.git tags/migration/20150715

for you to fetch changes up to 4915a5ad93e6c8ac77e2e5a007be178d50c7b019:

  migration: We also want to store the global state for savevm (2015-07-15 10:51:05 +0200)

----------------------------------------------------------------
migration/next for 20150715

----------------------------------------------------------------
Juan Quintela (5):
      migration: Only change state after migration has finished
      migration: Trace event and migration event are different things
      migration: Write documetation for events capabilites
      migration: Register global state section before loadvm
      migration: We also want to store the global state for savevm

Liang Li (1):
      migration: reduce the count of strlen call

 include/migration/migration.h |  1 +
 migration/migration.c         | 52 ++++++++++++++++++++++++-------------------
 migration/ram.c               | 10 ++++-----
 migration/savevm.c            |  6 +++++
 qmp-commands.hx               |  1 +
 vl.c                          |  2 +-
 6 files changed, 43 insertions(+), 29 deletions(-)

[Qemu-devel] [PULL 1/6] migration: Only change state after migration has finished

[Juan Quintela]

On previous change, we changed state at post load time if it was not
running, special casing the "running" change.  Now, we change any states
at the end of the migration.

Signed-off-by: Juan Quintela <quintela@redhat.com>
Tested-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
 migration/migration.c | 48 +++++++++++++++++++++++++++---------------------
 1 file changed, 27 insertions(+), 21 deletions(-)

diff --git a/migration/migration.c b/migration/migration.c
index 45719a0..ede432e 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -104,6 +104,8 @@ typedef struct {
     bool optional;
     uint32_t size;
     uint8_t runstate[100];
+    RunState state;
+    bool received;
 } GlobalState;

 static GlobalState global_state;
@@ -119,9 +121,14 @@ static int global_state_store(void)
     return 0;
 }

-static char *global_state_get_runstate(void)
+static bool global_state_received(void)
 {
-    return (char *)global_state.runstate;
+    return global_state.received;
+}
+
+static RunState global_state_get_runstate(void)
+{
+    return global_state.state;
 }

 void global_state_set_optional(void)
@@ -154,26 +161,25 @@ static bool global_state_needed(void *opaque)
 static int global_state_post_load(void *opaque, int version_id)
 {
     GlobalState *s = opaque;
-    int ret = 0;
+    Error *local_err = NULL;
+    int r;
     char *runstate = (char *)s->runstate;

+    s->received = true;
     trace_migrate_global_state_post_load(runstate);

-    if (strcmp(runstate, "running") != 0) {
-        Error *local_err = NULL;
-        int r = qapi_enum_parse(RunState_lookup, runstate, RUN_STATE_MAX,
+    r = qapi_enum_parse(RunState_lookup, runstate, RUN_STATE_MAX,
                                 -1, &local_err);

-        if (r == -1) {
-            if (local_err) {
-                error_report_err(local_err);
-            }
-            return -EINVAL;
+    if (r == -1) {
+        if (local_err) {
+            error_report_err(local_err);
         }
-        ret = vm_stop_force_state(r);
+        return -EINVAL;
     }
+    s->state = r;

-   return ret;
+    return 0;
 }

 static void global_state_pre_save(void *opaque)
@@ -202,6 +208,7 @@ void register_global_state(void)
 {
     /* We would use it independently that we receive it */
     strcpy((char *)&global_state.runstate, "");
+    global_state.received = false;
     vmstate_register(NULL, 0, &vmstate_globalstate, &global_state);
 }

@@ -283,20 +290,19 @@ static void process_incoming_migration_co(void *opaque)
         exit(EXIT_FAILURE);
     }

-    /* 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 global state section was not received or we are in running
+       state, we need to obey autostart. Any other state is set with
+       runstate_set. */

-    if (strcmp(global_state_get_runstate(), "running") == 0) {
-        vm_start();
-    } else if (strcmp(global_state_get_runstate(), "") == 0) {
+    if (!global_state_received() ||
+        global_state_get_runstate() == RUN_STATE_RUNNING) {
         if (autostart) {
             vm_start();
         } else {
             runstate_set(RUN_STATE_PAUSED);
         }
+    } else {
+        runstate_set(global_state_get_runstate());
     }
     migrate_decompress_threads_join();
 }
-- 
2.4.3

[Qemu-devel] [PULL 2/6] migration: Trace event and migration event are different things

[Juan Quintela]

We can want the trace event even without migration events enabled.

Reported-by:  Wen Congyang <ghostwcy@gmail.com>
Signed-off-by: Juan Quintela <quintela@redhat.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 migration/migration.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/migration/migration.c b/migration/migration.c
index ede432e..ba82ff6 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -216,7 +216,6 @@ static void migrate_generate_event(int new_state)
 {
     if (migrate_use_events()) {
         qapi_event_send_migration(new_state, &error_abort);
-        trace_migrate_set_state(new_state);
     }
 }

@@ -528,6 +527,7 @@ void qmp_migrate_set_parameters(bool has_compress_level,
 static void migrate_set_state(MigrationState *s, int old_state, int new_state)
 {
     if (atomic_cmpxchg(&s->state, old_state, new_state) == old_state) {
+        trace_migrate_set_state(new_state);
         migrate_generate_event(new_state);
     }
 }
-- 
2.4.3

[Qemu-devel] [PULL 3/6] migration: Write documetation for events capabilites

[Juan Quintela]

Reported-by: Jiri Denemark <jdenemar@redhat.com>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 qmp-commands.hx | 1 +
 1 file changed, 1 insertion(+)

diff --git a/qmp-commands.hx b/qmp-commands.hx
index e1bcc60..ba630b1 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -3406,6 +3406,7 @@ Enable/Disable migration capabilities
 - "rdma-pin-all": pin all pages when using RDMA during migration
 - "auto-converge": throttle down guest to help convergence of migration
 - "zero-blocks": compress zero blocks during block migration
+- "events": generate events for each migration state change

 Arguments:

-- 
2.4.3

[Qemu-devel] [PULL 4/6] migration: Register global state section before loadvm

[Juan Quintela]

Otherwise, it is not found

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 vl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/vl.c b/vl.c
index 3f269dc..5856396 100644
--- a/vl.c
+++ b/vl.c
@@ -4615,6 +4615,7 @@ int main(int argc, char **argv, char **envp)
     }

     qemu_system_reset(VMRESET_SILENT);
+    register_global_state();
     if (loadvm) {
         if (load_vmstate(loadvm) < 0) {
             autostart = 0;
@@ -4628,7 +4629,6 @@ 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.4.3

[Qemu-devel] [PULL 5/6] migration: reduce the count of strlen call

[Juan Quintela]

From: Liang Li <liang.z.li@intel.com>

'strlen' is called three times in 'save_page_header', it's
inefficient.

Signed-off-by: Liang Li <liang.z.li@intel.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
Reviewed-by: Amit Shat <amit.shah@redhat.com>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 migration/ram.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/migration/ram.c b/migration/ram.c
index 1e58cd3..7f007e6 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -382,16 +382,16 @@ void migrate_compress_threads_create(void)
  */
 static size_t save_page_header(QEMUFile *f, RAMBlock *block, ram_addr_t offset)
 {
-    size_t size;
+    size_t size, len;

     qemu_put_be64(f, offset);
     size = 8;

     if (!(offset & RAM_SAVE_FLAG_CONTINUE)) {
-        qemu_put_byte(f, strlen(block->idstr));
-        qemu_put_buffer(f, (uint8_t *)block->idstr,
-                        strlen(block->idstr));
-        size += 1 + strlen(block->idstr);
+        len = strlen(block->idstr);
+        qemu_put_byte(f, len);
+        qemu_put_buffer(f, (uint8_t *)block->idstr, len);
+        size += 1 + len;
     }
     return size;
 }
-- 
2.4.3

[Qemu-devel] [PULL 6/6] migration: We also want to store the global state for savevm

[Juan Quintela]

Commit df4b1024526cae3479da3492d6371fd4a7324a03 introduced global_state
section.  But it only filled the state while doing migration.  While
doing a savevm, we stored an empty string as state.  So when we did a
loadvm, it complained that state was invalid.

Fedora 21, 4.1.1, qemu 2.4.0-rc0
> ../../configure --target-list="x86_64-softmmu"

068 2s ... - output mismatch (see 068.out.bad)
--- /home/bos/jhuston/src/qemu/tests/qemu-iotests/068.out	2015-07-08
17:56:18.588164979 -0400
+++ 068.out.bad	2015-07-09 17:39:58.636651317 -0400
@@ -6,6 +6,8 @@
 QEMU X.Y.Z monitor - type 'help' for more information
 (qemu) savevm 0
 (qemu) quit
+qemu-system-x86_64: Unknown savevm section or instance 'globalstate' 0
+qemu-system-x86_64: Error -22 while loading VM state
 QEMU X.Y.Z monitor - type 'help' for more information
 (qemu) quit
 *** done
Failures: 068
Failed 1 of 1 tests

Actually, there were two problems here:
- we registered global_state too late for load_vm (fixed on another
  patch on the list)
- we didn't store a valid state for savevm (fixed by this patch).

Reported-by: John Snow <jsnow@redhat.com>
Signed-off-by: Juan Quintela <quintela@redhat.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Amit Shat <amit.shah@redhat.com>
---
 include/migration/migration.h | 1 +
 migration/migration.c         | 2 +-
 migration/savevm.c            | 6 ++++++
 3 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/include/migration/migration.h b/include/migration/migration.h
index b2711ef..a2f8ed0 100644
--- a/include/migration/migration.h
+++ b/include/migration/migration.h
@@ -202,4 +202,5 @@ void savevm_skip_section_footers(void);
 void register_global_state(void);
 void global_state_set_optional(void);
 void savevm_skip_configuration(void);
+int global_state_store(void);
 #endif
diff --git a/migration/migration.c b/migration/migration.c
index ba82ff6..86ca099 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -110,7 +110,7 @@ typedef struct {

 static GlobalState global_state;

-static int global_state_store(void)
+int global_state_store(void)
 {
     if (!runstate_store((char *)global_state.runstate,
                         sizeof(global_state.runstate))) {
diff --git a/migration/savevm.c b/migration/savevm.c
index 86735fc..81dbe58 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -1315,6 +1315,12 @@ void hmp_savevm(Monitor *mon, const QDict *qdict)
     }

     saved_vm_running = runstate_is_running();
+
+    ret = global_state_store();
+    if (ret) {
+        monitor_printf(mon, "Error saving global state\n");
+        return;
+    }
     vm_stop(RUN_STATE_SAVE_VM);

     memset(sn, 0, sizeof(*sn));
-- 
2.4.3

Re: [Qemu-devel] [PULL 0/6] Migration pull request

[Amit Shah]

On (Wed) 15 Jul 2015 [11:04:34], Juan Quintela wrote:
> Hi
> 
> This pull requset:
> - small optimization by Li
> - fixes global state with savevm (reported by John Snow)
> - get migration traces for migration state changes even if migration
>   events are not enabled
> - Change state after migration has finished (reported by Christian)
> - add documentation for events
> 
> Please, Apply.

Peter, can you fix the typos in my name in patches 5 and 6, or do you
want a new pull req?

Thanks,

		Amit

Re: [Qemu-devel] [PULL 0/6] Migration pull request

[Juan Quintela]

Self-Nack

Misspelled Amit name, resending with name fixed.  Sorry.

Juan Quintela <quintela@redhat.com> wrote:
> Hi
>
> This pull requset:
> - small optimization by Li
> - fixes global state with savevm (reported by John Snow)
> - get migration traces for migration state changes even if migration
>   events are not enabled
> - Change state after migration has finished (reported by Christian)
> - add documentation for events
>
> Please, Apply.
>
> PD.  Yes, it had to pass:
>    make check
>    virt-test
>    savevm by hand
>
>
> The following changes since commit 661725da09f47eb92d356fac10a4cf3b7ad1f61d:
>
>   Merge remote-tracking branch 'remotes/cohuck/tags/s390x-20150714' into staging (2015-07-14 18:50:17 +0100)
>
> are available in the git repository at:
>
>   git://github.com/juanquintela/qemu.git tags/migration/20150715
>
> for you to fetch changes up to 4915a5ad93e6c8ac77e2e5a007be178d50c7b019:
>
>   migration: We also want to store the global state for savevm (2015-07-15 10:51:05 +0200)
>
> ----------------------------------------------------------------
> migration/next for 20150715
>
> ----------------------------------------------------------------
> Juan Quintela (5):
>       migration: Only change state after migration has finished
>       migration: Trace event and migration event are different things
>       migration: Write documetation for events capabilites
>       migration: Register global state section before loadvm
>       migration: We also want to store the global state for savevm
>
> Liang Li (1):
>       migration: reduce the count of strlen call
>
>  include/migration/migration.h |  1 +
>  migration/migration.c         | 52 ++++++++++++++++++++++++-------------------
>  migration/ram.c               | 10 ++++-----
>  migration/savevm.c            |  6 +++++
>  qmp-commands.hx               |  1 +
>  vl.c                          |  2 +-
>  6 files changed, 43 insertions(+), 29 deletions(-)

[Qemu-devel] [PULL 6/6] migration: We also want to store the global state for savevm

[Juan Quintela]

Commit df4b1024526cae3479da3492d6371fd4a7324a03 introduced global_state
section.  But it only filled the state while doing migration.  While
doing a savevm, we stored an empty string as state.  So when we did a
loadvm, it complained that state was invalid.

Fedora 21, 4.1.1, qemu 2.4.0-rc0
> ../../configure --target-list="x86_64-softmmu"

068 2s ... - output mismatch (see 068.out.bad)
--- /home/bos/jhuston/src/qemu/tests/qemu-iotests/068.out	2015-07-08
17:56:18.588164979 -0400
+++ 068.out.bad	2015-07-09 17:39:58.636651317 -0400
@@ -6,6 +6,8 @@
 QEMU X.Y.Z monitor - type 'help' for more information
 (qemu) savevm 0
 (qemu) quit
+qemu-system-x86_64: Unknown savevm section or instance 'globalstate' 0
+qemu-system-x86_64: Error -22 while loading VM state
 QEMU X.Y.Z monitor - type 'help' for more information
 (qemu) quit
 *** done
Failures: 068
Failed 1 of 1 tests

Actually, there were two problems here:
- we registered global_state too late for load_vm (fixed on another
  patch on the list)
- we didn't store a valid state for savevm (fixed by this patch).

Reported-by: John Snow <jsnow@redhat.com>
Signed-off-by: Juan Quintela <quintela@redhat.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Amit Shah <amit.shah@redhat.com>
Tested-by:  Christian Borntraeger <borntraeger@de.ibm.com>
---
 include/migration/migration.h | 1 +
 migration/migration.c         | 2 +-
 migration/savevm.c            | 6 ++++++
 3 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/include/migration/migration.h b/include/migration/migration.h
index b2711ef..a2f8ed0 100644
--- a/include/migration/migration.h
+++ b/include/migration/migration.h
@@ -202,4 +202,5 @@ void savevm_skip_section_footers(void);
 void register_global_state(void);
 void global_state_set_optional(void);
 void savevm_skip_configuration(void);
+int global_state_store(void);
 #endif
diff --git a/migration/migration.c b/migration/migration.c
index ba82ff6..86ca099 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -110,7 +110,7 @@ typedef struct {

 static GlobalState global_state;

-static int global_state_store(void)
+int global_state_store(void)
 {
     if (!runstate_store((char *)global_state.runstate,
                         sizeof(global_state.runstate))) {
diff --git a/migration/savevm.c b/migration/savevm.c
index 86735fc..81dbe58 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -1315,6 +1315,12 @@ void hmp_savevm(Monitor *mon, const QDict *qdict)
     }

     saved_vm_running = runstate_is_running();
+
+    ret = global_state_store();
+    if (ret) {
+        monitor_printf(mon, "Error saving global state\n");
+        return;
+    }
     vm_stop(RUN_STATE_SAVE_VM);

     memset(sn, 0, sizeof(*sn));
-- 
2.4.3

Re: [Qemu-devel] [PULL 0/6] Migration pull request

[Peter Maydell]

On 15 July 2015 at 11:16, Amit Shah <amit.shah@redhat.com> wrote:
> On (Wed) 15 Jul 2015 [11:04:34], Juan Quintela wrote:
>> Hi
>>
>> This pull requset:
>> - small optimization by Li
>> - fixes global state with savevm (reported by John Snow)
>> - get migration traces for migration state changes even if migration
>>   events are not enabled
>> - Change state after migration has finished (reported by Christian)
>> - add documentation for events
>>
>> Please, Apply.
>
> Peter, can you fix the typos in my name in patches 5 and 6, or do you
> want a new pull req?

I've applied Juan's fixed pull. For the record, I always need a
new pullreq -- it's not possible to edit the commits in a pull,
I can only edit if I'm applying patches individually to master.

thanks
-- PMM

Re: [Qemu-devel] [PULL 0/6] Migration pull request

[Juan Quintela]

Peter Maydell <peter.maydell@linaro.org> wrote:
> On 15 July 2015 at 11:16, Amit Shah <amit.shah@redhat.com> wrote:
>> On (Wed) 15 Jul 2015 [11:04:34], Juan Quintela wrote:
>>> Hi
>>>
>>> This pull requset:
>>> - small optimization by Li
>>> - fixes global state with savevm (reported by John Snow)
>>> - get migration traces for migration state changes even if migration
>>>   events are not enabled
>>> - Change state after migration has finished (reported by Christian)
>>> - add documentation for events
>>>
>>> Please, Apply.
>>
>> Peter, can you fix the typos in my name in patches 5 and 6, or do you
>> want a new pull req?
>
> I've applied Juan's fixed pull. For the record, I always need a
> new pullreq -- it's not possible to edit the commits in a pull,
> I can only edit if I'm applying patches individually to master.

I used a different algorithm.

What is easier for Peter?

O:-)

Thanks, Juan.