[PATCH RFC 0/3] Migration regressions with Xen.

[PATCH RFC 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 first patch would fix the default case when we use the machine 'xenfv'.

But we can also use the machine 'pc' and I don't know how to fix this case.
The second and third patches are a step in the direction of fixing
migration with the machine 'pc'.

I have not look at save/restore for PV guest yet.

Thanks,

Anthony PERARD (3):
  migration: Fix regretion for xenfv machine.
  migration: Fix global state with Xen.
  migration: Add configuration section to vmstate with xen.

 hw/i386/pc_piix.c             | 3 +++
 include/migration/migration.h | 1 +
 migration/migration.c         | 7 +++++++
 migration/savevm.c            | 6 ++++++
 4 files changed, 17 insertions(+)

-- 
Anthony PERARD

[PATCH RFC 1/3] migration: Fix regretion for xenfv machine.

[Anthony PERARD]

From: Anthony PERARD <anthony.perard@gmail.com>

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 sectin and the section footers are not going to be
present in previous version of QEMU with xenfv machine, so we skip them.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
Tested migration from QEMU 2.2.1 to master.
---
 hw/i386/pc_piix.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index a896624..b18758d 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -455,6 +455,9 @@ static void pc_xen_hvm_init(MachineState *machine)
     if (bus != NULL) {
         pci_create_simple(bus, -1, "xen-platform");
     }
+    global_state_set_optional();
+    savevm_skip_configuration();
+    savevm_skip_section_footers();
 }
 #endif
 
-- 
Anthony PERARD

[PATCH RFC 2/3] migration: Fix global state with Xen.

[Anthony PERARD]

