[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-block] [PULL 26/29] throttle: Fix crash on reopen
From: |
Max Reitz |
Subject: |
[Qemu-block] [PULL 26/29] throttle: Fix crash on reopen |
Date: |
Mon, 11 Jun 2018 16:26:08 +0200 |
From: Alberto Garcia <address@hidden>
The throttle block filter can be reopened, and with this it is
possible to change the throttle group that the filter belongs to.
The way the code does that is the following:
- On throttle_reopen_prepare(): create a new ThrottleGroupMember
and attach it to the new throttle group.
- On throttle_reopen_commit(): detach the old ThrottleGroupMember,
delete it and replace it with the new one.
The problem with this is that by replacing the ThrottleGroupMember the
previous value of io_limits_disabled is lost, causing an assertion
failure in throttle_co_drain_end().
This problem can be reproduced by reopening a throttle node:
$QEMU -monitor stdio
-object throttle-group,id=tg0,x-iops-total=1000 \
-blockdev node-name=hd0,driver=qcow2,file.driver=file,file.filename=hd.qcow2
\
-blockdev
node-name=root,driver=throttle,throttle-group=tg0,file=hd0,read-only=on
(qemu) block_stream root
block/throttle.c:214: throttle_co_drain_end: Assertion
`tgm->io_limits_disabled' failed.
Since we only want to change the throttle group on reopen there's no
need to create a ThrottleGroupMember and discard the old one. It's
easier if we simply detach it from its current group and attach it to
the new one.
Signed-off-by: Alberto Garcia <address@hidden>
Message-id: address@hidden
Signed-off-by: Max Reitz <address@hidden>
---
block/throttle.c | 54 +++++++++++++++++++++++++++++-------------------
1 file changed, 33 insertions(+), 21 deletions(-)
diff --git a/block/throttle.c b/block/throttle.c
index e298827f95..f617f23a12 100644
--- a/block/throttle.c
+++ b/block/throttle.c
@@ -36,9 +36,12 @@ static QemuOptsList throttle_opts = {
},
};
-static int throttle_configure_tgm(BlockDriverState *bs,
- ThrottleGroupMember *tgm,
- QDict *options, Error **errp)
+/*
+ * If this function succeeds then the throttle group name is stored in
+ * @group and must be freed by the caller.
+ * If there's an error then @group remains unmodified.
+ */
+static int throttle_parse_options(QDict *options, char **group, Error **errp)
{
int ret;
const char *group_name;
@@ -63,8 +66,7 @@ static int throttle_configure_tgm(BlockDriverState *bs,
goto fin;
}
- /* Register membership to group with name group_name */
- throttle_group_register_tgm(tgm, group_name, bdrv_get_aio_context(bs));
+ *group = g_strdup(group_name);
ret = 0;
fin:
qemu_opts_del(opts);
@@ -75,6 +77,8 @@ static int throttle_open(BlockDriverState *bs, QDict *options,
int flags, Error **errp)
{
ThrottleGroupMember *tgm = bs->opaque;
+ char *group;
+ int ret;
bs->file = bdrv_open_child(NULL, options, "file", bs,
&child_file, false, errp);
@@ -86,7 +90,14 @@ static int throttle_open(BlockDriverState *bs, QDict
*options,
bs->supported_zero_flags = bs->file->bs->supported_zero_flags |
BDRV_REQ_WRITE_UNCHANGED;
- return throttle_configure_tgm(bs, tgm, options, errp);
+ ret = throttle_parse_options(options, &group, errp);
+ if (ret == 0) {
+ /* Register membership to group with name group_name */
+ throttle_group_register_tgm(tgm, group, bdrv_get_aio_context(bs));
+ g_free(group);
+ }
+
+ return ret;
}
static void throttle_close(BlockDriverState *bs)
@@ -162,35 +173,36 @@ static void throttle_attach_aio_context(BlockDriverState
*bs,
static int throttle_reopen_prepare(BDRVReopenState *reopen_state,
BlockReopenQueue *queue, Error **errp)
{
- ThrottleGroupMember *tgm;
+ int ret;
+ char *group = NULL;
assert(reopen_state != NULL);
assert(reopen_state->bs != NULL);
- reopen_state->opaque = g_new0(ThrottleGroupMember, 1);
- tgm = reopen_state->opaque;
-
- return throttle_configure_tgm(reopen_state->bs, tgm, reopen_state->options,
- errp);
+ ret = throttle_parse_options(reopen_state->options, &group, errp);
+ reopen_state->opaque = group;
+ return ret;
}
static void throttle_reopen_commit(BDRVReopenState *reopen_state)
{
- ThrottleGroupMember *old_tgm = reopen_state->bs->opaque;
- ThrottleGroupMember *new_tgm = reopen_state->opaque;
+ BlockDriverState *bs = reopen_state->bs;
+ ThrottleGroupMember *tgm = bs->opaque;
+ char *group = reopen_state->opaque;
+
+ assert(group);
- throttle_group_unregister_tgm(old_tgm);
- g_free(old_tgm);
- reopen_state->bs->opaque = new_tgm;
+ if (strcmp(group, throttle_group_get_name(tgm))) {
+ throttle_group_unregister_tgm(tgm);
+ throttle_group_register_tgm(tgm, group, bdrv_get_aio_context(bs));
+ }
+ g_free(reopen_state->opaque);
reopen_state->opaque = NULL;
}
static void throttle_reopen_abort(BDRVReopenState *reopen_state)
{
- ThrottleGroupMember *tgm = reopen_state->opaque;
-
- throttle_group_unregister_tgm(tgm);
- g_free(tgm);
+ g_free(reopen_state->opaque);
reopen_state->opaque = NULL;
}
--
2.17.1
- [Qemu-block] [PULL 17/29] iotests: Let 216 make use of qemu-io's exit code, (continued)
- [Qemu-block] [PULL 17/29] iotests: Let 216 make use of qemu-io's exit code, Max Reitz, 2018/06/11
- [Qemu-block] [PULL 16/29] iotests.py: Add qemu_io_silent, Max Reitz, 2018/06/11
- [Qemu-block] [PULL 18/29] qemu-img: Resolve relative backing paths in rebase, Max Reitz, 2018/06/11
- [Qemu-block] [PULL 14/29] qemu-io: Let command functions return error code, Max Reitz, 2018/06/11
- [Qemu-block] [PULL 13/29] qemu-io: Drop command functions' return values, Max Reitz, 2018/06/11
- [Qemu-block] [PULL 20/29] qemu-img: Special post-backing convert handling, Max Reitz, 2018/06/11
- [Qemu-block] [PULL 19/29] iotests: Add test for rebasing with relative paths, Max Reitz, 2018/06/11
- [Qemu-block] [PULL 21/29] iotests: Test post-backing convert target behavior, Max Reitz, 2018/06/11
- [Qemu-block] [PULL 22/29] iotests: improve pause_job, Max Reitz, 2018/06/11
- [Qemu-block] [PULL 23/29] iotests: Fix 219's timing, Max Reitz, 2018/06/11
- [Qemu-block] [PULL 26/29] throttle: Fix crash on reopen,
Max Reitz <=
- [Qemu-block] [PULL 25/29] block/qcow2-bitmap: fix free_bitmap_clusters, Max Reitz, 2018/06/11
- [Qemu-block] [PULL 28/29] qcow2: Do not mark inactive images corrupt, Max Reitz, 2018/06/11
- [Qemu-block] [PULL 24/29] qemu-img: Remove deprecated -s snapshot_id_or_name option, Max Reitz, 2018/06/11
- [Qemu-block] [PULL 27/29] block: Make bdrv_is_writable() public, Max Reitz, 2018/06/11
- [Qemu-block] [PULL 29/29] iotests: Add case for a corrupted inactive image, Max Reitz, 2018/06/11
- Re: [Qemu-block] [PULL 00/29] Block patches, Peter Maydell, 2018/06/11