* [Qemu-devel] [PATCH v1] migration/ram.c: do not set 'postcopy_running' in POSTCOPY_INCOMING_END
@ 2017-11-10 20:35 Daniel Henrique Barboza
2017-11-13 3:22 ` Peter Xu
2017-11-21 18:41 ` Juan Quintela
0 siblings, 2 replies; 6+ messages in thread
From: Daniel Henrique Barboza @ 2017-11-10 20:35 UTC (permalink / raw)
To: qemu-devel; +Cc: mdroth, quintela, dgilbert
When migrating a VM with 'migrate_set_capability postcopy-ram on'
a postcopy_state is set during the process, ending up with the
state POSTCOPY_INCOMING_END when the migration is over. This
postcopy_state is taken into account inside ram_load to check
how it will load the memory pages. This same ram_load is called when
in a loadvm command.
Inside ram_load, the logic to see if we're at postcopy_running state
is:
postcopy_running = postcopy_state_get() >= POSTCOPY_INCOMING_LISTENING
postcopy_state_get() returns this enum type:
typedef enum {
POSTCOPY_INCOMING_NONE = 0,
POSTCOPY_INCOMING_ADVISE,
POSTCOPY_INCOMING_DISCARD,
POSTCOPY_INCOMING_LISTENING,
POSTCOPY_INCOMING_RUNNING,
POSTCOPY_INCOMING_END
} PostcopyState;
In the case where ram_load is executed and postcopy_state is
POSTCOPY_INCOMING_END, postcopy_running will be set to 'true' and
ram_load will behave like a postcopy is in progress. This scenario isn't
achievable in a migration but it is reproducible when executing
savevm/loadvm after migrating with 'postcopy-ram on', causing loadvm
to fail with Error -22:
Source:
(qemu) migrate_set_capability postcopy-ram on
(qemu) migrate tcp:127.0.0.1:4444
Dest:
(qemu) migrate_set_capability postcopy-ram on
(qemu)
ubuntu1704-intel login:
Ubuntu 17.04 ubuntu1704-intel ttyS0
ubuntu1704-intel login: (qemu)
(qemu) savevm test1
(qemu) loadvm test1
Unknown combination of migration flags: 0x4 (postcopy mode)
error while loading state for instance 0x0 of device 'ram'
Error -22 while loading VM state
(qemu)
This patch fixes this problem by changing a bit the semantics
of postcopy_running inside ram_load, verifying first if
we're not in the POSTCOPY_INCOMING_END state. In this case,
postcopy_running is set to 'false'.
Signed-off-by: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com>
---
migration/ram.c | 22 +++++++++++++++-------
1 file changed, 15 insertions(+), 7 deletions(-)
diff --git a/migration/ram.c b/migration/ram.c
index 8620aa400a..43ed719668 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -2803,13 +2803,21 @@ static int ram_load(QEMUFile *f, void *opaque, int version_id)
int flags = 0, ret = 0, invalid_flags = 0;
static uint64_t seq_iter;
int len = 0;
- /*
- * If system is running in postcopy mode, page inserts to host memory must
- * be atomic
- */
- bool postcopy_running = postcopy_state_get() >= POSTCOPY_INCOMING_LISTENING;
- /* ADVISE is earlier, it shows the source has the postcopy capability on */
- bool postcopy_advised = postcopy_state_get() >= POSTCOPY_INCOMING_ADVISE;
+ bool postcopy_advised = false, postcopy_running = false;
+ uint8_t postcopy_state = postcopy_state_get();
+
+ if (postcopy_state != POSTCOPY_INCOMING_END) {
+ /*
+ * If system is running in postcopy mode, page inserts to host memory
+ * must be atomic
+ */
+ postcopy_running = postcopy_state >= POSTCOPY_INCOMING_LISTENING;
+
+ /* ADVISE is earlier, it shows the source has the postcopy
+ * capability on
+ */
+ postcopy_advised = postcopy_state >= POSTCOPY_INCOMING_ADVISE;
+ }
seq_iter++;
--
2.13.6
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH v1] migration/ram.c: do not set 'postcopy_running' in POSTCOPY_INCOMING_END
2017-11-10 20:35 [Qemu-devel] [PATCH v1] migration/ram.c: do not set 'postcopy_running' in POSTCOPY_INCOMING_END Daniel Henrique Barboza
@ 2017-11-13 3:22 ` Peter Xu
2017-11-13 11:39 ` Daniel Henrique Barboza
2017-11-21 18:41 ` Juan Quintela
1 sibling, 1 reply; 6+ messages in thread
From: Peter Xu @ 2017-11-13 3:22 UTC (permalink / raw)
To: Daniel Henrique Barboza; +Cc: qemu-devel, mdroth, dgilbert, quintela
On Fri, Nov 10, 2017 at 06:35:16PM -0200, Daniel Henrique Barboza wrote:
> When migrating a VM with 'migrate_set_capability postcopy-ram on'
> a postcopy_state is set during the process, ending up with the
> state POSTCOPY_INCOMING_END when the migration is over. This
> postcopy_state is taken into account inside ram_load to check
> how it will load the memory pages. This same ram_load is called when
> in a loadvm command.
>
> Inside ram_load, the logic to see if we're at postcopy_running state
> is:
>
> postcopy_running = postcopy_state_get() >= POSTCOPY_INCOMING_LISTENING
>
> postcopy_state_get() returns this enum type:
>
> typedef enum {
> POSTCOPY_INCOMING_NONE = 0,
> POSTCOPY_INCOMING_ADVISE,
> POSTCOPY_INCOMING_DISCARD,
> POSTCOPY_INCOMING_LISTENING,
> POSTCOPY_INCOMING_RUNNING,
> POSTCOPY_INCOMING_END
> } PostcopyState;
>
> In the case where ram_load is executed and postcopy_state is
> POSTCOPY_INCOMING_END, postcopy_running will be set to 'true' and
> ram_load will behave like a postcopy is in progress. This scenario isn't
> achievable in a migration but it is reproducible when executing
> savevm/loadvm after migrating with 'postcopy-ram on', causing loadvm
> to fail with Error -22:
>
> Source:
>
> (qemu) migrate_set_capability postcopy-ram on
> (qemu) migrate tcp:127.0.0.1:4444
>
> Dest:
>
> (qemu) migrate_set_capability postcopy-ram on
> (qemu)
> ubuntu1704-intel login:
> Ubuntu 17.04 ubuntu1704-intel ttyS0
>
> ubuntu1704-intel login: (qemu)
> (qemu) savevm test1
> (qemu) loadvm test1
> Unknown combination of migration flags: 0x4 (postcopy mode)
> error while loading state for instance 0x0 of device 'ram'
> Error -22 while loading VM state
> (qemu)
>
> This patch fixes this problem by changing a bit the semantics
> of postcopy_running inside ram_load, verifying first if
> we're not in the POSTCOPY_INCOMING_END state. In this case,
> postcopy_running is set to 'false'.
>
> Signed-off-by: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com>
> ---
> migration/ram.c | 22 +++++++++++++++-------
> 1 file changed, 15 insertions(+), 7 deletions(-)
>
> diff --git a/migration/ram.c b/migration/ram.c
> index 8620aa400a..43ed719668 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -2803,13 +2803,21 @@ static int ram_load(QEMUFile *f, void *opaque, int version_id)
> int flags = 0, ret = 0, invalid_flags = 0;
> static uint64_t seq_iter;
> int len = 0;
> - /*
> - * If system is running in postcopy mode, page inserts to host memory must
> - * be atomic
> - */
> - bool postcopy_running = postcopy_state_get() >= POSTCOPY_INCOMING_LISTENING;
> - /* ADVISE is earlier, it shows the source has the postcopy capability on */
> - bool postcopy_advised = postcopy_state_get() >= POSTCOPY_INCOMING_ADVISE;
> + bool postcopy_advised = false, postcopy_running = false;
> + uint8_t postcopy_state = postcopy_state_get();
> +
> + if (postcopy_state != POSTCOPY_INCOMING_END) {
> + /*
> + * If system is running in postcopy mode, page inserts to host memory
> + * must be atomic
> + */
> + postcopy_running = postcopy_state >= POSTCOPY_INCOMING_LISTENING;
> +
> + /* ADVISE is earlier, it shows the source has the postcopy
> + * capability on
> + */
> + postcopy_advised = postcopy_state >= POSTCOPY_INCOMING_ADVISE;
> + }
For me, this is not that clear. I would prefer something simpler like:
PostcopyState state = postcopy_state_get();
bool postcopy_running = state >= POSTCOPY_INCOMING_LISTENING && \
state < POSTCOPY_INCOMING_END;
Or we can introduce postcopy_is_running() helper. Same thing to
postcopy_advised. Thanks,
>
> seq_iter++;
>
> --
> 2.13.6
>
>
--
Peter Xu
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH v1] migration/ram.c: do not set 'postcopy_running' in POSTCOPY_INCOMING_END
2017-11-13 3:22 ` Peter Xu
@ 2017-11-13 11:39 ` Daniel Henrique Barboza
0 siblings, 0 replies; 6+ messages in thread
From: Daniel Henrique Barboza @ 2017-11-13 11:39 UTC (permalink / raw)
To: Peter Xu; +Cc: quintela, qemu-devel, dgilbert, mdroth
Hi Peter,
On 11/13/2017 01:22 AM, Peter Xu wrote:
> On Fri, Nov 10, 2017 at 06:35:16PM -0200, Daniel Henrique Barboza wrote:
>> When migrating a VM with 'migrate_set_capability postcopy-ram on'
>> a postcopy_state is set during the process, ending up with the
>> state POSTCOPY_INCOMING_END when the migration is over. This
>> postcopy_state is taken into account inside ram_load to check
>> how it will load the memory pages. This same ram_load is called when
>> in a loadvm command.
>>
>> Inside ram_load, the logic to see if we're at postcopy_running state
>> is:
>>
>> postcopy_running = postcopy_state_get() >= POSTCOPY_INCOMING_LISTENING
>>
>> postcopy_state_get() returns this enum type:
>>
>> typedef enum {
>> POSTCOPY_INCOMING_NONE = 0,
>> POSTCOPY_INCOMING_ADVISE,
>> POSTCOPY_INCOMING_DISCARD,
>> POSTCOPY_INCOMING_LISTENING,
>> POSTCOPY_INCOMING_RUNNING,
>> POSTCOPY_INCOMING_END
>> } PostcopyState;
>>
>> In the case where ram_load is executed and postcopy_state is
>> POSTCOPY_INCOMING_END, postcopy_running will be set to 'true' and
>> ram_load will behave like a postcopy is in progress. This scenario isn't
>> achievable in a migration but it is reproducible when executing
>> savevm/loadvm after migrating with 'postcopy-ram on', causing loadvm
>> to fail with Error -22:
>>
>> Source:
>>
>> (qemu) migrate_set_capability postcopy-ram on
>> (qemu) migrate tcp:127.0.0.1:4444
>>
>> Dest:
>>
>> (qemu) migrate_set_capability postcopy-ram on
>> (qemu)
>> ubuntu1704-intel login:
>> Ubuntu 17.04 ubuntu1704-intel ttyS0
>>
>> ubuntu1704-intel login: (qemu)
>> (qemu) savevm test1
>> (qemu) loadvm test1
>> Unknown combination of migration flags: 0x4 (postcopy mode)
>> error while loading state for instance 0x0 of device 'ram'
>> Error -22 while loading VM state
>> (qemu)
>>
>> This patch fixes this problem by changing a bit the semantics
>> of postcopy_running inside ram_load, verifying first if
>> we're not in the POSTCOPY_INCOMING_END state. In this case,
>> postcopy_running is set to 'false'.
>>
>> Signed-off-by: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com>
>> ---
>> migration/ram.c | 22 +++++++++++++++-------
>> 1 file changed, 15 insertions(+), 7 deletions(-)
>>
>> diff --git a/migration/ram.c b/migration/ram.c
>> index 8620aa400a..43ed719668 100644
>> --- a/migration/ram.c
>> +++ b/migration/ram.c
>> @@ -2803,13 +2803,21 @@ static int ram_load(QEMUFile *f, void *opaque, int version_id)
>> int flags = 0, ret = 0, invalid_flags = 0;
>> static uint64_t seq_iter;
>> int len = 0;
>> - /*
>> - * If system is running in postcopy mode, page inserts to host memory must
>> - * be atomic
>> - */
>> - bool postcopy_running = postcopy_state_get() >= POSTCOPY_INCOMING_LISTENING;
>> - /* ADVISE is earlier, it shows the source has the postcopy capability on */
>> - bool postcopy_advised = postcopy_state_get() >= POSTCOPY_INCOMING_ADVISE;
>> + bool postcopy_advised = false, postcopy_running = false;
>> + uint8_t postcopy_state = postcopy_state_get();
>> +
>> + if (postcopy_state != POSTCOPY_INCOMING_END) {
>> + /*
>> + * If system is running in postcopy mode, page inserts to host memory
>> + * must be atomic
>> + */
>> + postcopy_running = postcopy_state >= POSTCOPY_INCOMING_LISTENING;
>> +
>> + /* ADVISE is earlier, it shows the source has the postcopy
>> + * capability on
>> + */
>> + postcopy_advised = postcopy_state >= POSTCOPY_INCOMING_ADVISE;
>> + }
> For me, this is not that clear. I would prefer something simpler like:
>
> PostcopyState state = postcopy_state_get();
> bool postcopy_running = state >= POSTCOPY_INCOMING_LISTENING && \
> state < POSTCOPY_INCOMING_END;
>
> Or we can introduce postcopy_is_running() helper. Same thing to
> postcopy_advised. Thanks,
I like the idea of creating helpers to make the logic simpler. I'll make
this change in the next spin.
Thanks,
Daniel
>
>>
>> seq_iter++;
>>
>> --
>> 2.13.6
>>
>>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH v1] migration/ram.c: do not set 'postcopy_running' in POSTCOPY_INCOMING_END
2017-11-10 20:35 [Qemu-devel] [PATCH v1] migration/ram.c: do not set 'postcopy_running' in POSTCOPY_INCOMING_END Daniel Henrique Barboza
2017-11-13 3:22 ` Peter Xu
@ 2017-11-21 18:41 ` Juan Quintela
2017-11-21 18:54 ` Dr. David Alan Gilbert
1 sibling, 1 reply; 6+ messages in thread
From: Juan Quintela @ 2017-11-21 18:41 UTC (permalink / raw)
To: Daniel Henrique Barboza; +Cc: qemu-devel, mdroth, dgilbert
Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com> wrote:
> When migrating a VM with 'migrate_set_capability postcopy-ram on'
> a postcopy_state is set during the process, ending up with the
> state POSTCOPY_INCOMING_END when the migration is over. This
> postcopy_state is taken into account inside ram_load to check
> how it will load the memory pages. This same ram_load is called when
> in a loadvm command.
>
> Inside ram_load, the logic to see if we're at postcopy_running state
> is:
>
> postcopy_running = postcopy_state_get() >= POSTCOPY_INCOMING_LISTENING
>
> postcopy_state_get() returns this enum type:
>
> typedef enum {
> POSTCOPY_INCOMING_NONE = 0,
> POSTCOPY_INCOMING_ADVISE,
> POSTCOPY_INCOMING_DISCARD,
> POSTCOPY_INCOMING_LISTENING,
> POSTCOPY_INCOMING_RUNNING,
> POSTCOPY_INCOMING_END
> } PostcopyState;
>
> In the case where ram_load is executed and postcopy_state is
> POSTCOPY_INCOMING_END, postcopy_running will be set to 'true' and
> ram_load will behave like a postcopy is in progress. This scenario isn't
> achievable in a migration but it is reproducible when executing
> savevm/loadvm after migrating with 'postcopy-ram on', causing loadvm
> to fail with Error -22:
>
> Source:
>
> (qemu) migrate_set_capability postcopy-ram on
> (qemu) migrate tcp:127.0.0.1:4444
>
> Dest:
>
> (qemu) migrate_set_capability postcopy-ram on
> (qemu)
> ubuntu1704-intel login:
> Ubuntu 17.04 ubuntu1704-intel ttyS0
>
> ubuntu1704-intel login: (qemu)
> (qemu) savevm test1
> (qemu) loadvm test1
> Unknown combination of migration flags: 0x4 (postcopy mode)
> error while loading state for instance 0x0 of device 'ram'
> Error -22 while loading VM state
> (qemu)
>
> This patch fixes this problem by changing a bit the semantics
> of postcopy_running inside ram_load, verifying first if
> we're not in the POSTCOPY_INCOMING_END state. In this case,
> postcopy_running is set to 'false'.
>
> Signed-off-by: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
queued
> ---
> migration/ram.c | 22 +++++++++++++++-------
> 1 file changed, 15 insertions(+), 7 deletions(-)
>
> diff --git a/migration/ram.c b/migration/ram.c
> index 8620aa400a..43ed719668 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -2803,13 +2803,21 @@ static int ram_load(QEMUFile *f, void *opaque, int version_id)
> int flags = 0, ret = 0, invalid_flags = 0;
> static uint64_t seq_iter;
> int len = 0;
> - /*
> - * If system is running in postcopy mode, page inserts to host memory must
> - * be atomic
> - */
> - bool postcopy_running = postcopy_state_get() >= POSTCOPY_INCOMING_LISTENING;
> - /* ADVISE is earlier, it shows the source has the postcopy capability on */
> - bool postcopy_advised = postcopy_state_get() >= POSTCOPY_INCOMING_ADVISE;
> + bool postcopy_advised = false, postcopy_running = false;
> + uint8_t postcopy_state = postcopy_state_get();
> +
> + if (postcopy_state != POSTCOPY_INCOMING_END) {
> + /*
> + * If system is running in postcopy mode, page inserts to host memory
> + * must be atomic
> + */
> + postcopy_running = postcopy_state >= POSTCOPY_INCOMING_LISTENING;
> +
> + /* ADVISE is earlier, it shows the source has the postcopy
> + * capability on
> + */
> + postcopy_advised = postcopy_state >= POSTCOPY_INCOMING_ADVISE;
> + }
>
> seq_iter++;
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH v1] migration/ram.c: do not set 'postcopy_running' in POSTCOPY_INCOMING_END
2017-11-21 18:41 ` Juan Quintela
@ 2017-11-21 18:54 ` Dr. David Alan Gilbert
2017-11-22 8:22 ` Juan Quintela
0 siblings, 1 reply; 6+ messages in thread
From: Dr. David Alan Gilbert @ 2017-11-21 18:54 UTC (permalink / raw)
To: Juan Quintela; +Cc: Daniel Henrique Barboza, qemu-devel, mdroth
* Juan Quintela (quintela@redhat.com) wrote:
> Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com> wrote:
> > When migrating a VM with 'migrate_set_capability postcopy-ram on'
> > a postcopy_state is set during the process, ending up with the
> > state POSTCOPY_INCOMING_END when the migration is over. This
> > postcopy_state is taken into account inside ram_load to check
> > how it will load the memory pages. This same ram_load is called when
> > in a loadvm command.
> >
> > Inside ram_load, the logic to see if we're at postcopy_running state
> > is:
> >
> > postcopy_running = postcopy_state_get() >= POSTCOPY_INCOMING_LISTENING
> >
> > postcopy_state_get() returns this enum type:
> >
> > typedef enum {
> > POSTCOPY_INCOMING_NONE = 0,
> > POSTCOPY_INCOMING_ADVISE,
> > POSTCOPY_INCOMING_DISCARD,
> > POSTCOPY_INCOMING_LISTENING,
> > POSTCOPY_INCOMING_RUNNING,
> > POSTCOPY_INCOMING_END
> > } PostcopyState;
> >
> > In the case where ram_load is executed and postcopy_state is
> > POSTCOPY_INCOMING_END, postcopy_running will be set to 'true' and
> > ram_load will behave like a postcopy is in progress. This scenario isn't
> > achievable in a migration but it is reproducible when executing
> > savevm/loadvm after migrating with 'postcopy-ram on', causing loadvm
> > to fail with Error -22:
> >
> > Source:
> >
> > (qemu) migrate_set_capability postcopy-ram on
> > (qemu) migrate tcp:127.0.0.1:4444
> >
> > Dest:
> >
> > (qemu) migrate_set_capability postcopy-ram on
> > (qemu)
> > ubuntu1704-intel login:
> > Ubuntu 17.04 ubuntu1704-intel ttyS0
> >
> > ubuntu1704-intel login: (qemu)
> > (qemu) savevm test1
> > (qemu) loadvm test1
> > Unknown combination of migration flags: 0x4 (postcopy mode)
> > error while loading state for instance 0x0 of device 'ram'
> > Error -22 while loading VM state
> > (qemu)
> >
> > This patch fixes this problem by changing a bit the semantics
> > of postcopy_running inside ram_load, verifying first if
> > we're not in the POSTCOPY_INCOMING_END state. In this case,
> > postcopy_running is set to 'false'.
> >
> > Signed-off-by: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com>
>
> Reviewed-by: Juan Quintela <quintela@redhat.com>
>
> queued
Wrong version; v3 is:
http://lists.nongnu.org/archive/html/qemu-devel/2017-11/msg03188.html
Dave
> > ---
> > migration/ram.c | 22 +++++++++++++++-------
> > 1 file changed, 15 insertions(+), 7 deletions(-)
> >
> > diff --git a/migration/ram.c b/migration/ram.c
> > index 8620aa400a..43ed719668 100644
> > --- a/migration/ram.c
> > +++ b/migration/ram.c
> > @@ -2803,13 +2803,21 @@ static int ram_load(QEMUFile *f, void *opaque, int version_id)
> > int flags = 0, ret = 0, invalid_flags = 0;
> > static uint64_t seq_iter;
> > int len = 0;
> > - /*
> > - * If system is running in postcopy mode, page inserts to host memory must
> > - * be atomic
> > - */
> > - bool postcopy_running = postcopy_state_get() >= POSTCOPY_INCOMING_LISTENING;
> > - /* ADVISE is earlier, it shows the source has the postcopy capability on */
> > - bool postcopy_advised = postcopy_state_get() >= POSTCOPY_INCOMING_ADVISE;
> > + bool postcopy_advised = false, postcopy_running = false;
> > + uint8_t postcopy_state = postcopy_state_get();
> > +
> > + if (postcopy_state != POSTCOPY_INCOMING_END) {
> > + /*
> > + * If system is running in postcopy mode, page inserts to host memory
> > + * must be atomic
> > + */
> > + postcopy_running = postcopy_state >= POSTCOPY_INCOMING_LISTENING;
> > +
> > + /* ADVISE is earlier, it shows the source has the postcopy
> > + * capability on
> > + */
> > + postcopy_advised = postcopy_state >= POSTCOPY_INCOMING_ADVISE;
> > + }
> >
> > seq_iter++;
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH v1] migration/ram.c: do not set 'postcopy_running' in POSTCOPY_INCOMING_END
2017-11-21 18:54 ` Dr. David Alan Gilbert
@ 2017-11-22 8:22 ` Juan Quintela
0 siblings, 0 replies; 6+ messages in thread
From: Juan Quintela @ 2017-11-22 8:22 UTC (permalink / raw)
To: Dr. David Alan Gilbert; +Cc: Daniel Henrique Barboza, qemu-devel, mdroth
"Dr. David Alan Gilbert" <dgilbert@redhat.com> wrote:
> * Juan Quintela (quintela@redhat.com) wrote:
>> Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com> wrote:
>> > When migrating a VM with 'migrate_set_capability postcopy-ram on'
>> > a postcopy_state is set during the process, ending up with the
>> > state POSTCOPY_INCOMING_END when the migration is over. This
>> > postcopy_state is taken into account inside ram_load to check
>> > how it will load the memory pages. This same ram_load is called when
>> > in a loadvm command.
>> >
>> > Inside ram_load, the logic to see if we're at postcopy_running state
>> > is:
>> >
>> > postcopy_running = postcopy_state_get() >= POSTCOPY_INCOMING_LISTENING
>> >
>> > postcopy_state_get() returns this enum type:
>> >
>> > typedef enum {
>> > POSTCOPY_INCOMING_NONE = 0,
>> > POSTCOPY_INCOMING_ADVISE,
>> > POSTCOPY_INCOMING_DISCARD,
>> > POSTCOPY_INCOMING_LISTENING,
>> > POSTCOPY_INCOMING_RUNNING,
>> > POSTCOPY_INCOMING_END
>> > } PostcopyState;
>> >
>> > In the case where ram_load is executed and postcopy_state is
>> > POSTCOPY_INCOMING_END, postcopy_running will be set to 'true' and
>> > ram_load will behave like a postcopy is in progress. This scenario isn't
>> > achievable in a migration but it is reproducible when executing
>> > savevm/loadvm after migrating with 'postcopy-ram on', causing loadvm
>> > to fail with Error -22:
>> >
>> > Source:
>> >
>> > (qemu) migrate_set_capability postcopy-ram on
>> > (qemu) migrate tcp:127.0.0.1:4444
>> >
>> > Dest:
>> >
>> > (qemu) migrate_set_capability postcopy-ram on
>> > (qemu)
>> > ubuntu1704-intel login:
>> > Ubuntu 17.04 ubuntu1704-intel ttyS0
>> >
>> > ubuntu1704-intel login: (qemu)
>> > (qemu) savevm test1
>> > (qemu) loadvm test1
>> > Unknown combination of migration flags: 0x4 (postcopy mode)
>> > error while loading state for instance 0x0 of device 'ram'
>> > Error -22 while loading VM state
>> > (qemu)
>> >
>> > This patch fixes this problem by changing a bit the semantics
>> > of postcopy_running inside ram_load, verifying first if
>> > we're not in the POSTCOPY_INCOMING_END state. In this case,
>> > postcopy_running is set to 'false'.
>> >
>> > Signed-off-by: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com>
>>
>> Reviewed-by: Juan Quintela <quintela@redhat.com>
>>
>> queued
>
> Wrong version; v3 is:
My bad, sorry. Got a new one.
No clue why search on my INBOX didn't found this version.
compiling a new pull request.
>
> http://lists.nongnu.org/archive/html/qemu-devel/2017-11/msg03188.html
>
> Dave
>
>> > ---
>> > migration/ram.c | 22 +++++++++++++++-------
>> > 1 file changed, 15 insertions(+), 7 deletions(-)
>> >
>> > diff --git a/migration/ram.c b/migration/ram.c
>> > index 8620aa400a..43ed719668 100644
>> > --- a/migration/ram.c
>> > +++ b/migration/ram.c
>> > @@ -2803,13 +2803,21 @@ static int ram_load(QEMUFile *f, void *opaque, int version_id)
>> > int flags = 0, ret = 0, invalid_flags = 0;
>> > static uint64_t seq_iter;
>> > int len = 0;
>> > - /*
>> > - * If system is running in postcopy mode, page inserts to host memory must
>> > - * be atomic
>> > - */
>> > - bool postcopy_running = postcopy_state_get() >= POSTCOPY_INCOMING_LISTENING;
>> > - /* ADVISE is earlier, it shows the source has the postcopy capability on */
>> > - bool postcopy_advised = postcopy_state_get() >= POSTCOPY_INCOMING_ADVISE;
>> > + bool postcopy_advised = false, postcopy_running = false;
>> > + uint8_t postcopy_state = postcopy_state_get();
>> > +
>> > + if (postcopy_state != POSTCOPY_INCOMING_END) {
>> > + /*
>> > + * If system is running in postcopy mode, page inserts to host memory
>> > + * must be atomic
>> > + */
>> > + postcopy_running = postcopy_state >= POSTCOPY_INCOMING_LISTENING;
>> > +
>> > + /* ADVISE is earlier, it shows the source has the postcopy
>> > + * capability on
>> > + */
>> > + postcopy_advised = postcopy_state >= POSTCOPY_INCOMING_ADVISE;
>> > + }
>> >
>> > seq_iter++;
> --
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-11-22 8:22 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-11-10 20:35 [Qemu-devel] [PATCH v1] migration/ram.c: do not set 'postcopy_running' in POSTCOPY_INCOMING_END Daniel Henrique Barboza
2017-11-13 3:22 ` Peter Xu
2017-11-13 11:39 ` Daniel Henrique Barboza
2017-11-21 18:41 ` Juan Quintela
2017-11-21 18:54 ` Dr. David Alan Gilbert
2017-11-22 8:22 ` Juan Quintela
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).