[Qemu-devel] [PATCH for-2.4 v3 0/3] Migration regressions with Xen.

[Qemu-devel] [PATCH for-2.4 v3 0/3] Migration regressions with Xen.

[Anthony PERARD]

Hi,

We've spotted several regression which prevent migration with Xen using the
same version of QEMU or from a previous version of QEMU (tryied with 2.2).

Regression have been introduce by at least:
- df4b1024526cae3479da3492d6371fd4a7324a03
  migration: create new section to store global state
- 61964c23e5ddd5a33f15699e45ce126f879e3e33
  migration: Add configuration section

The last patch fix migration for 'xenfv' machine and for 'pc,accel=xen'
machine. 'xenpv' machine is not affected as there is no emulated device
state to save.

Change since in v2:
- replace memcpy() call by a strncpy().
- Move the skip_config_section and the like from 'xenfv' machine init to
  xen_init() which is called by accel_init_machine. (in 3rd patch)

Thanks,

Anthony PERARD (3):
  migration: Fix global state with Xen.
  migration: Add configuration section to vmstate with xen.
  migration: Fix regression for xenfv and pc,accel=xen machine.

 include/migration/migration.h | 1 +
 migration/migration.c         | 7 +++++++
 migration/savevm.c            | 6 ++++++
 xen-common.c                  | 5 +++++
 4 files changed, 19 insertions(+)

-- 
Anthony PERARD

[Qemu-devel] [PATCH for-2.4 v3 1/3] migration: Fix global state with Xen.

[Anthony PERARD]

When doing migration via the QMP command xen_save_devices_state, the
current runstate is not store into the global state section. Also the
current runstate is not the one we want on the receiver side.

During migration, the Xen toolstack paused QEMU before save the devices
state. Also, the toolstack expect QEMU to autostart when the migration is
finished.
So this patch store "running" as it's current runstate.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
 include/migration/migration.h | 1 +
 migration/migration.c         | 7 +++++++
 migration/savevm.c            | 1 +
 3 files changed, 9 insertions(+)

diff --git a/include/migration/migration.h b/include/migration/migration.h
index a2f8ed0..8334621 100644
--- a/include/migration/migration.h
+++ b/include/migration/migration.h
@@ -203,4 +203,5 @@ void register_global_state(void);
 void global_state_set_optional(void);
 void savevm_skip_configuration(void);
 int global_state_store(void);
+void global_state_store_running(void);
 #endif
diff --git a/migration/migration.c b/migration/migration.c
index fd4f99b..662e77e 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -122,6 +122,13 @@ int global_state_store(void)
     return 0;
 }
 
