* [Qemu-devel] [PATCH] migration: Fix block failure cases @ 2018-02-28 16:49 Dr. David Alan Gilbert (git) 2018-03-01 7:01 ` Peter Xu 0 siblings, 1 reply; 4+ messages in thread From: Dr. David Alan Gilbert (git) @ 2018-02-28 16:49 UTC (permalink / raw) To: qemu-devel, quintela; +Cc: peterx From: "Dr. David Alan Gilbert" <dgilbert@redhat.com> This fixes a couple of cases where the block migration capability doesn't get cleared when a migration failed. 1) When block migration is compiled out: (qemu) migrate -d -b "exec:cat > /dev/null" QEMU compiled without old-style (blk/-b, inc/-i) block migration Use drive_mirror+NBD instead. (qemu) migrate_set_capability xbzrle off QEMU compiled without old-style (blk/-b, inc/-i) block migration Use drive_mirror+NBD instead. This corresponds to https://bugzilla.redhat.com/show_bug.cgi?id=1550022 2) When a migration with a bad protocol is tried: (qemu) migrate -d -b "foo:bah" Parameter 'uri' expects a valid migration protocol (qemu) info migrate_capabilities xbzrle: off rdma-pin-all: off auto-converge: off zero-blocks: off compress: off events: off postcopy-ram: off x-colo: off release-ram: off block: on <<<<<<----- return-path: off pause-before-switchover: off x-multifd: off Fixes: 2833c59b947 Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com> --- migration/migration.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/migration/migration.c b/migration/migration.c index b913b98803..da0e4a1f56 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -1391,11 +1391,12 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk, return; } migrate_set_block_enabled(true, &local_err); + s->must_remove_block_options = true; if (local_err) { error_propagate(errp, local_err); + block_cleanup_parameters(s); return; } - s->must_remove_block_options = true; } if (has_inc && inc) { @@ -1417,11 +1418,10 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk, } else if (strstart(uri, "fd:", &p)) { fd_start_outgoing_migration(s, p, &local_err); } else { - error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "uri", + error_setg(&local_err, QERR_INVALID_PARAMETER_VALUE, "uri", "a valid migration protocol"); migrate_set_state(&s->state, MIGRATION_STATUS_SETUP, MIGRATION_STATUS_FAILED); - return; } if (local_err) { -- 2.14.3 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH] migration: Fix block failure cases 2018-02-28 16:49 [Qemu-devel] [PATCH] migration: Fix block failure cases Dr. David Alan Gilbert (git) @ 2018-03-01 7:01 ` Peter Xu 2018-03-01 15:32 ` Dr. David Alan Gilbert 0 siblings, 1 reply; 4+ messages in thread From: Peter Xu @ 2018-03-01 7:01 UTC (permalink / raw) To: Dr. David Alan Gilbert (git); +Cc: qemu-devel, quintela On Wed, Feb 28, 2018 at 04:49:37PM +0000, Dr. David Alan Gilbert (git) wrote: > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com> > > This fixes a couple of cases where the block migration capability > doesn't get cleared when a migration failed. > > 1) When block migration is compiled out: > (qemu) migrate -d -b "exec:cat > /dev/null" > QEMU compiled without old-style (blk/-b, inc/-i) block migration > Use drive_mirror+NBD instead. > (qemu) migrate_set_capability xbzrle off > QEMU compiled without old-style (blk/-b, inc/-i) block migration > Use drive_mirror+NBD instead. > > This corresponds to https://bugzilla.redhat.com/show_bug.cgi?id=1550022 > > 2) When a migration with a bad protocol is tried: > (qemu) migrate -d -b "foo:bah" > Parameter 'uri' expects a valid migration protocol > (qemu) info migrate_capabilities > xbzrle: off > rdma-pin-all: off > auto-converge: off > zero-blocks: off > compress: off > events: off > postcopy-ram: off > x-colo: off > release-ram: off > block: on <<<<<<----- > return-path: off > pause-before-switchover: off > x-multifd: off > > Fixes: 2833c59b947 > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com> > --- > migration/migration.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/migration/migration.c b/migration/migration.c > index b913b98803..da0e4a1f56 100644 > --- a/migration/migration.c > +++ b/migration/migration.c > @@ -1391,11 +1391,12 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk, > return; > } > migrate_set_block_enabled(true, &local_err); > + s->must_remove_block_options = true; > if (local_err) { > error_propagate(errp, local_err); > + block_cleanup_parameters(s); > return; > } > - s->must_remove_block_options = true; > } > > if (has_inc && inc) { > @@ -1417,11 +1418,10 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk, > } else if (strstart(uri, "fd:", &p)) { > fd_start_outgoing_migration(s, p, &local_err); > } else { > - error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "uri", > + error_setg(&local_err, QERR_INVALID_PARAMETER_VALUE, "uri", > "a valid migration protocol"); > migrate_set_state(&s->state, MIGRATION_STATUS_SETUP, > MIGRATION_STATUS_FAILED); > - return; > } > > if (local_err) { > -- > 2.14.3 > Ouch... Instead it seems to be my fault in 4a84214ebe ("migration: provide migrate_caps_check()", 2017-07-18). For now I cannot understand why I did that before since it's obviously strange if without this squashed... diff --git a/migration/migration.c b/migration/migration.c index 0aa596f867..88ed9375aa 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -747,13 +747,15 @@ void qmp_migrate_set_capabilities(MigrationCapabilityStatusList *params, { MigrationState *s = migrate_get_current(); MigrationCapabilityStatusList *cap; + bool cap_list[MIGRATION_CAPABILITY__MAX]; if (migration_is_setup_or_active(s->state)) { error_setg(errp, QERR_MIGRATION_ACTIVE); return; } - if (!migrate_caps_check(s->enabled_capabilities, params, errp)) { + memcpy(cap_list, s->enabled_capabilities, sizeof(cap_list)); + if (!migrate_caps_check(cap_list, params, errp)) { return; } Otherwise I'll get: (qemu) migrate_set_capability postcopy-ram on (qemu) migrate_set_capability compress on Postcopy is not currently compatible with compression (qemu) info migrate_capabilities xbzrle: off rdma-pin-all: off auto-converge: off zero-blocks: off compress: on <------------------------- :( events: off postcopy-ram: on x-colo: off release-ram: off block: off return-path: off pause-before-switchover: off x-multifd: off And it looks very likely that this should solve the block bug too. (So I think either I got a brain fart last July, or now...) Thanks, -- Peter Xu ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH] migration: Fix block failure cases 2018-03-01 7:01 ` Peter Xu @ 2018-03-01 15:32 ` Dr. David Alan Gilbert 2018-03-02 3:43 ` Peter Xu 0 siblings, 1 reply; 4+ messages in thread From: Dr. David Alan Gilbert @ 2018-03-01 15:32 UTC (permalink / raw) To: Peter Xu; +Cc: qemu-devel, quintela * Peter Xu (peterx@redhat.com) wrote: > On Wed, Feb 28, 2018 at 04:49:37PM +0000, Dr. David Alan Gilbert (git) wrote: > > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com> > > > > This fixes a couple of cases where the block migration capability > > doesn't get cleared when a migration failed. > > > > 1) When block migration is compiled out: > > (qemu) migrate -d -b "exec:cat > /dev/null" > > QEMU compiled without old-style (blk/-b, inc/-i) block migration > > Use drive_mirror+NBD instead. > > (qemu) migrate_set_capability xbzrle off > > QEMU compiled without old-style (blk/-b, inc/-i) block migration > > Use drive_mirror+NBD instead. > > > > This corresponds to https://bugzilla.redhat.com/show_bug.cgi?id=1550022 > > > > 2) When a migration with a bad protocol is tried: > > (qemu) migrate -d -b "foo:bah" > > Parameter 'uri' expects a valid migration protocol > > (qemu) info migrate_capabilities > > xbzrle: off > > rdma-pin-all: off > > auto-converge: off > > zero-blocks: off > > compress: off > > events: off > > postcopy-ram: off > > x-colo: off > > release-ram: off > > block: on <<<<<<----- > > return-path: off > > pause-before-switchover: off > > x-multifd: off > > > > Fixes: 2833c59b947 > > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com> > > --- > > migration/migration.c | 6 +++--- > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/migration/migration.c b/migration/migration.c > > index b913b98803..da0e4a1f56 100644 > > --- a/migration/migration.c > > +++ b/migration/migration.c > > @@ -1391,11 +1391,12 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk, > > return; > > } > > migrate_set_block_enabled(true, &local_err); > > + s->must_remove_block_options = true; > > if (local_err) { > > error_propagate(errp, local_err); > > + block_cleanup_parameters(s); > > return; > > } > > - s->must_remove_block_options = true; > > } > > > > if (has_inc && inc) { > > @@ -1417,11 +1418,10 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk, > > } else if (strstart(uri, "fd:", &p)) { > > fd_start_outgoing_migration(s, p, &local_err); > > } else { > > - error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "uri", > > + error_setg(&local_err, QERR_INVALID_PARAMETER_VALUE, "uri", > > "a valid migration protocol"); > > migrate_set_state(&s->state, MIGRATION_STATUS_SETUP, > > MIGRATION_STATUS_FAILED); > > - return; > > } > > > > if (local_err) { > > -- > > 2.14.3 > > > > Ouch... > > Instead it seems to be my fault in 4a84214ebe ("migration: provide > migrate_caps_check()", 2017-07-18). For now I cannot understand why I > did that before since it's obviously strange if without this > squashed... > > diff --git a/migration/migration.c b/migration/migration.c > index 0aa596f867..88ed9375aa 100644 > --- a/migration/migration.c > +++ b/migration/migration.c > @@ -747,13 +747,15 @@ void qmp_migrate_set_capabilities(MigrationCapabilityStatusList *params, > { > MigrationState *s = migrate_get_current(); > MigrationCapabilityStatusList *cap; > + bool cap_list[MIGRATION_CAPABILITY__MAX]; > > if (migration_is_setup_or_active(s->state)) { > error_setg(errp, QERR_MIGRATION_ACTIVE); > return; > } > > - if (!migrate_caps_check(s->enabled_capabilities, params, errp)) { > + memcpy(cap_list, s->enabled_capabilities, sizeof(cap_list)); > + if (!migrate_caps_check(cap_list, params, errp)) { > return; > } > > Otherwise I'll get: > > (qemu) migrate_set_capability postcopy-ram on > (qemu) migrate_set_capability compress on > Postcopy is not currently compatible with compression > (qemu) info migrate_capabilities > xbzrle: off > rdma-pin-all: off > auto-converge: off > zero-blocks: off > compress: on <------------------------- :( > events: off > postcopy-ram: on > x-colo: off > release-ram: off > block: off > return-path: off > pause-before-switchover: off > x-multifd: off > > And it looks very likely that this should solve the block bug too. Yes, it looks like it should - but it doesn't solve the 2nd of my cases; so we also need the 2nd half of my change. > (So I think either I got a brain fart last July, or now...) Of course it's the combination of two bugs :-) Dave > Thanks, > > -- > Peter Xu -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH] migration: Fix block failure cases 2018-03-01 15:32 ` Dr. David Alan Gilbert @ 2018-03-02 3:43 ` Peter Xu 0 siblings, 0 replies; 4+ messages in thread From: Peter Xu @ 2018-03-02 3:43 UTC (permalink / raw) To: Dr. David Alan Gilbert; +Cc: qemu-devel, quintela On Thu, Mar 01, 2018 at 03:32:19PM +0000, Dr. David Alan Gilbert wrote: > * Peter Xu (peterx@redhat.com) wrote: > > On Wed, Feb 28, 2018 at 04:49:37PM +0000, Dr. David Alan Gilbert (git) wrote: > > > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com> > > > > > > This fixes a couple of cases where the block migration capability > > > doesn't get cleared when a migration failed. > > > > > > 1) When block migration is compiled out: > > > (qemu) migrate -d -b "exec:cat > /dev/null" > > > QEMU compiled without old-style (blk/-b, inc/-i) block migration > > > Use drive_mirror+NBD instead. > > > (qemu) migrate_set_capability xbzrle off > > > QEMU compiled without old-style (blk/-b, inc/-i) block migration > > > Use drive_mirror+NBD instead. > > > > > > This corresponds to https://bugzilla.redhat.com/show_bug.cgi?id=1550022 > > > > > > 2) When a migration with a bad protocol is tried: > > > (qemu) migrate -d -b "foo:bah" > > > Parameter 'uri' expects a valid migration protocol > > > (qemu) info migrate_capabilities > > > xbzrle: off > > > rdma-pin-all: off > > > auto-converge: off > > > zero-blocks: off > > > compress: off > > > events: off > > > postcopy-ram: off > > > x-colo: off > > > release-ram: off > > > block: on <<<<<<----- > > > return-path: off > > > pause-before-switchover: off > > > x-multifd: off > > > > > > Fixes: 2833c59b947 > > > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com> > > > --- > > > migration/migration.c | 6 +++--- > > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > > > diff --git a/migration/migration.c b/migration/migration.c > > > index b913b98803..da0e4a1f56 100644 > > > --- a/migration/migration.c > > > +++ b/migration/migration.c > > > @@ -1391,11 +1391,12 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk, > > > return; > > > } > > > migrate_set_block_enabled(true, &local_err); > > > + s->must_remove_block_options = true; > > > if (local_err) { > > > error_propagate(errp, local_err); > > > + block_cleanup_parameters(s); > > > return; > > > } > > > - s->must_remove_block_options = true; > > > } > > > > > > if (has_inc && inc) { > > > @@ -1417,11 +1418,10 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk, > > > } else if (strstart(uri, "fd:", &p)) { > > > fd_start_outgoing_migration(s, p, &local_err); > > > } else { > > > - error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "uri", > > > + error_setg(&local_err, QERR_INVALID_PARAMETER_VALUE, "uri", > > > "a valid migration protocol"); > > > migrate_set_state(&s->state, MIGRATION_STATUS_SETUP, > > > MIGRATION_STATUS_FAILED); > > > - return; > > > } > > > > > > if (local_err) { > > > -- > > > 2.14.3 > > > > > > > Ouch... > > > > Instead it seems to be my fault in 4a84214ebe ("migration: provide > > migrate_caps_check()", 2017-07-18). For now I cannot understand why I > > did that before since it's obviously strange if without this > > squashed... > > > > diff --git a/migration/migration.c b/migration/migration.c > > index 0aa596f867..88ed9375aa 100644 > > --- a/migration/migration.c > > +++ b/migration/migration.c > > @@ -747,13 +747,15 @@ void qmp_migrate_set_capabilities(MigrationCapabilityStatusList *params, > > { > > MigrationState *s = migrate_get_current(); > > MigrationCapabilityStatusList *cap; > > + bool cap_list[MIGRATION_CAPABILITY__MAX]; > > > > if (migration_is_setup_or_active(s->state)) { > > error_setg(errp, QERR_MIGRATION_ACTIVE); > > return; > > } > > > > - if (!migrate_caps_check(s->enabled_capabilities, params, errp)) { > > + memcpy(cap_list, s->enabled_capabilities, sizeof(cap_list)); > > + if (!migrate_caps_check(cap_list, params, errp)) { > > return; > > } > > > > > Otherwise I'll get: > > > > (qemu) migrate_set_capability postcopy-ram on > > (qemu) migrate_set_capability compress on > > Postcopy is not currently compatible with compression > > (qemu) info migrate_capabilities > > xbzrle: off > > rdma-pin-all: off > > auto-converge: off > > zero-blocks: off > > compress: on <------------------------- :( > > events: off > > postcopy-ram: on > > x-colo: off > > release-ram: off > > block: off > > return-path: off > > pause-before-switchover: off > > x-multifd: off > > > > And it looks very likely that this should solve the block bug too. > > Yes, it looks like it should - but it doesn't solve the 2nd of my cases; > so we also need the 2nd half of my change. > > > (So I think either I got a brain fart last July, or now...) > > Of course it's the combination of two bugs :-) Ah sure. :) Please just let me know if you want me to post a patch, or I'll assume you'll handle this together in the next post (and thanks anyways :) -- Peter Xu ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2018-03-02 3:44 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-02-28 16:49 [Qemu-devel] [PATCH] migration: Fix block failure cases Dr. David Alan Gilbert (git) 2018-03-01 7:01 ` Peter Xu 2018-03-01 15:32 ` Dr. David Alan Gilbert 2018-03-02 3:43 ` Peter Xu
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).