* [Qemu-devel] [Qemu-devel [RFC] [WIP] v1] Adding feature to reconnect with -r option to migrate command
@ 2016-06-08 18:13 Md Haris Iqbal
2016-06-15 13:27 ` Dr. David Alan Gilbert
0 siblings, 1 reply; 2+ messages in thread
From: Md Haris Iqbal @ 2016-06-08 18:13 UTC (permalink / raw)
To: qemu-devel; +Cc: dgilbert, Md Haris Iqbal
---
hmp-commands.hx | 10 +++++---
hmp.c | 4 ++-
include/migration/migration.h | 1 +
migration/migration.c | 60 +++++++++++++++++++++++++++++++++----------
qapi-schema.json | 2 +-
qmp-commands.hx | 3 ++-
6 files changed, 60 insertions(+), 20 deletions(-)
diff --git a/hmp-commands.hx b/hmp-commands.hx
index 4f4f60a..2e7aed3 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -893,10 +893,11 @@ ETEXI
{
.name = "migrate",
- .args_type = "detach:-d,blk:-b,inc:-i,uri:s",
- .params = "[-d] [-b] [-i] uri",
+ .args_type = "detach:-d,recover:-r,blk:-b,inc:-i,uri:s",
+ .params = "[-d] [-r] [-b] [-i] uri",
.help = "migrate to URI (using -d to not wait for completion)"
- "\n\t\t\t -b for migration without shared storage with"
+ "\n\t\t\t -r to recover from a broken migration\n\t\t\t"
+ " -b for migration without shared storage with"
" full copy of disk\n\t\t\t -i for migration without "
"shared storage with incremental copy of disk "
"(base image shared between src and destination)",
@@ -905,9 +906,10 @@ ETEXI
STEXI
-@item migrate [-d] [-b] [-i] @var{uri}
+@item migrate [-d] [-r] [-b] [-i] @var{uri}
@findex migrate
Migrate to @var{uri} (using -d to not wait for completion).
+ -r to recover from a broken migration
-b for migration with full copy of disk
-i for migration with incremental copy of disk (base image is shared)
ETEXI
diff --git a/hmp.c b/hmp.c
index d510236..ec8fab4 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1544,12 +1544,14 @@ static void hmp_migrate_status_cb(void *opaque)
void hmp_migrate(Monitor *mon, const QDict *qdict)
{
bool detach = qdict_get_try_bool(qdict, "detach", false);
+ bool recover = qdict_get_try_bool(qdict, "recover", false);
bool blk = qdict_get_try_bool(qdict, "blk", false);
bool inc = qdict_get_try_bool(qdict, "inc", false);
const char *uri = qdict_get_str(qdict, "uri");
Error *err = NULL;
- qmp_migrate(uri, !!blk, blk, !!inc, inc, false, false, &err);
+ qmp_migrate(uri, !!recover, recover, !!blk, blk, !!inc, inc, false, false,
+ &err);
if (err) {
error_report_err(err);
return;
diff --git a/include/migration/migration.h b/include/migration/migration.h
index ac2c12c..4a3201b 100644
--- a/include/migration/migration.h
+++ b/include/migration/migration.h
@@ -139,6 +139,7 @@ struct MigrationState
int state;
MigrationParams params;
+ bool in_recovery;
/* State related to return path */
struct {
diff --git a/migration/migration.c b/migration/migration.c
index 991313a..a77f62e 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -933,6 +933,7 @@ MigrationState *migrate_init(const MigrationParams *params)
s->xfer_limit = 0;
s->cleanup_bh = 0;
s->to_dst_file = NULL;
+ s->in_recovery = false;
s->state = MIGRATION_STATUS_NONE;
s->params = *params;
s->rp_state.from_dst_file = NULL;
@@ -992,13 +993,14 @@ void qmp_migrate_incoming(const char *uri, Error **errp)
once = false;
}
-void qmp_migrate(const char *uri, bool has_blk, bool blk,
- bool has_inc, bool inc, bool has_detach, bool detach,
+void qmp_migrate(const char *uri, bool in_recover, bool recover, bool has_blk,
+ bool blk, bool has_inc, bool inc, bool has_detach, bool detach,
Error **errp)
{
Error *local_err = NULL;
MigrationState *s = migrate_get_current();
MigrationParams params;
+ bool recovery = in_recover && recover;
const char *p;
params.blk = has_blk && blk;
@@ -1023,7 +1025,31 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk,
return;
}
- s = migrate_init(¶ms);
+ if (recovery ^ atomic_mb_read(&s->in_recovery)) {
+ if (recovery) {
+ /* No VM is waiting for recovery and
+ * recovery option was set
+ */
+
+ error_setg(errp, QERR_UNDEFINED_ERROR);
+ return;
+ } else {
+ /* A VM is waiting for recovery and
+ * no recovery option is set
+ */
+
+ error_setg(errp, QERR_UNDEFINED_ERROR);
+ return;
+ }
+ } else {
+ if (!recovery) {
+ /* No VM is waiting for recovery and
+ * no recovery option is set
+ */
+ fprintf(stderr, "hello1\n");
+ s = migrate_init(¶ms);
+ }
+ }
if (strstart(uri, "tcp:", &p)) {
tcp_start_outgoing_migration(s, p, &local_err);
@@ -1767,16 +1793,6 @@ static void *migration_thread(void *opaque)
void migrate_fd_connect(MigrationState *s)
{
- /* This is a best 1st approximation. ns to ms */
- s->expected_downtime = max_downtime/1000000;
- s->cleanup_bh = qemu_bh_new(migrate_fd_cleanup, s);
-
- qemu_file_set_rate_limit(s->to_dst_file,
- s->bandwidth_limit / XFER_LIMIT_RATIO);
-
- /* Notify before starting migration thread */
- notifier_list_notify(&migration_state_notifiers, s);
-
/*
* Open the return path; currently for postcopy but other things might
* also want it.
@@ -1791,6 +1807,24 @@ void migrate_fd_connect(MigrationState *s)
}
}
+ if (atomic_mb_read(&s->in_recovery)) {
+ atomic_mb_set(&s->in_recovery, 0);
+
+ fprintf(stderr, "recovered\n");
+ return;
+ }
+
+ /* This is a best 1st approximation. ns to ms */
+ s->expected_downtime = max_downtime/1000000;
+ s->cleanup_bh = qemu_bh_new(migrate_fd_cleanup, s);
+
+ qemu_file_set_rate_limit(s->to_dst_file,
+ s->bandwidth_limit / XFER_LIMIT_RATIO);
+
+ /* Notify before starting migration thread */
+ notifier_list_notify(&migration_state_notifiers, s);
+
+
migrate_compress_threads_create();
qemu_thread_create(&s->thread, "migration", migration_thread, s,
QEMU_THREAD_JOINABLE);
diff --git a/qapi-schema.json b/qapi-schema.json
index 54634c4..1b7b1e1 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -2070,7 +2070,7 @@
# Since: 0.14.0
##
{ 'command': 'migrate',
- 'data': {'uri': 'str', '*blk': 'bool', '*inc': 'bool', '*detach': 'bool' } }
+ 'data': {'uri': 'str', '*recover': 'bool', '*blk': 'bool', '*inc': 'bool', '*detach': 'bool' } }
##
# @migrate-incoming
diff --git a/qmp-commands.hx b/qmp-commands.hx
index de896a5..c8a1a93 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -612,7 +612,7 @@ EQMP
{
.name = "migrate",
- .args_type = "detach:-d,blk:-b,inc:-i,uri:s",
+ .args_type = "detach:-d,recover:-r,blk:-b,inc:-i,uri:s",
.mhandler.cmd_new = qmp_marshal_migrate,
},
@@ -625,6 +625,7 @@ Migrate to URI.
Arguments:
- "blk": block migration, full disk copy (json-bool, optional)
+- "recover": recover migration (json-bool, optional)
- "inc": incremental disk copy (json-bool, optional)
- "uri": Destination URI (json-string)
--
2.7.4
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [Qemu-devel] [Qemu-devel [RFC] [WIP] v1] Adding feature to reconnect with -r option to migrate command
2016-06-08 18:13 [Qemu-devel] [Qemu-devel [RFC] [WIP] v1] Adding feature to reconnect with -r option to migrate command Md Haris Iqbal
@ 2016-06-15 13:27 ` Dr. David Alan Gilbert
0 siblings, 0 replies; 2+ messages in thread
From: Dr. David Alan Gilbert @ 2016-06-15 13:27 UTC (permalink / raw)
To: Md Haris Iqbal; +Cc: qemu-devel
* Md Haris Iqbal (haris.phnx@gmail.com) wrote:
> ---
> hmp-commands.hx | 10 +++++---
> hmp.c | 4 ++-
> include/migration/migration.h | 1 +
> migration/migration.c | 60 +++++++++++++++++++++++++++++++++----------
> qapi-schema.json | 2 +-
> qmp-commands.hx | 3 ++-
> 6 files changed, 60 insertions(+), 20 deletions(-)
>
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index 4f4f60a..2e7aed3 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -893,10 +893,11 @@ ETEXI
>
> {
> .name = "migrate",
> - .args_type = "detach:-d,blk:-b,inc:-i,uri:s",
> - .params = "[-d] [-b] [-i] uri",
> + .args_type = "detach:-d,recover:-r,blk:-b,inc:-i,uri:s",
> + .params = "[-d] [-r] [-b] [-i] uri",
> .help = "migrate to URI (using -d to not wait for completion)"
> - "\n\t\t\t -b for migration without shared storage with"
> + "\n\t\t\t -r to recover from a broken migration\n\t\t\t"
> + " -b for migration without shared storage with"
Is that a tab/spce indent problem?
> " full copy of disk\n\t\t\t -i for migration without "
> "shared storage with incremental copy of disk "
> "(base image shared between src and destination)",
> @@ -905,9 +906,10 @@ ETEXI
>
>
> STEXI
> -@item migrate [-d] [-b] [-i] @var{uri}
> +@item migrate [-d] [-r] [-b] [-i] @var{uri}
> @findex migrate
> Migrate to @var{uri} (using -d to not wait for completion).
> + -r to recover from a broken migration
Is that a tab/spce indent problem?
> -b for migration with full copy of disk
> -i for migration with incremental copy of disk (base image is shared)
> ETEXI
> diff --git a/hmp.c b/hmp.c
> index d510236..ec8fab4 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -1544,12 +1544,14 @@ static void hmp_migrate_status_cb(void *opaque)
> void hmp_migrate(Monitor *mon, const QDict *qdict)
> {
> bool detach = qdict_get_try_bool(qdict, "detach", false);
> + bool recover = qdict_get_try_bool(qdict, "recover", false);
> bool blk = qdict_get_try_bool(qdict, "blk", false);
> bool inc = qdict_get_try_bool(qdict, "inc", false);
> const char *uri = qdict_get_str(qdict, "uri");
> Error *err = NULL;
>
> - qmp_migrate(uri, !!blk, blk, !!inc, inc, false, false, &err);
> + qmp_migrate(uri, !!recover, recover, !!blk, blk, !!inc, inc, false, false,
> + &err);
> if (err) {
> error_report_err(err);
> return;
> diff --git a/include/migration/migration.h b/include/migration/migration.h
> index ac2c12c..4a3201b 100644
> --- a/include/migration/migration.h
> +++ b/include/migration/migration.h
> @@ -139,6 +139,7 @@ struct MigrationState
>
> int state;
> MigrationParams params;
> + bool in_recovery;
>
> /* State related to return path */
> struct {
> diff --git a/migration/migration.c b/migration/migration.c
> index 991313a..a77f62e 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -933,6 +933,7 @@ MigrationState *migrate_init(const MigrationParams *params)
> s->xfer_limit = 0;
> s->cleanup_bh = 0;
> s->to_dst_file = NULL;
> + s->in_recovery = false;
> s->state = MIGRATION_STATUS_NONE;
> s->params = *params;
> s->rp_state.from_dst_file = NULL;
> @@ -992,13 +993,14 @@ void qmp_migrate_incoming(const char *uri, Error **errp)
> once = false;
> }
>
> -void qmp_migrate(const char *uri, bool has_blk, bool blk,
> - bool has_inc, bool inc, bool has_detach, bool detach,
> +void qmp_migrate(const char *uri, bool in_recover, bool recover, bool has_blk,
> + bool blk, bool has_inc, bool inc, bool has_detach, bool detach,
> Error **errp)
> {
> Error *local_err = NULL;
> MigrationState *s = migrate_get_current();
> MigrationParams params;
> + bool recovery = in_recover && recover;
> const char *p;
You have an odd mix of things called recover and recovery.
>
> params.blk = has_blk && blk;
> @@ -1023,7 +1025,31 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk,
> return;
> }
>
> - s = migrate_init(¶ms);
> + if (recovery ^ atomic_mb_read(&s->in_recovery)) {
> + if (recovery) {
> + /* No VM is waiting for recovery and
> + * recovery option was set
> + */
> +
> + error_setg(errp, QERR_UNDEFINED_ERROR);
> + return;
> + } else {
> + /* A VM is waiting for recovery and
> + * no recovery option is set
> + */
> +
> + error_setg(errp, QERR_UNDEFINED_ERROR);
> + return;
Use error messages that explain the problem; e.g.
error_setg(errp, "Can't start a new migration while a migration is in recovery");
But I wonder; do you actually need an s->in_recovery? Can't you just use the current
migration state - it's postcopy_recovery then is that the same as your in_recovery
flag? Do we change state once we start the recovery, at the moment I think you're
going:
postcopy_active -> postmigrate_recovery
but should we go to :
postcopy_active -> postmigrate_recovery -> postmigrate_recovery_active ?
> + }
> + } else {
> + if (!recovery) {
> + /* No VM is waiting for recovery and
> + * no recovery option is set
> + */
> + fprintf(stderr, "hello1\n");
> + s = migrate_init(¶ms);
> + }
> + }
>
> if (strstart(uri, "tcp:", &p)) {
> tcp_start_outgoing_migration(s, p, &local_err);
> @@ -1767,16 +1793,6 @@ static void *migration_thread(void *opaque)
>
> void migrate_fd_connect(MigrationState *s)
> {
> - /* This is a best 1st approximation. ns to ms */
> - s->expected_downtime = max_downtime/1000000;
> - s->cleanup_bh = qemu_bh_new(migrate_fd_cleanup, s);
> -
> - qemu_file_set_rate_limit(s->to_dst_file,
> - s->bandwidth_limit / XFER_LIMIT_RATIO);
> -
> - /* Notify before starting migration thread */
> - notifier_list_notify(&migration_state_notifiers, s);
> -
> /*
> * Open the return path; currently for postcopy but other things might
> * also want it.
> @@ -1791,6 +1807,24 @@ void migrate_fd_connect(MigrationState *s)
> }
> }
>
> + if (atomic_mb_read(&s->in_recovery)) {
> + atomic_mb_set(&s->in_recovery, 0);
> +
> + fprintf(stderr, "recovered\n");
> + return;
> + }
Mark the places where you're going to add stuff with /* TODO: explanation */
> +
> + /* This is a best 1st approximation. ns to ms */
> + s->expected_downtime = max_downtime/1000000;
> + s->cleanup_bh = qemu_bh_new(migrate_fd_cleanup, s);
> +
> + qemu_file_set_rate_limit(s->to_dst_file,
> + s->bandwidth_limit / XFER_LIMIT_RATIO);
> +
> + /* Notify before starting migration thread */
> + notifier_list_notify(&migration_state_notifiers, s);
> +
> +
> migrate_compress_threads_create();
> qemu_thread_create(&s->thread, "migration", migration_thread, s,
> QEMU_THREAD_JOINABLE);
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 54634c4..1b7b1e1 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -2070,7 +2070,7 @@
> # Since: 0.14.0
> ##
> { 'command': 'migrate',
> - 'data': {'uri': 'str', '*blk': 'bool', '*inc': 'bool', '*detach': 'bool' } }
> + 'data': {'uri': 'str', '*recover': 'bool', '*blk': 'bool', '*inc': 'bool', '*detach': 'bool' } }
>
> ##
> # @migrate-incoming
> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index de896a5..c8a1a93 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -612,7 +612,7 @@ EQMP
>
> {
> .name = "migrate",
> - .args_type = "detach:-d,blk:-b,inc:-i,uri:s",
> + .args_type = "detach:-d,recover:-r,blk:-b,inc:-i,uri:s",
> .mhandler.cmd_new = qmp_marshal_migrate,
> },
>
> @@ -625,6 +625,7 @@ Migrate to URI.
> Arguments:
>
> - "blk": block migration, full disk copy (json-bool, optional)
> +- "recover": recover migration (json-bool, optional)
> - "inc": incremental disk copy (json-bool, optional)
> - "uri": Destination URI (json-string)
>
> --
> 2.7.4
>
Dave
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2016-06-15 13:28 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-06-08 18:13 [Qemu-devel] [Qemu-devel [RFC] [WIP] v1] Adding feature to reconnect with -r option to migrate command Md Haris Iqbal
2016-06-15 13:27 ` Dr. David Alan Gilbert
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).