Re: [Qemu-devel] [PATCH v2 1/1] migration/ram.c: do not set 'postcopy_running' in POSTCOPY_INCOMING_END
* Daniel Henrique Barboza (danie...@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: > > 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 the existing logic for > postcopy_advised and postcopy_running in ram_load, making them > 'false' if we're at POSTCOPY_INCOMING_END state. > > Signed-off-by: Daniel Henrique BarbozaThank you for spotting that. As Peter says those two functions might be better elsewhere, but they're OK there; so: Reviewed-by: Dr. David Alan Gilbert > --- > migration/ram.c | 16 ++-- > 1 file changed, 14 insertions(+), 2 deletions(-) > > diff --git a/migration/ram.c b/migration/ram.c > index 8620aa400a..021d583b9b 100644 > --- a/migration/ram.c > +++ b/migration/ram.c > @@ -2798,6 +2798,18 @@ static int ram_load_postcopy(QEMUFile *f) > return ret; > } > > +static bool postcopy_is_advised(void) > +{ > +PostcopyState ps = postcopy_state_get(); > +return ps >= POSTCOPY_INCOMING_ADVISE && ps < POSTCOPY_INCOMING_END; > +} > + > +static bool postcopy_is_running(void) > +{ > +PostcopyState ps = postcopy_state_get(); > +return ps >= POSTCOPY_INCOMING_LISTENING && ps < POSTCOPY_INCOMING_END; > +} > + > static int ram_load(QEMUFile *f, void *opaque, int version_id) > { > int flags = 0, ret = 0, invalid_flags = 0; > @@ -2807,9 +2819,9 @@ static int ram_load(QEMUFile *f, void *opaque, int > version_id) > * If system is running in postcopy mode, page inserts to host memory > must > * be atomic > */ > -bool postcopy_running = postcopy_state_get() >= > POSTCOPY_INCOMING_LISTENING; > +bool postcopy_running = postcopy_is_running(); > /* ADVISE is earlier, it shows the source has the postcopy capability on > */ > -bool postcopy_advised = postcopy_state_get() >= POSTCOPY_INCOMING_ADVISE; > +bool postcopy_advised = postcopy_is_advised(); > > seq_iter++; > > -- > 2.13.6 > -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [Qemu-devel] [PATCH v2 1/1] migration/ram.c: do not set 'postcopy_running' in POSTCOPY_INCOMING_END
On Mon, Nov 13, 2017 at 04:35:17PM -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: > > 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 the existing logic for > postcopy_advised and postcopy_running in ram_load, making them > 'false' if we're at POSTCOPY_INCOMING_END state. > > Signed-off-by: Daniel Henrique Barboza> --- > migration/ram.c | 16 ++-- > 1 file changed, 14 insertions(+), 2 deletions(-) > > diff --git a/migration/ram.c b/migration/ram.c > index 8620aa400a..021d583b9b 100644 > --- a/migration/ram.c > +++ b/migration/ram.c > @@ -2798,6 +2798,18 @@ static int ram_load_postcopy(QEMUFile *f) > return ret; > } > > +static bool postcopy_is_advised(void) > +{ > +PostcopyState ps = postcopy_state_get(); > +return ps >= POSTCOPY_INCOMING_ADVISE && ps < POSTCOPY_INCOMING_END; > +} > + > +static bool postcopy_is_running(void) > +{ > +PostcopyState ps = postcopy_state_get(); > +return ps >= POSTCOPY_INCOMING_LISTENING && ps < POSTCOPY_INCOMING_END; > +} > + If there is a new version, maybe we can put these into postcopy-ram.c. But may not worth a repost since after all we have postcopy helpers in ram.c already. Reviewed-by: Peter Xu > static int ram_load(QEMUFile *f, void *opaque, int version_id) > { > int flags = 0, ret = 0, invalid_flags = 0; > @@ -2807,9 +2819,9 @@ static int ram_load(QEMUFile *f, void *opaque, int > version_id) > * If system is running in postcopy mode, page inserts to host memory > must > * be atomic > */ > -bool postcopy_running = postcopy_state_get() >= > POSTCOPY_INCOMING_LISTENING; > +bool postcopy_running = postcopy_is_running(); > /* ADVISE is earlier, it shows the source has the postcopy capability on > */ > -bool postcopy_advised = postcopy_state_get() >= POSTCOPY_INCOMING_ADVISE; > +bool postcopy_advised = postcopy_is_advised(); > > seq_iter++; > > -- > 2.13.6 > -- Peter Xu
[Qemu-devel] [PATCH v2 1/1] migration/ram.c: do not set 'postcopy_running' in POSTCOPY_INCOMING_END
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: 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 the existing logic for postcopy_advised and postcopy_running in ram_load, making them 'false' if we're at POSTCOPY_INCOMING_END state. Signed-off-by: Daniel Henrique Barboza--- migration/ram.c | 16 ++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/migration/ram.c b/migration/ram.c index 8620aa400a..021d583b9b 100644 --- a/migration/ram.c +++ b/migration/ram.c @@ -2798,6 +2798,18 @@ static int ram_load_postcopy(QEMUFile *f) return ret; } +static bool postcopy_is_advised(void) +{ +PostcopyState ps = postcopy_state_get(); +return ps >= POSTCOPY_INCOMING_ADVISE && ps < POSTCOPY_INCOMING_END; +} + +static bool postcopy_is_running(void) +{ +PostcopyState ps = postcopy_state_get(); +return ps >= POSTCOPY_INCOMING_LISTENING && ps < POSTCOPY_INCOMING_END; +} + static int ram_load(QEMUFile *f, void *opaque, int version_id) { int flags = 0, ret = 0, invalid_flags = 0; @@ -2807,9 +2819,9 @@ static int ram_load(QEMUFile *f, void *opaque, int version_id) * If system is running in postcopy mode, page inserts to host memory must * be atomic */ -bool postcopy_running = postcopy_state_get() >= POSTCOPY_INCOMING_LISTENING; +bool postcopy_running = postcopy_is_running(); /* ADVISE is earlier, it shows the source has the postcopy capability on */ -bool postcopy_advised = postcopy_state_get() >= POSTCOPY_INCOMING_ADVISE; +bool postcopy_advised = postcopy_is_advised(); seq_iter++; -- 2.13.6