115 lines
4.3 KiB
Diff
115 lines
4.3 KiB
Diff
|
From: Jeff Cody <jcody@redhat.com>
|
||
|
Date: Mon, 1 Feb 2016 20:33:10 -0500
|
||
|
Subject: [PATCH] block: set device_list.tqe_prev to NULL on BDS removal
|
||
|
|
||
|
This fixes a regression introduced with commit 3f09bfbc7. Multiple
|
||
|
bugs arise in conjunction with live snapshots and mirroring operations
|
||
|
(which include active layer commit).
|
||
|
|
||
|
After a live snapshot occurs, the active layer and the base layer both
|
||
|
have a non-NULL tqe_prev field in the device_list, although the base
|
||
|
node's tqe_prev field points to a NULL entry. This non-NULL tqe_prev
|
||
|
field occurs after the bdrv_append() in the external snapshot calls
|
||
|
change_parent_backing_link().
|
||
|
|
||
|
In change_parent_backing_link(), when the previous active layer is
|
||
|
removed from device_list, the device_list.tqe_prev pointer is not
|
||
|
set to NULL.
|
||
|
|
||
|
The operating scheme in the block layer is to indicate that a BDS belongs
|
||
|
in the bdrv_states device_list iff the device_list.tqe_prev pointer
|
||
|
is non-NULL.
|
||
|
|
||
|
This patch does two things:
|
||
|
|
||
|
1.) Introduces a new block layer helper bdrv_device_remove() to remove a
|
||
|
BDS from the device_list, and
|
||
|
2.) uses that new API, which also fixes the regression once used in
|
||
|
change_parent_backing_link().
|
||
|
|
||
|
Signed-off-by: Jeff Cody <jcody@redhat.com>
|
||
|
Message-id: 0cd51e11c0666c04ddb7c05293fe94afeb551e89.1454376655.git.jcody@redhat.com
|
||
|
Reviewed-by: Max Reitz <mreitz@redhat.com>
|
||
|
Signed-off-by: Max Reitz <mreitz@redhat.com>
|
||
|
(cherry picked from commit f8aa905a4fec89863c82de4186352447d851871e)
|
||
|
---
|
||
|
block.c | 24 ++++++++++++++----------
|
||
|
blockdev.c | 3 +--
|
||
|
include/block/block.h | 1 +
|
||
|
3 files changed, 16 insertions(+), 12 deletions(-)
|
||
|
|
||
|
diff --git a/block.c b/block.c
|
||
|
index 3a7324b..3c172dd 100644
|
||
|
--- a/block.c
|
||
|
+++ b/block.c
|
||
|
@@ -1976,21 +1976,25 @@ void bdrv_close_all(void)
|
||
|
}
|
||
|
}
|
||
|
|
||
|
+/* Note that bs->device_list.tqe_prev is initially null,
|
||
|
+ * and gets set to non-null by QTAILQ_INSERT_TAIL(). Establish
|
||
|
+ * the useful invariant "bs in bdrv_states iff bs->tqe_prev" by
|
||
|
+ * resetting it to null on remove. */
|
||
|
+void bdrv_device_remove(BlockDriverState *bs)
|
||
|
+{
|
||
|
+ QTAILQ_REMOVE(&bdrv_states, bs, device_list);
|
||
|
+ bs->device_list.tqe_prev = NULL;
|
||
|
+}
|
||
|
+
|
||
|
/* make a BlockDriverState anonymous by removing from bdrv_state and
|
||
|
* graph_bdrv_state list.
|
||
|
Also, NULL terminate the device_name to prevent double remove */
|
||
|
void bdrv_make_anon(BlockDriverState *bs)
|
||
|
{
|
||
|
- /*
|
||
|
- * Take care to remove bs from bdrv_states only when it's actually
|
||
|
- * in it. Note that bs->device_list.tqe_prev is initially null,
|
||
|
- * and gets set to non-null by QTAILQ_INSERT_TAIL(). Establish
|
||
|
- * the useful invariant "bs in bdrv_states iff bs->tqe_prev" by
|
||
|
- * resetting it to null on remove.
|
||
|
- */
|
||
|
+ /* Take care to remove bs from bdrv_states only when it's actually
|
||
|
+ * in it. */
|
||
|
if (bs->device_list.tqe_prev) {
|
||
|
- QTAILQ_REMOVE(&bdrv_states, bs, device_list);
|
||
|
- bs->device_list.tqe_prev = NULL;
|
||
|
+ bdrv_device_remove(bs);
|
||
|
}
|
||
|
if (bs->node_name[0] != '\0') {
|
||
|
QTAILQ_REMOVE(&graph_bdrv_states, bs, node_list);
|
||
|
@@ -2031,7 +2035,7 @@ static void change_parent_backing_link(BlockDriverState *from,
|
||
|
if (!to->device_list.tqe_prev) {
|
||
|
QTAILQ_INSERT_BEFORE(from, to, device_list);
|
||
|
}
|
||
|
- QTAILQ_REMOVE(&bdrv_states, from, device_list);
|
||
|
+ bdrv_device_remove(from);
|
||
|
}
|
||
|
}
|
||
|
|
||
|
diff --git a/blockdev.c b/blockdev.c
|
||
|
index 80932e8..c236126 100644
|
||
|
--- a/blockdev.c
|
||
|
+++ b/blockdev.c
|
||
|
@@ -2384,8 +2384,7 @@ void qmp_x_blockdev_remove_medium(const char *device, Error **errp)
|
||
|
|
||
|
/* This follows the convention established by bdrv_make_anon() */
|
||
|
if (bs->device_list.tqe_prev) {
|
||
|
- QTAILQ_REMOVE(&bdrv_states, bs, device_list);
|
||
|
- bs->device_list.tqe_prev = NULL;
|
||
|
+ bdrv_device_remove(bs);
|
||
|
}
|
||
|
|
||
|
blk_remove_bs(blk);
|
||
|
diff --git a/include/block/block.h b/include/block/block.h
|
||
|
index 3477328..d83d420 100644
|
||
|
--- a/include/block/block.h
|
||
|
+++ b/include/block/block.h
|
||
|
@@ -196,6 +196,7 @@ int bdrv_create(BlockDriver *drv, const char* filename,
|
||
|
int bdrv_create_file(const char *filename, QemuOpts *opts, Error **errp);
|
||
|
BlockDriverState *bdrv_new_root(void);
|
||
|
BlockDriverState *bdrv_new(void);
|
||
|
+void bdrv_device_remove(BlockDriverState *bs);
|
||
|
void bdrv_make_anon(BlockDriverState *bs);
|
||
|
void bdrv_swap(BlockDriverState *bs_new, BlockDriverState *bs_old);
|
||
|
void bdrv_append(BlockDriverState *bs_new, BlockDriverState *bs_top);
|