* [Qemu-devel] [PATCH 05/37] migration: If there is one error, it makes no sense to continue
2011-10-20 0:11 [Qemu-devel] [PATCH v5 00/37] Migration errors & cleanup (the integrated version) Juan Quintela
@ 2011-10-20 0:11 ` Juan Quintela
2011-10-20 0:11 ` [Qemu-devel] [PATCH 10/37] migration: change has_error to contain errno values Juan Quintela
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Juan Quintela @ 2011-10-20 0:11 UTC (permalink / raw)
To: qemu-devel
Once there, add a comment about what each error mean.
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
buffered_file.c | 12 +++++++++---
1 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/buffered_file.c b/buffered_file.c
index 486af57..94ecbbc 100644
--- a/buffered_file.c
+++ b/buffered_file.c
@@ -189,13 +189,19 @@ static int buffered_close(void *opaque)
return ret;
}
+/*
+ * The meaning of the return values is:
+ * 0: We can continue sending
+ * 1: Time to stop
+ * -1: There has been an error
+ */
static int buffered_rate_limit(void *opaque)
{
QEMUFileBuffered *s = opaque;
- if (s->has_error)
- return 0;
-
+ if (s->has_error) {
+ return -1;
+ }
if (s->freeze_output)
return 1;
--
1.7.6.4
^ permalink raw reply related [flat|nested] 6+ messages in thread* [Qemu-devel] [PATCH 10/37] migration: change has_error to contain errno values
2011-10-20 0:11 [Qemu-devel] [PATCH v5 00/37] Migration errors & cleanup (the integrated version) Juan Quintela
2011-10-20 0:11 ` [Qemu-devel] [PATCH 05/37] migration: If there is one error, it makes no sense to continue Juan Quintela
@ 2011-10-20 0:11 ` Juan Quintela
2011-10-20 0:11 ` [Qemu-devel] [PATCH 14/37] migration: use qemu_file_get_error() return value when possible Juan Quintela
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Juan Quintela @ 2011-10-20 0:11 UTC (permalink / raw)
To: qemu-devel
We normally already have an errno value. When not, abuse EIO.
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
arch_init.c | 2 +-
block-migration.c | 11 ++++++-----
buffered_file.c | 4 ++--
hw/hw.h | 2 +-
migration.c | 2 +-
savevm.c | 8 ++++----
6 files changed, 15 insertions(+), 14 deletions(-)
diff --git a/arch_init.c b/arch_init.c
index a6c69c7..941d585 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -263,7 +263,7 @@ int ram_save_live(Monitor *mon, QEMUFile *f, int stage, void *opaque)
}
if (cpu_physical_sync_dirty_bitmap(0, TARGET_PHYS_ADDR_MAX) != 0) {
- qemu_file_set_error(f);
+ qemu_file_set_error(f, -EINVAL);
return 0;
}
diff --git a/block-migration.c b/block-migration.c
index e2775ee..325c905 100644
--- a/block-migration.c
+++ b/block-migration.c
@@ -263,7 +263,7 @@ static int mig_save_device_bulk(Monitor *mon, QEMUFile *f,
error:
monitor_printf(mon, "Error reading sector %" PRId64 "\n", cur_sector);
- qemu_file_set_error(f);
+ qemu_file_set_error(f, -EIO);
g_free(blk->buf);
g_free(blk);
return 0;
@@ -383,6 +383,7 @@ static int mig_save_device_dirty(Monitor *mon, QEMUFile *f,
int64_t total_sectors = bmds->total_sectors;
int64_t sector;
int nr_sectors;
+ int ret = -EIO;
for (sector = bmds->cur_dirty; sector < bmds->total_sectors;) {
if (bmds_aio_inflight(bmds, sector)) {
@@ -418,8 +419,8 @@ static int mig_save_device_dirty(Monitor *mon, QEMUFile *f,
block_mig_state.submitted++;
bmds_set_aio_inflight(bmds, sector, nr_sectors, 1);
} else {
- if (bdrv_read(bmds->bs, sector, blk->buf,
- nr_sectors) < 0) {
+ ret = bdrv_read(bmds->bs, sector, blk->buf, nr_sectors);
+ if (ret < 0) {
goto error;
}
blk_send(f, blk);
@@ -439,7 +440,7 @@ static int mig_save_device_dirty(Monitor *mon, QEMUFile *f,
error:
monitor_printf(mon, "Error reading sector %" PRId64 "\n", sector);
- qemu_file_set_error(f);
+ qemu_file_set_error(f, ret);
g_free(blk->buf);
g_free(blk);
return 0;
@@ -473,7 +474,7 @@ static void flush_blks(QEMUFile* f)
break;
}
if (blk->ret < 0) {
- qemu_file_set_error(f);
+ qemu_file_set_error(f, blk->ret);
break;
}
blk_send(f, blk);
diff --git a/buffered_file.c b/buffered_file.c
index 4f49763..94ca8d1 100644
--- a/buffered_file.c
+++ b/buffered_file.c
@@ -92,7 +92,7 @@ static void buffered_flush(QEMUFileBuffered *s)
if (ret <= 0) {
DPRINTF("error flushing data, %zd\n", ret);
- qemu_file_set_error(s->file);
+ qemu_file_set_error(s->file, ret);
break;
} else {
DPRINTF("flushed %zd byte(s)\n", ret);
@@ -138,7 +138,7 @@ static int buffered_put_buffer(void *opaque, const uint8_t *buf, int64_t pos, in
if (ret <= 0) {
DPRINTF("error putting\n");
- qemu_file_set_error(s->file);
+ qemu_file_set_error(s->file, ret);
offset = -EINVAL;
break;
}
diff --git a/hw/hw.h b/hw/hw.h
index a124da9..6cf8cd2 100644
--- a/hw/hw.h
+++ b/hw/hw.h
@@ -86,7 +86,7 @@ int qemu_file_rate_limit(QEMUFile *f);
int64_t qemu_file_set_rate_limit(QEMUFile *f, int64_t new_rate);
int64_t qemu_file_get_rate_limit(QEMUFile *f);
int qemu_file_has_error(QEMUFile *f);
-void qemu_file_set_error(QEMUFile *f);
+void qemu_file_set_error(QEMUFile *f, int error);
/* Try to send any outstanding data. This function is useful when output is
* halted due to rate limiting or EAGAIN errors occur as it can be used to
diff --git a/migration.c b/migration.c
index a682168..d5876a9 100644
--- a/migration.c
+++ b/migration.c
@@ -455,7 +455,7 @@ void migrate_fd_wait_for_unfreeze(void *opaque)
} while (ret == -1 && (s->get_error(s)) == EINTR);
if (ret == -1) {
- qemu_file_set_error(s->file);
+ qemu_file_set_error(s->file, -s->get_error(s));
}
}
diff --git a/savevm.c b/savevm.c
index bf4d0e7..8f00f0c 100644
--- a/savevm.c
+++ b/savevm.c
@@ -430,9 +430,9 @@ int qemu_file_has_error(QEMUFile *f)
return f->has_error;
}
-void qemu_file_set_error(QEMUFile *f)
+void qemu_file_set_error(QEMUFile *f, int ret)
{
- f->has_error = 1;
+ f->has_error = ret;
}
void qemu_fflush(QEMUFile *f)
@@ -447,7 +447,7 @@ void qemu_fflush(QEMUFile *f)
if (len > 0)
f->buf_offset += f->buf_index;
else
- f->has_error = 1;
+ f->has_error = -EINVAL;
f->buf_index = 0;
}
}
@@ -468,7 +468,7 @@ static void qemu_fill_buffer(QEMUFile *f)
f->buf_size = len;
f->buf_offset += len;
} else if (len != -EAGAIN)
- f->has_error = 1;
+ f->has_error = len;
}
int qemu_fclose(QEMUFile *f)
--
1.7.6.4
^ permalink raw reply related [flat|nested] 6+ messages in thread* [Qemu-devel] [PATCH 14/37] migration: use qemu_file_get_error() return value when possible
2011-10-20 0:11 [Qemu-devel] [PATCH v5 00/37] Migration errors & cleanup (the integrated version) Juan Quintela
2011-10-20 0:11 ` [Qemu-devel] [PATCH 05/37] migration: If there is one error, it makes no sense to continue Juan Quintela
2011-10-20 0:11 ` [Qemu-devel] [PATCH 10/37] migration: change has_error to contain errno values Juan Quintela
@ 2011-10-20 0:11 ` Juan Quintela
2011-10-20 0:11 ` [Qemu-devel] [PATCH 15/37] migration: make *save_live return errors Juan Quintela
2011-10-20 0:11 ` [Qemu-devel] [PATCH 23/37] migration: Introduce MIG_STATE_SETUP Juan Quintela
4 siblings, 0 replies; 6+ messages in thread
From: Juan Quintela @ 2011-10-20 0:11 UTC (permalink / raw)
To: qemu-devel
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
arch_init.c | 6 ++++--
block-migration.c | 7 ++++---
buffered_file.c | 23 ++++++++++++++---------
savevm.c | 4 ++--
4 files changed, 24 insertions(+), 16 deletions(-)
diff --git a/arch_init.c b/arch_init.c
index 9128be0..98daaf3 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -371,6 +371,7 @@ int ram_load(QEMUFile *f, void *opaque, int version_id)
{
ram_addr_t addr;
int flags;
+ int error;
if (version_id < 3 || version_id > 4) {
return -EINVAL;
@@ -451,8 +452,9 @@ int ram_load(QEMUFile *f, void *opaque, int version_id)
qemu_get_buffer(f, host, TARGET_PAGE_SIZE);
}
- if (qemu_file_get_error(f)) {
- return -EIO;
+ error = qemu_file_get_error(f);
+ if (error) {
+ return error;
}
} while (!(flags & RAM_SAVE_FLAG_EOS));
diff --git a/block-migration.c b/block-migration.c
index 56907a6..b8d19a1 100644
--- a/block-migration.c
+++ b/block-migration.c
@@ -647,6 +647,7 @@ static int block_load(QEMUFile *f, void *opaque, int version_id)
uint8_t *buf;
int64_t total_sectors = 0;
int nr_sectors;
+ int ret;
do {
addr = qemu_get_be64(f);
@@ -655,7 +656,6 @@ static int block_load(QEMUFile *f, void *opaque, int version_id)
addr >>= BDRV_SECTOR_BITS;
if (flags & BLK_MIG_FLAG_DEVICE_BLOCK) {
- int ret;
/* get device name */
len = qemu_get_byte(f);
qemu_get_buffer(f, (uint8_t *)device_name, len);
@@ -705,8 +705,9 @@ static int block_load(QEMUFile *f, void *opaque, int version_id)
fprintf(stderr, "Unknown flags\n");
return -EINVAL;
}
- if (qemu_file_get_error(f)) {
- return -EIO;
+ ret = qemu_file_get_error(f);
+ if (ret != 0) {
+ return ret;
}
} while (!(flags & BLK_MIG_FLAG_EOS));
diff --git a/buffered_file.c b/buffered_file.c
index 41c659c..fed9a22 100644
--- a/buffered_file.c
+++ b/buffered_file.c
@@ -71,9 +71,11 @@ static void buffered_append(QEMUFileBuffered *s,
static void buffered_flush(QEMUFileBuffered *s)
{
size_t offset = 0;
+ int error;
- if (qemu_file_get_error(s->file)) {
- DPRINTF("flush when error, bailing\n");
+ error = qemu_file_get_error(s->file);
+ if (error != 0) {
+ DPRINTF("flush when error, bailing: %s\n", strerror(-error));
return;
}
@@ -108,14 +110,15 @@ static void buffered_flush(QEMUFileBuffered *s)
static int buffered_put_buffer(void *opaque, const uint8_t *buf, int64_t pos, int size)
{
QEMUFileBuffered *s = opaque;
- int offset = 0;
+ int offset = 0, error;
ssize_t ret;
DPRINTF("putting %d bytes at %" PRId64 "\n", size, pos);
- if (qemu_file_get_error(s->file)) {
- DPRINTF("flush when error, bailing\n");
- return -EINVAL;
+ error = qemu_file_get_error(s->file);
+ if (error) {
+ DPRINTF("flush when error, bailing: %s\n", strerror(-error));
+ return error;
}
DPRINTF("unfreezing output\n");
@@ -192,14 +195,16 @@ static int buffered_close(void *opaque)
* The meaning of the return values is:
* 0: We can continue sending
* 1: Time to stop
- * -1: There has been an error
+ * negative: There has been an error
*/
static int buffered_rate_limit(void *opaque)
{
QEMUFileBuffered *s = opaque;
+ int ret;
- if (qemu_file_get_error(s->file)) {
- return -1;
+ ret = qemu_file_get_error(s->file);
+ if (ret) {
+ return ret;
}
if (s->freeze_output)
return 1;
diff --git a/savevm.c b/savevm.c
index b7a61c5..f27f474 100644
--- a/savevm.c
+++ b/savevm.c
@@ -1841,8 +1841,8 @@ out:
g_free(le);
}
- if (qemu_file_get_error(f)) {
- ret = -EIO;
+ if (ret == 0) {
+ ret = qemu_file_get_error(f);
}
return ret;
--
1.7.6.4
^ permalink raw reply related [flat|nested] 6+ messages in thread* [Qemu-devel] [PATCH 15/37] migration: make *save_live return errors
2011-10-20 0:11 [Qemu-devel] [PATCH v5 00/37] Migration errors & cleanup (the integrated version) Juan Quintela
` (2 preceding siblings ...)
2011-10-20 0:11 ` [Qemu-devel] [PATCH 14/37] migration: use qemu_file_get_error() return value when possible Juan Quintela
@ 2011-10-20 0:11 ` Juan Quintela
2011-10-20 0:11 ` [Qemu-devel] [PATCH 23/37] migration: Introduce MIG_STATE_SETUP Juan Quintela
4 siblings, 0 replies; 6+ messages in thread
From: Juan Quintela @ 2011-10-20 0:11 UTC (permalink / raw)
To: qemu-devel
Make *save_live() return negative values when there is one error, and
updates all callers to check for the error.
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
arch_init.c | 9 +++++++--
block-migration.c | 17 +++++++++++------
savevm.c | 14 +++++++++++---
3 files changed, 29 insertions(+), 11 deletions(-)
diff --git a/arch_init.c b/arch_init.c
index 98daaf3..a411fdf 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -256,6 +256,7 @@ int ram_save_live(Monitor *mon, QEMUFile *f, int stage, void *opaque)
uint64_t bytes_transferred_last;
double bwidth = 0;
uint64_t expected_time = 0;
+ int ret;
if (stage < 0) {
cpu_physical_memory_set_dirty_tracking(0);
@@ -264,7 +265,7 @@ int ram_save_live(Monitor *mon, QEMUFile *f, int stage, void *opaque)
if (cpu_physical_sync_dirty_bitmap(0, TARGET_PHYS_ADDR_MAX) != 0) {
qemu_file_set_error(f, -EINVAL);
- return 0;
+ return -EINVAL;
}
if (stage == 1) {
@@ -300,7 +301,7 @@ int ram_save_live(Monitor *mon, QEMUFile *f, int stage, void *opaque)
bytes_transferred_last = bytes_transferred;
bwidth = qemu_get_clock_ns(rt_clock);
- while (!qemu_file_rate_limit(f)) {
+ while ((ret = qemu_file_rate_limit(f)) == 0) {
int bytes_sent;
bytes_sent = ram_save_block(f);
@@ -310,6 +311,10 @@ int ram_save_live(Monitor *mon, QEMUFile *f, int stage, void *opaque)
}
}
+ if (ret < 0) {
+ return ret;
+ }
+
bwidth = qemu_get_clock_ns(rt_clock) - bwidth;
bwidth = (bytes_transferred - bytes_transferred_last) / bwidth;
diff --git a/block-migration.c b/block-migration.c
index b8d19a1..0bff075 100644
--- a/block-migration.c
+++ b/block-migration.c
@@ -557,6 +557,8 @@ static void blk_mig_cleanup(Monitor *mon)
static int block_save_live(Monitor *mon, QEMUFile *f, int stage, void *opaque)
{
+ int ret;
+
DPRINTF("Enter save live stage %d submitted %d transferred %d\n",
stage, block_mig_state.submitted, block_mig_state.transferred);
@@ -580,9 +582,10 @@ static int block_save_live(Monitor *mon, QEMUFile *f, int stage, void *opaque)
flush_blks(f);
- if (qemu_file_get_error(f)) {
+ ret = qemu_file_get_error(f);
+ if (ret) {
blk_mig_cleanup(mon);
- return 0;
+ return ret;
}
blk_mig_reset_dirty_cursor();
@@ -608,9 +611,10 @@ static int block_save_live(Monitor *mon, QEMUFile *f, int stage, void *opaque)
flush_blks(f);
- if (qemu_file_get_error(f)) {
+ ret = qemu_file_get_error(f);
+ if (ret) {
blk_mig_cleanup(mon);
- return 0;
+ return ret;
}
}
@@ -625,8 +629,9 @@ static int block_save_live(Monitor *mon, QEMUFile *f, int stage, void *opaque)
/* report completion */
qemu_put_be64(f, (100 << BDRV_SECTOR_BITS) | BLK_MIG_FLAG_PROGRESS);
- if (qemu_file_get_error(f)) {
- return 0;
+ ret = qemu_file_get_error(f);
+ if (ret) {
+ return ret;
}
monitor_printf(mon, "Block migration completed\n");
diff --git a/savevm.c b/savevm.c
index f27f474..9a5a369 100644
--- a/savevm.c
+++ b/savevm.c
@@ -1496,7 +1496,11 @@ int qemu_savevm_state_begin(Monitor *mon, QEMUFile *f, int blk_enable,
qemu_put_be32(f, se->instance_id);
qemu_put_be32(f, se->version_id);
- se->save_live_state(mon, f, QEMU_VM_SECTION_START, se->opaque);
+ ret = se->save_live_state(mon, f, QEMU_VM_SECTION_START, se->opaque);
+ if (ret < 0) {
+ qemu_savevm_state_cancel(mon, f);
+ return ret;
+ }
}
ret = qemu_file_get_error(f);
if (ret != 0) {
@@ -1527,7 +1531,7 @@ int qemu_savevm_state_iterate(Monitor *mon, QEMUFile *f)
qemu_put_be32(f, se->section_id);
ret = se->save_live_state(mon, f, QEMU_VM_SECTION_PART, se->opaque);
- if (!ret) {
+ if (ret <= 0) {
/* Do not proceed to the next vmstate before this one reported
completion of the current stage. This serializes the migration
and reduces the probability that a faster changing state is
@@ -1548,6 +1552,7 @@ int qemu_savevm_state_iterate(Monitor *mon, QEMUFile *f)
int qemu_savevm_state_complete(Monitor *mon, QEMUFile *f)
{
SaveStateEntry *se;
+ int ret;
cpu_synchronize_all_states();
@@ -1559,7 +1564,10 @@ int qemu_savevm_state_complete(Monitor *mon, QEMUFile *f)
qemu_put_byte(f, QEMU_VM_SECTION_END);
qemu_put_be32(f, se->section_id);
- se->save_live_state(mon, f, QEMU_VM_SECTION_END, se->opaque);
+ ret = se->save_live_state(mon, f, QEMU_VM_SECTION_END, se->opaque);
+ if (ret < 0) {
+ return ret;
+ }
}
QTAILQ_FOREACH(se, &savevm_handlers, entry) {
--
1.7.6.4
^ permalink raw reply related [flat|nested] 6+ messages in thread* [Qemu-devel] [PATCH 23/37] migration: Introduce MIG_STATE_SETUP
2011-10-20 0:11 [Qemu-devel] [PATCH v5 00/37] Migration errors & cleanup (the integrated version) Juan Quintela
` (3 preceding siblings ...)
2011-10-20 0:11 ` [Qemu-devel] [PATCH 15/37] migration: make *save_live return errors Juan Quintela
@ 2011-10-20 0:11 ` Juan Quintela
4 siblings, 0 replies; 6+ messages in thread
From: Juan Quintela @ 2011-10-20 0:11 UTC (permalink / raw)
To: qemu-devel
Use MIG_STATE_ACTIVE only when migration has really started. Use this
new state to setup migration parameters. Change defines for an
anonymous struct.
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 ca038ec..281fbae 100644
--- a/migration.c
+++ b/migration.c
@@ -239,6 +239,9 @@ void do_info_migrate(Monitor *mon, QObject **ret_data)
MigrationState *s = current_migration;
switch (s->get_status(current_migration)) {
+ case MIG_STATE_SETUP:
+ /* no migration has happened ever */
+ break;
case MIG_STATE_ACTIVE:
qdict = qdict_new();
qdict_put(qdict, "status", qstring_from_str("active"));
@@ -478,6 +481,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,
@@ -507,7 +511,7 @@ static MigrationState *migrate_new(Monitor *mon, int64_t bandwidth_limit,
s->shared = inc;
s->mon = NULL;
s->bandwidth_limit = bandwidth_limit;
- s->state = MIG_STATE_ACTIVE;
+ s->state = MIG_STATE_SETUP;
if (!detach) {
migrate_fd_monitor_suspend(s, mon);
diff --git a/migration.h b/migration.h
index 14c3ebc..fed1cf1 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 {
+ MIG_STATE_ERROR,
+ MIG_STATE_SETUP,
+ MIG_STATE_CANCELLED,
+ MIG_STATE_ACTIVE,
+ MIG_STATE_COMPLETED,
+};
typedef struct MigrationState MigrationState;
--
1.7.6.4
^ permalink raw reply related [flat|nested] 6+ messages in thread