aboutsummaryrefslogtreecommitdiff
path: root/block
diff options
context:
space:
mode:
authorKevin Wolf <kwolf@redhat.com>2016-05-20 18:49:07 +0200
committerKevin Wolf <kwolf@redhat.com>2016-05-25 19:04:10 +0200
commit88be7b4be4aa17c88247e162bdd7577ea79db94f (patch)
treecec7d9da5079f6dba0c65945925aedfd8c77267c /block
parent287db79df8af8e31f18e262feb5e05103a09e4d4 (diff)
downloadqemu-88be7b4be4aa17c88247e162bdd7577ea79db94f.zip
qemu-88be7b4be4aa17c88247e162bdd7577ea79db94f.tar.gz
qemu-88be7b4be4aa17c88247e162bdd7577ea79db94f.tar.bz2
block: Fix bdrv_next() memory leak
The bdrv_next() users all leaked the BdrvNextIterator after completing the iteration. Simply changing bdrv_next() to free the iterator before returning NULL at the end of list doesn't work because some callers exit the loop before looking at all BDSes. This patch moves the BdrvNextIterator from the heap to the stack of the caller and switches to a bdrv_first()/bdrv_next() interface for initialising the iterator. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Fam Zheng <famz@redhat.com>
Diffstat (limited to 'block')
-rw-r--r--block/block-backend.c41
-rw-r--r--block/io.c10
-rw-r--r--block/snapshot.c55
3 files changed, 63 insertions, 43 deletions
diff --git a/block/block-backend.c b/block/block-backend.c
index 6928d61..9f306f8 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -286,25 +286,11 @@ BlockBackend *blk_next(BlockBackend *blk)
: QTAILQ_FIRST(&monitor_block_backends);
}
-struct BdrvNextIterator {
- enum {
- BDRV_NEXT_BACKEND_ROOTS,
- BDRV_NEXT_MONITOR_OWNED,
- } phase;
- BlockBackend *blk;
- BlockDriverState *bs;
-};
-
/* Iterates over all top-level BlockDriverStates, i.e. BDSs that are owned by
* the monitor or attached to a BlockBackend */
-BdrvNextIterator *bdrv_next(BdrvNextIterator *it, BlockDriverState **bs)
+BlockDriverState *bdrv_next(BdrvNextIterator *it)
{
- if (!it) {
- it = g_new(BdrvNextIterator, 1);
- *it = (BdrvNextIterator) {
- .phase = BDRV_NEXT_BACKEND_ROOTS,
- };
- }
+ BlockDriverState *bs;
/* First, return all root nodes of BlockBackends. In order to avoid
* returning a BDS twice when multiple BBs refer to it, we only return it
@@ -312,11 +298,11 @@ BdrvNextIterator *bdrv_next(BdrvNextIterator *it, BlockDriverState **bs)
if (it->phase == BDRV_NEXT_BACKEND_ROOTS) {
do {
it->blk = blk_all_next(it->blk);
- *bs = it->blk ? blk_bs(it->blk) : NULL;
- } while (it->blk && (*bs == NULL || bdrv_first_blk(*bs) != it->blk));
+ bs = it->blk ? blk_bs(it->blk) : NULL;
+ } while (it->blk && (bs == NULL || bdrv_first_blk(bs) != it->blk));
- if (*bs) {
- return it;
+ if (bs) {
+ return bs;
}
it->phase = BDRV_NEXT_MONITOR_OWNED;
}
@@ -326,10 +312,19 @@ BdrvNextIterator *bdrv_next(BdrvNextIterator *it, BlockDriverState **bs)
* by the above block already */
do {
it->bs = bdrv_next_monitor_owned(it->bs);
- *bs = it->bs;
- } while (*bs && bdrv_has_blk(*bs));
+ bs = it->bs;
+ } while (bs && bdrv_has_blk(bs));
+
+ return bs;
+}
+
+BlockDriverState *bdrv_first(BdrvNextIterator *it)
+{
+ *it = (BdrvNextIterator) {
+ .phase = BDRV_NEXT_BACKEND_ROOTS,
+ };
- return *bs ? it : NULL;
+ return bdrv_next(it);
}
/*
diff --git a/block/io.c b/block/io.c
index 60a6bd8..2a28d63 100644
--- a/block/io.c
+++ b/block/io.c
@@ -271,10 +271,10 @@ void bdrv_drain_all(void)
/* Always run first iteration so any pending completion BHs run */
bool busy = true;
BlockDriverState *bs;
- BdrvNextIterator *it = NULL;
+ BdrvNextIterator it;
GSList *aio_ctxs = NULL, *ctx;
- while ((it = bdrv_next(it, &bs))) {
+ for (bs = bdrv_first(&it); bs; bs = bdrv_next(&it)) {
AioContext *aio_context = bdrv_get_aio_context(bs);
aio_context_acquire(aio_context);
@@ -302,10 +302,9 @@ void bdrv_drain_all(void)
for (ctx = aio_ctxs; ctx != NULL; ctx = ctx->next) {
AioContext *aio_context = ctx->data;
- it = NULL;
aio_context_acquire(aio_context);
- while ((it = bdrv_next(it, &bs))) {
+ for (bs = bdrv_first(&it); bs; bs = bdrv_next(&it)) {
if (aio_context == bdrv_get_aio_context(bs)) {
if (bdrv_requests_pending(bs)) {
busy = true;
@@ -318,8 +317,7 @@ void bdrv_drain_all(void)
}
}
- it = NULL;
- while ((it = bdrv_next(it, &bs))) {
+ for (bs = bdrv_first(&it); bs; bs = bdrv_next(&it)) {
AioContext *aio_context = bdrv_get_aio_context(bs);
aio_context_acquire(aio_context);
diff --git a/block/snapshot.c b/block/snapshot.c
index 3917ec5..6e6e34f 100644
--- a/block/snapshot.c
+++ b/block/snapshot.c
@@ -374,9 +374,9 @@ bool bdrv_all_can_snapshot(BlockDriverState **first_bad_bs)
{
bool ok = true;
BlockDriverState *bs;
- BdrvNextIterator *it = NULL;
+ BdrvNextIterator it;
- while (ok && (it = bdrv_next(it, &bs))) {
+ for (bs = bdrv_first(&it); bs; bs = bdrv_next(&it)) {
AioContext *ctx = bdrv_get_aio_context(bs);
aio_context_acquire(ctx);
@@ -384,8 +384,12 @@ bool bdrv_all_can_snapshot(BlockDriverState **first_bad_bs)
ok = bdrv_can_snapshot(bs);
}
aio_context_release(ctx);
+ if (!ok) {
+ goto fail;
+ }
}
+fail:
*first_bad_bs = bs;
return ok;
}
@@ -395,20 +399,27 @@ int bdrv_all_delete_snapshot(const char *name, BlockDriverState **first_bad_bs,
{
int ret = 0;
BlockDriverState *bs;
- BdrvNextIterator *it = NULL;
+ BdrvNextIterator it;
QEMUSnapshotInfo sn1, *snapshot = &sn1;
- while (ret == 0 && (it = bdrv_next(it, &bs))) {
+ for (bs = bdrv_first(&it); bs; bs = bdrv_next(&it)) {
AioContext *ctx = bdrv_get_aio_context(bs);
aio_context_acquire(ctx);
if (bdrv_can_snapshot(bs) &&
bdrv_snapshot_find(bs, snapshot, name) >= 0) {
ret = bdrv_snapshot_delete_by_id_or_name(bs, name, err);
+ if (ret < 0) {
+ goto fail;
+ }
}
aio_context_release(ctx);
+ if (ret < 0) {
+ goto fail;
+ }
}
+fail:
*first_bad_bs = bs;
return ret;
}
@@ -418,9 +429,9 @@ int bdrv_all_goto_snapshot(const char *name, BlockDriverState **first_bad_bs)
{
int err = 0;
BlockDriverState *bs;
- BdrvNextIterator *it = NULL;
+ BdrvNextIterator it;
- while (err == 0 && (it = bdrv_next(it, &bs))) {
+ for (bs = bdrv_first(&it); bs; bs = bdrv_next(&it)) {
AioContext *ctx = bdrv_get_aio_context(bs);
aio_context_acquire(ctx);
@@ -428,8 +439,12 @@ int bdrv_all_goto_snapshot(const char *name, BlockDriverState **first_bad_bs)
err = bdrv_snapshot_goto(bs, name);
}
aio_context_release(ctx);
+ if (err < 0) {
+ goto fail;
+ }
}
+fail:
*first_bad_bs = bs;
return err;
}
@@ -439,9 +454,9 @@ int bdrv_all_find_snapshot(const char *name, BlockDriverState **first_bad_bs)
QEMUSnapshotInfo sn;
int err = 0;
BlockDriverState *bs;
- BdrvNextIterator *it = NULL;
+ BdrvNextIterator it;
- while (err == 0 && (it = bdrv_next(it, &bs))) {
+ for (bs = bdrv_first(&it); bs; bs = bdrv_next(&it)) {
AioContext *ctx = bdrv_get_aio_context(bs);
aio_context_acquire(ctx);
@@ -449,8 +464,12 @@ int bdrv_all_find_snapshot(const char *name, BlockDriverState **first_bad_bs)
err = bdrv_snapshot_find(bs, &sn, name);
}
aio_context_release(ctx);
+ if (err < 0) {
+ goto fail;
+ }
}
+fail:
*first_bad_bs = bs;
return err;
}
@@ -462,9 +481,9 @@ int bdrv_all_create_snapshot(QEMUSnapshotInfo *sn,
{
int err = 0;
BlockDriverState *bs;
- BdrvNextIterator *it = NULL;
+ BdrvNextIterator it;
- while (err == 0 && (it = bdrv_next(it, &bs))) {
+ for (bs = bdrv_first(&it); bs; bs = bdrv_next(&it)) {
AioContext *ctx = bdrv_get_aio_context(bs);
aio_context_acquire(ctx);
@@ -476,24 +495,32 @@ int bdrv_all_create_snapshot(QEMUSnapshotInfo *sn,
err = bdrv_snapshot_create(bs, sn);
}
aio_context_release(ctx);
+ if (err < 0) {
+ goto fail;
+ }
}
+fail:
*first_bad_bs = bs;
return err;
}
BlockDriverState *bdrv_all_find_vmstate_bs(void)
{
- bool not_found = true;
BlockDriverState *bs;
- BdrvNextIterator *it = NULL;
+ BdrvNextIterator it;
- while (not_found && (it = bdrv_next(it, &bs))) {
+ for (bs = bdrv_first(&it); bs; bs = bdrv_next(&it)) {
AioContext *ctx = bdrv_get_aio_context(bs);
+ bool found;
aio_context_acquire(ctx);
- not_found = !bdrv_can_snapshot(bs);
+ found = bdrv_can_snapshot(bs);
aio_context_release(ctx);
+
+ if (found) {
+ break;
+ }
}
return bs;
}