[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[PULL 16/24] migration/block-dirty-bitmap: relax error handling in incom
From: |
Eric Blake |
Subject: |
[PULL 16/24] migration/block-dirty-bitmap: relax error handling in incoming part |
Date: |
Mon, 27 Jul 2020 15:55:35 -0500 |
From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Bitmaps data is not critical, and we should not fail the migration (or
use postcopy recovering) because of dirty-bitmaps migration failure.
Instead we should just lose unfinished bitmaps.
Still we have to report io stream violation errors, as they affect the
whole migration stream.
While touching this, tighten code that was previously blindly calling
malloc on a size read from the migration stream, as a corrupted stream
(perhaps from a malicious user) should not be able to convince us to
allocate an inordinate amount of memory.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20200727194236.19551-16-vsementsov@virtuozzo.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
[eblake: typo fixes, enhance commit message]
Signed-off-by: Eric Blake <eblake@redhat.com>
---
migration/block-dirty-bitmap.c | 162 +++++++++++++++++++++++++--------
1 file changed, 126 insertions(+), 36 deletions(-)
diff --git a/migration/block-dirty-bitmap.c b/migration/block-dirty-bitmap.c
index eb4ffeac4d1b..f91015a4f88f 100644
--- a/migration/block-dirty-bitmap.c
+++ b/migration/block-dirty-bitmap.c
@@ -145,6 +145,15 @@ typedef struct DBMLoadState {
bool before_vm_start_handled; /* set in dirty_bitmap_mig_before_vm_start */
+ /*
+ * cancelled
+ * Incoming migration is cancelled for some reason. That means that we
+ * still should read our chunks from migration stream, to not affect other
+ * migration objects (like RAM), but just ignore them and do not touch any
+ * bitmaps or nodes.
+ */
+ bool cancelled;
+
GSList *bitmaps;
QemuMutex lock; /* protect bitmaps */
} DBMLoadState;
@@ -531,6 +540,10 @@ static int dirty_bitmap_load_start(QEMUFile *f,
DBMLoadState *s)
uint8_t flags = qemu_get_byte(f);
LoadBitmapState *b;
+ if (s->cancelled) {
+ return 0;
+ }
+
if (s->bitmap) {
error_report("Bitmap with the same name ('%s') already exists on "
"destination", bdrv_dirty_bitmap_name(s->bitmap));
@@ -613,14 +626,48 @@ void dirty_bitmap_mig_before_vm_start(void)
qemu_mutex_unlock(&s->lock);
}
+static void cancel_incoming_locked(DBMLoadState *s)
+{
+ GSList *item;
+
+ if (s->cancelled) {
+ return;
+ }
+
+ s->cancelled = true;
+ s->bs = NULL;
+ s->bitmap = NULL;
+
+ /* Drop all unfinished bitmaps */
+ for (item = s->bitmaps; item; item = g_slist_next(item)) {
+ LoadBitmapState *b = item->data;
+
+ /*
+ * Bitmap must be unfinished, as finished bitmaps should already be
+ * removed from the list.
+ */
+ assert(!s->before_vm_start_handled || !b->migrated);
+ if (bdrv_dirty_bitmap_has_successor(b->bitmap)) {
+ bdrv_reclaim_dirty_bitmap(b->bitmap, &error_abort);
+ }
+ bdrv_release_dirty_bitmap(b->bitmap);
+ }
+
+ g_slist_free_full(s->bitmaps, g_free);
+ s->bitmaps = NULL;
+}
+
static void dirty_bitmap_load_complete(QEMUFile *f, DBMLoadState *s)
{
GSList *item;
trace_dirty_bitmap_load_complete();
+
+ if (s->cancelled) {
+ return;
+ }
+
bdrv_dirty_bitmap_deserialize_finish(s->bitmap);
- qemu_mutex_lock(&s->lock);
-
if (bdrv_dirty_bitmap_has_successor(s->bitmap)) {
bdrv_reclaim_dirty_bitmap(s->bitmap, &error_abort);
}
@@ -637,8 +684,6 @@ static void dirty_bitmap_load_complete(QEMUFile *f,
DBMLoadState *s)
break;
}
}
-
- qemu_mutex_unlock(&s->lock);
}
static int dirty_bitmap_load_bits(QEMUFile *f, DBMLoadState *s)
@@ -650,15 +695,46 @@ static int dirty_bitmap_load_bits(QEMUFile *f,
DBMLoadState *s)
if (s->flags & DIRTY_BITMAP_MIG_FLAG_ZEROES) {
trace_dirty_bitmap_load_bits_zeroes();
- bdrv_dirty_bitmap_deserialize_zeroes(s->bitmap, first_byte, nr_bytes,
- false);
+ if (!s->cancelled) {
+ bdrv_dirty_bitmap_deserialize_zeroes(s->bitmap, first_byte,
+ nr_bytes, false);
+ }
} else {
size_t ret;
- uint8_t *buf;
+ g_autofree uint8_t *buf = NULL;
uint64_t buf_size = qemu_get_be64(f);
- uint64_t needed_size =
- bdrv_dirty_bitmap_serialization_size(s->bitmap,
- first_byte, nr_bytes);
+ uint64_t needed_size;
+
+ /*
+ * The actual check for buf_size is done a bit later. We can't do it in
+ * cancelled mode as we don't have the bitmap to check the constraints
+ * (so, we allocate a buffer and read prior to the check). On the other
+ * hand, we shouldn't blindly g_malloc the number from the stream.
+ * Actually one chunk should not be larger than CHUNK_SIZE. Let's allow
+ * a bit larger (which means that bitmap migration will fail anyway and
+ * the whole migration will most probably fail soon due to broken
+ * stream).
+ */
+ if (buf_size > 10 * CHUNK_SIZE) {
+ error_report("Bitmap migration stream buffer allocation request "
+ "is too large");
+ return -EIO;
+ }
+
+ buf = g_malloc(buf_size);
+ ret = qemu_get_buffer(f, buf, buf_size);
+ if (ret != buf_size) {
+ error_report("Failed to read bitmap bits");
+ return -EIO;
+ }
+
+ if (s->cancelled) {
+ return 0;
+ }
+
+ needed_size = bdrv_dirty_bitmap_serialization_size(s->bitmap,
+ first_byte,
+ nr_bytes);
if (needed_size > buf_size ||
buf_size > QEMU_ALIGN_UP(needed_size, 4 * sizeof(long))
@@ -667,20 +743,12 @@ static int dirty_bitmap_load_bits(QEMUFile *f,
DBMLoadState *s)
error_report("Migrated bitmap granularity doesn't "
"match the destination bitmap '%s' granularity",
bdrv_dirty_bitmap_name(s->bitmap));
- return -EINVAL;
- }
-
- buf = g_malloc(buf_size);
- ret = qemu_get_buffer(f, buf, buf_size);
- if (ret != buf_size) {
- error_report("Failed to read bitmap bits");
- g_free(buf);
- return -EIO;
+ cancel_incoming_locked(s);
+ return 0;
}
bdrv_dirty_bitmap_deserialize_part(s->bitmap, buf, first_byte,
nr_bytes,
false);
- g_free(buf);
}
return 0;
@@ -700,14 +768,16 @@ static int dirty_bitmap_load_header(QEMUFile *f,
DBMLoadState *s)
error_report("Unable to read node name string");
return -EINVAL;
}
- s->bs = bdrv_lookup_bs(s->node_name, s->node_name, &local_err);
- if (!s->bs) {
- error_report_err(local_err);
- return -EINVAL;
+ if (!s->cancelled) {
+ s->bs = bdrv_lookup_bs(s->node_name, s->node_name, &local_err);
+ if (!s->bs) {
+ error_report_err(local_err);
+ cancel_incoming_locked(s);
+ }
}
- } else if (!s->bs && !nothing) {
+ } else if (!s->bs && !nothing && !s->cancelled) {
error_report("Error: block device name is not set");
- return -EINVAL;
+ cancel_incoming_locked(s);
}
if (s->flags & DIRTY_BITMAP_MIG_FLAG_BITMAP_NAME) {
@@ -715,24 +785,38 @@ static int dirty_bitmap_load_header(QEMUFile *f,
DBMLoadState *s)
error_report("Unable to read bitmap name string");
return -EINVAL;
}
- s->bitmap = bdrv_find_dirty_bitmap(s->bs, s->bitmap_name);
+ if (!s->cancelled) {
+ s->bitmap = bdrv_find_dirty_bitmap(s->bs, s->bitmap_name);
- /* bitmap may be NULL here, it wouldn't be an error if it is the
- * first occurrence of the bitmap */
- if (!s->bitmap && !(s->flags & DIRTY_BITMAP_MIG_FLAG_START)) {
- error_report("Error: unknown dirty bitmap "
- "'%s' for block device '%s'",
- s->bitmap_name, s->node_name);
- return -EINVAL;
+ /*
+ * bitmap may be NULL here, it wouldn't be an error if it is the
+ * first occurrence of the bitmap
+ */
+ if (!s->bitmap && !(s->flags & DIRTY_BITMAP_MIG_FLAG_START)) {
+ error_report("Error: unknown dirty bitmap "
+ "'%s' for block device '%s'",
+ s->bitmap_name, s->node_name);
+ cancel_incoming_locked(s);
+ }
}
- } else if (!s->bitmap && !nothing) {
+ } else if (!s->bitmap && !nothing && !s->cancelled) {
error_report("Error: block device name is not set");
- return -EINVAL;
+ cancel_incoming_locked(s);
}
return 0;
}
+/*
+ * dirty_bitmap_load
+ *
+ * Load sequence of dirty bitmap chunks. Return error only on fatal io stream
+ * violations. On other errors just cancel bitmaps incoming migration and
return
+ * 0.
+ *
+ * Note, than when incoming bitmap migration is canceled, we still must read
all
+ * our chunks (and just ignore them), to not affect other migration objects.
+ */
static int dirty_bitmap_load(QEMUFile *f, void *opaque, int version_id)
{
DBMLoadState *s = &((DBMState *)opaque)->load;
@@ -741,12 +825,17 @@ static int dirty_bitmap_load(QEMUFile *f, void *opaque,
int version_id)
trace_dirty_bitmap_load_enter();
if (version_id != 1) {
+ QEMU_LOCK_GUARD(&s->lock);
+ cancel_incoming_locked(s);
return -EINVAL;
}
do {
+ QEMU_LOCK_GUARD(&s->lock);
+
ret = dirty_bitmap_load_header(f, s);
if (ret < 0) {
+ cancel_incoming_locked(s);
return ret;
}
@@ -763,6 +852,7 @@ static int dirty_bitmap_load(QEMUFile *f, void *opaque, int
version_id)
}
if (ret) {
+ cancel_incoming_locked(s);
return ret;
}
} while (!(s->flags & DIRTY_BITMAP_MIG_FLAG_EOS));
--
2.27.0
- [PULL 06/24] qemu-iotests/199: change discard patterns, (continued)
- [PULL 06/24] qemu-iotests/199: change discard patterns, Eric Blake, 2020/07/27
- [PULL 07/24] qemu-iotests/199: increase postcopy period, Eric Blake, 2020/07/27
- [PULL 08/24] migration/block-dirty-bitmap: fix dirty_bitmap_mig_before_vm_start, Eric Blake, 2020/07/27
- [PULL 09/24] migration/block-dirty-bitmap: rename state structure types, Eric Blake, 2020/07/27
- [PULL 11/24] migration/block-dirty-bitmap: move mutex init to dirty_bitmap_mig_init, Eric Blake, 2020/07/27
- [PULL 12/24] migration/block-dirty-bitmap: refactor state global variables, Eric Blake, 2020/07/27
- [PULL 10/24] migration/block-dirty-bitmap: rename dirty_bitmap_mig_cleanup, Eric Blake, 2020/07/27
- [PULL 15/24] migration/block-dirty-bitmap: keep bitmap state for all bitmaps, Eric Blake, 2020/07/27
- [PULL 14/24] migration/block-dirty-bitmap: simplify dirty_bitmap_load_complete, Eric Blake, 2020/07/27
- [PULL 13/24] migration/block-dirty-bitmap: rename finish_lock to just lock, Eric Blake, 2020/07/27
- [PULL 16/24] migration/block-dirty-bitmap: relax error handling in incoming part,
Eric Blake <=
- [PULL 17/24] migration/block-dirty-bitmap: cancel migration on shutdown, Eric Blake, 2020/07/27
- [PULL 18/24] migration/savevm: don't worry if bitmap migration postcopy failed, Eric Blake, 2020/07/27
- [PULL 20/24] qemu-iotests/199: check persistent bitmaps, Eric Blake, 2020/07/27
- [PULL 22/24] qemu-iotests/199: add source-killed case to bitmaps postcopy, Eric Blake, 2020/07/27
- [PULL 19/24] qemu-iotests/199: prepare for new test-cases addition, Eric Blake, 2020/07/27
- [PULL 21/24] qemu-iotests/199: add early shutdown case to bitmaps postcopy, Eric Blake, 2020/07/27
- [PULL 23/24] iotests: Adjust which migration tests are quick, Eric Blake, 2020/07/27
- [PULL 24/24] migration: Fix typos in bitmap migration comments, Eric Blake, 2020/07/27
- Re: [PULL 00/24] bitmaps patches for -rc2, 2020-07-27, Peter Maydell, 2020/07/28