From: Anthony PERARD <anthony.perard@gmail.com>

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..175a397 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];
+    memcpy((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

[PATCH RFC 3/3] migration: Add configuration section to vmstate with xen.

[Anthony PERARD]

From: Anthony PERARD <anthony.perard@gmail.com>

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

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

[Anthony PERARD]

This is a critical issue for Xen as migration either with the same version
of QEMU, or from a previous version of QEMU is broken.

Any suggestion on how to move forward?

On Tue, Jul 28, 2015 at 04:54:42PM +0100, 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 first patch would fix the default case when we use the machine 'xenfv'.
> 
> But we can also use the machine 'pc' and I don't know how to fix this case.
> The second and third patches are a step in the direction of fixing
> migration with the machine 'pc'.
> 
> I have not look at save/restore for PV guest yet.
> 
> Thanks,
> 
> Anthony PERARD (3):
>   migration: Fix regretion for xenfv machine.
>   migration: Fix global state with Xen.
>   migration: Add configuration section to vmstate with xen.
> 
>  hw/i386/pc_piix.c             | 3 +++
>  include/migration/migration.h | 1 +
>  migration/migration.c         | 7 +++++++
>  migration/savevm.c            | 6 ++++++
>  4 files changed, 17 insertions(+)

-- 
Anthony PERARD

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

[Juan Quintela]

Anthony PERARD <anthony.perard@citrix.com> wrote:
> This is a critical issue for Xen as migration either with the same version
> of QEMU, or from a previous version of QEMU is broken.
>
> Any suggestion on how to move forward?

Will send a pull requset tomorrow.

Thinking about creating a single function that is called for all needed
places, just to avoid this problem in the future.

Thanks, Juan.

>
> On Tue, Jul 28, 2015 at 04:54:42PM +0100, 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 first patch would fix the default case when we use the machine 'xenfv'.
>> 
>> But we can also use the machine 'pc' and I don't know how to fix this case.
>> The second and third patches are a step in the direction of fixing
>> migration with the machine 'pc'.
>> 
>> I have not look at save/restore for PV guest yet.
>> 
>> Thanks,
>> 
>> Anthony PERARD (3):
>>   migration: Fix regretion for xenfv machine.
>>   migration: Fix global state with Xen.
>>   migration: Add configuration section to vmstate with xen.
>> 
>>  hw/i386/pc_piix.c             | 3 +++
>>  include/migration/migration.h | 1 +
>>  migration/migration.c         | 7 +++++++
>>  migration/savevm.c            | 6 ++++++
>>  4 files changed, 17 insertions(+)

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

[Stefano Stabellini]

On Thu, 30 Jul 2015, Juan Quintela wrote:
> Anthony PERARD <anthony.perard@citrix.com> wrote:
> > This is a critical issue for Xen as migration either with the same version
> > of QEMU, or from a previous version of QEMU is broken.
> >
> > Any suggestion on how to move forward?
> 
> Will send a pull requset tomorrow.
> 
> Thinking about creating a single function that is called for all needed
> places, just to avoid this problem in the future.
> 

Hi Juan,

thanks for looking into this!
Do you have patches already we can look at and help you test to make
sure they fix the issue?

Thanks,

Stefano

> 
> >
> > On Tue, Jul 28, 2015 at 04:54:42PM +0100, 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 first patch would fix the default case when we use the machine 'xenfv'.
> >> 
> >> But we can also use the machine 'pc' and I don't know how to fix this case.
> >> The second and third patches are a step in the direction of fixing
> >> migration with the machine 'pc'.
> >> 
> >> I have not look at save/restore for PV guest yet.
> >> 
> >> Thanks,
> >> 
> >> Anthony PERARD (3):
> >>   migration: Fix regretion for xenfv machine.
> >>   migration: Fix global state with Xen.
> >>   migration: Add configuration section to vmstate with xen.
> >> 
> >>  hw/i386/pc_piix.c             | 3 +++
> >>  include/migration/migration.h | 1 +
> >>  migration/migration.c         | 7 +++++++
> >>  migration/savevm.c            | 6 ++++++
> >>  4 files changed, 17 insertions(+)
> 

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

[Stefano Stabellini]

On Thu, 30 Jul 2015, Stefano Stabellini wrote:
> On Thu, 30 Jul 2015, Juan Quintela wrote:
> > Anthony PERARD <anthony.perard@citrix.com> wrote:
> > > This is a critical issue for Xen as migration either with the same version
> > > of QEMU, or from a previous version of QEMU is broken.
> > >
> > > Any suggestion on how to move forward?
> > 
> > Will send a pull requset tomorrow.
> > 
> > Thinking about creating a single function that is called for all needed
> > places, just to avoid this problem in the future.
> > 
> 
> Hi Juan,
> 
> thanks for looking into this!
> Do you have patches already we can look at and help you test to make
> sure they fix the issue?

Any updates?


> 
> > 
> > >
> > > On Tue, Jul 28, 2015 at 04:54:42PM +0100, 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 first patch would fix the default case when we use the machine 'xenfv'.
> > >> 
> > >> But we can also use the machine 'pc' and I don't know how to fix this case.
> > >> The second and third patches are a step in the direction of fixing
> > >> migration with the machine 'pc'.
> > >> 
> > >> I have not look at save/restore for PV guest yet.
> > >> 
> > >> Thanks,
> > >> 
> > >> Anthony PERARD (3):
> > >>   migration: Fix regretion for xenfv machine.
> > >>   migration: Fix global state with Xen.
> > >>   migration: Add configuration section to vmstate with xen.
> > >> 
> > >>  hw/i386/pc_piix.c             | 3 +++
> > >>  include/migration/migration.h | 1 +
> > >>  migration/migration.c         | 7 +++++++
> > >>  migration/savevm.c            | 6 ++++++
> > >>  4 files changed, 17 insertions(+)
> > 
> 

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

[Amit Shah]

On (Fri) 31 Jul 2015 [10:59:47], Stefano Stabellini wrote:
> On Thu, 30 Jul 2015, Stefano Stabellini wrote:
> > On Thu, 30 Jul 2015, Juan Quintela wrote:
> > > Anthony PERARD <anthony.perard@citrix.com> wrote:
> > > > This is a critical issue for Xen as migration either with the same version
> > > > of QEMU, or from a previous version of QEMU is broken.
> > > >
> > > > Any suggestion on how to move forward?
> > > 
> > > Will send a pull requset tomorrow.
> > > 
> > > Thinking about creating a single function that is called for all needed
> > > places, just to avoid this problem in the future.
> > > 
> > 
> > Hi Juan,
> > 
> > thanks for looking into this!
> > Do you have patches already we can look at and help you test to make
> > sure they fix the issue?
> 
> Any updates?

That bigger overhaul would be 2.5 stuff now.

The only thing that tripped here was the RFC tag in the subject line.

Also, I think Dave had a comment about this but I think that was made
on IRC.  Dave, do you see a problem with this series?

Thanks,


		Amit

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

[Dr. David Alan Gilbert]

* Amit Shah (amit.shah@redhat.com) wrote:
> On (Fri) 31 Jul 2015 [10:59:47], Stefano Stabellini wrote:
> > On Thu, 30 Jul 2015, Stefano Stabellini wrote:
> > > On Thu, 30 Jul 2015, Juan Quintela wrote:
> > > > Anthony PERARD <anthony.perard@citrix.com> wrote:
> > > > > This is a critical issue for Xen as migration either with the same version
> > > > > of QEMU, or from a previous version of QEMU is broken.
> > > > >
> > > > > Any suggestion on how to move forward?
> > > > 
> > > > Will send a pull requset tomorrow.
> > > > 
> > > > Thinking about creating a single function that is called for all needed
> > > > places, just to avoid this problem in the future.
> > > > 
> > > 
> > > Hi Juan,
> > > 
> > > thanks for looking into this!
> > > Do you have patches already we can look at and help you test to make
> > > sure they fix the issue?
> > 
> > Any updates?
> 
> That bigger overhaul would be 2.5 stuff now.
> 
> The only thing that tripped here was the RFC tag in the subject line.
> 
> Also, I think Dave had a comment about this but I think that was made
> on IRC.  Dave, do you see a problem with this series?

Most of it is in Xen only code, so mostly no, however, the followign code looks
wrong to me:

+void global_state_store_running(void)
+{
+    const char *state = RunState_lookup[RUN_STATE_RUNNING];
+    memcpy((char *)global_state.runstate,
+           state, sizeof(global_state.runstate));
+}

Shouldn't that just be a strcpy ?

Dave

> Thanks,
> 
> 
> 		Amit
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

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

[Stefano Stabellini]

On Mon, 3 Aug 2015, Amit Shah wrote:
> On (Fri) 31 Jul 2015 [10:59:47], Stefano Stabellini wrote:
> > On Thu, 30 Jul 2015, Stefano Stabellini wrote:
> > > On Thu, 30 Jul 2015, Juan Quintela wrote:
> > > > Anthony PERARD <anthony.perard@citrix.com> wrote:
> > > > > This is a critical issue for Xen as migration either with the same version
> > > > > of QEMU, or from a previous version of QEMU is broken.
> > > > >
> > > > > Any suggestion on how to move forward?
> > > > 
> > > > Will send a pull requset tomorrow.
> > > > 
> > > > Thinking about creating a single function that is called for all needed
> > > > places, just to avoid this problem in the future.
> > > > 
> > > 
> > > Hi Juan,
> > > 
> > > thanks for looking into this!
> > > Do you have patches already we can look at and help you test to make
> > > sure they fix the issue?
> > 
> > Any updates?
> 
> That bigger overhaul would be 2.5 stuff now.

That's fine, but I would appreaciate if we could get a small fix in for
the release.


> The only thing that tripped here was the RFC tag in the subject line.
> 
> Also, I think Dave had a comment about this but I think that was made
> on IRC.  Dave, do you see a problem with this series?

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

[Amit Shah]

On (Mon) 03 Aug 2015 [10:37:26], Stefano Stabellini wrote:
> On Mon, 3 Aug 2015, Amit Shah wrote:
> > On (Fri) 31 Jul 2015 [10:59:47], Stefano Stabellini wrote:
> > > On Thu, 30 Jul 2015, Stefano Stabellini wrote:
> > > > On Thu, 30 Jul 2015, Juan Quintela wrote:
> > > > > Anthony PERARD <anthony.perard@citrix.com> wrote:
> > > > > > This is a critical issue for Xen as migration either with the same version
> > > > > > of QEMU, or from a previous version of QEMU is broken.
> > > > > >
> > > > > > Any suggestion on how to move forward?
> > > > > 
> > > > > Will send a pull requset tomorrow.
> > > > > 
> > > > > Thinking about creating a single function that is called for all needed
> > > > > places, just to avoid this problem in the future.
> > > > > 
> > > > 
> > > > Hi Juan,
> > > > 
> > > > thanks for looking into this!
> > > > Do you have patches already we can look at and help you test to make
> > > > sure they fix the issue?
> > > 
> > > Any updates?
> > 
> > That bigger overhaul would be 2.5 stuff now.
> 
> That's fine, but I would appreaciate if we could get a small fix in for
> the release.

Of course, pull req sent.

		Amit