[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] [PATCH 104/113] throttle: Fix crash on reopen
From: |
Michael Roth |
Subject: |
[Qemu-devel] [PATCH 104/113] throttle: Fix crash on reopen |
Date: |
Mon, 18 Jun 2018 20:43:10 -0500 |
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>
(cherry picked from commit bc33c047d1ec0b35c9cd8be62bcefae2da28654f)
Signed-off-by: Michael Roth <address@hidden>
---
block/throttle.c | 54 +++++++++++++++++++++++++++++++++---------------------
1 file changed, 33 insertions(+), 21 deletions(-)
diff --git a/block/throttle.c b/block/throttle.c
index 833175ac77..d5903784c0 100644
--- a/block/throttle.c
+++ b/block/throttle.c
@@ -35,9 +35,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;
@@ -62,8 +65,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);
@@ -74,6 +76,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);
@@ -83,7 +87,14 @@ static int throttle_open(BlockDriverState *bs, QDict
*options,
bs->supported_write_flags = bs->file->bs->supported_write_flags;
bs->supported_zero_flags = bs->file->bs->supported_zero_flags;
- 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)
@@ -159,35 +170,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.11.0
- [Qemu-devel] [PATCH 00/113] Patch Round-up for stable 2.11.2, freeze on 2018-06-22, Michael Roth, 2018/06/18
- [Qemu-devel] [PATCH 099/113] ahci: fix PxCI register race, Michael Roth, 2018/06/18
- [Qemu-devel] [PATCH 100/113] arm_gicv3_kvm: kvm_dist_get/put: skip the registers banked by GICR, Michael Roth, 2018/06/18
- [Qemu-devel] [PATCH 009/113] spapr: use spapr->vsmt to compute VCPU ids, Michael Roth, 2018/06/18
- [Qemu-devel] [PATCH 101/113] block: Make bdrv_is_writable() public, Michael Roth, 2018/06/18
- [Qemu-devel] [PATCH 102/113] qcow2: Do not mark inactive images corrupt, Michael Roth, 2018/06/18
- [Qemu-devel] [PATCH 103/113] iotests: Add case for a corrupted inactive image, Michael Roth, 2018/06/18
- [Qemu-devel] [PATCH 104/113] throttle: Fix crash on reopen,
Michael Roth <=
- [Qemu-devel] [PATCH 105/113] vga: fix region calculation, Michael Roth, 2018/06/18
- [Qemu-devel] [PATCH 106/113] i386: define the 'ssbd' CPUID feature bit (CVE-2018-3639), Michael Roth, 2018/06/18
- [Qemu-devel] [PATCH 107/113] i386: Define the Virt SSBD MSR and handling of it (CVE-2018-3639), Michael Roth, 2018/06/18
- [Qemu-devel] [PATCH 108/113] i386: define the AMD 'virt-ssbd' CPUID feature bit (CVE-2018-3639), Michael Roth, 2018/06/18
- [Qemu-devel] [PATCH 109/113] tap: set vhostfd passed from qemu cli to non-blocking, Michael Roth, 2018/06/18
- [Qemu-devel] [PATCH 110/113] vhost-user: delete net client if necessary, Michael Roth, 2018/06/18
- [Qemu-devel] [PATCH 010/113] spapr: move VCPU calculation to core machine code, Michael Roth, 2018/06/18
- [Qemu-devel] [PATCH 111/113] qemu-img: Fix assert when mapping unaligned raw file, Michael Roth, 2018/06/18
- [Qemu-devel] [PATCH 112/113] iotests: Add test 221 to catch qemu-img map regression, Michael Roth, 2018/06/18
- [Qemu-devel] [PATCH 113/113] arm_gicv3_kvm: kvm_dist_get/put_priority: skip the registers banked by GICR_IPRIORITYR, Michael Roth, 2018/06/18