mirrors/qemu
0
0
Fork 0
mirror of https://github.com/qemu/qemu.git synced 2024-11-23 10:53:37 +08:00
Code Issues Packages Projects Releases Wiki Activity

block: Fix bdrv_next() memory leak

Browse Source 
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>
This commit is contained in:
Kevin Wolf 2016-05-20 18:49:07 +02:00
parent 287db79df8
commit 88be7b4be4
9 changed files with 92 additions and 64 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

18
block.c
View File

@ -3195,9 +3195,9 @@ void bdrv_invalidate_cache_all(Error **errp)
{
BlockDriverState *bs;
Error *local_err = NULL;
BdrvNextIterator *it = NULL;
BdrvNextIterator it;
while ((it = bdrv_next(it, &bs)) != NULL) {
for (bs = bdrv_first(&it); bs; bs = bdrv_next(&it)) {
AioContext *aio_context = bdrv_get_aio_context(bs);
aio_context_acquire(aio_context);
@ -3239,11 +3239,11 @@ static int bdrv_inactivate_recurse(BlockDriverState *bs,
int bdrv_inactivate_all(void)
{
BlockDriverState *bs = NULL;
BdrvNextIterator *it = NULL;
BdrvNextIterator it;
int ret = 0;
int pass;
while ((it = bdrv_next(it, &bs)) != NULL) {
for (bs = bdrv_first(&it); bs; bs = bdrv_next(&it)) {
aio_context_acquire(bdrv_get_aio_context(bs));
}
@ -3252,8 +3252,7 @@ int bdrv_inactivate_all(void)
* the second pass sets the BDRV_O_INACTIVE flag so that no further write
* is allowed. */
for (pass = 0; pass < 2; pass++) {
it = NULL;
while ((it = bdrv_next(it, &bs)) != NULL) {
for (bs = bdrv_first(&it); bs; bs = bdrv_next(&it)) {
ret = bdrv_inactivate_recurse(bs, pass);
if (ret < 0) {
goto out;
@ -3262,8 +3261,7 @@ int bdrv_inactivate_all(void)
}
out:
it = NULL;
while ((it = bdrv_next(it, &bs)) != NULL) {
for (bs = bdrv_first(&it); bs; bs = bdrv_next(&it)) {
aio_context_release(bdrv_get_aio_context(bs));
}
@ -3753,10 +3751,10 @@ bool bdrv_recurse_is_first_non_filter(BlockDriverState *bs,
bool bdrv_is_first_non_filter(BlockDriverState *candidate)
{
BlockDriverState *bs;
BdrvNextIterator *it = NULL;
BdrvNextIterator it;
/* walk down the bs forest recursively */
while ((it = bdrv_next(it, &bs)) != NULL) {
for (bs = bdrv_first(&it); bs; bs = bdrv_next(&it)) {
bool perm;
/* try to recurse in this top level bs */

41
block/block-backend.c
View File

@ -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 ? it : NULL;
return bs;
}
BlockDriverState *bdrv_first(BdrvNextIterator *it)
{
*it = (BdrvNextIterator) {
.phase = BDRV_NEXT_BACKEND_ROOTS,
};
return bdrv_next(it);
}
/*

10
block/io.c
View File

@ -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);

55
block/snapshot.c
View File

@ -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;
}

4
blockdev.c
View File

@ -4164,9 +4164,9 @@ BlockJobInfoList *qmp_query_block_jobs(Error **errp)
{
BlockJobInfoList *head = NULL, **p_next = &head;
BlockDriverState *bs;
BdrvNextIterator *it = NULL;
BdrvNextIterator it;
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);

15
include/block/block.h
View File

@ -17,7 +17,6 @@ typedef struct BlockJob BlockJob;
typedef struct BdrvChild BdrvChild;
typedef struct BdrvChildRole BdrvChildRole;
typedef struct BlockJobTxn BlockJobTxn;
typedef struct BdrvNextIterator BdrvNextIterator;
typedef struct BlockDriverInfo {
/* in bytes, 0 if irrelevant */
@ -402,7 +401,19 @@ BlockDriverState *bdrv_lookup_bs(const char *device,
Error **errp);
bool bdrv_chain_contains(BlockDriverState *top, BlockDriverState *base);
BlockDriverState *bdrv_next_node(BlockDriverState *bs);
BdrvNextIterator *bdrv_next(BdrvNextIterator *it, BlockDriverState **bs);
typedef struct BdrvNextIterator {
enum {
BDRV_NEXT_BACKEND_ROOTS,
BDRV_NEXT_MONITOR_OWNED,
} phase;
BlockBackend *blk;
BlockDriverState *bs;
} BdrvNextIterator;
BlockDriverState *bdrv_first(BdrvNextIterator *it);
BlockDriverState *bdrv_next(BdrvNextIterator *it);
BlockDriverState *bdrv_next_monitor_owned(BlockDriverState *bs);
int bdrv_is_encrypted(BlockDriverState *bs);
int bdrv_key_required(BlockDriverState *bs);

4
migration/block.c
View File

@ -383,7 +383,7 @@ static void init_blk_migration(QEMUFile *f)
BlockDriverState *bs;
BlkMigDevState *bmds;
int64_t sectors;
BdrvNextIterator *it = NULL;
BdrvNextIterator it;
block_mig_state.submitted = 0;
block_mig_state.read_done = 0;
@ -394,7 +394,7 @@ static void init_blk_migration(QEMUFile *f)
block_mig_state.zero_blocks = migrate_zero_blocks();
while ((it = bdrv_next(it, &bs))) {
for (bs = bdrv_first(&it); bs; bs = bdrv_next(&it)) {
if (bdrv_is_read_only(bs)) {
continue;
}

4
monitor.c
View File

@ -3432,12 +3432,12 @@ static void vm_completion(ReadLineState *rs, const char *str)
{
size_t len;
BlockDriverState *bs;
BdrvNextIterator *it = NULL;
BdrvNextIterator it;
len = strlen(str);
readline_set_completion_index(rs, len);
while ((it = bdrv_next(it, &bs))) {
for (bs = bdrv_first(&it); bs; bs = bdrv_next(&it)) {
SnapshotInfoList *snapshots, *snapshot;
AioContext *ctx = bdrv_get_aio_context(bs);
bool ok = false;

5
qmp.c
View File

@ -181,7 +181,7 @@ void qmp_cont(Error **errp)
Error *local_err = NULL;
BlockBackend *blk;
BlockDriverState *bs;
BdrvNextIterator *it;
BdrvNextIterator it;
/* if there is a dump in background, we should wait until the dump
* finished */
@ -201,8 +201,7 @@ void qmp_cont(Error **errp)
blk_iostatus_reset(blk);
}
it = NULL;
while ((it = bdrv_next(it, &bs))) {
for (bs = bdrv_first(&it); bs; bs = bdrv_next(&it)) {
bdrv_add_key(bs, NULL, &local_err);
if (local_err) {
error_propagate(errp, local_err);