* [Qemu-devel] [PATCH 01/23] migration: Make *start_outgoing_migration return FdMigrationState
2011-09-23 12:56 [Qemu-devel] [PATCH v3 00/23] Refactor and cleanup migration code Juan Quintela
@ 2011-09-23 12:56 ` Juan Quintela
2011-09-23 12:56 ` [Qemu-devel] [PATCH 02/23] migration: Use FdMigrationState instead of MigrationState when possible Juan Quintela
` (22 subsequent siblings)
23 siblings, 0 replies; 45+ messages in thread
From: Juan Quintela @ 2011-09-23 12:56 UTC (permalink / raw)
To: qemu-devel
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
migration-exec.c | 4 ++--
migration-fd.c | 4 ++--
migration-tcp.c | 4 ++--
migration-unix.c | 4 ++--
migration.c | 4 ++--
migration.h | 8 ++++----
6 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/migration-exec.c b/migration-exec.c
index 2cfb6f2..759aa79 100644
--- a/migration-exec.c
+++ b/migration-exec.c
@@ -61,7 +61,7 @@ static int exec_close(FdMigrationState *s)
return ret;
}
-MigrationState *exec_start_outgoing_migration(Monitor *mon,
+FdMigrationState *exec_start_outgoing_migration(Monitor *mon,
const char *command,
int64_t bandwidth_limit,
int detach,
@@ -108,7 +108,7 @@ MigrationState *exec_start_outgoing_migration(Monitor *mon,
}
migrate_fd_connect(s);
- return &s->mig_state;
+ return s;
err_after_open:
pclose(f);
diff --git a/migration-fd.c b/migration-fd.c
index aee690a..8036a27 100644
--- a/migration-fd.c
+++ b/migration-fd.c
@@ -50,7 +50,7 @@ static int fd_close(FdMigrationState *s)
return 0;
}
-MigrationState *fd_start_outgoing_migration(Monitor *mon,
+FdMigrationState *fd_start_outgoing_migration(Monitor *mon,
const char *fdname,
int64_t bandwidth_limit,
int detach,
@@ -91,7 +91,7 @@ MigrationState *fd_start_outgoing_migration(Monitor *mon,
}
migrate_fd_connect(s);
- return &s->mig_state;
+ return s;
err_after_open:
close(s->fd);
diff --git a/migration-tcp.c b/migration-tcp.c
index c431e03..05a121f 100644
--- a/migration-tcp.c
+++ b/migration-tcp.c
@@ -75,7 +75,7 @@ static void tcp_wait_for_connect(void *opaque)
}
}
-MigrationState *tcp_start_outgoing_migration(Monitor *mon,
+FdMigrationState *tcp_start_outgoing_migration(Monitor *mon,
const char *host_port,
int64_t bandwidth_limit,
int detach,
@@ -131,7 +131,7 @@ MigrationState *tcp_start_outgoing_migration(Monitor *mon,
} else if (ret >= 0)
migrate_fd_connect(s);
- return &s->mig_state;
+ return s;
}
static void tcp_accept_incoming_migration(void *opaque)
diff --git a/migration-unix.c b/migration-unix.c
index 6dc985d..0eeedde 100644
--- a/migration-unix.c
+++ b/migration-unix.c
@@ -74,7 +74,7 @@ static void unix_wait_for_connect(void *opaque)
}
}
-MigrationState *unix_start_outgoing_migration(Monitor *mon,
+FdMigrationState *unix_start_outgoing_migration(Monitor *mon,
const char *path,
int64_t bandwidth_limit,
int detach,
@@ -132,7 +132,7 @@ MigrationState *unix_start_outgoing_migration(Monitor *mon,
if (ret >= 0)
migrate_fd_connect(s);
- return &s->mig_state;
+ return s;
err_after_open:
close(s->fd);
diff --git a/migration.c b/migration.c
index 08688aa..fae540f 100644
--- a/migration.c
+++ b/migration.c
@@ -79,7 +79,7 @@ void process_incoming_migration(QEMUFile *f)
int do_migrate(Monitor *mon, const QDict *qdict, QObject **ret_data)
{
- MigrationState *s = NULL;
+ FdMigrationState *s = NULL;
const char *p;
int detach = qdict_get_try_bool(qdict, "detach", 0);
int blk = qdict_get_try_bool(qdict, "blk", 0);
@@ -124,7 +124,7 @@ int do_migrate(Monitor *mon, const QDict *qdict, QObject **ret_data)
current_migration->release(current_migration);
}
- current_migration = s;
+ current_migration = &s->mig_state;
notifier_list_notify(&migration_state_notifiers, NULL);
return 0;
}
diff --git a/migration.h b/migration.h
index 050c56c..58354c7 100644
--- a/migration.h
+++ b/migration.h
@@ -72,7 +72,7 @@ void do_info_migrate(Monitor *mon, QObject **ret_data);
int exec_start_incoming_migration(const char *host_port);
-MigrationState *exec_start_outgoing_migration(Monitor *mon,
+FdMigrationState *exec_start_outgoing_migration(Monitor *mon,
const char *host_port,
int64_t bandwidth_limit,
int detach,
@@ -81,7 +81,7 @@ MigrationState *exec_start_outgoing_migration(Monitor *mon,
int tcp_start_incoming_migration(const char *host_port);
-MigrationState *tcp_start_outgoing_migration(Monitor *mon,
+FdMigrationState *tcp_start_outgoing_migration(Monitor *mon,
const char *host_port,
int64_t bandwidth_limit,
int detach,
@@ -90,7 +90,7 @@ MigrationState *tcp_start_outgoing_migration(Monitor *mon,
int unix_start_incoming_migration(const char *path);
-MigrationState *unix_start_outgoing_migration(Monitor *mon,
+FdMigrationState *unix_start_outgoing_migration(Monitor *mon,
const char *path,
int64_t bandwidth_limit,
int detach,
@@ -99,7 +99,7 @@ MigrationState *unix_start_outgoing_migration(Monitor *mon,
int fd_start_incoming_migration(const char *path);
-MigrationState *fd_start_outgoing_migration(Monitor *mon,
+FdMigrationState *fd_start_outgoing_migration(Monitor *mon,
const char *fdname,
int64_t bandwidth_limit,
int detach,
--
1.7.6.2
^ permalink raw reply related [flat|nested] 45+ messages in thread* [Qemu-devel] [PATCH 02/23] migration: Use FdMigrationState instead of MigrationState when possible
2011-09-23 12:56 [Qemu-devel] [PATCH v3 00/23] Refactor and cleanup migration code Juan Quintela
2011-09-23 12:56 ` [Qemu-devel] [PATCH 01/23] migration: Make *start_outgoing_migration return FdMigrationState Juan Quintela
@ 2011-09-23 12:56 ` Juan Quintela
2011-09-23 12:56 ` [Qemu-devel] [PATCH 03/23] migration: Fold MigrationState into FdMigrationState Juan Quintela
` (21 subsequent siblings)
23 siblings, 0 replies; 45+ messages in thread
From: Juan Quintela @ 2011-09-23 12:56 UTC (permalink / raw)
To: qemu-devel
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
migration.c | 34 ++++++++++++++++------------------
migration.h | 16 ++++++++--------
2 files changed, 24 insertions(+), 26 deletions(-)
diff --git a/migration.c b/migration.c
index fae540f..5262dd0 100644
--- a/migration.c
+++ b/migration.c
@@ -34,7 +34,7 @@
/* Migration speed throttling */
static int64_t max_throttle = (32 << 20);
-static MigrationState *current_migration;
+static FdMigrationState *current_migration;
static NotifierList migration_state_notifiers =
NOTIFIER_LIST_INITIALIZER(migration_state_notifiers);
@@ -87,7 +87,8 @@ int do_migrate(Monitor *mon, const QDict *qdict, QObject **ret_data)
const char *uri = qdict_get_str(qdict, "uri");
if (current_migration &&
- current_migration->get_status(current_migration) == MIG_STATE_ACTIVE) {
+ current_migration->mig_state.get_status(current_migration) ==
+ MIG_STATE_ACTIVE) {
monitor_printf(mon, "migration already in progress\n");
return -1;
}
@@ -121,20 +122,20 @@ int do_migrate(Monitor *mon, const QDict *qdict, QObject **ret_data)
}
if (current_migration) {
- current_migration->release(current_migration);
+ current_migration->mig_state.release(current_migration);
}
- current_migration = &s->mig_state;
+ current_migration = s;
notifier_list_notify(&migration_state_notifiers, NULL);
return 0;
}
int do_migrate_cancel(Monitor *mon, const QDict *qdict, QObject **ret_data)
{
- MigrationState *s = current_migration;
+ FdMigrationState *s = current_migration;
- if (s && s->get_status(s) == MIG_STATE_ACTIVE) {
- s->cancel(s);
+ if (s && s->mig_state.get_status(s) == MIG_STATE_ACTIVE) {
+ s->mig_state.cancel(s);
}
return 0;
}
@@ -150,7 +151,7 @@ int do_migrate_set_speed(Monitor *mon, const QDict *qdict, QObject **ret_data)
}
max_throttle = d;
- s = migrate_to_fms(current_migration);
+ s = current_migration;
if (s && s->file) {
qemu_file_set_rate_limit(s->file, max_throttle);
}
@@ -228,10 +229,11 @@ static void migrate_put_status(QDict *qdict, const char *name,
void do_info_migrate(Monitor *mon, QObject **ret_data)
{
QDict *qdict;
- MigrationState *s = current_migration;
- if (s) {
- switch (s->get_status(s)) {
+ if (current_migration) {
+ MigrationState *s = ¤t_migration->mig_state;
+
+ switch (s->get_status(current_migration)) {
case MIG_STATE_ACTIVE:
qdict = qdict_new();
qdict_put(qdict, "status", qstring_from_str("active"));
@@ -404,16 +406,13 @@ void migrate_fd_put_ready(void *opaque)
}
}
-int migrate_fd_get_status(MigrationState *mig_state)
+int migrate_fd_get_status(FdMigrationState *s)
{
- FdMigrationState *s = migrate_to_fms(mig_state);
return s->state;
}
-void migrate_fd_cancel(MigrationState *mig_state)
+void migrate_fd_cancel(FdMigrationState *s)
{
- FdMigrationState *s = migrate_to_fms(mig_state);
-
if (s->state != MIG_STATE_ACTIVE)
return;
@@ -426,9 +425,8 @@ void migrate_fd_cancel(MigrationState *mig_state)
migrate_fd_cleanup(s);
}
-void migrate_fd_release(MigrationState *mig_state)
+void migrate_fd_release(FdMigrationState *s)
{
- FdMigrationState *s = migrate_to_fms(mig_state);
DPRINTF("releasing state\n");
diff --git a/migration.h b/migration.h
index 58354c7..b10bb6e 100644
--- a/migration.h
+++ b/migration.h
@@ -25,18 +25,18 @@
typedef struct MigrationState MigrationState;
+typedef struct FdMigrationState FdMigrationState;
+
struct MigrationState
{
/* FIXME: add more accessors to print migration info */
- void (*cancel)(MigrationState *s);
- int (*get_status)(MigrationState *s);
- void (*release)(MigrationState *s);
+ void (*cancel)(FdMigrationState *s);
+ int (*get_status)(FdMigrationState *s);
+ void (*release)(FdMigrationState *s);
int blk;
int shared;
};
-typedef struct FdMigrationState FdMigrationState;
-
struct FdMigrationState
{
MigrationState mig_state;
@@ -120,11 +120,11 @@ void migrate_fd_connect(FdMigrationState *s);
void migrate_fd_put_ready(void *opaque);
-int migrate_fd_get_status(MigrationState *mig_state);
+int migrate_fd_get_status(FdMigrationState *mig_state);
-void migrate_fd_cancel(MigrationState *mig_state);
+void migrate_fd_cancel(FdMigrationState *mig_state);
-void migrate_fd_release(MigrationState *mig_state);
+void migrate_fd_release(FdMigrationState *mig_state);
void migrate_fd_wait_for_unfreeze(void *opaque);
--
1.7.6.2
^ permalink raw reply related [flat|nested] 45+ messages in thread* [Qemu-devel] [PATCH 03/23] migration: Fold MigrationState into FdMigrationState
2011-09-23 12:56 [Qemu-devel] [PATCH v3 00/23] Refactor and cleanup migration code Juan Quintela
2011-09-23 12:56 ` [Qemu-devel] [PATCH 01/23] migration: Make *start_outgoing_migration return FdMigrationState Juan Quintela
2011-09-23 12:56 ` [Qemu-devel] [PATCH 02/23] migration: Use FdMigrationState instead of MigrationState when possible Juan Quintela
@ 2011-09-23 12:56 ` Juan Quintela
2011-09-23 12:56 ` [Qemu-devel] [PATCH 04/23] migration: Rename FdMigrationState MigrationState Juan Quintela
` (20 subsequent siblings)
23 siblings, 0 replies; 45+ messages in thread
From: Juan Quintela @ 2011-09-23 12:56 UTC (permalink / raw)
To: qemu-devel
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
migration-exec.c | 10 +++++-----
migration-fd.c | 10 +++++-----
migration-tcp.c | 10 +++++-----
migration-unix.c | 10 +++++-----
migration.c | 14 ++++++--------
migration.h | 23 +++++------------------
6 files changed, 31 insertions(+), 46 deletions(-)
diff --git a/migration-exec.c b/migration-exec.c
index 759aa79..39fd416 100644
--- a/migration-exec.c
+++ b/migration-exec.c
@@ -92,12 +92,12 @@ FdMigrationState *exec_start_outgoing_migration(Monitor *mon,
s->close = exec_close;
s->get_error = file_errno;
s->write = file_write;
- s->mig_state.cancel = migrate_fd_cancel;
- s->mig_state.get_status = migrate_fd_get_status;
- s->mig_state.release = migrate_fd_release;
+ s->cancel = migrate_fd_cancel;
+ s->get_status = migrate_fd_get_status;
+ s->release = migrate_fd_release;
- s->mig_state.blk = blk;
- s->mig_state.shared = inc;
+ s->blk = blk;
+ s->shared = inc;
s->state = MIG_STATE_ACTIVE;
s->mon = NULL;
diff --git a/migration-fd.c b/migration-fd.c
index 8036a27..c3c7b0e 100644
--- a/migration-fd.c
+++ b/migration-fd.c
@@ -75,12 +75,12 @@ FdMigrationState *fd_start_outgoing_migration(Monitor *mon,
s->get_error = fd_errno;
s->write = fd_write;
s->close = fd_close;
- s->mig_state.cancel = migrate_fd_cancel;
- s->mig_state.get_status = migrate_fd_get_status;
- s->mig_state.release = migrate_fd_release;
+ s->cancel = migrate_fd_cancel;
+ s->get_status = migrate_fd_get_status;
+ s->release = migrate_fd_release;
- s->mig_state.blk = blk;
- s->mig_state.shared = inc;
+ s->blk = blk;
+ s->shared = inc;
s->state = MIG_STATE_ACTIVE;
s->mon = NULL;
diff --git a/migration-tcp.c b/migration-tcp.c
index 05a121f..5ce93d9 100644
--- a/migration-tcp.c
+++ b/migration-tcp.c
@@ -94,12 +94,12 @@ FdMigrationState *tcp_start_outgoing_migration(Monitor *mon,
s->get_error = socket_errno;
s->write = socket_write;
s->close = tcp_close;
- s->mig_state.cancel = migrate_fd_cancel;
- s->mig_state.get_status = migrate_fd_get_status;
- s->mig_state.release = migrate_fd_release;
+ s->cancel = migrate_fd_cancel;
+ s->get_status = migrate_fd_get_status;
+ s->release = migrate_fd_release;
- s->mig_state.blk = blk;
- s->mig_state.shared = inc;
+ s->blk = blk;
+ s->shared = inc;
s->state = MIG_STATE_ACTIVE;
s->mon = NULL;
diff --git a/migration-unix.c b/migration-unix.c
index 0eeedde..00a6ed5 100644
--- a/migration-unix.c
+++ b/migration-unix.c
@@ -93,12 +93,12 @@ FdMigrationState *unix_start_outgoing_migration(Monitor *mon,
s->get_error = unix_errno;
s->write = unix_write;
s->close = unix_close;
- s->mig_state.cancel = migrate_fd_cancel;
- s->mig_state.get_status = migrate_fd_get_status;
- s->mig_state.release = migrate_fd_release;
+ s->cancel = migrate_fd_cancel;
+ s->get_status = migrate_fd_get_status;
+ s->release = migrate_fd_release;
- s->mig_state.blk = blk;
- s->mig_state.shared = inc;
+ s->blk = blk;
+ s->shared = inc;
s->state = MIG_STATE_ACTIVE;
s->mon = NULL;
diff --git a/migration.c b/migration.c
index 5262dd0..c48ed72 100644
--- a/migration.c
+++ b/migration.c
@@ -87,8 +87,7 @@ int do_migrate(Monitor *mon, const QDict *qdict, QObject **ret_data)
const char *uri = qdict_get_str(qdict, "uri");
if (current_migration &&
- current_migration->mig_state.get_status(current_migration) ==
- MIG_STATE_ACTIVE) {
+ current_migration->get_status(current_migration) == MIG_STATE_ACTIVE) {
monitor_printf(mon, "migration already in progress\n");
return -1;
}
@@ -122,7 +121,7 @@ int do_migrate(Monitor *mon, const QDict *qdict, QObject **ret_data)
}
if (current_migration) {
- current_migration->mig_state.release(current_migration);
+ current_migration->release(current_migration);
}
current_migration = s;
@@ -134,8 +133,8 @@ int do_migrate_cancel(Monitor *mon, const QDict *qdict, QObject **ret_data)
{
FdMigrationState *s = current_migration;
- if (s && s->mig_state.get_status(s) == MIG_STATE_ACTIVE) {
- s->mig_state.cancel(s);
+ if (s && s->get_status(s) == MIG_STATE_ACTIVE) {
+ s->cancel(s);
}
return 0;
}
@@ -231,7 +230,7 @@ void do_info_migrate(Monitor *mon, QObject **ret_data)
QDict *qdict;
if (current_migration) {
- MigrationState *s = ¤t_migration->mig_state;
+ FdMigrationState *s = current_migration;
switch (s->get_status(current_migration)) {
case MIG_STATE_ACTIVE:
@@ -355,8 +354,7 @@ void migrate_fd_connect(FdMigrationState *s)
migrate_fd_close);
DPRINTF("beginning savevm\n");
- ret = qemu_savevm_state_begin(s->mon, s->file, s->mig_state.blk,
- s->mig_state.shared);
+ ret = qemu_savevm_state_begin(s->mon, s->file, s->blk, s->shared);
if (ret < 0) {
DPRINTF("failed, %d\n", ret);
migrate_fd_error(s);
diff --git a/migration.h b/migration.h
index b10bb6e..f0caf7b 100644
--- a/migration.h
+++ b/migration.h
@@ -23,23 +23,10 @@
#define MIG_STATE_CANCELLED 1
#define MIG_STATE_ACTIVE 2
-typedef struct MigrationState MigrationState;
-
typedef struct FdMigrationState FdMigrationState;
-struct MigrationState
-{
- /* FIXME: add more accessors to print migration info */
- void (*cancel)(FdMigrationState *s);
- int (*get_status)(FdMigrationState *s);
- void (*release)(FdMigrationState *s);
- int blk;
- int shared;
-};
-
struct FdMigrationState
{
- MigrationState mig_state;
int64_t bandwidth_limit;
QEMUFile *file;
int fd;
@@ -48,7 +35,12 @@ struct FdMigrationState
int (*get_error)(struct FdMigrationState*);
int (*close)(struct FdMigrationState*);
int (*write)(struct FdMigrationState*, const void *, size_t);
+ void (*cancel)(FdMigrationState *s);
+ int (*get_status)(FdMigrationState *s);
+ void (*release)(FdMigrationState *s);
void *opaque;
+ int blk;
+ int shared;
};
void process_incoming_migration(QEMUFile *f);
@@ -130,11 +122,6 @@ void migrate_fd_wait_for_unfreeze(void *opaque);
int migrate_fd_close(void *opaque);
-static inline FdMigrationState *migrate_to_fms(MigrationState *mig_state)
-{
- return container_of(mig_state, FdMigrationState, mig_state);
-}
-
void add_migration_state_change_notifier(Notifier *notify);
void remove_migration_state_change_notifier(Notifier *notify);
int get_migration_state(void);
--
1.7.6.2
^ permalink raw reply related [flat|nested] 45+ messages in thread* [Qemu-devel] [PATCH 04/23] migration: Rename FdMigrationState MigrationState
2011-09-23 12:56 [Qemu-devel] [PATCH v3 00/23] Refactor and cleanup migration code Juan Quintela
` (2 preceding siblings ...)
2011-09-23 12:56 ` [Qemu-devel] [PATCH 03/23] migration: Fold MigrationState into FdMigrationState Juan Quintela
@ 2011-09-23 12:56 ` Juan Quintela
2011-09-23 12:56 ` [Qemu-devel] [PATCH 05/23] migration: Refactor MigrationState creation Juan Quintela
` (19 subsequent siblings)
23 siblings, 0 replies; 45+ messages in thread
From: Juan Quintela @ 2011-09-23 12:56 UTC (permalink / raw)
To: qemu-devel
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
migration-exec.c | 10 +++++-----
migration-fd.c | 10 +++++-----
migration-tcp.c | 12 ++++++------
migration-unix.c | 12 ++++++------
migration.c | 34 +++++++++++++++++-----------------
migration.h | 38 +++++++++++++++++++-------------------
6 files changed, 58 insertions(+), 58 deletions(-)
diff --git a/migration-exec.c b/migration-exec.c
index 39fd416..0ed5976 100644
--- a/migration-exec.c
+++ b/migration-exec.c
@@ -32,17 +32,17 @@
do { } while (0)
#endif
-static int file_errno(FdMigrationState *s)
+static int file_errno(MigrationState *s)
{
return errno;
}
-static int file_write(FdMigrationState *s, const void * buf, size_t size)
+static int file_write(MigrationState *s, const void * buf, size_t size)
{
return write(s->fd, buf, size);
}
-static int exec_close(FdMigrationState *s)
+static int exec_close(MigrationState *s)
{
int ret = 0;
DPRINTF("exec_close\n");
@@ -61,14 +61,14 @@ static int exec_close(FdMigrationState *s)
return ret;
}
-FdMigrationState *exec_start_outgoing_migration(Monitor *mon,
+MigrationState *exec_start_outgoing_migration(Monitor *mon,
const char *command,
int64_t bandwidth_limit,
int detach,
int blk,
int inc)
{
- FdMigrationState *s;
+ MigrationState *s;
FILE *f;
s = g_malloc0(sizeof(*s));
diff --git a/migration-fd.c b/migration-fd.c
index c3c7b0e..e78fd4e 100644
--- a/migration-fd.c
+++ b/migration-fd.c
@@ -30,17 +30,17 @@
do { } while (0)
#endif
-static int fd_errno(FdMigrationState *s)
+static int fd_errno(MigrationState *s)
{
return errno;
}
-static int fd_write(FdMigrationState *s, const void * buf, size_t size)
+static int fd_write(MigrationState *s, const void * buf, size_t size)
{
return write(s->fd, buf, size);
}
-static int fd_close(FdMigrationState *s)
+static int fd_close(MigrationState *s)
{
DPRINTF("fd_close\n");
if (s->fd != -1) {
@@ -50,14 +50,14 @@ static int fd_close(FdMigrationState *s)
return 0;
}
-FdMigrationState *fd_start_outgoing_migration(Monitor *mon,
+MigrationState *fd_start_outgoing_migration(Monitor *mon,
const char *fdname,
int64_t bandwidth_limit,
int detach,
int blk,
int inc)
{
- FdMigrationState *s;
+ MigrationState *s;
s = g_malloc0(sizeof(*s));
diff --git a/migration-tcp.c b/migration-tcp.c
index 5ce93d9..d6feb23 100644
--- a/migration-tcp.c
+++ b/migration-tcp.c
@@ -28,17 +28,17 @@
do { } while (0)
#endif
-static int socket_errno(FdMigrationState *s)
+static int socket_errno(MigrationState *s)
{
return socket_error();
}
-static int socket_write(FdMigrationState *s, const void * buf, size_t size)
+static int socket_write(MigrationState *s, const void * buf, size_t size)
{
return send(s->fd, buf, size, 0);
}
-static int tcp_close(FdMigrationState *s)
+static int tcp_close(MigrationState *s)
{
DPRINTF("tcp_close\n");
if (s->fd != -1) {
@@ -51,7 +51,7 @@ static int tcp_close(FdMigrationState *s)
static void tcp_wait_for_connect(void *opaque)
{
- FdMigrationState *s = opaque;
+ MigrationState *s = opaque;
int val, ret;
socklen_t valsize = sizeof(val);
@@ -75,7 +75,7 @@ static void tcp_wait_for_connect(void *opaque)
}
}
-FdMigrationState *tcp_start_outgoing_migration(Monitor *mon,
+MigrationState *tcp_start_outgoing_migration(Monitor *mon,
const char *host_port,
int64_t bandwidth_limit,
int detach,
@@ -83,7 +83,7 @@ FdMigrationState *tcp_start_outgoing_migration(Monitor *mon,
int inc)
{
struct sockaddr_in addr;
- FdMigrationState *s;
+ MigrationState *s;
int ret;
if (parse_host_port(&addr, host_port) < 0)
diff --git a/migration-unix.c b/migration-unix.c
index 00a6ed5..3b9017b 100644
--- a/migration-unix.c
+++ b/migration-unix.c
@@ -28,17 +28,17 @@
do { } while (0)
#endif
-static int unix_errno(FdMigrationState *s)
+static int unix_errno(MigrationState *s)
{
return errno;
}
-static int unix_write(FdMigrationState *s, const void * buf, size_t size)
+static int unix_write(MigrationState *s, const void * buf, size_t size)
{
return write(s->fd, buf, size);
}
-static int unix_close(FdMigrationState *s)
+static int unix_close(MigrationState *s)
{
DPRINTF("unix_close\n");
if (s->fd != -1) {
@@ -50,7 +50,7 @@ static int unix_close(FdMigrationState *s)
static void unix_wait_for_connect(void *opaque)
{
- FdMigrationState *s = opaque;
+ MigrationState *s = opaque;
int val, ret;
socklen_t valsize = sizeof(val);
@@ -74,14 +74,14 @@ static void unix_wait_for_connect(void *opaque)
}
}
-FdMigrationState *unix_start_outgoing_migration(Monitor *mon,
+MigrationState *unix_start_outgoing_migration(Monitor *mon,
const char *path,
int64_t bandwidth_limit,
int detach,
int blk,
int inc)
{
- FdMigrationState *s;
+ MigrationState *s;
struct sockaddr_un addr;
int ret;
diff --git a/migration.c b/migration.c
index c48ed72..b27a322 100644
--- a/migration.c
+++ b/migration.c
@@ -34,7 +34,7 @@
/* Migration speed throttling */
static int64_t max_throttle = (32 << 20);
-static FdMigrationState *current_migration;
+static MigrationState *current_migration;
static NotifierList migration_state_notifiers =
NOTIFIER_LIST_INITIALIZER(migration_state_notifiers);
@@ -79,7 +79,7 @@ void process_incoming_migration(QEMUFile *f)
int do_migrate(Monitor *mon, const QDict *qdict, QObject **ret_data)
{
- FdMigrationState *s = NULL;
+ MigrationState *s = NULL;
const char *p;
int detach = qdict_get_try_bool(qdict, "detach", 0);
int blk = qdict_get_try_bool(qdict, "blk", 0);
@@ -131,7 +131,7 @@ int do_migrate(Monitor *mon, const QDict *qdict, QObject **ret_data)
int do_migrate_cancel(Monitor *mon, const QDict *qdict, QObject **ret_data)
{
- FdMigrationState *s = current_migration;
+ MigrationState *s = current_migration;
if (s && s->get_status(s) == MIG_STATE_ACTIVE) {
s->cancel(s);
@@ -142,7 +142,7 @@ int do_migrate_cancel(Monitor *mon, const QDict *qdict, QObject **ret_data)
int do_migrate_set_speed(Monitor *mon, const QDict *qdict, QObject **ret_data)
{
int64_t d;
- FdMigrationState *s;
+ MigrationState *s;
d = qdict_get_int(qdict, "value");
if (d < 0) {
@@ -230,7 +230,7 @@ void do_info_migrate(Monitor *mon, QObject **ret_data)
QDict *qdict;
if (current_migration) {
- FdMigrationState *s = current_migration;
+ MigrationState *s = current_migration;
switch (s->get_status(current_migration)) {
case MIG_STATE_ACTIVE:
@@ -263,7 +263,7 @@ void do_info_migrate(Monitor *mon, QObject **ret_data)
/* shared migration helpers */
-void migrate_fd_monitor_suspend(FdMigrationState *s, Monitor *mon)
+void migrate_fd_monitor_suspend(MigrationState *s, Monitor *mon)
{
s->mon = mon;
if (monitor_suspend(mon) == 0) {
@@ -274,7 +274,7 @@ void migrate_fd_monitor_suspend(FdMigrationState *s, Monitor *mon)
}
}
-void migrate_fd_error(FdMigrationState *s)
+void migrate_fd_error(MigrationState *s)
{
DPRINTF("setting error state\n");
s->state = MIG_STATE_ERROR;
@@ -282,7 +282,7 @@ void migrate_fd_error(FdMigrationState *s)
migrate_fd_cleanup(s);
}
-int migrate_fd_cleanup(FdMigrationState *s)
+int migrate_fd_cleanup(MigrationState *s)
{
int ret = 0;
@@ -310,7 +310,7 @@ int migrate_fd_cleanup(FdMigrationState *s)
void migrate_fd_put_notify(void *opaque)
{
- FdMigrationState *s = opaque;
+ MigrationState *s = opaque;
qemu_set_fd_handler2(s->fd, NULL, NULL, NULL, NULL);
qemu_file_put_notify(s->file);
@@ -321,7 +321,7 @@ void migrate_fd_put_notify(void *opaque)
ssize_t migrate_fd_put_buffer(void *opaque, const void *data, size_t size)
{
- FdMigrationState *s = opaque;
+ MigrationState *s = opaque;
ssize_t ret;
if (s->state != MIG_STATE_ACTIVE) {
@@ -342,7 +342,7 @@ ssize_t migrate_fd_put_buffer(void *opaque, const void *data, size_t size)
return ret;
}
-void migrate_fd_connect(FdMigrationState *s)
+void migrate_fd_connect(MigrationState *s)
{
int ret;
@@ -366,7 +366,7 @@ void migrate_fd_connect(FdMigrationState *s)
void migrate_fd_put_ready(void *opaque)
{
- FdMigrationState *s = opaque;
+ MigrationState *s = opaque;
int ret;
if (s->state != MIG_STATE_ACTIVE) {
@@ -404,12 +404,12 @@ void migrate_fd_put_ready(void *opaque)
}
}
-int migrate_fd_get_status(FdMigrationState *s)
+int migrate_fd_get_status(MigrationState *s)
{
return s->state;
}
-void migrate_fd_cancel(FdMigrationState *s)
+void migrate_fd_cancel(MigrationState *s)
{
if (s->state != MIG_STATE_ACTIVE)
return;
@@ -423,7 +423,7 @@ void migrate_fd_cancel(FdMigrationState *s)
migrate_fd_cleanup(s);
}
-void migrate_fd_release(FdMigrationState *s)
+void migrate_fd_release(MigrationState *s)
{
DPRINTF("releasing state\n");
@@ -438,7 +438,7 @@ void migrate_fd_release(FdMigrationState *s)
void migrate_fd_wait_for_unfreeze(void *opaque)
{
- FdMigrationState *s = opaque;
+ MigrationState *s = opaque;
int ret;
DPRINTF("wait for unfreeze\n");
@@ -461,7 +461,7 @@ void migrate_fd_wait_for_unfreeze(void *opaque)
int migrate_fd_close(void *opaque)
{
- FdMigrationState *s = opaque;
+ MigrationState *s = opaque;
if (s->mon) {
monitor_resume(s->mon);
diff --git a/migration.h b/migration.h
index f0caf7b..e24efed 100644
--- a/migration.h
+++ b/migration.h
@@ -23,21 +23,21 @@
#define MIG_STATE_CANCELLED 1
#define MIG_STATE_ACTIVE 2
-typedef struct FdMigrationState FdMigrationState;
+typedef struct MigrationState MigrationState;
-struct FdMigrationState
+struct MigrationState
{
int64_t bandwidth_limit;
QEMUFile *file;
int fd;
Monitor *mon;
int state;
- int (*get_error)(struct FdMigrationState*);
- int (*close)(struct FdMigrationState*);
- int (*write)(struct FdMigrationState*, const void *, size_t);
- void (*cancel)(FdMigrationState *s);
- int (*get_status)(FdMigrationState *s);
- void (*release)(FdMigrationState *s);
+ int (*get_error)(MigrationState *s);
+ int (*close)(MigrationState *s);
+ int (*write)(MigrationState *s, const void *buff, size_t size);
+ void (*cancel)(MigrationState *s);
+ int (*get_status)(MigrationState *s);
+ void (*release)(MigrationState *s);
void *opaque;
int blk;
int shared;
@@ -64,7 +64,7 @@ void do_info_migrate(Monitor *mon, QObject **ret_data);
int exec_start_incoming_migration(const char *host_port);
-FdMigrationState *exec_start_outgoing_migration(Monitor *mon,
+MigrationState *exec_start_outgoing_migration(Monitor *mon,
const char *host_port,
int64_t bandwidth_limit,
int detach,
@@ -73,7 +73,7 @@ FdMigrationState *exec_start_outgoing_migration(Monitor *mon,
int tcp_start_incoming_migration(const char *host_port);
-FdMigrationState *tcp_start_outgoing_migration(Monitor *mon,
+MigrationState *tcp_start_outgoing_migration(Monitor *mon,
const char *host_port,
int64_t bandwidth_limit,
int detach,
@@ -82,7 +82,7 @@ FdMigrationState *tcp_start_outgoing_migration(Monitor *mon,
int unix_start_incoming_migration(const char *path);
-FdMigrationState *unix_start_outgoing_migration(Monitor *mon,
+MigrationState *unix_start_outgoing_migration(Monitor *mon,
const char *path,
int64_t bandwidth_limit,
int detach,
@@ -91,32 +91,32 @@ FdMigrationState *unix_start_outgoing_migration(Monitor *mon,
int fd_start_incoming_migration(const char *path);
-FdMigrationState *fd_start_outgoing_migration(Monitor *mon,
+MigrationState *fd_start_outgoing_migration(Monitor *mon,
const char *fdname,
int64_t bandwidth_limit,
int detach,
int blk,
int inc);
-void migrate_fd_monitor_suspend(FdMigrationState *s, Monitor *mon);
+void migrate_fd_monitor_suspend(MigrationState *s, Monitor *mon);
-void migrate_fd_error(FdMigrationState *s);
+void migrate_fd_error(MigrationState *s);
-int migrate_fd_cleanup(FdMigrationState *s);
+int migrate_fd_cleanup(MigrationState *s);
void migrate_fd_put_notify(void *opaque);
ssize_t migrate_fd_put_buffer(void *opaque, const void *data, size_t size);
-void migrate_fd_connect(FdMigrationState *s);
+void migrate_fd_connect(MigrationState *s);
void migrate_fd_put_ready(void *opaque);
-int migrate_fd_get_status(FdMigrationState *mig_state);
+int migrate_fd_get_status(MigrationState *mig_state);
-void migrate_fd_cancel(FdMigrationState *mig_state);
+void migrate_fd_cancel(MigrationState *mig_state);
-void migrate_fd_release(FdMigrationState *mig_state);
+void migrate_fd_release(MigrationState *mig_state);
void migrate_fd_wait_for_unfreeze(void *opaque);
--
1.7.6.2
^ permalink raw reply related [flat|nested] 45+ messages in thread* [Qemu-devel] [PATCH 05/23] migration: Refactor MigrationState creation
2011-09-23 12:56 [Qemu-devel] [PATCH v3 00/23] Refactor and cleanup migration code Juan Quintela
` (3 preceding siblings ...)
2011-09-23 12:56 ` [Qemu-devel] [PATCH 04/23] migration: Rename FdMigrationState MigrationState Juan Quintela
@ 2011-09-23 12:56 ` Juan Quintela
2011-10-04 14:23 ` Anthony Liguori
2011-09-23 12:56 ` [Qemu-devel] [PATCH 06/23] migration: Make all posible migration functions static Juan Quintela
` (18 subsequent siblings)
23 siblings, 1 reply; 45+ messages in thread
From: Juan Quintela @ 2011-09-23 12:56 UTC (permalink / raw)
To: qemu-devel
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
migration-exec.c | 16 +---------------
migration-fd.c | 16 +---------------
migration-tcp.c | 15 +--------------
migration-unix.c | 15 +--------------
migration.c | 29 +++++++++++++++++++++++++----
migration.h | 11 +++--------
6 files changed, 32 insertions(+), 70 deletions(-)
diff --git a/migration-exec.c b/migration-exec.c
index 0ed5976..db11374 100644
--- a/migration-exec.c
+++ b/migration-exec.c
@@ -71,7 +71,7 @@ MigrationState *exec_start_outgoing_migration(Monitor *mon,
MigrationState *s;
FILE *f;
- s = g_malloc0(sizeof(*s));
+ s = migrate_create_state(mon, bandwidth_limit, detach, blk, inc);
f = popen(command, "w");
if (f == NULL) {
@@ -92,20 +92,6 @@ MigrationState *exec_start_outgoing_migration(Monitor *mon,
s->close = exec_close;
s->get_error = file_errno;
s->write = file_write;
- s->cancel = migrate_fd_cancel;
- s->get_status = migrate_fd_get_status;
- s->release = migrate_fd_release;
-
- s->blk = blk;
- s->shared = inc;
-
- s->state = MIG_STATE_ACTIVE;
- s->mon = NULL;
- s->bandwidth_limit = bandwidth_limit;
-
- if (!detach) {
- migrate_fd_monitor_suspend(s, mon);
- }
migrate_fd_connect(s);
return s;
diff --git a/migration-fd.c b/migration-fd.c
index e78fd4e..2235a2f 100644
--- a/migration-fd.c
+++ b/migration-fd.c
@@ -59,7 +59,7 @@ MigrationState *fd_start_outgoing_migration(Monitor *mon,
{
MigrationState *s;
- s = g_malloc0(sizeof(*s));
+ s = migrate_create_state(mon, bandwidth_limit, detach, blk, inc);
s->fd = monitor_get_fd(mon, fdname);
if (s->fd == -1) {
@@ -75,20 +75,6 @@ MigrationState *fd_start_outgoing_migration(Monitor *mon,
s->get_error = fd_errno;
s->write = fd_write;
s->close = fd_close;
- s->cancel = migrate_fd_cancel;
- s->get_status = migrate_fd_get_status;
- s->release = migrate_fd_release;
-
- s->blk = blk;
- s->shared = inc;
-
- s->state = MIG_STATE_ACTIVE;
- s->mon = NULL;
- s->bandwidth_limit = bandwidth_limit;
-
- if (!detach) {
- migrate_fd_monitor_suspend(s, mon);
- }
migrate_fd_connect(s);
return s;
diff --git a/migration-tcp.c b/migration-tcp.c
index d6feb23..db8cea2 100644
--- a/migration-tcp.c
+++ b/migration-tcp.c
@@ -89,21 +89,12 @@ MigrationState *tcp_start_outgoing_migration(Monitor *mon,
if (parse_host_port(&addr, host_port) < 0)
return NULL;
- s = g_malloc0(sizeof(*s));
+ s = migrate_create_state(mon, bandwidth_limit, detach, blk, inc);
s->get_error = socket_errno;
s->write = socket_write;
s->close = tcp_close;
- s->cancel = migrate_fd_cancel;
- s->get_status = migrate_fd_get_status;
- s->release = migrate_fd_release;
- s->blk = blk;
- s->shared = inc;
-
- s->state = MIG_STATE_ACTIVE;
- s->mon = NULL;
- s->bandwidth_limit = bandwidth_limit;
s->fd = qemu_socket(PF_INET, SOCK_STREAM, 0);
if (s->fd == -1) {
g_free(s);
@@ -112,10 +103,6 @@ MigrationState *tcp_start_outgoing_migration(Monitor *mon,
socket_set_nonblock(s->fd);
- if (!detach) {
- migrate_fd_monitor_suspend(s, mon);
- }
-
do {
ret = connect(s->fd, (struct sockaddr *)&addr, sizeof(addr));
if (ret == -1)
diff --git a/migration-unix.c b/migration-unix.c
index 3b9017b..74c6dde 100644
--- a/migration-unix.c
+++ b/migration-unix.c
@@ -88,21 +88,12 @@ MigrationState *unix_start_outgoing_migration(Monitor *mon,
addr.sun_family = AF_UNIX;
snprintf(addr.sun_path, sizeof(addr.sun_path), "%s", path);
- s = g_malloc0(sizeof(*s));
+ s = migrate_create_state(mon, bandwidth_limit, detach, blk, inc);
s->get_error = unix_errno;
s->write = unix_write;
s->close = unix_close;
- s->cancel = migrate_fd_cancel;
- s->get_status = migrate_fd_get_status;
- s->release = migrate_fd_release;
- s->blk = blk;
- s->shared = inc;
-
- s->state = MIG_STATE_ACTIVE;
- s->mon = NULL;
- s->bandwidth_limit = bandwidth_limit;
s->fd = qemu_socket(PF_UNIX, SOCK_STREAM, 0);
if (s->fd < 0) {
DPRINTF("Unable to open socket");
@@ -125,10 +116,6 @@ MigrationState *unix_start_outgoing_migration(Monitor *mon,
goto err_after_open;
}
- if (!detach) {
- migrate_fd_monitor_suspend(s, mon);
- }
-
if (ret >= 0)
migrate_fd_connect(s);
diff --git a/migration.c b/migration.c
index b27a322..990667e 100644
--- a/migration.c
+++ b/migration.c
@@ -263,7 +263,7 @@ void do_info_migrate(Monitor *mon, QObject **ret_data)
/* shared migration helpers */
-void migrate_fd_monitor_suspend(MigrationState *s, Monitor *mon)
+static void migrate_fd_monitor_suspend(MigrationState *s, Monitor *mon)
{
s->mon = mon;
if (monitor_suspend(mon) == 0) {
@@ -404,12 +404,12 @@ void migrate_fd_put_ready(void *opaque)
}
}
-int migrate_fd_get_status(MigrationState *s)
+static int migrate_fd_get_status(MigrationState *s)
{
return s->state;
}
-void migrate_fd_cancel(MigrationState *s)
+static void migrate_fd_cancel(MigrationState *s)
{
if (s->state != MIG_STATE_ACTIVE)
return;
@@ -423,7 +423,7 @@ void migrate_fd_cancel(MigrationState *s)
migrate_fd_cleanup(s);
}
-void migrate_fd_release(MigrationState *s)
+static void migrate_fd_release(MigrationState *s)
{
DPRINTF("releasing state\n");
@@ -488,3 +488,24 @@ int get_migration_state(void)
return MIG_STATE_ERROR;
}
}
+
+MigrationState *migrate_create_state(Monitor *mon, int64_t bandwidth_limit,
+ int detach, int blk, int inc)
+{
+ MigrationState *s = g_malloc0(sizeof(*s));
+
+ s->cancel = migrate_fd_cancel;
+ s->get_status = migrate_fd_get_status;
+ s->release = migrate_fd_release;
+ s->blk = blk;
+ s->shared = inc;
+ s->mon = NULL;
+ s->bandwidth_limit = bandwidth_limit;
+ s->state = MIG_STATE_ACTIVE;
+
+ if (!detach) {
+ migrate_fd_monitor_suspend(s, mon);
+ }
+
+ return s;
+}
diff --git a/migration.h b/migration.h
index e24efed..32462f7 100644
--- a/migration.h
+++ b/migration.h
@@ -98,8 +98,6 @@ MigrationState *fd_start_outgoing_migration(Monitor *mon,
int blk,
int inc);
-void migrate_fd_monitor_suspend(MigrationState *s, Monitor *mon);
-
void migrate_fd_error(MigrationState *s);
int migrate_fd_cleanup(MigrationState *s);
@@ -112,16 +110,13 @@ void migrate_fd_connect(MigrationState *s);
void migrate_fd_put_ready(void *opaque);
-int migrate_fd_get_status(MigrationState *mig_state);
-
-void migrate_fd_cancel(MigrationState *mig_state);
-
-void migrate_fd_release(MigrationState *mig_state);
-
void migrate_fd_wait_for_unfreeze(void *opaque);
int migrate_fd_close(void *opaque);
+MigrationState *migrate_create_state(Monitor *mon, int64_t bandwidth_limit,
+ int detach, int blk, int inc);
+
void add_migration_state_change_notifier(Notifier *notify);
void remove_migration_state_change_notifier(Notifier *notify);
int get_migration_state(void);
--
1.7.6.2
^ permalink raw reply related [flat|nested] 45+ messages in thread* Re: [Qemu-devel] [PATCH 05/23] migration: Refactor MigrationState creation
2011-09-23 12:56 ` [Qemu-devel] [PATCH 05/23] migration: Refactor MigrationState creation Juan Quintela
@ 2011-10-04 14:23 ` Anthony Liguori
2011-10-04 14:44 ` Juan Quintela
0 siblings, 1 reply; 45+ messages in thread
From: Anthony Liguori @ 2011-10-04 14:23 UTC (permalink / raw)
To: Juan Quintela; +Cc: qemu-devel
On 09/23/2011 07:56 AM, Juan Quintela wrote:
> Signed-off-by: Juan Quintela<quintela@redhat.com>
> ---
> migration-exec.c | 16 +---------------
> migration-fd.c | 16 +---------------
> migration-tcp.c | 15 +--------------
> migration-unix.c | 15 +--------------
> migration.c | 29 +++++++++++++++++++++++++----
> migration.h | 11 +++--------
> 6 files changed, 32 insertions(+), 70 deletions(-)
>
> diff --git a/migration-exec.c b/migration-exec.c
> index 0ed5976..db11374 100644
> --- a/migration-exec.c
> +++ b/migration-exec.c
> @@ -71,7 +71,7 @@ MigrationState *exec_start_outgoing_migration(Monitor *mon,
> MigrationState *s;
> FILE *f;
>
> - s = g_malloc0(sizeof(*s));
> + s = migrate_create_state(mon, bandwidth_limit, detach, blk, inc);
>
> f = popen(command, "w");
> if (f == NULL) {
> @@ -92,20 +92,6 @@ MigrationState *exec_start_outgoing_migration(Monitor *mon,
> s->close = exec_close;
> s->get_error = file_errno;
> s->write = file_write;
> - s->cancel = migrate_fd_cancel;
> - s->get_status = migrate_fd_get_status;
> - s->release = migrate_fd_release;
> -
> - s->blk = blk;
> - s->shared = inc;
> -
> - s->state = MIG_STATE_ACTIVE;
> - s->mon = NULL;
> - s->bandwidth_limit = bandwidth_limit;
> -
> - if (!detach) {
> - migrate_fd_monitor_suspend(s, mon);
> - }
>
> migrate_fd_connect(s);
> return s;
> diff --git a/migration-fd.c b/migration-fd.c
> index e78fd4e..2235a2f 100644
> --- a/migration-fd.c
> +++ b/migration-fd.c
> @@ -59,7 +59,7 @@ MigrationState *fd_start_outgoing_migration(Monitor *mon,
> {
> MigrationState *s;
>
> - s = g_malloc0(sizeof(*s));
> + s = migrate_create_state(mon, bandwidth_limit, detach, blk, inc);
>
> s->fd = monitor_get_fd(mon, fdname);
> if (s->fd == -1) {
> @@ -75,20 +75,6 @@ MigrationState *fd_start_outgoing_migration(Monitor *mon,
> s->get_error = fd_errno;
> s->write = fd_write;
> s->close = fd_close;
> - s->cancel = migrate_fd_cancel;
> - s->get_status = migrate_fd_get_status;
> - s->release = migrate_fd_release;
> -
> - s->blk = blk;
> - s->shared = inc;
> -
> - s->state = MIG_STATE_ACTIVE;
> - s->mon = NULL;
> - s->bandwidth_limit = bandwidth_limit;
> -
> - if (!detach) {
> - migrate_fd_monitor_suspend(s, mon);
> - }
>
> migrate_fd_connect(s);
> return s;
> diff --git a/migration-tcp.c b/migration-tcp.c
> index d6feb23..db8cea2 100644
> --- a/migration-tcp.c
> +++ b/migration-tcp.c
> @@ -89,21 +89,12 @@ MigrationState *tcp_start_outgoing_migration(Monitor *mon,
> if (parse_host_port(&addr, host_port)< 0)
> return NULL;
>
> - s = g_malloc0(sizeof(*s));
> + s = migrate_create_state(mon, bandwidth_limit, detach, blk, inc);
>
> s->get_error = socket_errno;
> s->write = socket_write;
> s->close = tcp_close;
> - s->cancel = migrate_fd_cancel;
> - s->get_status = migrate_fd_get_status;
> - s->release = migrate_fd_release;
>
> - s->blk = blk;
> - s->shared = inc;
> -
> - s->state = MIG_STATE_ACTIVE;
> - s->mon = NULL;
> - s->bandwidth_limit = bandwidth_limit;
> s->fd = qemu_socket(PF_INET, SOCK_STREAM, 0);
> if (s->fd == -1) {
> g_free(s);
> @@ -112,10 +103,6 @@ MigrationState *tcp_start_outgoing_migration(Monitor *mon,
>
> socket_set_nonblock(s->fd);
>
> - if (!detach) {
> - migrate_fd_monitor_suspend(s, mon);
> - }
> -
> do {
> ret = connect(s->fd, (struct sockaddr *)&addr, sizeof(addr));
> if (ret == -1)
> diff --git a/migration-unix.c b/migration-unix.c
> index 3b9017b..74c6dde 100644
> --- a/migration-unix.c
> +++ b/migration-unix.c
> @@ -88,21 +88,12 @@ MigrationState *unix_start_outgoing_migration(Monitor *mon,
> addr.sun_family = AF_UNIX;
> snprintf(addr.sun_path, sizeof(addr.sun_path), "%s", path);
>
> - s = g_malloc0(sizeof(*s));
> + s = migrate_create_state(mon, bandwidth_limit, detach, blk, inc);
>
> s->get_error = unix_errno;
> s->write = unix_write;
> s->close = unix_close;
> - s->cancel = migrate_fd_cancel;
> - s->get_status = migrate_fd_get_status;
> - s->release = migrate_fd_release;
>
> - s->blk = blk;
> - s->shared = inc;
> -
> - s->state = MIG_STATE_ACTIVE;
> - s->mon = NULL;
> - s->bandwidth_limit = bandwidth_limit;
> s->fd = qemu_socket(PF_UNIX, SOCK_STREAM, 0);
> if (s->fd< 0) {
> DPRINTF("Unable to open socket");
> @@ -125,10 +116,6 @@ MigrationState *unix_start_outgoing_migration(Monitor *mon,
> goto err_after_open;
> }
>
> - if (!detach) {
> - migrate_fd_monitor_suspend(s, mon);
> - }
> -
> if (ret>= 0)
> migrate_fd_connect(s);
>
> diff --git a/migration.c b/migration.c
> index b27a322..990667e 100644
> --- a/migration.c
> +++ b/migration.c
> @@ -263,7 +263,7 @@ void do_info_migrate(Monitor *mon, QObject **ret_data)
>
> /* shared migration helpers */
>
> -void migrate_fd_monitor_suspend(MigrationState *s, Monitor *mon)
> +static void migrate_fd_monitor_suspend(MigrationState *s, Monitor *mon)
> {
> s->mon = mon;
> if (monitor_suspend(mon) == 0) {
> @@ -404,12 +404,12 @@ void migrate_fd_put_ready(void *opaque)
> }
> }
>
> -int migrate_fd_get_status(MigrationState *s)
> +static int migrate_fd_get_status(MigrationState *s)
> {
> return s->state;
> }
>
> -void migrate_fd_cancel(MigrationState *s)
> +static void migrate_fd_cancel(MigrationState *s)
> {
> if (s->state != MIG_STATE_ACTIVE)
> return;
> @@ -423,7 +423,7 @@ void migrate_fd_cancel(MigrationState *s)
> migrate_fd_cleanup(s);
> }
>
> -void migrate_fd_release(MigrationState *s)
> +static void migrate_fd_release(MigrationState *s)
> {
>
> DPRINTF("releasing state\n");
> @@ -488,3 +488,24 @@ int get_migration_state(void)
> return MIG_STATE_ERROR;
> }
> }
> +
> +MigrationState *migrate_create_state(Monitor *mon, int64_t bandwidth_limit,
> + int detach, int blk, int inc)
I think this would fit better if it were migrate_new() or migrate_open().
It definitely should be in the form noun_verb and not noun_verb_noun().
Regards,
Anthony Liguori
> +{
> + MigrationState *s = g_malloc0(sizeof(*s));
> +
> + s->cancel = migrate_fd_cancel;
> + s->get_status = migrate_fd_get_status;
> + s->release = migrate_fd_release;
> + s->blk = blk;
> + s->shared = inc;
> + s->mon = NULL;
> + s->bandwidth_limit = bandwidth_limit;
> + s->state = MIG_STATE_ACTIVE;
> +
> + if (!detach) {
> + migrate_fd_monitor_suspend(s, mon);
> + }
> +
> + return s;
> +}
> diff --git a/migration.h b/migration.h
> index e24efed..32462f7 100644
> --- a/migration.h
> +++ b/migration.h
> @@ -98,8 +98,6 @@ MigrationState *fd_start_outgoing_migration(Monitor *mon,
> int blk,
> int inc);
>
> -void migrate_fd_monitor_suspend(MigrationState *s, Monitor *mon);
> -
> void migrate_fd_error(MigrationState *s);
>
> int migrate_fd_cleanup(MigrationState *s);
> @@ -112,16 +110,13 @@ void migrate_fd_connect(MigrationState *s);
>
> void migrate_fd_put_ready(void *opaque);
>
> -int migrate_fd_get_status(MigrationState *mig_state);
> -
> -void migrate_fd_cancel(MigrationState *mig_state);
> -
> -void migrate_fd_release(MigrationState *mig_state);
> -
> void migrate_fd_wait_for_unfreeze(void *opaque);
>
> int migrate_fd_close(void *opaque);
>
> +MigrationState *migrate_create_state(Monitor *mon, int64_t bandwidth_limit,
> + int detach, int blk, int inc);
> +
> void add_migration_state_change_notifier(Notifier *notify);
> void remove_migration_state_change_notifier(Notifier *notify);
> int get_migration_state(void);
^ permalink raw reply [flat|nested] 45+ messages in thread* Re: [Qemu-devel] [PATCH 05/23] migration: Refactor MigrationState creation
2011-10-04 14:23 ` Anthony Liguori
@ 2011-10-04 14:44 ` Juan Quintela
2011-10-04 14:55 ` Anthony Liguori
0 siblings, 1 reply; 45+ messages in thread
From: Juan Quintela @ 2011-10-04 14:44 UTC (permalink / raw)
To: Anthony Liguori; +Cc: qemu-devel
Anthony Liguori <anthony@codemonkey.ws> wrote:
> On 09/23/2011 07:56 AM, Juan Quintela wrote:
>> Signed-off-by: Juan Quintela<quintela@redhat.com>
>
>
> I think this would fit better if it were migrate_new() or migrate_open().
>
> It definitely should be in the form noun_verb and not noun_verb_noun().
>
> Regards,
You are the native speaker, so I change this one O:-)
Later, Juan.
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [Qemu-devel] [PATCH 05/23] migration: Refactor MigrationState creation
2011-10-04 14:44 ` Juan Quintela
@ 2011-10-04 14:55 ` Anthony Liguori
0 siblings, 0 replies; 45+ messages in thread
From: Anthony Liguori @ 2011-10-04 14:55 UTC (permalink / raw)
To: quintela; +Cc: qemu-devel
On 10/04/2011 09:44 AM, Juan Quintela wrote:
> Anthony Liguori<anthony@codemonkey.ws> wrote:
>> On 09/23/2011 07:56 AM, Juan Quintela wrote:
>>> Signed-off-by: Juan Quintela<quintela@redhat.com>
>>
>>
>> I think this would fit better if it were migrate_new() or migrate_open().
>>
>> It definitely should be in the form noun_verb and not noun_verb_noun().
>>
>> Regards,
>
> You are the native speaker, so I change this one O:-)
It's just consistency, not about what sounds good.
bdrv_new(), bdrv_open(), qemu_chr_open(), memory_region_init(), etc.
Regards,
Anthony Liguori
>
> Later, Juan.
>
^ permalink raw reply [flat|nested] 45+ messages in thread
* [Qemu-devel] [PATCH 06/23] migration: Make all posible migration functions static
2011-09-23 12:56 [Qemu-devel] [PATCH v3 00/23] Refactor and cleanup migration code Juan Quintela
` (4 preceding siblings ...)
2011-09-23 12:56 ` [Qemu-devel] [PATCH 05/23] migration: Refactor MigrationState creation Juan Quintela
@ 2011-09-23 12:56 ` Juan Quintela
2011-09-23 12:56 ` [Qemu-devel] [PATCH 07/23] migration: move migrate_create_state to do_migrate Juan Quintela
` (17 subsequent siblings)
23 siblings, 0 replies; 45+ messages in thread
From: Juan Quintela @ 2011-09-23 12:56 UTC (permalink / raw)
To: qemu-devel
I have to move two functions postions to avoid forward declarations
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
migration.c | 72 +++++++++++++++++++++++++++++-----------------------------
migration.h | 12 ---------
2 files changed, 36 insertions(+), 48 deletions(-)
diff --git a/migration.c b/migration.c
index 990667e..a87ed4d 100644
--- a/migration.c
+++ b/migration.c
@@ -274,15 +274,7 @@ static void migrate_fd_monitor_suspend(MigrationState *s, Monitor *mon)
}
}
-void migrate_fd_error(MigrationState *s)
-{
- DPRINTF("setting error state\n");
- s->state = MIG_STATE_ERROR;
- notifier_list_notify(&migration_state_notifiers, NULL);
- migrate_fd_cleanup(s);
-}
-
-int migrate_fd_cleanup(MigrationState *s)
+static int migrate_fd_cleanup(MigrationState *s)
{
int ret = 0;
@@ -308,7 +300,15 @@ int migrate_fd_cleanup(MigrationState *s)
return ret;
}
-void migrate_fd_put_notify(void *opaque)
+void migrate_fd_error(MigrationState *s)
+{
+ DPRINTF("setting error state\n");
+ s->state = MIG_STATE_ERROR;
+ notifier_list_notify(&migration_state_notifiers, NULL);
+ migrate_fd_cleanup(s);
+}
+
+static void migrate_fd_put_notify(void *opaque)
{
MigrationState *s = opaque;
@@ -319,7 +319,8 @@ void migrate_fd_put_notify(void *opaque)
}
}
-ssize_t migrate_fd_put_buffer(void *opaque, const void *data, size_t size)
+static ssize_t migrate_fd_put_buffer(void *opaque, const void *data,
+ size_t size)
{
MigrationState *s = opaque;
ssize_t ret;
@@ -342,29 +343,7 @@ ssize_t migrate_fd_put_buffer(void *opaque, const void *data, size_t size)
return ret;
}
-void migrate_fd_connect(MigrationState *s)
-{
- int ret;
-
- s->file = qemu_fopen_ops_buffered(s,
- s->bandwidth_limit,
- migrate_fd_put_buffer,
- migrate_fd_put_ready,
- migrate_fd_wait_for_unfreeze,
- migrate_fd_close);
-
- DPRINTF("beginning savevm\n");
- ret = qemu_savevm_state_begin(s->mon, s->file, s->blk, s->shared);
- if (ret < 0) {
- DPRINTF("failed, %d\n", ret);
- migrate_fd_error(s);
- return;
- }
-
- migrate_fd_put_ready(s);
-}
-
-void migrate_fd_put_ready(void *opaque)
+static void migrate_fd_put_ready(void *opaque)
{
MigrationState *s = opaque;
int ret;
@@ -436,7 +415,7 @@ static void migrate_fd_release(MigrationState *s)
g_free(s);
}
-void migrate_fd_wait_for_unfreeze(void *opaque)
+static void migrate_fd_wait_for_unfreeze(void *opaque)
{
MigrationState *s = opaque;
int ret;
@@ -459,7 +438,7 @@ void migrate_fd_wait_for_unfreeze(void *opaque)
}
}
-int migrate_fd_close(void *opaque)
+static int migrate_fd_close(void *opaque)
{
MigrationState *s = opaque;
@@ -489,6 +468,27 @@ int get_migration_state(void)
}
}
+void migrate_fd_connect(MigrationState *s)
+{
+ int ret;
+
+ s->file = qemu_fopen_ops_buffered(s,
+ s->bandwidth_limit,
+ migrate_fd_put_buffer,
+ migrate_fd_put_ready,
+ migrate_fd_wait_for_unfreeze,
+ migrate_fd_close);
+
+ DPRINTF("beginning savevm\n");
+ ret = qemu_savevm_state_begin(s->mon, s->file, s->blk, s->shared);
+ if (ret < 0) {
+ DPRINTF("failed, %d\n", ret);
+ migrate_fd_error(s);
+ return;
+ }
+ migrate_fd_put_ready(s);
+}
+
MigrationState *migrate_create_state(Monitor *mon, int64_t bandwidth_limit,
int detach, int blk, int inc)
{
diff --git a/migration.h b/migration.h
index 32462f7..4ce4ddc 100644
--- a/migration.h
+++ b/migration.h
@@ -100,20 +100,8 @@ MigrationState *fd_start_outgoing_migration(Monitor *mon,
void migrate_fd_error(MigrationState *s);
-int migrate_fd_cleanup(MigrationState *s);
-
-void migrate_fd_put_notify(void *opaque);
-
-ssize_t migrate_fd_put_buffer(void *opaque, const void *data, size_t size);
-
void migrate_fd_connect(MigrationState *s);
-void migrate_fd_put_ready(void *opaque);
-
-void migrate_fd_wait_for_unfreeze(void *opaque);
-
-int migrate_fd_close(void *opaque);
-
MigrationState *migrate_create_state(Monitor *mon, int64_t bandwidth_limit,
int detach, int blk, int inc);
--
1.7.6.2
^ permalink raw reply related [flat|nested] 45+ messages in thread* [Qemu-devel] [PATCH 07/23] migration: move migrate_create_state to do_migrate
2011-09-23 12:56 [Qemu-devel] [PATCH v3 00/23] Refactor and cleanup migration code Juan Quintela
` (5 preceding siblings ...)
2011-09-23 12:56 ` [Qemu-devel] [PATCH 06/23] migration: Make all posible migration functions static Juan Quintela
@ 2011-09-23 12:56 ` Juan Quintela
2011-09-23 12:56 ` [Qemu-devel] [PATCH 08/23] migration: Introduce MIG_STATE_NONE Juan Quintela
` (16 subsequent siblings)
23 siblings, 0 replies; 45+ messages in thread
From: Juan Quintela @ 2011-09-23 12:56 UTC (permalink / raw)
To: qemu-devel
Once there, remove all parameters that don't need to be passed to
*start_outgoing_migration() functions
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
migration-exec.c | 19 +++++--------------
migration-fd.c | 22 ++++++----------------
migration-tcp.c | 22 +++++++---------------
migration-unix.c | 19 +++++--------------
migration.c | 34 +++++++++++++++++++++-------------
migration.h | 31 ++++---------------------------
6 files changed, 48 insertions(+), 99 deletions(-)
diff --git a/migration-exec.c b/migration-exec.c
index db11374..b7b1055 100644
--- a/migration-exec.c
+++ b/migration-exec.c
@@ -61,22 +61,14 @@ static int exec_close(MigrationState *s)
return ret;
}
-MigrationState *exec_start_outgoing_migration(Monitor *mon,
- const char *command,
- int64_t bandwidth_limit,
- int detach,
- int blk,
- int inc)
+int exec_start_outgoing_migration(MigrationState *s, const char *command)
{
- MigrationState *s;
FILE *f;
- s = migrate_create_state(mon, bandwidth_limit, detach, blk, inc);
-
f = popen(command, "w");
if (f == NULL) {
DPRINTF("Unable to popen exec target\n");
- goto err_after_alloc;
+ goto err_after_popen;
}
s->fd = fileno(f);
@@ -94,13 +86,12 @@ MigrationState *exec_start_outgoing_migration(Monitor *mon,
s->write = file_write;
migrate_fd_connect(s);
- return s;
+ return 0;
err_after_open:
pclose(f);
-err_after_alloc:
- g_free(s);
- return NULL;
+err_after_popen:
+ return -1;
}
static void exec_accept_incoming_migration(void *opaque)
diff --git a/migration-fd.c b/migration-fd.c
index 2235a2f..d0aec89 100644
--- a/migration-fd.c
+++ b/migration-fd.c
@@ -50,21 +50,12 @@ static int fd_close(MigrationState *s)
return 0;
}
-MigrationState *fd_start_outgoing_migration(Monitor *mon,
- const char *fdname,
- int64_t bandwidth_limit,
- int detach,
- int blk,
- int inc)
+int fd_start_outgoing_migration(MigrationState *s, const char *fdname)
{
- MigrationState *s;
-
- s = migrate_create_state(mon, bandwidth_limit, detach, blk, inc);
-
- s->fd = monitor_get_fd(mon, fdname);
+ s->fd = monitor_get_fd(s->mon, fdname);
if (s->fd == -1) {
DPRINTF("fd_migration: invalid file descriptor identifier\n");
- goto err_after_alloc;
+ goto err_after_get_fd;
}
if (fcntl(s->fd, F_SETFL, O_NONBLOCK) == -1) {
@@ -77,13 +68,12 @@ MigrationState *fd_start_outgoing_migration(Monitor *mon,
s->close = fd_close;
migrate_fd_connect(s);
- return s;
+ return 0;
err_after_open:
close(s->fd);
-err_after_alloc:
- g_free(s);
- return NULL;
+err_after_get_fd:
+ return -1;
}
static void fd_accept_incoming_migration(void *opaque)
diff --git a/migration-tcp.c b/migration-tcp.c
index db8cea2..f6b2288 100644
--- a/migration-tcp.c
+++ b/migration-tcp.c
@@ -75,30 +75,22 @@ static void tcp_wait_for_connect(void *opaque)
}
}
-MigrationState *tcp_start_outgoing_migration(Monitor *mon,
- const char *host_port,
- int64_t bandwidth_limit,
- int detach,
- int blk,
- int inc)
+int tcp_start_outgoing_migration(MigrationState *s, const char *host_port)
{
struct sockaddr_in addr;
- MigrationState *s;
int ret;
- if (parse_host_port(&addr, host_port) < 0)
- return NULL;
-
- s = migrate_create_state(mon, bandwidth_limit, detach, blk, inc);
-
+ ret = parse_host_port(&addr, host_port);
+ if (ret < 0) {
+ return ret;
+ }
s->get_error = socket_errno;
s->write = socket_write;
s->close = tcp_close;
s->fd = qemu_socket(PF_INET, SOCK_STREAM, 0);
if (s->fd == -1) {
- g_free(s);
- return NULL;
+ return -1;
}
socket_set_nonblock(s->fd);
@@ -118,7 +110,7 @@ MigrationState *tcp_start_outgoing_migration(Monitor *mon,
} else if (ret >= 0)
migrate_fd_connect(s);
- return s;
+ return 0;
}
static void tcp_accept_incoming_migration(void *opaque)
diff --git a/migration-unix.c b/migration-unix.c
index 74c6dde..d6f315a 100644
--- a/migration-unix.c
+++ b/migration-unix.c
@@ -74,22 +74,14 @@ static void unix_wait_for_connect(void *opaque)
}
}
-MigrationState *unix_start_outgoing_migration(Monitor *mon,
- const char *path,
- int64_t bandwidth_limit,
- int detach,
- int blk,
- int inc)
+int unix_start_outgoing_migration(MigrationState *s, const char *path)
{
- MigrationState *s;
struct sockaddr_un addr;
int ret;
addr.sun_family = AF_UNIX;
snprintf(addr.sun_path, sizeof(addr.sun_path), "%s", path);
- s = migrate_create_state(mon, bandwidth_limit, detach, blk, inc);
-
s->get_error = unix_errno;
s->write = unix_write;
s->close = unix_close;
@@ -97,7 +89,7 @@ MigrationState *unix_start_outgoing_migration(Monitor *mon,
s->fd = qemu_socket(PF_UNIX, SOCK_STREAM, 0);
if (s->fd < 0) {
DPRINTF("Unable to open socket");
- goto err_after_alloc;
+ goto err_after_socket;
}
socket_set_nonblock(s->fd);
@@ -119,14 +111,13 @@ MigrationState *unix_start_outgoing_migration(Monitor *mon,
if (ret >= 0)
migrate_fd_connect(s);
- return s;
+ return 0;
err_after_open:
close(s->fd);
-err_after_alloc:
- g_free(s);
- return NULL;
+err_after_socket:
+ return -1;
}
static void unix_accept_incoming_migration(void *opaque)
diff --git a/migration.c b/migration.c
index a87ed4d..88f4c23 100644
--- a/migration.c
+++ b/migration.c
@@ -77,6 +77,10 @@ void process_incoming_migration(QEMUFile *f)
}
}
+static MigrationState *migrate_create_state(Monitor *mon,
+ int64_t bandwidth_limit,
+ int detach, int blk, int inc);
+
int do_migrate(Monitor *mon, const QDict *qdict, QObject **ret_data)
{
MigrationState *s = NULL;
@@ -85,6 +89,7 @@ int do_migrate(Monitor *mon, const QDict *qdict, QObject **ret_data)
int blk = qdict_get_try_bool(qdict, "blk", 0);
int inc = qdict_get_try_bool(qdict, "inc", 0);
const char *uri = qdict_get_str(qdict, "uri");
+ int ret;
if (current_migration &&
current_migration->get_status(current_migration) == MIG_STATE_ACTIVE) {
@@ -96,28 +101,27 @@ int do_migrate(Monitor *mon, const QDict *qdict, QObject **ret_data)
return -1;
}
+ s = migrate_create_state(mon, max_throttle, detach, blk, inc);
+
if (strstart(uri, "tcp:", &p)) {
- s = tcp_start_outgoing_migration(mon, p, max_throttle, detach,
- blk, inc);
+ ret = tcp_start_outgoing_migration(s, p);
#if !defined(WIN32)
} else if (strstart(uri, "exec:", &p)) {
- s = exec_start_outgoing_migration(mon, p, max_throttle, detach,
- blk, inc);
+ ret = exec_start_outgoing_migration(s, p);
} else if (strstart(uri, "unix:", &p)) {
- s = unix_start_outgoing_migration(mon, p, max_throttle, detach,
- blk, inc);
+ ret = unix_start_outgoing_migration(s, p);
} else if (strstart(uri, "fd:", &p)) {
- s = fd_start_outgoing_migration(mon, p, max_throttle, detach,
- blk, inc);
+ ret = fd_start_outgoing_migration(s, p);
#endif
} else {
monitor_printf(mon, "unknown migration protocol: %s\n", uri);
- return -1;
+ ret = -EINVAL;
+ goto free_migrate_state;
}
- if (s == NULL) {
+ if (ret < 0) {
monitor_printf(mon, "migration failed\n");
- return -1;
+ goto free_migrate_state;
}
if (current_migration) {
@@ -127,6 +131,9 @@ int do_migrate(Monitor *mon, const QDict *qdict, QObject **ret_data)
current_migration = s;
notifier_list_notify(&migration_state_notifiers, NULL);
return 0;
+free_migrate_state:
+ g_free(s);
+ return -1;
}
int do_migrate_cancel(Monitor *mon, const QDict *qdict, QObject **ret_data)
@@ -489,8 +496,9 @@ void migrate_fd_connect(MigrationState *s)
migrate_fd_put_ready(s);
}
-MigrationState *migrate_create_state(Monitor *mon, int64_t bandwidth_limit,
- int detach, int blk, int inc)
+static MigrationState *migrate_create_state(Monitor *mon,
+ int64_t bandwidth_limit,
+ int detach, int blk, int inc)
{
MigrationState *s = g_malloc0(sizeof(*s));
diff --git a/migration.h b/migration.h
index 4ce4ddc..14c3ebc 100644
--- a/migration.h
+++ b/migration.h
@@ -64,47 +64,24 @@ void do_info_migrate(Monitor *mon, QObject **ret_data);
int exec_start_incoming_migration(const char *host_port);
-MigrationState *exec_start_outgoing_migration(Monitor *mon,
- const char *host_port,
- int64_t bandwidth_limit,
- int detach,
- int blk,
- int inc);
+int exec_start_outgoing_migration(MigrationState *s, const char *host_port);
int tcp_start_incoming_migration(const char *host_port);
-MigrationState *tcp_start_outgoing_migration(Monitor *mon,
- const char *host_port,
- int64_t bandwidth_limit,
- int detach,
- int blk,
- int inc);
+int tcp_start_outgoing_migration(MigrationState *s, const char *host_port);
int unix_start_incoming_migration(const char *path);
-MigrationState *unix_start_outgoing_migration(Monitor *mon,
- const char *path,
- int64_t bandwidth_limit,
- int detach,
- int blk,
- int inc);
+int unix_start_outgoing_migration(MigrationState *s, const char *path);
int fd_start_incoming_migration(const char *path);
-MigrationState *fd_start_outgoing_migration(Monitor *mon,
- const char *fdname,
- int64_t bandwidth_limit,
- int detach,
- int blk,
- int inc);
+int fd_start_outgoing_migration(MigrationState *s, const char *fdname);
void migrate_fd_error(MigrationState *s);
void migrate_fd_connect(MigrationState *s);
-MigrationState *migrate_create_state(Monitor *mon, int64_t bandwidth_limit,
- int detach, int blk, int inc);
-
void add_migration_state_change_notifier(Notifier *notify);
void remove_migration_state_change_notifier(Notifier *notify);
int get_migration_state(void);
--
1.7.6.2
^ permalink raw reply related [flat|nested] 45+ messages in thread* [Qemu-devel] [PATCH 08/23] migration: Introduce MIG_STATE_NONE
2011-09-23 12:56 [Qemu-devel] [PATCH v3 00/23] Refactor and cleanup migration code Juan Quintela
` (6 preceding siblings ...)
2011-09-23 12:56 ` [Qemu-devel] [PATCH 07/23] migration: move migrate_create_state to do_migrate Juan Quintela
@ 2011-09-23 12:56 ` Juan Quintela
2011-09-23 12:56 ` [Qemu-devel] [PATCH 09/23] migration: Refactor and simplify error checking in migrate_fd_put_ready Juan Quintela
` (15 subsequent siblings)
23 siblings, 0 replies; 45+ messages in thread
From: Juan Quintela @ 2011-09-23 12:56 UTC (permalink / raw)
To: qemu-devel
Use MIG_STATE_ACTIVE only when migration has really started
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
migration.c | 6 +++++-
migration.h | 11 +++++++----
2 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/migration.c b/migration.c
index 88f4c23..abb99ea 100644
--- a/migration.c
+++ b/migration.c
@@ -240,6 +240,9 @@ void do_info_migrate(Monitor *mon, QObject **ret_data)
MigrationState *s = current_migration;
switch (s->get_status(current_migration)) {
+ case MIG_STATE_NONE:
+ /* no migration has happened ever */
+ break;
case MIG_STATE_ACTIVE:
qdict = qdict_new();
qdict_put(qdict, "status", qstring_from_str("active"));
@@ -479,6 +482,7 @@ void migrate_fd_connect(MigrationState *s)
{
int ret;
+ s->state = MIG_STATE_ACTIVE;
s->file = qemu_fopen_ops_buffered(s,
s->bandwidth_limit,
migrate_fd_put_buffer,
@@ -509,7 +513,7 @@ static MigrationState *migrate_create_state(Monitor *mon,
s->shared = inc;
s->mon = NULL;
s->bandwidth_limit = bandwidth_limit;
- s->state = MIG_STATE_ACTIVE;
+ s->state = MIG_STATE_NONE;
if (!detach) {
migrate_fd_monitor_suspend(s, mon);
diff --git a/migration.h b/migration.h
index 14c3ebc..8792655 100644
--- a/migration.h
+++ b/migration.h
@@ -18,10 +18,13 @@
#include "qemu-common.h"
#include "notify.h"
-#define MIG_STATE_ERROR -1
-#define MIG_STATE_COMPLETED 0
-#define MIG_STATE_CANCELLED 1
-#define MIG_STATE_ACTIVE 2
+enum migration_state {
+ MIG_STATE_ERROR,
+ MIG_STATE_NONE,
+ MIG_STATE_CANCELLED,
+ MIG_STATE_ACTIVE,
+ MIG_STATE_COMPLETED,
+};
typedef struct MigrationState MigrationState;
--
1.7.6.2
^ permalink raw reply related [flat|nested] 45+ messages in thread* [Qemu-devel] [PATCH 09/23] migration: Refactor and simplify error checking in migrate_fd_put_ready
2011-09-23 12:56 [Qemu-devel] [PATCH v3 00/23] Refactor and cleanup migration code Juan Quintela
` (7 preceding siblings ...)
2011-09-23 12:56 ` [Qemu-devel] [PATCH 08/23] migration: Introduce MIG_STATE_NONE Juan Quintela
@ 2011-09-23 12:56 ` Juan Quintela
2011-09-23 12:56 ` [Qemu-devel] [PATCH 10/23] migration: Introduce migrate_fd_completed() for consistency Juan Quintela
` (14 subsequent siblings)
23 siblings, 0 replies; 45+ messages in thread
From: Juan Quintela @ 2011-09-23 12:56 UTC (permalink / raw)
To: qemu-devel
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
migration.c | 21 ++++++++++-----------
1 files changed, 10 insertions(+), 11 deletions(-)
diff --git a/migration.c b/migration.c
index abb99ea..740483d 100644
--- a/migration.c
+++ b/migration.c
@@ -373,23 +373,22 @@ static void migrate_fd_put_ready(void *opaque)
DPRINTF("done iterating\n");
vm_stop(RSTATE_PRE_MIGRATE);
- if ((qemu_savevm_state_complete(s->mon, s->file)) < 0) {
- if (old_vm_running) {
- vm_start();
+ if (qemu_savevm_state_complete(s->mon, s->file) < 0) {
+ migrate_fd_error(s);
+ } else {
+ if (migrate_fd_cleanup(s) < 0) {
+ migrate_fd_error(s);
+ } else {
+ s->state = MIG_STATE_COMPLETED;
+ runstate_set(RSTATE_POST_MIGRATE);
+ notifier_list_notify(&migration_state_notifiers, NULL);
}
- s->state = MIG_STATE_ERROR;
}
- if (migrate_fd_cleanup(s) < 0) {
+ if (s->get_status(s) != MIG_STATE_COMPLETED) {
if (old_vm_running) {
vm_start();
}
- s->state = MIG_STATE_ERROR;
}
- if (s->state == MIG_STATE_ACTIVE) {
- s->state = MIG_STATE_COMPLETED;
- runstate_set(RSTATE_POST_MIGRATE);
- }
- notifier_list_notify(&migration_state_notifiers, NULL);
}
}
--
1.7.6.2
^ permalink raw reply related [flat|nested] 45+ messages in thread* [Qemu-devel] [PATCH 10/23] migration: Introduce migrate_fd_completed() for consistency
2011-09-23 12:56 [Qemu-devel] [PATCH v3 00/23] Refactor and cleanup migration code Juan Quintela
` (8 preceding siblings ...)
2011-09-23 12:56 ` [Qemu-devel] [PATCH 09/23] migration: Refactor and simplify error checking in migrate_fd_put_ready Juan Quintela
@ 2011-09-23 12:56 ` Juan Quintela
2011-09-23 12:57 ` [Qemu-devel] [PATCH 11/23] migration: Our release callback was just free Juan Quintela
` (13 subsequent siblings)
23 siblings, 0 replies; 45+ messages in thread
From: Juan Quintela @ 2011-09-23 12:56 UTC (permalink / raw)
To: qemu-devel
This function is a bit different of the others that change the state,
in the sense that if migrate_fd_cleanup() returns an error, it set the
status to error, not completed.
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
migration.c | 20 +++++++++++++-------
1 files changed, 13 insertions(+), 7 deletions(-)
diff --git a/migration.c b/migration.c
index 740483d..40fda7d 100644
--- a/migration.c
+++ b/migration.c
@@ -318,6 +318,18 @@ void migrate_fd_error(MigrationState *s)
migrate_fd_cleanup(s);
}
+static void migrate_fd_completed(MigrationState *s)
+{
+ DPRINTF("setting completed state\n");
+ if (migrate_fd_cleanup(s) < 0) {
+ s->state = MIG_STATE_ERROR;
+ } else {
+ s->state = MIG_STATE_COMPLETED;
+ runstate_set(RSTATE_POST_MIGRATE);
+ }
+ notifier_list_notify(&migration_state_notifiers, NULL);
+}
+
static void migrate_fd_put_notify(void *opaque)
{
MigrationState *s = opaque;
@@ -376,13 +388,7 @@ static void migrate_fd_put_ready(void *opaque)
if (qemu_savevm_state_complete(s->mon, s->file) < 0) {
migrate_fd_error(s);
} else {
- if (migrate_fd_cleanup(s) < 0) {
- migrate_fd_error(s);
- } else {
- s->state = MIG_STATE_COMPLETED;
- runstate_set(RSTATE_POST_MIGRATE);
- notifier_list_notify(&migration_state_notifiers, NULL);
- }
+ migrate_fd_completed(s);
}
if (s->get_status(s) != MIG_STATE_COMPLETED) {
if (old_vm_running) {
--
1.7.6.2
^ permalink raw reply related [flat|nested] 45+ messages in thread* [Qemu-devel] [PATCH 11/23] migration: Our release callback was just free
2011-09-23 12:56 [Qemu-devel] [PATCH v3 00/23] Refactor and cleanup migration code Juan Quintela
` (9 preceding siblings ...)
2011-09-23 12:56 ` [Qemu-devel] [PATCH 10/23] migration: Introduce migrate_fd_completed() for consistency Juan Quintela
@ 2011-09-23 12:57 ` Juan Quintela
2011-09-23 12:57 ` [Qemu-devel] [PATCH 12/23] migration: Remove get_status() accessor Juan Quintela
` (12 subsequent siblings)
23 siblings, 0 replies; 45+ messages in thread
From: Juan Quintela @ 2011-09-23 12:57 UTC (permalink / raw)
To: qemu-devel
We called it from a single place, and always with state !=
MIG_STATE_ACTIVE. Just remove the whole callback. For users of the
notifier, notice that this is exactly the case where they don't care,
we are just freeing the state from previous failed migration (it can't
be a sucessful one, otherwise we would not be running on that machine
in the first place).
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
migration.c | 19 +------------------
migration.h | 1 -
2 files changed, 1 insertions(+), 19 deletions(-)
diff --git a/migration.c b/migration.c
index 40fda7d..48c05b9 100644
--- a/migration.c
+++ b/migration.c
@@ -124,10 +124,7 @@ int do_migrate(Monitor *mon, const QDict *qdict, QObject **ret_data)
goto free_migrate_state;
}
- if (current_migration) {
- current_migration->release(current_migration);
- }
-
+ g_free(current_migration);
current_migration = s;
notifier_list_notify(&migration_state_notifiers, NULL);
return 0;
@@ -417,19 +414,6 @@ static void migrate_fd_cancel(MigrationState *s)
migrate_fd_cleanup(s);
}
-static void migrate_fd_release(MigrationState *s)
-{
-
- DPRINTF("releasing state\n");
-
- if (s->state == MIG_STATE_ACTIVE) {
- s->state = MIG_STATE_CANCELLED;
- notifier_list_notify(&migration_state_notifiers, NULL);
- migrate_fd_cleanup(s);
- }
- g_free(s);
-}
-
static void migrate_fd_wait_for_unfreeze(void *opaque)
{
MigrationState *s = opaque;
@@ -513,7 +497,6 @@ static MigrationState *migrate_create_state(Monitor *mon,
s->cancel = migrate_fd_cancel;
s->get_status = migrate_fd_get_status;
- s->release = migrate_fd_release;
s->blk = blk;
s->shared = inc;
s->mon = NULL;
diff --git a/migration.h b/migration.h
index 8792655..9ea1ad6 100644
--- a/migration.h
+++ b/migration.h
@@ -40,7 +40,6 @@ struct MigrationState
int (*write)(MigrationState *s, const void *buff, size_t size);
void (*cancel)(MigrationState *s);
int (*get_status)(MigrationState *s);
- void (*release)(MigrationState *s);
void *opaque;
int blk;
int shared;
--
1.7.6.2
^ permalink raw reply related [flat|nested] 45+ messages in thread* [Qemu-devel] [PATCH 12/23] migration: Remove get_status() accessor
2011-09-23 12:56 [Qemu-devel] [PATCH v3 00/23] Refactor and cleanup migration code Juan Quintela
` (10 preceding siblings ...)
2011-09-23 12:57 ` [Qemu-devel] [PATCH 11/23] migration: Our release callback was just free Juan Quintela
@ 2011-09-23 12:57 ` Juan Quintela
2011-09-23 12:57 ` [Qemu-devel] [PATCH 13/23] migration: Remove migration cancel() callback Juan Quintela
` (11 subsequent siblings)
23 siblings, 0 replies; 45+ messages in thread
From: Juan Quintela @ 2011-09-23 12:57 UTC (permalink / raw)
To: qemu-devel
It is only used inside migration.c, and fields on that struct are
accessed all around the place on that file.
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
migration.c | 16 +++++-----------
migration.h | 1 -
2 files changed, 5 insertions(+), 12 deletions(-)
diff --git a/migration.c b/migration.c
index 48c05b9..7cb0404 100644
--- a/migration.c
+++ b/migration.c
@@ -92,7 +92,7 @@ int do_migrate(Monitor *mon, const QDict *qdict, QObject **ret_data)
int ret;
if (current_migration &&
- current_migration->get_status(current_migration) == MIG_STATE_ACTIVE) {
+ current_migration->state == MIG_STATE_ACTIVE) {
monitor_printf(mon, "migration already in progress\n");
return -1;
}
@@ -137,7 +137,7 @@ int do_migrate_cancel(Monitor *mon, const QDict *qdict, QObject **ret_data)
{
MigrationState *s = current_migration;
- if (s && s->get_status(s) == MIG_STATE_ACTIVE) {
+ if (s && s->state == MIG_STATE_ACTIVE) {
s->cancel(s);
}
return 0;
@@ -236,7 +236,7 @@ void do_info_migrate(Monitor *mon, QObject **ret_data)
if (current_migration) {
MigrationState *s = current_migration;
- switch (s->get_status(current_migration)) {
+ switch (s->state) {
case MIG_STATE_NONE:
/* no migration has happened ever */
break;
@@ -387,7 +387,7 @@ static void migrate_fd_put_ready(void *opaque)
} else {
migrate_fd_completed(s);
}
- if (s->get_status(s) != MIG_STATE_COMPLETED) {
+ if (s->state != MIG_STATE_COMPLETED) {
if (old_vm_running) {
vm_start();
}
@@ -395,11 +395,6 @@ static void migrate_fd_put_ready(void *opaque)
}
}
-static int migrate_fd_get_status(MigrationState *s)
-{
- return s->state;
-}
-
static void migrate_fd_cancel(MigrationState *s)
{
if (s->state != MIG_STATE_ACTIVE)
@@ -461,7 +456,7 @@ void remove_migration_state_change_notifier(Notifier *notify)
int get_migration_state(void)
{
if (current_migration) {
- return migrate_fd_get_status(current_migration);
+ return current_migration->state;
} else {
return MIG_STATE_ERROR;
}
@@ -496,7 +491,6 @@ static MigrationState *migrate_create_state(Monitor *mon,
MigrationState *s = g_malloc0(sizeof(*s));
s->cancel = migrate_fd_cancel;
- s->get_status = migrate_fd_get_status;
s->blk = blk;
s->shared = inc;
s->mon = NULL;
diff --git a/migration.h b/migration.h
index 9ea1ad6..6d8d1ad 100644
--- a/migration.h
+++ b/migration.h
@@ -39,7 +39,6 @@ struct MigrationState
int (*close)(MigrationState *s);
int (*write)(MigrationState *s, const void *buff, size_t size);
void (*cancel)(MigrationState *s);
- int (*get_status)(MigrationState *s);
void *opaque;
int blk;
int shared;
--
1.7.6.2
^ permalink raw reply related [flat|nested] 45+ messages in thread* [Qemu-devel] [PATCH 13/23] migration: Remove migration cancel() callback
2011-09-23 12:56 [Qemu-devel] [PATCH v3 00/23] Refactor and cleanup migration code Juan Quintela
` (11 preceding siblings ...)
2011-09-23 12:57 ` [Qemu-devel] [PATCH 12/23] migration: Remove get_status() accessor Juan Quintela
@ 2011-09-23 12:57 ` Juan Quintela
2011-09-23 12:57 ` [Qemu-devel] [PATCH 14/23] migration: Move exported functions to the end of the file Juan Quintela
` (10 subsequent siblings)
23 siblings, 0 replies; 45+ messages in thread
From: Juan Quintela @ 2011-09-23 12:57 UTC (permalink / raw)
To: qemu-devel
It is used only in one place
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
migration.c | 9 ++++-----
migration.h | 1 -
2 files changed, 4 insertions(+), 6 deletions(-)
diff --git a/migration.c b/migration.c
index 7cb0404..9bc7ffa 100644
--- a/migration.c
+++ b/migration.c
@@ -133,12 +133,12 @@ free_migrate_state:
return -1;
}
+static void migrate_fd_cancel(MigrationState *s);
+
int do_migrate_cancel(Monitor *mon, const QDict *qdict, QObject **ret_data)
{
- MigrationState *s = current_migration;
-
- if (s && s->state == MIG_STATE_ACTIVE) {
- s->cancel(s);
+ if (current_migration) {
+ migrate_fd_cancel(current_migration);
}
return 0;
}
@@ -490,7 +490,6 @@ static MigrationState *migrate_create_state(Monitor *mon,
{
MigrationState *s = g_malloc0(sizeof(*s));
- s->cancel = migrate_fd_cancel;
s->blk = blk;
s->shared = inc;
s->mon = NULL;
diff --git a/migration.h b/migration.h
index 6d8d1ad..f1a7452 100644
--- a/migration.h
+++ b/migration.h
@@ -38,7 +38,6 @@ struct MigrationState
int (*get_error)(MigrationState *s);
int (*close)(MigrationState *s);
int (*write)(MigrationState *s, const void *buff, size_t size);
- void (*cancel)(MigrationState *s);
void *opaque;
int blk;
int shared;
--
1.7.6.2
^ permalink raw reply related [flat|nested] 45+ messages in thread* [Qemu-devel] [PATCH 14/23] migration: Move exported functions to the end of the file
2011-09-23 12:56 [Qemu-devel] [PATCH v3 00/23] Refactor and cleanup migration code Juan Quintela
` (12 preceding siblings ...)
2011-09-23 12:57 ` [Qemu-devel] [PATCH 13/23] migration: Remove migration cancel() callback Juan Quintela
@ 2011-09-23 12:57 ` Juan Quintela
2011-10-04 14:24 ` Anthony Liguori
2011-09-23 12:57 ` [Qemu-devel] [PATCH 15/23] migration: use global variable directly Juan Quintela
` (9 subsequent siblings)
23 siblings, 1 reply; 45+ messages in thread
From: Juan Quintela @ 2011-09-23 12:57 UTC (permalink / raw)
To: qemu-devel
This means we can remove the two forward declarations.
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
migration.c | 188 ++++++++++++++++++++++++++++------------------------------
1 files changed, 91 insertions(+), 97 deletions(-)
diff --git a/migration.c b/migration.c
index 9bc7ffa..9bb089a 100644
--- a/migration.c
+++ b/migration.c
@@ -77,91 +77,6 @@ void process_incoming_migration(QEMUFile *f)
}
}
-static MigrationState *migrate_create_state(Monitor *mon,
- int64_t bandwidth_limit,
- int detach, int blk, int inc);
-
-int do_migrate(Monitor *mon, const QDict *qdict, QObject **ret_data)
-{
- MigrationState *s = NULL;
- const char *p;
- int detach = qdict_get_try_bool(qdict, "detach", 0);
- int blk = qdict_get_try_bool(qdict, "blk", 0);
- int inc = qdict_get_try_bool(qdict, "inc", 0);
- const char *uri = qdict_get_str(qdict, "uri");
- int ret;
-
- if (current_migration &&
- current_migration->state == MIG_STATE_ACTIVE) {
- monitor_printf(mon, "migration already in progress\n");
- return -1;
- }
-
- if (qemu_savevm_state_blocked(mon)) {
- return -1;
- }
-
- s = migrate_create_state(mon, max_throttle, detach, blk, inc);
-
- if (strstart(uri, "tcp:", &p)) {
- ret = tcp_start_outgoing_migration(s, p);
-#if !defined(WIN32)
- } else if (strstart(uri, "exec:", &p)) {
- ret = exec_start_outgoing_migration(s, p);
- } else if (strstart(uri, "unix:", &p)) {
- ret = unix_start_outgoing_migration(s, p);
- } else if (strstart(uri, "fd:", &p)) {
- ret = fd_start_outgoing_migration(s, p);
-#endif
- } else {
- monitor_printf(mon, "unknown migration protocol: %s\n", uri);
- ret = -EINVAL;
- goto free_migrate_state;
- }
-
- if (ret < 0) {
- monitor_printf(mon, "migration failed\n");
- goto free_migrate_state;
- }
-
- g_free(current_migration);
- current_migration = s;
- notifier_list_notify(&migration_state_notifiers, NULL);
- return 0;
-free_migrate_state:
- g_free(s);
- return -1;
-}
-
-static void migrate_fd_cancel(MigrationState *s);
-
-int do_migrate_cancel(Monitor *mon, const QDict *qdict, QObject **ret_data)
-{
- if (current_migration) {
- migrate_fd_cancel(current_migration);
- }
- return 0;
-}
-
-int do_migrate_set_speed(Monitor *mon, const QDict *qdict, QObject **ret_data)
-{
- int64_t d;
- MigrationState *s;
-
- d = qdict_get_int(qdict, "value");
- if (d < 0) {
- d = 0;
- }
- max_throttle = d;
-
- s = current_migration;
- if (s && s->file) {
- qemu_file_set_rate_limit(s->file, max_throttle);
- }
-
- return 0;
-}
-
/* amount of nanoseconds we are willing to wait for migration to be down.
* the choice of nanoseconds is because it is the maximum resolution that
* get_clock() can achieve. It is an internal measure. All user-visible
@@ -173,18 +88,6 @@ uint64_t migrate_max_downtime(void)
return max_downtime;
}
-int do_migrate_set_downtime(Monitor *mon, const QDict *qdict,
- QObject **ret_data)
-{
- double d;
-
- d = qdict_get_double(qdict, "value") * 1e9;
- d = MAX(0, MIN(UINT64_MAX, d));
- max_downtime = (uint64_t)d;
-
- return 0;
-}
-
static void migrate_print_status(Monitor *mon, const char *name,
const QDict *status_dict)
{
@@ -502,3 +405,94 @@ static MigrationState *migrate_create_state(Monitor *mon,
return s;
}
+
+int do_migrate(Monitor *mon, const QDict *qdict, QObject **ret_data)
+{
+ MigrationState *s = NULL;
+ const char *p;
+ int detach = qdict_get_try_bool(qdict, "detach", 0);
+ int blk = qdict_get_try_bool(qdict, "blk", 0);
+ int inc = qdict_get_try_bool(qdict, "inc", 0);
+ const char *uri = qdict_get_str(qdict, "uri");
+ int ret;
+
+ if (current_migration &&
+ current_migration->state == MIG_STATE_ACTIVE) {
+ monitor_printf(mon, "migration already in progress\n");
+ return -1;
+ }
+
+ if (qemu_savevm_state_blocked(mon)) {
+ return -1;
+ }
+
+ s = migrate_create_state(mon, max_throttle, detach, blk, inc);
+
+ if (strstart(uri, "tcp:", &p)) {
+ ret = tcp_start_outgoing_migration(s, p);
+#if !defined(WIN32)
+ } else if (strstart(uri, "exec:", &p)) {
+ ret = exec_start_outgoing_migration(s, p);
+ } else if (strstart(uri, "unix:", &p)) {
+ ret = unix_start_outgoing_migration(s, p);
+ } else if (strstart(uri, "fd:", &p)) {
+ ret = fd_start_outgoing_migration(s, p);
+#endif
+ } else {
+ monitor_printf(mon, "unknown migration protocol: %s\n", uri);
+ ret = -EINVAL;
+ goto free_migrate_state;
+ }
+
+ if (ret < 0) {
+ monitor_printf(mon, "migration failed\n");
+ goto free_migrate_state;
+ }
+
+ g_free(current_migration);
+ current_migration = s;
+ notifier_list_notify(&migration_state_notifiers, NULL);
+ return 0;
+free_migrate_state:
+ g_free(s);
+ return -1;
+}
+
+int do_migrate_cancel(Monitor *mon, const QDict *qdict, QObject **ret_data)
+{
+ if (current_migration) {
+ migrate_fd_cancel(current_migration);
+ }
+ return 0;
+}
+
+int do_migrate_set_speed(Monitor *mon, const QDict *qdict, QObject **ret_data)
+{
+ int64_t d;
+ MigrationState *s;
+
+ d = qdict_get_int(qdict, "value");
+ if (d < 0) {
+ d = 0;
+ }
+ max_throttle = d;
+
+ s = current_migration;
+ if (s && s->file) {
+ qemu_file_set_rate_limit(s->file, max_throttle);
+ }
+
+ return 0;
+}
+
+int do_migrate_set_downtime(Monitor *mon, const QDict *qdict,
+ QObject **ret_data)
+{
+ double d;
+
+ d = qdict_get_double(qdict, "value") * 1e9;
+ d = MAX(0, MIN(UINT64_MAX, d));
+ max_downtime = (uint64_t)d;
+
+ return 0;
+}
--
1.7.6.2
^ permalink raw reply related [flat|nested] 45+ messages in thread* Re: [Qemu-devel] [PATCH 14/23] migration: Move exported functions to the end of the file
2011-09-23 12:57 ` [Qemu-devel] [PATCH 14/23] migration: Move exported functions to the end of the file Juan Quintela
@ 2011-10-04 14:24 ` Anthony Liguori
0 siblings, 0 replies; 45+ messages in thread
From: Anthony Liguori @ 2011-10-04 14:24 UTC (permalink / raw)
To: Juan Quintela; +Cc: qemu-devel
On 09/23/2011 07:57 AM, Juan Quintela wrote:
> This means we can remove the two forward declarations.
>
> Signed-off-by: Juan Quintela<quintela@redhat.com>
Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>
Regards,
Anthony Liguori
> ---
> migration.c | 188 ++++++++++++++++++++++++++++------------------------------
> 1 files changed, 91 insertions(+), 97 deletions(-)
>
> diff --git a/migration.c b/migration.c
> index 9bc7ffa..9bb089a 100644
> --- a/migration.c
> +++ b/migration.c
> @@ -77,91 +77,6 @@ void process_incoming_migration(QEMUFile *f)
> }
> }
>
> -static MigrationState *migrate_create_state(Monitor *mon,
> - int64_t bandwidth_limit,
> - int detach, int blk, int inc);
> -
> -int do_migrate(Monitor *mon, const QDict *qdict, QObject **ret_data)
> -{
> - MigrationState *s = NULL;
> - const char *p;
> - int detach = qdict_get_try_bool(qdict, "detach", 0);
> - int blk = qdict_get_try_bool(qdict, "blk", 0);
> - int inc = qdict_get_try_bool(qdict, "inc", 0);
> - const char *uri = qdict_get_str(qdict, "uri");
> - int ret;
> -
> - if (current_migration&&
> - current_migration->state == MIG_STATE_ACTIVE) {
> - monitor_printf(mon, "migration already in progress\n");
> - return -1;
> - }
> -
> - if (qemu_savevm_state_blocked(mon)) {
> - return -1;
> - }
> -
> - s = migrate_create_state(mon, max_throttle, detach, blk, inc);
> -
> - if (strstart(uri, "tcp:",&p)) {
> - ret = tcp_start_outgoing_migration(s, p);
> -#if !defined(WIN32)
> - } else if (strstart(uri, "exec:",&p)) {
> - ret = exec_start_outgoing_migration(s, p);
> - } else if (strstart(uri, "unix:",&p)) {
> - ret = unix_start_outgoing_migration(s, p);
> - } else if (strstart(uri, "fd:",&p)) {
> - ret = fd_start_outgoing_migration(s, p);
> -#endif
> - } else {
> - monitor_printf(mon, "unknown migration protocol: %s\n", uri);
> - ret = -EINVAL;
> - goto free_migrate_state;
> - }
> -
> - if (ret< 0) {
> - monitor_printf(mon, "migration failed\n");
> - goto free_migrate_state;
> - }
> -
> - g_free(current_migration);
> - current_migration = s;
> - notifier_list_notify(&migration_state_notifiers, NULL);
> - return 0;
> -free_migrate_state:
> - g_free(s);
> - return -1;
> -}
> -
> -static void migrate_fd_cancel(MigrationState *s);
> -
> -int do_migrate_cancel(Monitor *mon, const QDict *qdict, QObject **ret_data)
> -{
> - if (current_migration) {
> - migrate_fd_cancel(current_migration);
> - }
> - return 0;
> -}
> -
> -int do_migrate_set_speed(Monitor *mon, const QDict *qdict, QObject **ret_data)
> -{
> - int64_t d;
> - MigrationState *s;
> -
> - d = qdict_get_int(qdict, "value");
> - if (d< 0) {
> - d = 0;
> - }
> - max_throttle = d;
> -
> - s = current_migration;
> - if (s&& s->file) {
> - qemu_file_set_rate_limit(s->file, max_throttle);
> - }
> -
> - return 0;
> -}
> -
> /* amount of nanoseconds we are willing to wait for migration to be down.
> * the choice of nanoseconds is because it is the maximum resolution that
> * get_clock() can achieve. It is an internal measure. All user-visible
> @@ -173,18 +88,6 @@ uint64_t migrate_max_downtime(void)
> return max_downtime;
> }
>
> -int do_migrate_set_downtime(Monitor *mon, const QDict *qdict,
> - QObject **ret_data)
> -{
> - double d;
> -
> - d = qdict_get_double(qdict, "value") * 1e9;
> - d = MAX(0, MIN(UINT64_MAX, d));
> - max_downtime = (uint64_t)d;
> -
> - return 0;
> -}
> -
> static void migrate_print_status(Monitor *mon, const char *name,
> const QDict *status_dict)
> {
> @@ -502,3 +405,94 @@ static MigrationState *migrate_create_state(Monitor *mon,
>
> return s;
> }
> +
> +int do_migrate(Monitor *mon, const QDict *qdict, QObject **ret_data)
> +{
> + MigrationState *s = NULL;
> + const char *p;
> + int detach = qdict_get_try_bool(qdict, "detach", 0);
> + int blk = qdict_get_try_bool(qdict, "blk", 0);
> + int inc = qdict_get_try_bool(qdict, "inc", 0);
> + const char *uri = qdict_get_str(qdict, "uri");
> + int ret;
> +
> + if (current_migration&&
> + current_migration->state == MIG_STATE_ACTIVE) {
> + monitor_printf(mon, "migration already in progress\n");
> + return -1;
> + }
> +
> + if (qemu_savevm_state_blocked(mon)) {
> + return -1;
> + }
> +
> + s = migrate_create_state(mon, max_throttle, detach, blk, inc);
> +
> + if (strstart(uri, "tcp:",&p)) {
> + ret = tcp_start_outgoing_migration(s, p);
> +#if !defined(WIN32)
> + } else if (strstart(uri, "exec:",&p)) {
> + ret = exec_start_outgoing_migration(s, p);
> + } else if (strstart(uri, "unix:",&p)) {
> + ret = unix_start_outgoing_migration(s, p);
> + } else if (strstart(uri, "fd:",&p)) {
> + ret = fd_start_outgoing_migration(s, p);
> +#endif
> + } else {
> + monitor_printf(mon, "unknown migration protocol: %s\n", uri);
> + ret = -EINVAL;
> + goto free_migrate_state;
> + }
> +
> + if (ret< 0) {
> + monitor_printf(mon, "migration failed\n");
> + goto free_migrate_state;
> + }
> +
> + g_free(current_migration);
> + current_migration = s;
> + notifier_list_notify(&migration_state_notifiers, NULL);
> + return 0;
> +free_migrate_state:
> + g_free(s);
> + return -1;
> +}
> +
> +int do_migrate_cancel(Monitor *mon, const QDict *qdict, QObject **ret_data)
> +{
> + if (current_migration) {
> + migrate_fd_cancel(current_migration);
> + }
> + return 0;
> +}
> +
> +int do_migrate_set_speed(Monitor *mon, const QDict *qdict, QObject **ret_data)
> +{
> + int64_t d;
> + MigrationState *s;
> +
> + d = qdict_get_int(qdict, "value");
> + if (d< 0) {
> + d = 0;
> + }
> + max_throttle = d;
> +
> + s = current_migration;
> + if (s&& s->file) {
> + qemu_file_set_rate_limit(s->file, max_throttle);
> + }
> +
> + return 0;
> +}
> +
> +int do_migrate_set_downtime(Monitor *mon, const QDict *qdict,
> + QObject **ret_data)
> +{
> + double d;
> +
> + d = qdict_get_double(qdict, "value") * 1e9;
> + d = MAX(0, MIN(UINT64_MAX, d));
> + max_downtime = (uint64_t)d;
> +
> + return 0;
> +}
^ permalink raw reply [flat|nested] 45+ messages in thread
* [Qemu-devel] [PATCH 15/23] migration: use global variable directly
2011-09-23 12:56 [Qemu-devel] [PATCH v3 00/23] Refactor and cleanup migration code Juan Quintela
` (13 preceding siblings ...)
2011-09-23 12:57 ` [Qemu-devel] [PATCH 14/23] migration: Move exported functions to the end of the file Juan Quintela
@ 2011-09-23 12:57 ` Juan Quintela
2011-10-04 14:27 ` Anthony Liguori
2011-09-23 12:57 ` [Qemu-devel] [PATCH 16/23] migration: another case of global variable assigned to local one Juan Quintela
` (8 subsequent siblings)
23 siblings, 1 reply; 45+ messages in thread
From: Juan Quintela @ 2011-09-23 12:57 UTC (permalink / raw)
To: qemu-devel
We are setting a pointer to a local variable in the previous line, just use
the global variable directly. We remove the ->file test because it is already
done inside qemu_file_set_rate_limit() function.
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
migration.c | 6 ++----
1 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/migration.c b/migration.c
index 9bb089a..d5e0eb0 100644
--- a/migration.c
+++ b/migration.c
@@ -469,7 +469,6 @@ int do_migrate_cancel(Monitor *mon, const QDict *qdict, QObject **ret_data)
int do_migrate_set_speed(Monitor *mon, const QDict *qdict, QObject **ret_data)
{
int64_t d;
- MigrationState *s;
d = qdict_get_int(qdict, "value");
if (d < 0) {
@@ -477,9 +476,8 @@ int do_migrate_set_speed(Monitor *mon, const QDict *qdict, QObject **ret_data)
}
max_throttle = d;
- s = current_migration;
- if (s && s->file) {
- qemu_file_set_rate_limit(s->file, max_throttle);
+ if (current_migration) {
+ qemu_file_set_rate_limit(current_migration->file, max_throttle);
}
return 0;
--
1.7.6.2
^ permalink raw reply related [flat|nested] 45+ messages in thread* Re: [Qemu-devel] [PATCH 15/23] migration: use global variable directly
2011-09-23 12:57 ` [Qemu-devel] [PATCH 15/23] migration: use global variable directly Juan Quintela
@ 2011-10-04 14:27 ` Anthony Liguori
2011-10-04 14:46 ` Juan Quintela
0 siblings, 1 reply; 45+ messages in thread
From: Anthony Liguori @ 2011-10-04 14:27 UTC (permalink / raw)
To: Juan Quintela; +Cc: qemu-devel
On 09/23/2011 07:57 AM, Juan Quintela wrote:
> We are setting a pointer to a local variable in the previous line, just use
> the global variable directly. We remove the ->file test because it is already
> done inside qemu_file_set_rate_limit() function.
>
> Signed-off-by: Juan Quintela<quintela@redhat.com>
> ---
> migration.c | 6 ++----
> 1 files changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/migration.c b/migration.c
> index 9bb089a..d5e0eb0 100644
> --- a/migration.c
> +++ b/migration.c
> @@ -469,7 +469,6 @@ int do_migrate_cancel(Monitor *mon, const QDict *qdict, QObject **ret_data)
> int do_migrate_set_speed(Monitor *mon, const QDict *qdict, QObject **ret_data)
> {
> int64_t d;
> - MigrationState *s;
>
> d = qdict_get_int(qdict, "value");
> if (d< 0) {
> @@ -477,9 +476,8 @@ int do_migrate_set_speed(Monitor *mon, const QDict *qdict, QObject **ret_data)
> }
> max_throttle = d;
>
> - s = current_migration;
> - if (s&& s->file) {
> - qemu_file_set_rate_limit(s->file, max_throttle);
> + if (current_migration) {
> + qemu_file_set_rate_limit(current_migration->file, max_throttle);
> }
How about we compromise and add a:
MigrationState *migrate_get_current(void);
I'm strongly opposed to propagating direct usage of a global. If it's at least
a function call, that's a bit nicer.
Regards,
Anthony Liguori
>
> return 0;
^ permalink raw reply [flat|nested] 45+ messages in thread* Re: [Qemu-devel] [PATCH 15/23] migration: use global variable directly
2011-10-04 14:27 ` Anthony Liguori
@ 2011-10-04 14:46 ` Juan Quintela
0 siblings, 0 replies; 45+ messages in thread
From: Juan Quintela @ 2011-10-04 14:46 UTC (permalink / raw)
To: Anthony Liguori; +Cc: qemu-devel
Anthony Liguori <anthony@codemonkey.ws> wrote:
> How about we compromise and add a:
>
> MigrationState *migrate_get_current(void);
I can agree with this one.
> I'm strongly opposed to propagating direct usage of a global. If it's
> at least a function call, that's a bit nicer.
What I am against is with trying to "hide" the fact that we are using a
global. function accessor looks ok enough.
^ permalink raw reply [flat|nested] 45+ messages in thread
* [Qemu-devel] [PATCH 16/23] migration: another case of global variable assigned to local one
2011-09-23 12:56 [Qemu-devel] [PATCH v3 00/23] Refactor and cleanup migration code Juan Quintela
` (14 preceding siblings ...)
2011-09-23 12:57 ` [Qemu-devel] [PATCH 15/23] migration: use global variable directly Juan Quintela
@ 2011-09-23 12:57 ` Juan Quintela
2011-09-23 12:57 ` [Qemu-devel] [PATCH 17/23] migration: make sure we always have a migration state Juan Quintela
` (7 subsequent siblings)
23 siblings, 0 replies; 45+ messages in thread
From: Juan Quintela @ 2011-09-23 12:57 UTC (permalink / raw)
To: qemu-devel
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
migration.c | 3 +--
1 files changed, 1 insertions(+), 2 deletions(-)
diff --git a/migration.c b/migration.c
index d5e0eb0..1cc21f0 100644
--- a/migration.c
+++ b/migration.c
@@ -137,9 +137,8 @@ void do_info_migrate(Monitor *mon, QObject **ret_data)
QDict *qdict;
if (current_migration) {
- MigrationState *s = current_migration;
- switch (s->state) {
+ switch (current_migration->state) {
case MIG_STATE_NONE:
/* no migration has happened ever */
break;
--
1.7.6.2
^ permalink raw reply related [flat|nested] 45+ messages in thread* [Qemu-devel] [PATCH 17/23] migration: make sure we always have a migration state
2011-09-23 12:56 [Qemu-devel] [PATCH v3 00/23] Refactor and cleanup migration code Juan Quintela
` (15 preceding siblings ...)
2011-09-23 12:57 ` [Qemu-devel] [PATCH 16/23] migration: another case of global variable assigned to local one Juan Quintela
@ 2011-09-23 12:57 ` Juan Quintela
2011-10-04 14:29 ` Anthony Liguori
2011-09-23 12:57 ` [Qemu-devel] [PATCH 18/23] migration: Use bandwidth_limit directly Juan Quintela
` (6 subsequent siblings)
23 siblings, 1 reply; 45+ messages in thread
From: Juan Quintela @ 2011-09-23 12:57 UTC (permalink / raw)
To: qemu-devel
This cleans up a lot the code as we don't have to check anymore if
the variable is NULL or not.
We don't make it static, because when we integrate fault tolerance, we
can have several migrations at once.
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
migration.c | 126 +++++++++++++++++++++++++---------------------------------
1 files changed, 54 insertions(+), 72 deletions(-)
diff --git a/migration.c b/migration.c
index 1cc21f0..c382383 100644
--- a/migration.c
+++ b/migration.c
@@ -34,7 +34,13 @@
/* Migration speed throttling */
static int64_t max_throttle = (32 << 20);
-static MigrationState *current_migration;
+/* When we add fault tolerance, we could have several
+ migrations at once. For now we don't need to add
+ dynamic creation of migration */
+static MigrationState current_migration_static = {
+ .state = MIG_STATE_NONE,
+};
+static MigrationState *current_migration = ¤t_migration_static;
static NotifierList migration_state_notifiers =
NOTIFIER_LIST_INITIALIZER(migration_state_notifiers);
@@ -136,37 +142,34 @@ void do_info_migrate(Monitor *mon, QObject **ret_data)
{
QDict *qdict;
- if (current_migration) {
-
- switch (current_migration->state) {
- case MIG_STATE_NONE:
- /* no migration has happened ever */
- break;
- case MIG_STATE_ACTIVE:
- qdict = qdict_new();
- qdict_put(qdict, "status", qstring_from_str("active"));
-
- migrate_put_status(qdict, "ram", ram_bytes_transferred(),
- ram_bytes_remaining(), ram_bytes_total());
-
- if (blk_mig_active()) {
- migrate_put_status(qdict, "disk", blk_mig_bytes_transferred(),
- blk_mig_bytes_remaining(),
- blk_mig_bytes_total());
- }
-
- *ret_data = QOBJECT(qdict);
- break;
- case MIG_STATE_COMPLETED:
- *ret_data = qobject_from_jsonf("{ 'status': 'completed' }");
- break;
- case MIG_STATE_ERROR:
- *ret_data = qobject_from_jsonf("{ 'status': 'failed' }");
- break;
- case MIG_STATE_CANCELLED:
- *ret_data = qobject_from_jsonf("{ 'status': 'cancelled' }");
- break;
+ switch (current_migration->state) {
+ case MIG_STATE_NONE:
+ /* no migration has happened ever */
+ break;
+ case MIG_STATE_ACTIVE:
+ qdict = qdict_new();
+ qdict_put(qdict, "status", qstring_from_str("active"));
+
+ migrate_put_status(qdict, "ram", ram_bytes_transferred(),
+ ram_bytes_remaining(), ram_bytes_total());
+
+ if (blk_mig_active()) {
+ migrate_put_status(qdict, "disk", blk_mig_bytes_transferred(),
+ blk_mig_bytes_remaining(),
+ blk_mig_bytes_total());
}
+
+ *ret_data = QOBJECT(qdict);
+ break;
+ case MIG_STATE_COMPLETED:
+ *ret_data = qobject_from_jsonf("{ 'status': 'completed' }");
+ break;
+ case MIG_STATE_ERROR:
+ *ret_data = qobject_from_jsonf("{ 'status': 'failed' }");
+ break;
+ case MIG_STATE_CANCELLED:
+ *ret_data = qobject_from_jsonf("{ 'status': 'cancelled' }");
+ break;
}
}
@@ -357,11 +360,7 @@ void remove_migration_state_change_notifier(Notifier *notify)
int get_migration_state(void)
{
- if (current_migration) {
- return current_migration->state;
- } else {
- return MIG_STATE_ERROR;
- }
+ return current_migration->state;
}
void migrate_fd_connect(MigrationState *s)
@@ -386,28 +385,23 @@ void migrate_fd_connect(MigrationState *s)
migrate_fd_put_ready(s);
}
-static MigrationState *migrate_create_state(Monitor *mon,
- int64_t bandwidth_limit,
- int detach, int blk, int inc)
+static void migrate_init_state(Monitor *mon, int64_t bandwidth_limit,
+ int detach, int blk, int inc)
{
- MigrationState *s = g_malloc0(sizeof(*s));
-
- s->blk = blk;
- s->shared = inc;
- s->mon = NULL;
- s->bandwidth_limit = bandwidth_limit;
- s->state = MIG_STATE_NONE;
+ memset(current_migration, 0, sizeof(current_migration));
+ current_migration->blk = blk;
+ current_migration->shared = inc;
+ current_migration->mon = NULL;
+ current_migration->bandwidth_limit = bandwidth_limit;
+ current_migration->state = MIG_STATE_NONE;
if (!detach) {
- migrate_fd_monitor_suspend(s, mon);
+ migrate_fd_monitor_suspend(current_migration, mon);
}
-
- return s;
}
int do_migrate(Monitor *mon, const QDict *qdict, QObject **ret_data)
{
- MigrationState *s = NULL;
const char *p;
int detach = qdict_get_try_bool(qdict, "detach", 0);
int blk = qdict_get_try_bool(qdict, "blk", 0);
@@ -415,8 +409,7 @@ int do_migrate(Monitor *mon, const QDict *qdict, QObject **ret_data)
const char *uri = qdict_get_str(qdict, "uri");
int ret;
- if (current_migration &&
- current_migration->state == MIG_STATE_ACTIVE) {
+ if (current_migration->state == MIG_STATE_ACTIVE) {
monitor_printf(mon, "migration already in progress\n");
return -1;
}
@@ -425,43 +418,35 @@ int do_migrate(Monitor *mon, const QDict *qdict, QObject **ret_data)
return -1;
}
- s = migrate_create_state(mon, max_throttle, detach, blk, inc);
+ migrate_init_state(mon, max_throttle, detach, blk, inc);
if (strstart(uri, "tcp:", &p)) {
- ret = tcp_start_outgoing_migration(s, p);
+ ret = tcp_start_outgoing_migration(current_migration, p);
#if !defined(WIN32)
} else if (strstart(uri, "exec:", &p)) {
- ret = exec_start_outgoing_migration(s, p);
+ ret = exec_start_outgoing_migration(current_migration, p);
} else if (strstart(uri, "unix:", &p)) {
- ret = unix_start_outgoing_migration(s, p);
+ ret = unix_start_outgoing_migration(current_migration, p);
} else if (strstart(uri, "fd:", &p)) {
- ret = fd_start_outgoing_migration(s, p);
+ ret = fd_start_outgoing_migration(current_migration, p);
#endif
} else {
monitor_printf(mon, "unknown migration protocol: %s\n", uri);
- ret = -EINVAL;
- goto free_migrate_state;
+ return -EINVAL;
}
if (ret < 0) {
monitor_printf(mon, "migration failed\n");
- goto free_migrate_state;
+ return ret;
}
- g_free(current_migration);
- current_migration = s;
notifier_list_notify(&migration_state_notifiers, NULL);
return 0;
-free_migrate_state:
- g_free(s);
- return -1;
}
int do_migrate_cancel(Monitor *mon, const QDict *qdict, QObject **ret_data)
{
- if (current_migration) {
- migrate_fd_cancel(current_migration);
- }
+ migrate_fd_cancel(current_migration);
return 0;
}
@@ -475,10 +460,7 @@ int do_migrate_set_speed(Monitor *mon, const QDict *qdict, QObject **ret_data)
}
max_throttle = d;
- if (current_migration) {
- qemu_file_set_rate_limit(current_migration->file, max_throttle);
- }
-
+ qemu_file_set_rate_limit(current_migration->file, max_throttle);
return 0;
}
--
1.7.6.2
^ permalink raw reply related [flat|nested] 45+ messages in thread* Re: [Qemu-devel] [PATCH 17/23] migration: make sure we always have a migration state
2011-09-23 12:57 ` [Qemu-devel] [PATCH 17/23] migration: make sure we always have a migration state Juan Quintela
@ 2011-10-04 14:29 ` Anthony Liguori
2011-10-04 14:49 ` Juan Quintela
0 siblings, 1 reply; 45+ messages in thread
From: Anthony Liguori @ 2011-10-04 14:29 UTC (permalink / raw)
To: Juan Quintela; +Cc: qemu-devel
On 09/23/2011 07:57 AM, Juan Quintela wrote:
> This cleans up a lot the code as we don't have to check anymore if
> the variable is NULL or not.
>
> We don't make it static, because when we integrate fault tolerance, we
> can have several migrations at once.
>
> Signed-off-by: Juan Quintela<quintela@redhat.com>
> ---
> migration.c | 126 +++++++++++++++++++++++++---------------------------------
> 1 files changed, 54 insertions(+), 72 deletions(-)
>
> diff --git a/migration.c b/migration.c
> index 1cc21f0..c382383 100644
> --- a/migration.c
> +++ b/migration.c
> @@ -34,7 +34,13 @@
> /* Migration speed throttling */
> static int64_t max_throttle = (32<< 20);
>
> -static MigrationState *current_migration;
> +/* When we add fault tolerance, we could have several
> + migrations at once. For now we don't need to add
> + dynamic creation of migration */
> +static MigrationState current_migration_static = {
> + .state = MIG_STATE_NONE,
> +};
> +static MigrationState *current_migration =¤t_migration_static;
This doesn't make sense to me. We can have two migration states that are both
in the MIG_STATE_NONE? What does that actually mean?
I don't see the point in making migration state nullable via the state member
other than avoiding the if (s == NULL) check.
Regards,
Anthony Liguori
>
> static NotifierList migration_state_notifiers =
> NOTIFIER_LIST_INITIALIZER(migration_state_notifiers);
> @@ -136,37 +142,34 @@ void do_info_migrate(Monitor *mon, QObject **ret_data)
> {
> QDict *qdict;
>
> - if (current_migration) {
> -
> - switch (current_migration->state) {
> - case MIG_STATE_NONE:
> - /* no migration has happened ever */
> - break;
> - case MIG_STATE_ACTIVE:
> - qdict = qdict_new();
> - qdict_put(qdict, "status", qstring_from_str("active"));
> -
> - migrate_put_status(qdict, "ram", ram_bytes_transferred(),
> - ram_bytes_remaining(), ram_bytes_total());
> -
> - if (blk_mig_active()) {
> - migrate_put_status(qdict, "disk", blk_mig_bytes_transferred(),
> - blk_mig_bytes_remaining(),
> - blk_mig_bytes_total());
> - }
> -
> - *ret_data = QOBJECT(qdict);
> - break;
> - case MIG_STATE_COMPLETED:
> - *ret_data = qobject_from_jsonf("{ 'status': 'completed' }");
> - break;
> - case MIG_STATE_ERROR:
> - *ret_data = qobject_from_jsonf("{ 'status': 'failed' }");
> - break;
> - case MIG_STATE_CANCELLED:
> - *ret_data = qobject_from_jsonf("{ 'status': 'cancelled' }");
> - break;
> + switch (current_migration->state) {
> + case MIG_STATE_NONE:
> + /* no migration has happened ever */
> + break;
> + case MIG_STATE_ACTIVE:
> + qdict = qdict_new();
> + qdict_put(qdict, "status", qstring_from_str("active"));
> +
> + migrate_put_status(qdict, "ram", ram_bytes_transferred(),
> + ram_bytes_remaining(), ram_bytes_total());
> +
> + if (blk_mig_active()) {
> + migrate_put_status(qdict, "disk", blk_mig_bytes_transferred(),
> + blk_mig_bytes_remaining(),
> + blk_mig_bytes_total());
> }
> +
> + *ret_data = QOBJECT(qdict);
> + break;
> + case MIG_STATE_COMPLETED:
> + *ret_data = qobject_from_jsonf("{ 'status': 'completed' }");
> + break;
> + case MIG_STATE_ERROR:
> + *ret_data = qobject_from_jsonf("{ 'status': 'failed' }");
> + break;
> + case MIG_STATE_CANCELLED:
> + *ret_data = qobject_from_jsonf("{ 'status': 'cancelled' }");
> + break;
> }
> }
>
> @@ -357,11 +360,7 @@ void remove_migration_state_change_notifier(Notifier *notify)
>
> int get_migration_state(void)
> {
> - if (current_migration) {
> - return current_migration->state;
> - } else {
> - return MIG_STATE_ERROR;
> - }
> + return current_migration->state;
> }
>
> void migrate_fd_connect(MigrationState *s)
> @@ -386,28 +385,23 @@ void migrate_fd_connect(MigrationState *s)
> migrate_fd_put_ready(s);
> }
>
> -static MigrationState *migrate_create_state(Monitor *mon,
> - int64_t bandwidth_limit,
> - int detach, int blk, int inc)
> +static void migrate_init_state(Monitor *mon, int64_t bandwidth_limit,
> + int detach, int blk, int inc)
> {
> - MigrationState *s = g_malloc0(sizeof(*s));
> -
> - s->blk = blk;
> - s->shared = inc;
> - s->mon = NULL;
> - s->bandwidth_limit = bandwidth_limit;
> - s->state = MIG_STATE_NONE;
> + memset(current_migration, 0, sizeof(current_migration));
> + current_migration->blk = blk;
> + current_migration->shared = inc;
> + current_migration->mon = NULL;
> + current_migration->bandwidth_limit = bandwidth_limit;
> + current_migration->state = MIG_STATE_NONE;
>
> if (!detach) {
> - migrate_fd_monitor_suspend(s, mon);
> + migrate_fd_monitor_suspend(current_migration, mon);
> }
> -
> - return s;
> }
>
> int do_migrate(Monitor *mon, const QDict *qdict, QObject **ret_data)
> {
> - MigrationState *s = NULL;
> const char *p;
> int detach = qdict_get_try_bool(qdict, "detach", 0);
> int blk = qdict_get_try_bool(qdict, "blk", 0);
> @@ -415,8 +409,7 @@ int do_migrate(Monitor *mon, const QDict *qdict, QObject **ret_data)
> const char *uri = qdict_get_str(qdict, "uri");
> int ret;
>
> - if (current_migration&&
> - current_migration->state == MIG_STATE_ACTIVE) {
> + if (current_migration->state == MIG_STATE_ACTIVE) {
> monitor_printf(mon, "migration already in progress\n");
> return -1;
> }
> @@ -425,43 +418,35 @@ int do_migrate(Monitor *mon, const QDict *qdict, QObject **ret_data)
> return -1;
> }
>
> - s = migrate_create_state(mon, max_throttle, detach, blk, inc);
> + migrate_init_state(mon, max_throttle, detach, blk, inc);
>
> if (strstart(uri, "tcp:",&p)) {
> - ret = tcp_start_outgoing_migration(s, p);
> + ret = tcp_start_outgoing_migration(current_migration, p);
> #if !defined(WIN32)
> } else if (strstart(uri, "exec:",&p)) {
> - ret = exec_start_outgoing_migration(s, p);
> + ret = exec_start_outgoing_migration(current_migration, p);
> } else if (strstart(uri, "unix:",&p)) {
> - ret = unix_start_outgoing_migration(s, p);
> + ret = unix_start_outgoing_migration(current_migration, p);
> } else if (strstart(uri, "fd:",&p)) {
> - ret = fd_start_outgoing_migration(s, p);
> + ret = fd_start_outgoing_migration(current_migration, p);
> #endif
> } else {
> monitor_printf(mon, "unknown migration protocol: %s\n", uri);
> - ret = -EINVAL;
> - goto free_migrate_state;
> + return -EINVAL;
> }
>
> if (ret< 0) {
> monitor_printf(mon, "migration failed\n");
> - goto free_migrate_state;
> + return ret;
> }
>
> - g_free(current_migration);
> - current_migration = s;
> notifier_list_notify(&migration_state_notifiers, NULL);
> return 0;
> -free_migrate_state:
> - g_free(s);
> - return -1;
> }
>
> int do_migrate_cancel(Monitor *mon, const QDict *qdict, QObject **ret_data)
> {
> - if (current_migration) {
> - migrate_fd_cancel(current_migration);
> - }
> + migrate_fd_cancel(current_migration);
> return 0;
> }
>
> @@ -475,10 +460,7 @@ int do_migrate_set_speed(Monitor *mon, const QDict *qdict, QObject **ret_data)
> }
> max_throttle = d;
>
> - if (current_migration) {
> - qemu_file_set_rate_limit(current_migration->file, max_throttle);
> - }
> -
> + qemu_file_set_rate_limit(current_migration->file, max_throttle);
> return 0;
> }
>
^ permalink raw reply [flat|nested] 45+ messages in thread* Re: [Qemu-devel] [PATCH 17/23] migration: make sure we always have a migration state
2011-10-04 14:29 ` Anthony Liguori
@ 2011-10-04 14:49 ` Juan Quintela
2011-10-05 20:02 ` Anthony Liguori
0 siblings, 1 reply; 45+ messages in thread
From: Juan Quintela @ 2011-10-04 14:49 UTC (permalink / raw)
To: Anthony Liguori; +Cc: qemu-devel
Anthony Liguori <anthony@codemonkey.ws> wrote:
> On 09/23/2011 07:57 AM, Juan Quintela wrote:
>> This cleans up a lot the code as we don't have to check anymore if
>> the variable is NULL or not.
>>
>> We don't make it static, because when we integrate fault tolerance, we
>> can have several migrations at once.
>>
>> Signed-off-by: Juan Quintela<quintela@redhat.com>
>> ---
>> migration.c | 126 +++++++++++++++++++++++++---------------------------------
>> 1 files changed, 54 insertions(+), 72 deletions(-)
>>
>> diff --git a/migration.c b/migration.c
>> index 1cc21f0..c382383 100644
>> --- a/migration.c
>> +++ b/migration.c
>> @@ -34,7 +34,13 @@
>> /* Migration speed throttling */
>> static int64_t max_throttle = (32<< 20);
>>
>> -static MigrationState *current_migration;
>> +/* When we add fault tolerance, we could have several
>> + migrations at once. For now we don't need to add
>> + dynamic creation of migration */
>> +static MigrationState current_migration_static = {
>> + .state = MIG_STATE_NONE,
>> +};
>> +static MigrationState *current_migration =¤t_migration_static;
>
> This doesn't make sense to me. We can have two migration states that
> are both in the MIG_STATE_NONE? What does that actually mean?
>
> I don't see the point in making migration state nullable via the state
> member other than avoiding the if (s == NULL) check.
It avoids s==NULL checks, and it also avoids having to have new
variables (max_throotle) just because we don't have a migration state
handy.
Obviously the problem can't be the size (the variable is smaller that
all the code tests for NULL).
For me, it is easier to understand that we have an state (migration
never attempted) with the same states that the other migrations states.
Later, Juan.
^ permalink raw reply [flat|nested] 45+ messages in thread* Re: [Qemu-devel] [PATCH 17/23] migration: make sure we always have a migration state
2011-10-04 14:49 ` Juan Quintela
@ 2011-10-05 20:02 ` Anthony Liguori
2011-10-05 20:25 ` Juan Quintela
0 siblings, 1 reply; 45+ messages in thread
From: Anthony Liguori @ 2011-10-05 20:02 UTC (permalink / raw)
To: quintela; +Cc: qemu-devel
On 10/04/2011 09:49 AM, Juan Quintela wrote:
> Anthony Liguori<anthony@codemonkey.ws> wrote:
>> On 09/23/2011 07:57 AM, Juan Quintela wrote:
>>> This cleans up a lot the code as we don't have to check anymore if
>>> the variable is NULL or not.
>>>
>>> We don't make it static, because when we integrate fault tolerance, we
>>> can have several migrations at once.
>>>
>>> Signed-off-by: Juan Quintela<quintela@redhat.com>
>>> ---
>>> migration.c | 126 +++++++++++++++++++++++++---------------------------------
>>> 1 files changed, 54 insertions(+), 72 deletions(-)
>>>
>>> diff --git a/migration.c b/migration.c
>>> index 1cc21f0..c382383 100644
>>> --- a/migration.c
>>> +++ b/migration.c
>>> @@ -34,7 +34,13 @@
>>> /* Migration speed throttling */
>>> static int64_t max_throttle = (32<< 20);
>>>
>>> -static MigrationState *current_migration;
>>> +/* When we add fault tolerance, we could have several
>>> + migrations at once. For now we don't need to add
>>> + dynamic creation of migration */
>>> +static MigrationState current_migration_static = {
>>> + .state = MIG_STATE_NONE,
>>> +};
>>> +static MigrationState *current_migration =¤t_migration_static;
>>
>> This doesn't make sense to me. We can have two migration states that
>> are both in the MIG_STATE_NONE? What does that actually mean?
>>
>> I don't see the point in making migration state nullable via the state
>> member other than avoiding the if (s == NULL) check.
>
> It avoids s==NULL checks,
In favor of s->state == MIG_STATE_NONE.
> and it also avoids having to have new
> variables (max_throotle) just because we don't have a migration state
> handy.
The intention of the global variable is to set a default for new sessions. I
can imagine a world where if you had parallel migrations, you could either
toggle the default (the global variable) or the specific parameter within a
single migration session.
But it's not about features. It's a general reluctances to heavily modify the
code to rely on a global instead of passing the state around. Here's a pretty
good short write up of why this is a Bad Thing.
http://stackoverflow.com/questions/1392315/problems-with-singleton-pattern
Ultimately, MIG_STATE_NONE shouldn't be needed because we should not be relying
on a singleton accessor to get the MigrationState. A better cleanup would be to
further pass MigrationState between functions and eliminate the need for a
global at all.
Regards,
Anthony Liguori
> Obviously the problem can't be the size (the variable is smaller that
> all the code tests for NULL).
>
> For me, it is easier to understand that we have an state (migration
> never attempted) with the same states that the other migrations states.
>
> Later, Juan.
>
^ permalink raw reply [flat|nested] 45+ messages in thread* Re: [Qemu-devel] [PATCH 17/23] migration: make sure we always have a migration state
2011-10-05 20:02 ` Anthony Liguori
@ 2011-10-05 20:25 ` Juan Quintela
2011-10-06 9:09 ` Orit Wasserman
0 siblings, 1 reply; 45+ messages in thread
From: Juan Quintela @ 2011-10-05 20:25 UTC (permalink / raw)
To: Anthony Liguori; +Cc: qemu-devel
Anthony Liguori <anthony@codemonkey.ws> wrote:
> On 10/04/2011 09:49 AM, Juan Quintela wrote:
>> Anthony Liguori<anthony@codemonkey.ws> wrote:
>>> On 09/23/2011 07:57 AM, Juan Quintela wrote:
[ much more stuff ]
>> It avoids s==NULL checks,
>
> In favor of s->state == MIG_STATE_NONE.
>
>> and it also avoids having to have new
>> variables (max_throotle) just because we don't have a migration state
>> handy.
>
> The intention of the global variable is to set a default for new
> sessions. I can imagine a world where if you had parallel migrations,
> you could either toggle the default (the global variable) or the
> specific parameter within a single migration session.
>
> But it's not about features. It's a general reluctances to heavily
> modify the code to rely on a global instead of passing the state
> around. Here's a pretty good short write up of why this is a Bad
> Thing.
>
> http://stackoverflow.com/questions/1392315/problems-with-singleton-pattern
>
> Ultimately, MIG_STATE_NONE shouldn't be needed because we should not
> be relying on a singleton accessor to get the MigrationState. A
> better cleanup would be to further pass MigrationState between
> functions and eliminate the need for a global at all.
I understand the singleton problem, but the reason to put STATE_NONE is
not for that O:-) (it just happens that we only have a migration now).
Why I want it?
Just now, the only thing that we are "setting" for a migration before it
starts is the "bandwidth". I see the future when migration becomes
something like:
migration_set_speed ....
migration_set_target ....
migration_set_<whatever else>
migrate
as you can see, we are "preparing" migration, and we don't have a
"STATE" now that describes this state, migration not started, but we
want to prepare it.
Perhaps a better name that STATE_NONE is in order. I got NONE in the
sense that "migration" has not been attemted yet.
Later, Juan.
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [Qemu-devel] [PATCH 17/23] migration: make sure we always have a migration state
2011-10-05 20:25 ` Juan Quintela
@ 2011-10-06 9:09 ` Orit Wasserman
0 siblings, 0 replies; 45+ messages in thread
From: Orit Wasserman @ 2011-10-06 9:09 UTC (permalink / raw)
To: qemu-devel
[-- Attachment #1: Type: text/plain, Size: 2098 bytes --]
On 10/05/2011 10:25 PM, Juan Quintela wrote:
> Anthony Liguori<anthony@codemonkey.ws> wrote:
>> On 10/04/2011 09:49 AM, Juan Quintela wrote:
>>> Anthony Liguori<anthony@codemonkey.ws> wrote:
>>>> On 09/23/2011 07:57 AM, Juan Quintela wrote:
> [ much more stuff ]
>
>>> It avoids s==NULL checks,
>> In favor of s->state == MIG_STATE_NONE.
>>
>>> and it also avoids having to have new
>>> variables (max_throotle) just because we don't have a migration state
>>> handy.
>> The intention of the global variable is to set a default for new
>> sessions. I can imagine a world where if you had parallel migrations,
>> you could either toggle the default (the global variable) or the
>> specific parameter within a single migration session.
>>
>> But it's not about features. It's a general reluctances to heavily
>> modify the code to rely on a global instead of passing the state
>> around. Here's a pretty good short write up of why this is a Bad
>> Thing.
>>
>> http://stackoverflow.com/questions/1392315/problems-with-singleton-pattern
>>
>> Ultimately, MIG_STATE_NONE shouldn't be needed because we should not
>> be relying on a singleton accessor to get the MigrationState. A
>> better cleanup would be to further pass MigrationState between
>> functions and eliminate the need for a global at all.
> I understand the singleton problem, but the reason to put STATE_NONE is
> not for that O:-) (it just happens that we only have a migration now).
>
> Why I want it?
>
> Just now, the only thing that we are "setting" for a migration before it
> starts is the "bandwidth". I see the future when migration becomes
> something like:
>
> migration_set_speed ....
> migration_set_target ....
> migration_set_<whatever else>
> migrate
>
> as you can see, we are "preparing" migration, and we don't have a
> "STATE" now that describes this state, migration not started, but we
> want to prepare it.
>
> Perhaps a better name that STATE_NONE is in order. I got NONE in the
> sense that "migration" has not been attemted yet.
How about STATE_SETUP or STATE_PREPARE ?
Cheers,
Orit
> Later, Juan.
>
[-- Attachment #2: Type: text/html, Size: 3305 bytes --]
^ permalink raw reply [flat|nested] 45+ messages in thread
* [Qemu-devel] [PATCH 18/23] migration: Use bandwidth_limit directly
2011-09-23 12:56 [Qemu-devel] [PATCH v3 00/23] Refactor and cleanup migration code Juan Quintela
` (16 preceding siblings ...)
2011-09-23 12:57 ` [Qemu-devel] [PATCH 17/23] migration: make sure we always have a migration state Juan Quintela
@ 2011-09-23 12:57 ` Juan Quintela
2011-10-04 14:29 ` Anthony Liguori
2011-09-23 12:57 ` [Qemu-devel] [PATCH 19/23] migration: Export a function that tells if the migration has finished correctly Juan Quintela
` (5 subsequent siblings)
23 siblings, 1 reply; 45+ messages in thread
From: Juan Quintela @ 2011-09-23 12:57 UTC (permalink / raw)
To: qemu-devel
Now that current_migration always exist, there is no reason for
max_throotle variable.
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
migration.c | 19 ++++++++++---------
1 files changed, 10 insertions(+), 9 deletions(-)
diff --git a/migration.c b/migration.c
index c382383..ea50a6f 100644
--- a/migration.c
+++ b/migration.c
@@ -31,14 +31,14 @@
do { } while (0)
#endif
-/* Migration speed throttling */
-static int64_t max_throttle = (32 << 20);
+#define MAX_THROTTLE (32 << 20) /* Migration speed throttling */
/* When we add fault tolerance, we could have several
migrations at once. For now we don't need to add
dynamic creation of migration */
static MigrationState current_migration_static = {
.state = MIG_STATE_NONE,
+ .bandwidth_limit = MAX_THROTTLE,
};
static MigrationState *current_migration = ¤t_migration_static;
@@ -385,14 +385,14 @@ void migrate_fd_connect(MigrationState *s)
migrate_fd_put_ready(s);
}
-static void migrate_init_state(Monitor *mon, int64_t bandwidth_limit,
- int detach, int blk, int inc)
+static void migrate_init_state(Monitor *mon, int detach, int blk, int inc)
{
+ int64_t bandwidth_limit = current_migration->bandwidth_limit;
+
memset(current_migration, 0, sizeof(current_migration));
+ current_migration->bandwidth_limit = bandwidth_limit;
current_migration->blk = blk;
current_migration->shared = inc;
- current_migration->mon = NULL;
- current_migration->bandwidth_limit = bandwidth_limit;
current_migration->state = MIG_STATE_NONE;
if (!detach) {
@@ -418,7 +418,7 @@ int do_migrate(Monitor *mon, const QDict *qdict, QObject **ret_data)
return -1;
}
- migrate_init_state(mon, max_throttle, detach, blk, inc);
+ migrate_init_state(mon, detach, blk, inc);
if (strstart(uri, "tcp:", &p)) {
ret = tcp_start_outgoing_migration(current_migration, p);
@@ -458,9 +458,10 @@ int do_migrate_set_speed(Monitor *mon, const QDict *qdict, QObject **ret_data)
if (d < 0) {
d = 0;
}
- max_throttle = d;
+ current_migration->bandwidth_limit = d;
- qemu_file_set_rate_limit(current_migration->file, max_throttle);
+ qemu_file_set_rate_limit(current_migration->file,
+ current_migration->bandwidth_limit);
return 0;
}
--
1.7.6.2
^ permalink raw reply related [flat|nested] 45+ messages in thread* Re: [Qemu-devel] [PATCH 18/23] migration: Use bandwidth_limit directly
2011-09-23 12:57 ` [Qemu-devel] [PATCH 18/23] migration: Use bandwidth_limit directly Juan Quintela
@ 2011-10-04 14:29 ` Anthony Liguori
0 siblings, 0 replies; 45+ messages in thread
From: Anthony Liguori @ 2011-10-04 14:29 UTC (permalink / raw)
To: Juan Quintela; +Cc: qemu-devel
On 09/23/2011 07:57 AM, Juan Quintela wrote:
> Now that current_migration always exist, there is no reason for
> max_throotle variable.
>
> Signed-off-by: Juan Quintela<quintela@redhat.com>
If we can have multiple MigrationStates, this doesn't really make sense.
Regards,
Anthony Liguori
> ---
> migration.c | 19 ++++++++++---------
> 1 files changed, 10 insertions(+), 9 deletions(-)
>
> diff --git a/migration.c b/migration.c
> index c382383..ea50a6f 100644
> --- a/migration.c
> +++ b/migration.c
> @@ -31,14 +31,14 @@
> do { } while (0)
> #endif
>
> -/* Migration speed throttling */
> -static int64_t max_throttle = (32<< 20);
> +#define MAX_THROTTLE (32<< 20) /* Migration speed throttling */
>
> /* When we add fault tolerance, we could have several
> migrations at once. For now we don't need to add
> dynamic creation of migration */
> static MigrationState current_migration_static = {
> .state = MIG_STATE_NONE,
> + .bandwidth_limit = MAX_THROTTLE,
> };
> static MigrationState *current_migration =¤t_migration_static;
>
> @@ -385,14 +385,14 @@ void migrate_fd_connect(MigrationState *s)
> migrate_fd_put_ready(s);
> }
>
> -static void migrate_init_state(Monitor *mon, int64_t bandwidth_limit,
> - int detach, int blk, int inc)
> +static void migrate_init_state(Monitor *mon, int detach, int blk, int inc)
> {
> + int64_t bandwidth_limit = current_migration->bandwidth_limit;
> +
> memset(current_migration, 0, sizeof(current_migration));
> + current_migration->bandwidth_limit = bandwidth_limit;
> current_migration->blk = blk;
> current_migration->shared = inc;
> - current_migration->mon = NULL;
> - current_migration->bandwidth_limit = bandwidth_limit;
> current_migration->state = MIG_STATE_NONE;
>
> if (!detach) {
> @@ -418,7 +418,7 @@ int do_migrate(Monitor *mon, const QDict *qdict, QObject **ret_data)
> return -1;
> }
>
> - migrate_init_state(mon, max_throttle, detach, blk, inc);
> + migrate_init_state(mon, detach, blk, inc);
>
> if (strstart(uri, "tcp:",&p)) {
> ret = tcp_start_outgoing_migration(current_migration, p);
> @@ -458,9 +458,10 @@ int do_migrate_set_speed(Monitor *mon, const QDict *qdict, QObject **ret_data)
> if (d< 0) {
> d = 0;
> }
> - max_throttle = d;
> + current_migration->bandwidth_limit = d;
>
> - qemu_file_set_rate_limit(current_migration->file, max_throttle);
> + qemu_file_set_rate_limit(current_migration->file,
> + current_migration->bandwidth_limit);
> return 0;
> }
>
^ permalink raw reply [flat|nested] 45+ messages in thread
* [Qemu-devel] [PATCH 19/23] migration: Export a function that tells if the migration has finished correctly
2011-09-23 12:56 [Qemu-devel] [PATCH v3 00/23] Refactor and cleanup migration code Juan Quintela
` (17 preceding siblings ...)
2011-09-23 12:57 ` [Qemu-devel] [PATCH 18/23] migration: Use bandwidth_limit directly Juan Quintela
@ 2011-09-23 12:57 ` Juan Quintela
2011-10-04 14:31 ` Anthony Liguori
2011-09-23 12:57 ` [Qemu-devel] [PATCH 20/23] migration: Make state definitions local Juan Quintela
` (4 subsequent siblings)
23 siblings, 1 reply; 45+ messages in thread
From: Juan Quintela @ 2011-09-23 12:57 UTC (permalink / raw)
To: qemu-devel
This will allows us to hide the status values.
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
migration.c | 4 ++--
migration.h | 2 +-
ui/spice-core.c | 4 +---
3 files changed, 4 insertions(+), 6 deletions(-)
diff --git a/migration.c b/migration.c
index ea50a6f..580f546 100644
--- a/migration.c
+++ b/migration.c
@@ -358,9 +358,9 @@ void remove_migration_state_change_notifier(Notifier *notify)
notifier_list_remove(&migration_state_notifiers, notify);
}
-int get_migration_state(void)
+bool migration_has_finished(void)
{
- return current_migration->state;
+ return current_migration->state == MIG_STATE_COMPLETED;
}
void migrate_fd_connect(MigrationState *s)
diff --git a/migration.h b/migration.h
index f1a7452..6641a26 100644
--- a/migration.h
+++ b/migration.h
@@ -84,7 +84,7 @@ void migrate_fd_connect(MigrationState *s);
void add_migration_state_change_notifier(Notifier *notify);
void remove_migration_state_change_notifier(Notifier *notify);
-int get_migration_state(void);
+bool migration_has_finished(void);
uint64_t ram_bytes_remaining(void);
uint64_t ram_bytes_transferred(void);
diff --git a/ui/spice-core.c b/ui/spice-core.c
index 3cbc721..1202993 100644
--- a/ui/spice-core.c
+++ b/ui/spice-core.c
@@ -447,9 +447,7 @@ void do_info_spice(Monitor *mon, QObject **ret_data)
static void migration_state_notifier(Notifier *notifier, void *data)
{
- int state = get_migration_state();
-
- if (state == MIG_STATE_COMPLETED) {
+ if (migration_has_finished()) {
#if SPICE_SERVER_VERSION >= 0x000701 /* 0.7.1 */
spice_server_migrate_switch(spice_server);
#endif
--
1.7.6.2
^ permalink raw reply related [flat|nested] 45+ messages in thread* Re: [Qemu-devel] [PATCH 19/23] migration: Export a function that tells if the migration has finished correctly
2011-09-23 12:57 ` [Qemu-devel] [PATCH 19/23] migration: Export a function that tells if the migration has finished correctly Juan Quintela
@ 2011-10-04 14:31 ` Anthony Liguori
2011-10-05 12:24 ` Juan Quintela
0 siblings, 1 reply; 45+ messages in thread
From: Anthony Liguori @ 2011-10-04 14:31 UTC (permalink / raw)
To: Juan Quintela; +Cc: qemu-devel
On 09/23/2011 07:57 AM, Juan Quintela wrote:
> This will allows us to hide the status values.
>
> Signed-off-by: Juan Quintela<quintela@redhat.com>
> ---
> migration.c | 4 ++--
> migration.h | 2 +-
> ui/spice-core.c | 4 +---
> 3 files changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/migration.c b/migration.c
> index ea50a6f..580f546 100644
> --- a/migration.c
> +++ b/migration.c
> @@ -358,9 +358,9 @@ void remove_migration_state_change_notifier(Notifier *notify)
> notifier_list_remove(&migration_state_notifiers, notify);
> }
>
> -int get_migration_state(void)
> +bool migration_has_finished(void)
> {
> - return current_migration->state;
> + return current_migration->state == MIG_STATE_COMPLETED;
> }
>
> void migrate_fd_connect(MigrationState *s)
> diff --git a/migration.h b/migration.h
> index f1a7452..6641a26 100644
> --- a/migration.h
> +++ b/migration.h
> @@ -84,7 +84,7 @@ void migrate_fd_connect(MigrationState *s);
>
> void add_migration_state_change_notifier(Notifier *notify);
> void remove_migration_state_change_notifier(Notifier *notify);
> -int get_migration_state(void);
> +bool migration_has_finished(void);
>
> uint64_t ram_bytes_remaining(void);
> uint64_t ram_bytes_transferred(void);
> diff --git a/ui/spice-core.c b/ui/spice-core.c
> index 3cbc721..1202993 100644
> --- a/ui/spice-core.c
> +++ b/ui/spice-core.c
> @@ -447,9 +447,7 @@ void do_info_spice(Monitor *mon, QObject **ret_data)
>
> static void migration_state_notifier(Notifier *notifier, void *data)
> {
> - int state = get_migration_state();
> -
> - if (state == MIG_STATE_COMPLETED) {
> + if (migration_has_finished()) {
> #if SPICE_SERVER_VERSION>= 0x000701 /* 0.7.1 */
> spice_server_migrate_switch(spice_server);
> #endif
I think the bug here is migration_state_notifier. It should take an additional
argument of MigrationState. Otherwise, how does this code work with FT?
Regards,
Anthony Liguori
^ permalink raw reply [flat|nested] 45+ messages in thread* Re: [Qemu-devel] [PATCH 19/23] migration: Export a function that tells if the migration has finished correctly
2011-10-04 14:31 ` Anthony Liguori
@ 2011-10-05 12:24 ` Juan Quintela
2011-10-05 14:58 ` Anthony Liguori
0 siblings, 1 reply; 45+ messages in thread
From: Juan Quintela @ 2011-10-05 12:24 UTC (permalink / raw)
To: Anthony Liguori; +Cc: qemu-devel
Anthony Liguori <anthony@codemonkey.ws> wrote:
> On 09/23/2011 07:57 AM, Juan Quintela wrote:
>> This will allows us to hide the status values.
>>
>> --- a/ui/spice-core.c
>> +++ b/ui/spice-core.c
>> @@ -447,9 +447,7 @@ void do_info_spice(Monitor *mon, QObject **ret_data)
>>
>> static void migration_state_notifier(Notifier *notifier, void *data)
>> {
>> - int state = get_migration_state();
>> -
>> - if (state == MIG_STATE_COMPLETED) {
>> + if (migration_has_finished()) {
>> #if SPICE_SERVER_VERSION>= 0x000701 /* 0.7.1 */
>> spice_server_migrate_switch(spice_server);
>> #endif
>
> I think the bug here is migration_state_notifier. It should take an
> additional argument of MigrationState. Otherwise, how does this code
> work with FT?
Thinking about it, we need to pass MigrationState and export the
function that see if migration has finished (otherwise we also need to
export all STATE definitions, or worse, the whole MigrationState
definition.
Moving to have a function
bool migration_has_finished(MIgrationState *s);
That does the obvious thing.
What do you think?
Later, Juan.
^ permalink raw reply [flat|nested] 45+ messages in thread* Re: [Qemu-devel] [PATCH 19/23] migration: Export a function that tells if the migration has finished correctly
2011-10-05 12:24 ` Juan Quintela
@ 2011-10-05 14:58 ` Anthony Liguori
0 siblings, 0 replies; 45+ messages in thread
From: Anthony Liguori @ 2011-10-05 14:58 UTC (permalink / raw)
To: quintela; +Cc: qemu-devel
On 10/05/2011 07:24 AM, Juan Quintela wrote:
> Anthony Liguori<anthony@codemonkey.ws> wrote:
>> On 09/23/2011 07:57 AM, Juan Quintela wrote:
>>> This will allows us to hide the status values.
>>>
>>> --- a/ui/spice-core.c
>>> +++ b/ui/spice-core.c
>>> @@ -447,9 +447,7 @@ void do_info_spice(Monitor *mon, QObject **ret_data)
>>>
>>> static void migration_state_notifier(Notifier *notifier, void *data)
>>> {
>>> - int state = get_migration_state();
>>> -
>>> - if (state == MIG_STATE_COMPLETED) {
>>> + if (migration_has_finished()) {
>>> #if SPICE_SERVER_VERSION>= 0x000701 /* 0.7.1 */
>>> spice_server_migrate_switch(spice_server);
>>> #endif
>>
>> I think the bug here is migration_state_notifier. It should take an
>> additional argument of MigrationState. Otherwise, how does this code
>> work with FT?
>
> Thinking about it, we need to pass MigrationState and export the
> function that see if migration has finished (otherwise we also need to
> export all STATE definitions, or worse, the whole MigrationState
> definition.
>
> Moving to have a function
>
> bool migration_has_finished(MIgrationState *s);
>
> That does the obvious thing.
>
> What do you think?
Yeah, that was what I was advocating.
Regards,
Anthony Liguori
>
> Later, Juan.
^ permalink raw reply [flat|nested] 45+ messages in thread
* [Qemu-devel] [PATCH 20/23] migration: Make state definitions local
2011-09-23 12:56 [Qemu-devel] [PATCH v3 00/23] Refactor and cleanup migration code Juan Quintela
` (18 preceding siblings ...)
2011-09-23 12:57 ` [Qemu-devel] [PATCH 19/23] migration: Export a function that tells if the migration has finished correctly Juan Quintela
@ 2011-09-23 12:57 ` Juan Quintela
2011-09-23 12:57 ` [Qemu-devel] [PATCH 21/23] migration: Don't use callback on file defining it Juan Quintela
` (3 subsequent siblings)
23 siblings, 0 replies; 45+ messages in thread
From: Juan Quintela @ 2011-09-23 12:57 UTC (permalink / raw)
To: qemu-devel
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
migration.c | 8 ++++++++
migration.h | 8 --------
2 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/migration.c b/migration.c
index 580f546..0213d5c 100644
--- a/migration.c
+++ b/migration.c
@@ -31,6 +31,14 @@
do { } while (0)
#endif
+enum migration_state {
+ MIG_STATE_ERROR,
+ MIG_STATE_NONE,
+ MIG_STATE_CANCELLED,
+ MIG_STATE_ACTIVE,
+ MIG_STATE_COMPLETED,
+};
+
#define MAX_THROTTLE (32 << 20) /* Migration speed throttling */
/* When we add fault tolerance, we could have several
diff --git a/migration.h b/migration.h
index 6641a26..2090e7f 100644
--- a/migration.h
+++ b/migration.h
@@ -18,14 +18,6 @@
#include "qemu-common.h"
#include "notify.h"
-enum migration_state {
- MIG_STATE_ERROR,
- MIG_STATE_NONE,
- MIG_STATE_CANCELLED,
- MIG_STATE_ACTIVE,
- MIG_STATE_COMPLETED,
-};
-
typedef struct MigrationState MigrationState;
struct MigrationState
--
1.7.6.2
^ permalink raw reply related [flat|nested] 45+ messages in thread* [Qemu-devel] [PATCH 21/23] migration: Don't use callback on file defining it
2011-09-23 12:56 [Qemu-devel] [PATCH v3 00/23] Refactor and cleanup migration code Juan Quintela
` (19 preceding siblings ...)
2011-09-23 12:57 ` [Qemu-devel] [PATCH 20/23] migration: Make state definitions local Juan Quintela
@ 2011-09-23 12:57 ` Juan Quintela
2011-10-04 14:32 ` Anthony Liguori
2011-09-23 12:57 ` [Qemu-devel] [PATCH 22/23] migration: propagate error correctly Juan Quintela
` (2 subsequent siblings)
23 siblings, 1 reply; 45+ messages in thread
From: Juan Quintela @ 2011-09-23 12:57 UTC (permalink / raw)
To: qemu-devel
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
migration-tcp.c | 4 ++--
migration-unix.c | 6 +++---
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/migration-tcp.c b/migration-tcp.c
index f6b2288..bd3aa3a 100644
--- a/migration-tcp.c
+++ b/migration-tcp.c
@@ -58,7 +58,7 @@ static void tcp_wait_for_connect(void *opaque)
DPRINTF("connect completed\n");
do {
ret = getsockopt(s->fd, SOL_SOCKET, SO_ERROR, (void *) &val, &valsize);
- } while (ret == -1 && (s->get_error(s)) == EINTR);
+ } while (ret == -1 && (socket_error()) == EINTR);
if (ret < 0) {
migrate_fd_error(s);
@@ -98,7 +98,7 @@ int tcp_start_outgoing_migration(MigrationState *s, const char *host_port)
do {
ret = connect(s->fd, (struct sockaddr *)&addr, sizeof(addr));
if (ret == -1)
- ret = -(s->get_error(s));
+ ret = -(socket_error());
if (ret == -EINPROGRESS || ret == -EWOULDBLOCK)
qemu_set_fd_handler2(s->fd, NULL, NULL, tcp_wait_for_connect, s);
diff --git a/migration-unix.c b/migration-unix.c
index d6f315a..7205b25 100644
--- a/migration-unix.c
+++ b/migration-unix.c
@@ -57,7 +57,7 @@ static void unix_wait_for_connect(void *opaque)
DPRINTF("connect completed\n");
do {
ret = getsockopt(s->fd, SOL_SOCKET, SO_ERROR, (void *) &val, &valsize);
- } while (ret == -1 && (s->get_error(s)) == EINTR);
+ } while (ret == -1 && errno == EINTR);
if (ret < 0) {
migrate_fd_error(s);
@@ -97,7 +97,7 @@ int unix_start_outgoing_migration(MigrationState *s, const char *path)
do {
ret = connect(s->fd, (struct sockaddr *)&addr, sizeof(addr));
if (ret == -1)
- ret = -(s->get_error(s));
+ ret = -errno;
if (ret == -EINPROGRESS || ret == -EWOULDBLOCK)
qemu_set_fd_handler2(s->fd, NULL, NULL, unix_wait_for_connect, s);
@@ -130,7 +130,7 @@ static void unix_accept_incoming_migration(void *opaque)
do {
c = qemu_accept(s, (struct sockaddr *)&addr, &addrlen);
- } while (c == -1 && socket_error() == EINTR);
+ } while (c == -1 && errno == EINTR);
DPRINTF("accepted migration\n");
--
1.7.6.2
^ permalink raw reply related [flat|nested] 45+ messages in thread* Re: [Qemu-devel] [PATCH 21/23] migration: Don't use callback on file defining it
2011-09-23 12:57 ` [Qemu-devel] [PATCH 21/23] migration: Don't use callback on file defining it Juan Quintela
@ 2011-10-04 14:32 ` Anthony Liguori
2011-10-04 23:50 ` Juan Quintela
0 siblings, 1 reply; 45+ messages in thread
From: Anthony Liguori @ 2011-10-04 14:32 UTC (permalink / raw)
To: Juan Quintela; +Cc: qemu-devel
On 09/23/2011 07:57 AM, Juan Quintela wrote:
> Signed-off-by: Juan Quintela<quintela@redhat.com>
I don't think this is better. It just eliminates the possibility of having
useful trace points in get_error.
Regards,
Anthony Liguori
> ---
> migration-tcp.c | 4 ++--
> migration-unix.c | 6 +++---
> 2 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/migration-tcp.c b/migration-tcp.c
> index f6b2288..bd3aa3a 100644
> --- a/migration-tcp.c
> +++ b/migration-tcp.c
> @@ -58,7 +58,7 @@ static void tcp_wait_for_connect(void *opaque)
> DPRINTF("connect completed\n");
> do {
> ret = getsockopt(s->fd, SOL_SOCKET, SO_ERROR, (void *)&val,&valsize);
> - } while (ret == -1&& (s->get_error(s)) == EINTR);
> + } while (ret == -1&& (socket_error()) == EINTR);
>
> if (ret< 0) {
> migrate_fd_error(s);
> @@ -98,7 +98,7 @@ int tcp_start_outgoing_migration(MigrationState *s, const char *host_port)
> do {
> ret = connect(s->fd, (struct sockaddr *)&addr, sizeof(addr));
> if (ret == -1)
> - ret = -(s->get_error(s));
> + ret = -(socket_error());
>
> if (ret == -EINPROGRESS || ret == -EWOULDBLOCK)
> qemu_set_fd_handler2(s->fd, NULL, NULL, tcp_wait_for_connect, s);
> diff --git a/migration-unix.c b/migration-unix.c
> index d6f315a..7205b25 100644
> --- a/migration-unix.c
> +++ b/migration-unix.c
> @@ -57,7 +57,7 @@ static void unix_wait_for_connect(void *opaque)
> DPRINTF("connect completed\n");
> do {
> ret = getsockopt(s->fd, SOL_SOCKET, SO_ERROR, (void *)&val,&valsize);
> - } while (ret == -1&& (s->get_error(s)) == EINTR);
> + } while (ret == -1&& errno == EINTR);
>
> if (ret< 0) {
> migrate_fd_error(s);
> @@ -97,7 +97,7 @@ int unix_start_outgoing_migration(MigrationState *s, const char *path)
> do {
> ret = connect(s->fd, (struct sockaddr *)&addr, sizeof(addr));
> if (ret == -1)
> - ret = -(s->get_error(s));
> + ret = -errno;
>
> if (ret == -EINPROGRESS || ret == -EWOULDBLOCK)
> qemu_set_fd_handler2(s->fd, NULL, NULL, unix_wait_for_connect, s);
> @@ -130,7 +130,7 @@ static void unix_accept_incoming_migration(void *opaque)
>
> do {
> c = qemu_accept(s, (struct sockaddr *)&addr,&addrlen);
> - } while (c == -1&& socket_error() == EINTR);
> + } while (c == -1&& errno == EINTR);
>
> DPRINTF("accepted migration\n");
>
^ permalink raw reply [flat|nested] 45+ messages in thread* Re: [Qemu-devel] [PATCH 21/23] migration: Don't use callback on file defining it
2011-10-04 14:32 ` Anthony Liguori
@ 2011-10-04 23:50 ` Juan Quintela
0 siblings, 0 replies; 45+ messages in thread
From: Juan Quintela @ 2011-10-04 23:50 UTC (permalink / raw)
To: Anthony Liguori; +Cc: qemu-devel
Anthony Liguori <anthony@codemonkey.ws> wrote:
> On 09/23/2011 07:57 AM, Juan Quintela wrote:
>> Signed-off-by: Juan Quintela<quintela@redhat.com>
>
> I don't think this is better. It just eliminates the possibility of
> having useful trace points in get_error.
It's use is not consistent at all (same places use get_error() and the
others the native ones).
And after this change, it gets clear what get_error() does, just four
uses:
migration.c:266: } while (ret == -1 && ((s->get_error(s)) == EINTR));
migration.c:269: ret = -(s->get_error(s));
migration.c:341: } while (ret == -1 && (s->get_error(s)) == EINTR);
migration.c:344: qemu_file_set_error(s->file, s->get_error(s));
The first two are the same one (just that we don't store the value in a
variable), same for the other two.
And everything uses errno for it, except migration-tcp on windows, that
is the only valid case for windows.
So, what we really need here is a way to get error of last system call,
abstracted by OS, not by migration method, and another call that we can
remove.
After the patch, this is easy to find (before it is more complicated).
So, patch stand and will search for a way to abstract for errno on
migration code. (just using socket_error() will work.
What do you think?
Later, Juan.
^ permalink raw reply [flat|nested] 45+ messages in thread
* [Qemu-devel] [PATCH 22/23] migration: propagate error correctly
2011-09-23 12:56 [Qemu-devel] [PATCH v3 00/23] Refactor and cleanup migration code Juan Quintela
` (20 preceding siblings ...)
2011-09-23 12:57 ` [Qemu-devel] [PATCH 21/23] migration: Don't use callback on file defining it Juan Quintela
@ 2011-09-23 12:57 ` Juan Quintela
2011-10-04 14:33 ` Anthony Liguori
2011-09-23 12:57 ` [Qemu-devel] [PATCH 23/23] migration: make migration-{tcp, unix} consistent Juan Quintela
2011-10-04 14:36 ` [Qemu-devel] [PATCH v3 00/23] Refactor and cleanup migration code Anthony Liguori
23 siblings, 1 reply; 45+ messages in thread
From: Juan Quintela @ 2011-09-23 12:57 UTC (permalink / raw)
To: qemu-devel
unix and tcp outgoing migration have error values, but didn't returned
it. Make them return the error. Notice that EINPROGRESS & EWOULDBLOCK
are not considered errors as callwill be retry later.
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
migration-tcp.c | 20 +++++++++++---------
migration-unix.c | 26 ++++++++++----------------
2 files changed, 21 insertions(+), 25 deletions(-)
diff --git a/migration-tcp.c b/migration-tcp.c
index bd3aa3a..619df21 100644
--- a/migration-tcp.c
+++ b/migration-tcp.c
@@ -90,26 +90,28 @@ int tcp_start_outgoing_migration(MigrationState *s, const char *host_port)
s->fd = qemu_socket(PF_INET, SOCK_STREAM, 0);
if (s->fd == -1) {
- return -1;
+ return -socket_error();
}
socket_set_nonblock(s->fd);
do {
ret = connect(s->fd, (struct sockaddr *)&addr, sizeof(addr));
- if (ret == -1)
- ret = -(socket_error());
-
- if (ret == -EINPROGRESS || ret == -EWOULDBLOCK)
+ if (ret == -1) {
+ ret = -socket_error();
+ }
+ if (ret == -EINPROGRESS || ret == -EWOULDBLOCK) {
qemu_set_fd_handler2(s->fd, NULL, NULL, tcp_wait_for_connect, s);
+ return 0;
+ }
} while (ret == -EINTR);
- if (ret < 0 && ret != -EINPROGRESS && ret != -EWOULDBLOCK) {
+ if (ret < 0) {
DPRINTF("connect failed\n");
migrate_fd_error(s);
- } else if (ret >= 0)
- migrate_fd_connect(s);
-
+ return ret;
+ }
+ migrate_fd_connect(s);
return 0;
}
diff --git a/migration-unix.c b/migration-unix.c
index 7205b25..428fe66 100644
--- a/migration-unix.c
+++ b/migration-unix.c
@@ -89,35 +89,29 @@ int unix_start_outgoing_migration(MigrationState *s, const char *path)
s->fd = qemu_socket(PF_UNIX, SOCK_STREAM, 0);
if (s->fd < 0) {
DPRINTF("Unable to open socket");
- goto err_after_socket;
+ return -errno;
}
socket_set_nonblock(s->fd);
do {
ret = connect(s->fd, (struct sockaddr *)&addr, sizeof(addr));
- if (ret == -1)
+ if (ret == -1) {
ret = -errno;
-
- if (ret == -EINPROGRESS || ret == -EWOULDBLOCK)
+ }
+ if (ret == -EINPROGRESS || ret == -EWOULDBLOCK) {
qemu_set_fd_handler2(s->fd, NULL, NULL, unix_wait_for_connect, s);
+ return 0;
+ }
} while (ret == -EINTR);
- if (ret < 0 && ret != -EINPROGRESS && ret != -EWOULDBLOCK) {
+ if (ret < 0) {
DPRINTF("connect failed\n");
- goto err_after_open;
+ migrate_fd_error(s);
+ return ret;
}
-
- if (ret >= 0)
- migrate_fd_connect(s);
-
+ migrate_fd_connect(s);
return 0;
-
-err_after_open:
- close(s->fd);
-
-err_after_socket:
- return -1;
}
static void unix_accept_incoming_migration(void *opaque)
--
1.7.6.2
^ permalink raw reply related [flat|nested] 45+ messages in thread* Re: [Qemu-devel] [PATCH 22/23] migration: propagate error correctly
2011-09-23 12:57 ` [Qemu-devel] [PATCH 22/23] migration: propagate error correctly Juan Quintela
@ 2011-10-04 14:33 ` Anthony Liguori
0 siblings, 0 replies; 45+ messages in thread
From: Anthony Liguori @ 2011-10-04 14:33 UTC (permalink / raw)
To: Juan Quintela; +Cc: qemu-devel
On 09/23/2011 07:57 AM, Juan Quintela wrote:
> unix and tcp outgoing migration have error values, but didn't returned
> it. Make them return the error. Notice that EINPROGRESS& EWOULDBLOCK
> are not considered errors as callwill be retry later.
>
> Signed-off-by: Juan Quintela<quintela@redhat.com>
Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>
Regards,
Anthony Liguori
> ---
> migration-tcp.c | 20 +++++++++++---------
> migration-unix.c | 26 ++++++++++----------------
> 2 files changed, 21 insertions(+), 25 deletions(-)
>
> diff --git a/migration-tcp.c b/migration-tcp.c
> index bd3aa3a..619df21 100644
> --- a/migration-tcp.c
> +++ b/migration-tcp.c
> @@ -90,26 +90,28 @@ int tcp_start_outgoing_migration(MigrationState *s, const char *host_port)
>
> s->fd = qemu_socket(PF_INET, SOCK_STREAM, 0);
> if (s->fd == -1) {
> - return -1;
> + return -socket_error();
> }
>
> socket_set_nonblock(s->fd);
>
> do {
> ret = connect(s->fd, (struct sockaddr *)&addr, sizeof(addr));
> - if (ret == -1)
> - ret = -(socket_error());
> -
> - if (ret == -EINPROGRESS || ret == -EWOULDBLOCK)
> + if (ret == -1) {
> + ret = -socket_error();
> + }
> + if (ret == -EINPROGRESS || ret == -EWOULDBLOCK) {
> qemu_set_fd_handler2(s->fd, NULL, NULL, tcp_wait_for_connect, s);
> + return 0;
> + }
> } while (ret == -EINTR);
>
> - if (ret< 0&& ret != -EINPROGRESS&& ret != -EWOULDBLOCK) {
> + if (ret< 0) {
> DPRINTF("connect failed\n");
> migrate_fd_error(s);
> - } else if (ret>= 0)
> - migrate_fd_connect(s);
> -
> + return ret;
> + }
> + migrate_fd_connect(s);
> return 0;
> }
>
> diff --git a/migration-unix.c b/migration-unix.c
> index 7205b25..428fe66 100644
> --- a/migration-unix.c
> +++ b/migration-unix.c
> @@ -89,35 +89,29 @@ int unix_start_outgoing_migration(MigrationState *s, const char *path)
> s->fd = qemu_socket(PF_UNIX, SOCK_STREAM, 0);
> if (s->fd< 0) {
> DPRINTF("Unable to open socket");
> - goto err_after_socket;
> + return -errno;
> }
>
> socket_set_nonblock(s->fd);
>
> do {
> ret = connect(s->fd, (struct sockaddr *)&addr, sizeof(addr));
> - if (ret == -1)
> + if (ret == -1) {
> ret = -errno;
> -
> - if (ret == -EINPROGRESS || ret == -EWOULDBLOCK)
> + }
> + if (ret == -EINPROGRESS || ret == -EWOULDBLOCK) {
> qemu_set_fd_handler2(s->fd, NULL, NULL, unix_wait_for_connect, s);
> + return 0;
> + }
> } while (ret == -EINTR);
>
> - if (ret< 0&& ret != -EINPROGRESS&& ret != -EWOULDBLOCK) {
> + if (ret< 0) {
> DPRINTF("connect failed\n");
> - goto err_after_open;
> + migrate_fd_error(s);
> + return ret;
> }
> -
> - if (ret>= 0)
> - migrate_fd_connect(s);
> -
> + migrate_fd_connect(s);
> return 0;
> -
> -err_after_open:
> - close(s->fd);
> -
> -err_after_socket:
> - return -1;
> }
>
> static void unix_accept_incoming_migration(void *opaque)
^ permalink raw reply [flat|nested] 45+ messages in thread
* [Qemu-devel] [PATCH 23/23] migration: make migration-{tcp, unix} consistent
2011-09-23 12:56 [Qemu-devel] [PATCH v3 00/23] Refactor and cleanup migration code Juan Quintela
` (21 preceding siblings ...)
2011-09-23 12:57 ` [Qemu-devel] [PATCH 22/23] migration: propagate error correctly Juan Quintela
@ 2011-09-23 12:57 ` Juan Quintela
2011-10-04 14:34 ` Anthony Liguori
2011-10-04 14:36 ` [Qemu-devel] [PATCH v3 00/23] Refactor and cleanup migration code Anthony Liguori
23 siblings, 1 reply; 45+ messages in thread
From: Juan Quintela @ 2011-09-23 12:57 UTC (permalink / raw)
To: qemu-devel
Files are almost identical in functionality, just remove the
differences that make no sense.
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
migration-tcp.c | 15 ++++++++++-----
migration-unix.c | 46 +++++++++++++++++++++++++---------------------
2 files changed, 35 insertions(+), 26 deletions(-)
diff --git a/migration-tcp.c b/migration-tcp.c
index 619df21..5aa742c 100644
--- a/migration-tcp.c
+++ b/migration-tcp.c
@@ -48,7 +48,6 @@ static int tcp_close(MigrationState *s)
return 0;
}
-
static void tcp_wait_for_connect(void *opaque)
{
MigrationState *s = opaque;
@@ -84,12 +83,14 @@ int tcp_start_outgoing_migration(MigrationState *s, const char *host_port)
if (ret < 0) {
return ret;
}
+
s->get_error = socket_errno;
s->write = socket_write;
s->close = tcp_close;
s->fd = qemu_socket(PF_INET, SOCK_STREAM, 0);
if (s->fd == -1) {
+ DPRINTF("Unable to open socket");
return -socket_error();
}
@@ -155,23 +156,27 @@ int tcp_start_incoming_migration(const char *host_port)
int val;
int s;
+ DPRINTF("Attempting to start an incoming migration\n");
+
if (parse_host_port(&addr, host_port) < 0) {
fprintf(stderr, "invalid host/port combination: %s\n", host_port);
return -EINVAL;
}
s = qemu_socket(PF_INET, SOCK_STREAM, 0);
- if (s == -1)
+ if (s == -1) {
return -socket_error();
+ }
val = 1;
setsockopt(s, SOL_SOCKET, SO_REUSEADDR, (const char *)&val, sizeof(val));
- if (bind(s, (struct sockaddr *)&addr, sizeof(addr)) == -1)
+ if (bind(s, (struct sockaddr *)&addr, sizeof(addr)) == -1) {
goto err;
-
- if (listen(s, 1) == -1)
+ }
+ if (listen(s, 1) == -1) {
goto err;
+ }
qemu_set_fd_handler2(s, NULL, tcp_accept_incoming_migration, NULL,
(void *)(intptr_t)s);
diff --git a/migration-unix.c b/migration-unix.c
index 428fe66..f9d34b9 100644
--- a/migration-unix.c
+++ b/migration-unix.c
@@ -87,7 +87,7 @@ int unix_start_outgoing_migration(MigrationState *s, const char *path)
s->close = unix_close;
s->fd = qemu_socket(PF_UNIX, SOCK_STREAM, 0);
- if (s->fd < 0) {
+ if (s->fd == -1) {
DPRINTF("Unable to open socket");
return -errno;
}
@@ -130,7 +130,7 @@ static void unix_accept_incoming_migration(void *opaque)
if (c == -1) {
fprintf(stderr, "could not accept migration connection\n");
- return;
+ goto out2;
}
f = qemu_fopen_socket(c);
@@ -142,45 +142,49 @@ static void unix_accept_incoming_migration(void *opaque)
process_incoming_migration(f);
qemu_fclose(f);
out:
+ close(c);
+out2:
qemu_set_fd_handler2(s, NULL, NULL, NULL, NULL);
close(s);
- close(c);
}
int unix_start_incoming_migration(const char *path)
{
- struct sockaddr_un un;
- int sock;
+ struct sockaddr_un addr;
+ int s;
+ int ret;
DPRINTF("Attempting to start an incoming migration\n");
- sock = qemu_socket(PF_UNIX, SOCK_STREAM, 0);
- if (sock < 0) {
+ s = qemu_socket(PF_UNIX, SOCK_STREAM, 0);
+ if (s == -1) {
fprintf(stderr, "Could not open unix socket: %s\n", strerror(errno));
- return -EINVAL;
+ return -errno;
}
- memset(&un, 0, sizeof(un));
- un.sun_family = AF_UNIX;
- snprintf(un.sun_path, sizeof(un.sun_path), "%s", path);
+ memset(&addr, 0, sizeof(addr));
+ addr.sun_family = AF_UNIX;
+ snprintf(addr.sun_path, sizeof(addr.sun_path), "%s", path);
- unlink(un.sun_path);
- if (bind(sock, (struct sockaddr*) &un, sizeof(un)) < 0) {
- fprintf(stderr, "bind(unix:%s): %s\n", un.sun_path, strerror(errno));
+ unlink(addr.sun_path);
+ if (bind(s, (struct sockaddr *) &addr, sizeof(addr)) < 0) {
+ ret = -errno;
+ fprintf(stderr, "bind(unix:%s): %s\n", addr.sun_path, strerror(errno));
goto err;
}
- if (listen(sock, 1) < 0) {
- fprintf(stderr, "listen(unix:%s): %s\n", un.sun_path, strerror(errno));
+ if (listen(s, 1) == -1) {
+ fprintf(stderr, "listen(unix:%s): %s\n", addr.sun_path,
+ strerror(errno));
+ ret = -errno;
goto err;
}
- qemu_set_fd_handler2(sock, NULL, unix_accept_incoming_migration, NULL,
- (void *)(intptr_t)sock);
+ qemu_set_fd_handler2(s, NULL, unix_accept_incoming_migration, NULL,
+ (void *)(intptr_t)s);
return 0;
err:
- close(sock);
-
- return -EINVAL;
+ close(s);
+ return ret;
}
--
1.7.6.2
^ permalink raw reply related [flat|nested] 45+ messages in thread* Re: [Qemu-devel] [PATCH 23/23] migration: make migration-{tcp, unix} consistent
2011-09-23 12:57 ` [Qemu-devel] [PATCH 23/23] migration: make migration-{tcp, unix} consistent Juan Quintela
@ 2011-10-04 14:34 ` Anthony Liguori
0 siblings, 0 replies; 45+ messages in thread
From: Anthony Liguori @ 2011-10-04 14:34 UTC (permalink / raw)
To: Juan Quintela; +Cc: qemu-devel
On 09/23/2011 07:57 AM, Juan Quintela wrote:
> Files are almost identical in functionality, just remove the
> differences that make no sense.
>
> Signed-off-by: Juan Quintela<quintela@redhat.com>
Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>
Regards,
Anthony Liguori
> ---
> migration-tcp.c | 15 ++++++++++-----
> migration-unix.c | 46 +++++++++++++++++++++++++---------------------
> 2 files changed, 35 insertions(+), 26 deletions(-)
>
> diff --git a/migration-tcp.c b/migration-tcp.c
> index 619df21..5aa742c 100644
> --- a/migration-tcp.c
> +++ b/migration-tcp.c
> @@ -48,7 +48,6 @@ static int tcp_close(MigrationState *s)
> return 0;
> }
>
> -
> static void tcp_wait_for_connect(void *opaque)
> {
> MigrationState *s = opaque;
> @@ -84,12 +83,14 @@ int tcp_start_outgoing_migration(MigrationState *s, const char *host_port)
> if (ret< 0) {
> return ret;
> }
> +
> s->get_error = socket_errno;
> s->write = socket_write;
> s->close = tcp_close;
>
> s->fd = qemu_socket(PF_INET, SOCK_STREAM, 0);
> if (s->fd == -1) {
> + DPRINTF("Unable to open socket");
> return -socket_error();
> }
>
> @@ -155,23 +156,27 @@ int tcp_start_incoming_migration(const char *host_port)
> int val;
> int s;
>
> + DPRINTF("Attempting to start an incoming migration\n");
> +
> if (parse_host_port(&addr, host_port)< 0) {
> fprintf(stderr, "invalid host/port combination: %s\n", host_port);
> return -EINVAL;
> }
>
> s = qemu_socket(PF_INET, SOCK_STREAM, 0);
> - if (s == -1)
> + if (s == -1) {
> return -socket_error();
> + }
>
> val = 1;
> setsockopt(s, SOL_SOCKET, SO_REUSEADDR, (const char *)&val, sizeof(val));
>
> - if (bind(s, (struct sockaddr *)&addr, sizeof(addr)) == -1)
> + if (bind(s, (struct sockaddr *)&addr, sizeof(addr)) == -1) {
> goto err;
> -
> - if (listen(s, 1) == -1)
> + }
> + if (listen(s, 1) == -1) {
> goto err;
> + }
>
> qemu_set_fd_handler2(s, NULL, tcp_accept_incoming_migration, NULL,
> (void *)(intptr_t)s);
> diff --git a/migration-unix.c b/migration-unix.c
> index 428fe66..f9d34b9 100644
> --- a/migration-unix.c
> +++ b/migration-unix.c
> @@ -87,7 +87,7 @@ int unix_start_outgoing_migration(MigrationState *s, const char *path)
> s->close = unix_close;
>
> s->fd = qemu_socket(PF_UNIX, SOCK_STREAM, 0);
> - if (s->fd< 0) {
> + if (s->fd == -1) {
> DPRINTF("Unable to open socket");
> return -errno;
> }
> @@ -130,7 +130,7 @@ static void unix_accept_incoming_migration(void *opaque)
>
> if (c == -1) {
> fprintf(stderr, "could not accept migration connection\n");
> - return;
> + goto out2;
> }
>
> f = qemu_fopen_socket(c);
> @@ -142,45 +142,49 @@ static void unix_accept_incoming_migration(void *opaque)
> process_incoming_migration(f);
> qemu_fclose(f);
> out:
> + close(c);
> +out2:
> qemu_set_fd_handler2(s, NULL, NULL, NULL, NULL);
> close(s);
> - close(c);
> }
>
> int unix_start_incoming_migration(const char *path)
> {
> - struct sockaddr_un un;
> - int sock;
> + struct sockaddr_un addr;
> + int s;
> + int ret;
>
> DPRINTF("Attempting to start an incoming migration\n");
>
> - sock = qemu_socket(PF_UNIX, SOCK_STREAM, 0);
> - if (sock< 0) {
> + s = qemu_socket(PF_UNIX, SOCK_STREAM, 0);
> + if (s == -1) {
> fprintf(stderr, "Could not open unix socket: %s\n", strerror(errno));
> - return -EINVAL;
> + return -errno;
> }
>
> - memset(&un, 0, sizeof(un));
> - un.sun_family = AF_UNIX;
> - snprintf(un.sun_path, sizeof(un.sun_path), "%s", path);
> + memset(&addr, 0, sizeof(addr));
> + addr.sun_family = AF_UNIX;
> + snprintf(addr.sun_path, sizeof(addr.sun_path), "%s", path);
>
> - unlink(un.sun_path);
> - if (bind(sock, (struct sockaddr*)&un, sizeof(un))< 0) {
> - fprintf(stderr, "bind(unix:%s): %s\n", un.sun_path, strerror(errno));
> + unlink(addr.sun_path);
> + if (bind(s, (struct sockaddr *)&addr, sizeof(addr))< 0) {
> + ret = -errno;
> + fprintf(stderr, "bind(unix:%s): %s\n", addr.sun_path, strerror(errno));
> goto err;
> }
> - if (listen(sock, 1)< 0) {
> - fprintf(stderr, "listen(unix:%s): %s\n", un.sun_path, strerror(errno));
> + if (listen(s, 1) == -1) {
> + fprintf(stderr, "listen(unix:%s): %s\n", addr.sun_path,
> + strerror(errno));
> + ret = -errno;
> goto err;
> }
>
> - qemu_set_fd_handler2(sock, NULL, unix_accept_incoming_migration, NULL,
> - (void *)(intptr_t)sock);
> + qemu_set_fd_handler2(s, NULL, unix_accept_incoming_migration, NULL,
> + (void *)(intptr_t)s);
>
> return 0;
>
> err:
> - close(sock);
> -
> - return -EINVAL;
> + close(s);
> + return ret;
> }
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [Qemu-devel] [PATCH v3 00/23] Refactor and cleanup migration code
2011-09-23 12:56 [Qemu-devel] [PATCH v3 00/23] Refactor and cleanup migration code Juan Quintela
` (22 preceding siblings ...)
2011-09-23 12:57 ` [Qemu-devel] [PATCH 23/23] migration: make migration-{tcp, unix} consistent Juan Quintela
@ 2011-10-04 14:36 ` Anthony Liguori
23 siblings, 0 replies; 45+ messages in thread
From: Anthony Liguori @ 2011-10-04 14:36 UTC (permalink / raw)
To: Juan Quintela; +Cc: qemu-devel
I peeled off the first 4 patches to apply. Once I finish testing I'll push them.
Most of the rest of the series looks good. I really don't like making
MigrationState nullable with MIG_STATE_NONE. I don't think it makes sense with
multiple MigrationStates.
The direct use of globals is not something I'm happy about, but I'd be inclined
to hold my nose and apply if it weren't for MIG_STATE_NONE.
Regards,
Anthony Liguori
On 09/23/2011 07:56 AM, Juan Quintela wrote:
> Hi
>
> v4:
>
> rebase on top of new qemu and new migration-errors series
>
>
> v3:
> this patch applies on top of my previous "migration error"
> patches. All error handling has been moved to that series,
> except for "propagate error correctly", without this
> refactoring, it is quite complicated to apply it.
>
> Please, review.
>
> Later, Juan.
>
> v3:
> - more checkpatch.pl happines
> - split error handling in a previous series
> - make Anthony happy. current_migration is still a pointer, but points to
> a static variable. We can change current_migration when we integrate
> kemari.
>
> v2:
> - make Jan^Wcheckpatch.pl happy
> - Yoshiaki Tamura suggestions:
> - include its two patches to clean things
> - MAX_THROTTLE define
> - migration_state enum
> - I removed spurious differences between migration-{tcp,unix}
> - better error propagation, after this patch:
> migrate -d "tcp:name_don_exist:port"
> migrate -d "tcp:name:port_dont_exist"
> migrate -d "exec: prog_dont_exist"
> migrate -d "exec: gzip> /path/dont/exist"
> fail as expected. Last two used to enter an infinite loop.
>
> The fixes part should be backported to 0.14, waiting for the review to do that.
>
> Later, Juan.
>
> v1:
> This series:
> - Fold MigrationState into FdMigrationState (and then rename)
> - Factorize migration statec creation in a single place
> - Make use of MIG_STATE_*, setup through helpers and make them local
> - remove relase& cancel callbacks (where used only one in same
> file than defined)
> - get_status() is no more, just access directly to .state
> - current_migration use cleanup, and make variable static
> - max_throotle is gone, now inside current_migration
> - change get_migration_status() to migration_has_finished()
> and actualize single user.
>
> Please review.
>
> Later, Juan.
>
> Juan Quintela (23):
> migration: Make *start_outgoing_migration return FdMigrationState
> migration: Use FdMigrationState instead of MigrationState when
> possible
> migration: Fold MigrationState into FdMigrationState
> migration: Rename FdMigrationState MigrationState
> migration: Refactor MigrationState creation
> migration: Make all posible migration functions static
> migration: move migrate_create_state to do_migrate
> migration: Introduce MIG_STATE_NONE
> migration: Refactor and simplify error checking in
> migrate_fd_put_ready
> migration: Introduce migrate_fd_completed() for consistency
> migration: Our release callback was just free
> migration: Remove get_status() accessor
> migration: Remove migration cancel() callback
> migration: Move exported functions to the end of the file
> migration: use global variable directly
> migration: another case of global variable assigned to local one
> migration: make sure we always have a migration state
> migration: Use bandwidth_limit directly
> migration: Export a function that tells if the migration has finished
> correctly
> migration: Make state definitions local
> migration: Don't use callback on file defining it
> migration: propagate error correctly
> migration: make migration-{tcp,unix} consistent
>
> migration-exec.c | 39 +----
> migration-fd.c | 42 ++-----
> migration-tcp.c | 76 ++++------
> migration-unix.c | 112 ++++++---------
> migration.c | 404 ++++++++++++++++++++++++++---------------------------
> migration.h | 85 ++----------
> ui/spice-core.c | 4 +-
> 7 files changed, 303 insertions(+), 459 deletions(-)
>
^ permalink raw reply [flat|nested] 45+ messages in thread