+void global_state_store_running(void)
+{
+    const char *state = RunState_lookup[RUN_STATE_RUNNING];
+    strncpy((char *)global_state.runstate,
+           state, sizeof(global_state.runstate));
+}
+
 static bool global_state_received(void)
 {
     return global_state.received;
diff --git a/migration/savevm.c b/migration/savevm.c
index 81dbe58..6071215 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -1394,6 +1394,7 @@ void qmp_xen_save_devices_state(const char *filename, Error **errp)
 
     saved_vm_running = runstate_is_running();
     vm_stop(RUN_STATE_SAVE_VM);
+    global_state_store_running();
 
     f = qemu_fopen(filename, "wb");
     if (!f) {
-- 
Anthony PERARD

[Qemu-devel] [PATCH for-2.4 v3 2/3] migration: Add configuration section to vmstate with xen.

[Anthony PERARD]

This adds the configuration section in the vmstate saved by
xen_save_devices_state.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
 migration/savevm.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/migration/savevm.c b/migration/savevm.c
index 6071215..b3f605c 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -961,6 +961,11 @@ static int qemu_save_device_state(QEMUFile *f)
 
     cpu_synchronize_all_states();
 
+    if (!savevm_state.skip_configuration) {
+        qemu_put_byte(f, QEMU_VM_CONFIGURATION);
+        vmstate_save_state(f, &vmstate_configuration, &savevm_state, 0);
+    }
+
     QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
         if (se->is_ram) {
             continue;
-- 
Anthony PERARD

[Qemu-devel] [PATCH for-2.4 v3 3/3] migration: Fix regression for xenfv and pc, accel=xen machine.

[Anthony PERARD]

This fix migration from the same QEMU version and from previous QEMU
version.

>From the global state section, we don't need runstate with Xen. Right now,
the way the Xen toolstack knows when QEMU is ready is when QEMU reach
"running" runstate.

The configuration section and the section footers are not going to be
present in previous version of QEMU with xenfv machine, so we skip them.

The Xen toolstack libxenlight does not specify a particular version of the
'pc' machine, so migration from older version of QEMU used by Xen to newer
one would break due to missing "configuration" section and section footers.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
 xen-common.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/xen-common.c b/xen-common.c
index 56359ca..0dcdbc3 100644
--- a/xen-common.c
+++ b/xen-common.c
@@ -12,6 +12,7 @@
 #include "qmp-commands.h"
 #include "sysemu/char.h"
 #include "sysemu/accel.h"
+#include "migration/migration.h"
 
 //#define DEBUG_XEN
 
@@ -119,6 +120,10 @@ static int xen_init(MachineState *ms)
     }
     qemu_add_vm_change_state_handler(xen_change_state_handler, NULL);
 
+    global_state_set_optional();
+    savevm_skip_configuration();
+    savevm_skip_section_footers();
+
     return 0;
 }
 
-- 
Anthony PERARD

Re: [Qemu-devel] [PATCH for-2.4 v3 2/3] migration: Add configuration section to vmstate with xen.

[Stefano Stabellini]

On Mon, 3 Aug 2015, Anthony PERARD wrote:
> This adds the configuration section in the vmstate saved by
> xen_save_devices_state.
> 
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>

It doesn't seem to me that this patch is actually necessary to fix the
issue, is that right? 

If that is the case, I would rather skip it.


>  migration/savevm.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/migration/savevm.c b/migration/savevm.c
> index 6071215..b3f605c 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -961,6 +961,11 @@ static int qemu_save_device_state(QEMUFile *f)
>  
>      cpu_synchronize_all_states();
>  
> +    if (!savevm_state.skip_configuration) {
> +        qemu_put_byte(f, QEMU_VM_CONFIGURATION);
> +        vmstate_save_state(f, &vmstate_configuration, &savevm_state, 0);
> +    }
> +
>      QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
>          if (se->is_ram) {
>              continue;
> -- 
> Anthony PERARD
> 

Re: [Qemu-devel] [PATCH for-2.4 v3 2/3] migration: Add configuration section to vmstate with xen.

[Anthony PERARD]

On Mon, Aug 03, 2015 at 03:48:18PM +0100, Stefano Stabellini wrote:
> On Mon, 3 Aug 2015, Anthony PERARD wrote:
> > This adds the configuration section in the vmstate saved by
> > xen_save_devices_state.
> > 
> > Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> 
> It doesn't seem to me that this patch is actually necessary to fix the
> issue, is that right? 

No, it is not. This is going to be skipped with the next patch and with
accel=xen.

> If that is the case, I would rather skip it.
> 
> 
> >  migration/savevm.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/migration/savevm.c b/migration/savevm.c
> > index 6071215..b3f605c 100644
> > --- a/migration/savevm.c
> > +++ b/migration/savevm.c
> > @@ -961,6 +961,11 @@ static int qemu_save_device_state(QEMUFile *f)
> >  
> >      cpu_synchronize_all_states();
> >  
> > +    if (!savevm_state.skip_configuration) {
> > +        qemu_put_byte(f, QEMU_VM_CONFIGURATION);
> > +        vmstate_save_state(f, &vmstate_configuration, &savevm_state, 0);
> > +    }
> > +
> >      QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
> >          if (se->is_ram) {
> >              continue;
> > -- 
> > Anthony PERARD
> > 

-- 
Anthony PERARD

Re: [Qemu-devel] [PATCH for-2.4 v3 3/3] migration: Fix regression for xenfv and pc, accel=xen machine.

[Stefano Stabellini]

On Mon, 3 Aug 2015, Anthony PERARD wrote:
> This fix migration from the same QEMU version and from previous QEMU
> version.
> 
> >From the global state section, we don't need runstate with Xen. Right now,
> the way the Xen toolstack knows when QEMU is ready is when QEMU reach
> "running" runstate.
> 
> The configuration section and the section footers are not going to be
> present in previous version of QEMU with xenfv machine, so we skip them.
> 
> The Xen toolstack libxenlight does not specify a particular version of the
> 'pc' machine, so migration from older version of QEMU used by Xen to newer
> one would break due to missing "configuration" section and section footers.
> 
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>

Reviewed-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>


>  xen-common.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/xen-common.c b/xen-common.c
> index 56359ca..0dcdbc3 100644
> --- a/xen-common.c
> +++ b/xen-common.c
> @@ -12,6 +12,7 @@
>  #include "qmp-commands.h"
>  #include "sysemu/char.h"
>  #include "sysemu/accel.h"
> +#include "migration/migration.h"
>  
>  //#define DEBUG_XEN
>  
> @@ -119,6 +120,10 @@ static int xen_init(MachineState *ms)
>      }
>      qemu_add_vm_change_state_handler(xen_change_state_handler, NULL);
>  
> +    global_state_set_optional();
> +    savevm_skip_configuration();
> +    savevm_skip_section_footers();
> +
>      return 0;
>  }
>  
> -- 
> Anthony PERARD
> 

Re: [Qemu-devel] [PATCH for-2.4 v3 1/3] migration: Fix global state with Xen.

[Stefano Stabellini]

On Mon, 3 Aug 2015, Anthony PERARD wrote:
> When doing migration via the QMP command xen_save_devices_state, the
> current runstate is not store into the global state section. Also the
> current runstate is not the one we want on the receiver side.
> 
> During migration, the Xen toolstack paused QEMU before save the devices
> state. Also, the toolstack expect QEMU to autostart when the migration is
> finished.
> So this patch store "running" as it's current runstate.
> 
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>

Amit, Juan,

if you are OK with this patch, I'll send a pull request with it and
patch #3.

Thanks,

Stefano


>  include/migration/migration.h | 1 +
>  migration/migration.c         | 7 +++++++
>  migration/savevm.c            | 1 +
>  3 files changed, 9 insertions(+)
> 
> diff --git a/include/migration/migration.h b/include/migration/migration.h
> index a2f8ed0..8334621 100644
> --- a/include/migration/migration.h
> +++ b/include/migration/migration.h
> @@ -203,4 +203,5 @@ void register_global_state(void);
>  void global_state_set_optional(void);
>  void savevm_skip_configuration(void);
>  int global_state_store(void);
> +void global_state_store_running(void);
>  #endif
> diff --git a/migration/migration.c b/migration/migration.c
> index fd4f99b..662e77e 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -122,6 +122,13 @@ int global_state_store(void)
>      return 0;
>  }
>  
> +void global_state_store_running(void)
> +{
> +    const char *state = RunState_lookup[RUN_STATE_RUNNING];
> +    strncpy((char *)global_state.runstate,
> +           state, sizeof(global_state.runstate));
> +}
> +
>  static bool global_state_received(void)
>  {
>      return global_state.received;
> diff --git a/migration/savevm.c b/migration/savevm.c
> index 81dbe58..6071215 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -1394,6 +1394,7 @@ void qmp_xen_save_devices_state(const char *filename, Error **errp)
>  
>      saved_vm_running = runstate_is_running();
>      vm_stop(RUN_STATE_SAVE_VM);
> +    global_state_store_running();
>  
>      f = qemu_fopen(filename, "wb");
>      if (!f) {
> -- 
> Anthony PERARD
> 

Re: [Qemu-devel] [PATCH for-2.4 v3 1/3] migration: Fix global state with Xen.

[Amit Shah]

On (Mon) 03 Aug 2015 [15:53:57], Stefano Stabellini wrote:
> On Mon, 3 Aug 2015, Anthony PERARD wrote:
> > When doing migration via the QMP command xen_save_devices_state, the
> > current runstate is not store into the global state section. Also the
> > current runstate is not the one we want on the receiver side.
> > 
> > During migration, the Xen toolstack paused QEMU before save the devices
> > state. Also, the toolstack expect QEMU to autostart when the migration is
> > finished.
> > So this patch store "running" as it's current runstate.
> > 
> > Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> 
> Amit, Juan,
> 
> if you are OK with this patch, I'll send a pull request with it and
> patch #3.

Juan is away for the week.

I'm fine with patches 1 and 3 here; you may pick them up, thanks.


		Amit

Re: [Qemu-devel] [PATCH for-2.4 v3 0/3] Migration regressions with Xen.

[Amit Shah]

On (Mon) 03 Aug 2015 [15:29:18], Anthony PERARD wrote:
> Hi,
> 
> We've spotted several regression which prevent migration with Xen using the
> same version of QEMU or from a previous version of QEMU (tryied with 2.2).
> 
> Regression have been introduce by at least:
> - df4b1024526cae3479da3492d6371fd4a7324a03
>   migration: create new section to store global state
> - 61964c23e5ddd5a33f15699e45ce126f879e3e33
>   migration: Add configuration section
> 
> The last patch fix migration for 'xenfv' machine and for 'pc,accel=xen'
> machine. 'xenpv' machine is not affected as there is no emulated device
> state to save.
> 
> Change since in v2:
> - replace memcpy() call by a strncpy().
> - Move the skip_config_section and the like from 'xenfv' machine init to
>   xen_init() which is called by accel_init_machine. (in 3rd patch)

BTW I didn't see v2.  Was the previous series marked RFC v2?  The
changes seem to indicate there wasn't anything else in between.

		Amit

Re: [Qemu-devel] [Xen-devel] [PATCH for-2.4 v3 0/3] Migration regressions with Xen.

[Stefano Stabellini]

On Mon, 3 Aug 2015, Amit Shah wrote:
> On (Mon) 03 Aug 2015 [15:29:18], Anthony PERARD wrote:
> > Hi,
> > 
> > We've spotted several regression which prevent migration with Xen using the
> > same version of QEMU or from a previous version of QEMU (tryied with 2.2).
> > 
> > Regression have been introduce by at least:
> > - df4b1024526cae3479da3492d6371fd4a7324a03
> >   migration: create new section to store global state
> > - 61964c23e5ddd5a33f15699e45ce126f879e3e33
> >   migration: Add configuration section
> > 
> > The last patch fix migration for 'xenfv' machine and for 'pc,accel=xen'
> > machine. 'xenpv' machine is not affected as there is no emulated device
> > state to save.
> > 
> > Change since in v2:
> > - replace memcpy() call by a strncpy().
> > - Move the skip_config_section and the like from 'xenfv' machine init to
> >   xen_init() which is called by accel_init_machine. (in 3rd patch)
> 
> BTW I didn't see v2.  Was the previous series marked RFC v2?  The
> changes seem to indicate there wasn't anything else in between.

I am pretty sure that Anthony simply made a mistake with the version
number.

Re: [Qemu-devel] [PATCH for-2.4 v3 0/3] Migration regressions with Xen.

[Anthony PERARD]

On Mon, Aug 03, 2015 at 09:37:07PM +0530, Amit Shah wrote:
> On (Mon) 03 Aug 2015 [15:29:18], Anthony PERARD wrote:
> > Hi,
> > 
> > We've spotted several regression which prevent migration with Xen using the
> > same version of QEMU or from a previous version of QEMU (tryied with 2.2).
> > 
> > Regression have been introduce by at least:
> > - df4b1024526cae3479da3492d6371fd4a7324a03
> >   migration: create new section to store global state
> > - 61964c23e5ddd5a33f15699e45ce126f879e3e33
> >   migration: Add configuration section
> > 
> > The last patch fix migration for 'xenfv' machine and for 'pc,accel=xen'
> > machine. 'xenpv' machine is not affected as there is no emulated device
> > state to save.
> > 
> > Change since in v2:
> > - replace memcpy() call by a strncpy().
> > - Move the skip_config_section and the like from 'xenfv' machine init to
> >   xen_init() which is called by accel_init_machine. (in 3rd patch)
> 
> BTW I didn't see v2.  Was the previous series marked RFC v2?  The
> changes seem to indicate there wasn't anything else in between.

That's suppose to be the v2. And I change my mind while writing "Change
since v1" and decide to write "Change in v2" but "since" is still there...

-- 
Anthony PERARD