* [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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.