Skip to content

Commit

Permalink
blkdebug: protect rules and suspended_reqs with a lock
Browse files Browse the repository at this point in the history 
First, categorize the structure fields to identify what needs
to be protected and what doesn't.

We essentially need to protect only .state, and the 3 lists in
BDRVBlkdebugState.

Then, add the lock and mark the functions accordingly.

Co-developed-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
Message-Id: <20210614082931.24925-7-eesposit@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
  • Loading branch information
@esposem @XanClic
esposem authored and XanClic committed Jul 19, 2021
1 parent 4153b55 commit 36109bf
Showing 1 changed file with 39 additions and 10 deletions.
49 changes: 39 additions & 10 deletions block/blkdebug.c
View file
Expand Up @@ -38,24 +38,27 @@
#include "qapi/qobject-input-visitor.h"
#include "sysemu/qtest.h"

/* All APIs are thread-safe */

typedef struct BDRVBlkdebugState {
int state;
/* IN: initialized in blkdebug_open() and never changed */
uint64_t align;
uint64_t max_transfer;
uint64_t opt_write_zero;
uint64_t max_write_zero;
uint64_t opt_discard;
uint64_t max_discard;

char *config_file; /* For blkdebug_refresh_filename() */
/* initialized in blkdebug_parse_perms() */
uint64_t take_child_perms;
uint64_t unshare_child_perms;

/* For blkdebug_refresh_filename() */
char *config_file;

/* State. Protected by lock */
int state;
QLIST_HEAD(, BlkdebugRule) rules[BLKDBG__MAX];
QSIMPLEQ_HEAD(, BlkdebugRule) active_rules;
QLIST_HEAD(, BlkdebugSuspendedReq) suspended_reqs;
QemuMutex lock;
} BDRVBlkdebugState;

typedef struct BlkdebugAIOCB {
Expand All @@ -64,8 +67,11 @@ typedef struct BlkdebugAIOCB {
} BlkdebugAIOCB;

typedef struct BlkdebugSuspendedReq {
/* IN: initialized in suspend_request() */
Coroutine *co;
char *tag;

/* List entry protected BDRVBlkdebugState's lock */
QLIST_ENTRY(BlkdebugSuspendedReq) next;
} BlkdebugSuspendedReq;

Expand All @@ -77,6 +83,7 @@ enum {
};

typedef struct BlkdebugRule {
/* IN: initialized in add_rule() or blkdebug_debug_breakpoint() */
BlkdebugEvent event;
int action;
int state;
Expand All @@ -95,6 +102,8 @@ typedef struct BlkdebugRule {
char *tag;
} suspend;
} options;

/* List entries protected BDRVBlkdebugState's lock */
QLIST_ENTRY(BlkdebugRule) next;
QSIMPLEQ_ENTRY(BlkdebugRule) active_next;
} BlkdebugRule;
Expand Down Expand Up @@ -244,11 +253,14 @@ static int add_rule(void *opaque, QemuOpts *opts, Error **errp)
};

/* Add the rule */
qemu_mutex_lock(&s->lock);
QLIST_INSERT_HEAD(&s->rules[event], rule, next);
qemu_mutex_unlock(&s->lock);

return 0;
}

/* Called with lock held or from .bdrv_close */
static void remove_rule(BlkdebugRule *rule)
{
switch (rule->action) {
Expand Down Expand Up @@ -467,6 +479,7 @@ static int blkdebug_open(BlockDriverState *bs, QDict *options, int flags,
int ret;
uint64_t align;

qemu_mutex_init(&s->lock);
opts = qemu_opts_create(&runtime_opts, NULL, 0, &error_abort);
if (!qemu_opts_absorb_qdict(opts, options, errp)) {
ret = -EINVAL;
Expand Down Expand Up @@ -567,6 +580,7 @@ static int blkdebug_open(BlockDriverState *bs, QDict *options, int flags,
ret = 0;
out:
if (ret < 0) {
qemu_mutex_destroy(&s->lock);
g_free(s->config_file);
}
qemu_opts_del(opts);
Expand All @@ -581,6 +595,7 @@ static int rule_check(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
int error;
bool immediately;

qemu_mutex_lock(&s->lock);
QSIMPLEQ_FOREACH(rule, &s->active_rules, active_next) {
uint64_t inject_offset = rule->options.inject.offset;

Expand All @@ -594,6 +609,7 @@ static int rule_check(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
}

if (!rule || !rule->options.inject.error) {
qemu_mutex_unlock(&s->lock);
return 0;
}

Expand All @@ -605,6 +621,7 @@ static int rule_check(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
remove_rule(rule);
}

qemu_mutex_unlock(&s->lock);
if (!immediately) {
aio_co_schedule(qemu_get_current_aio_context(), qemu_coroutine_self());
qemu_coroutine_yield();
Expand Down Expand Up @@ -770,8 +787,10 @@ static void blkdebug_close(BlockDriverState *bs)
}

g_free(s->config_file);
qemu_mutex_destroy(&s->lock);
}

/* Called with lock held. */
static void suspend_request(BlockDriverState *bs, BlkdebugRule *rule)
{
BDRVBlkdebugState *s = bs->opaque;
Expand All @@ -790,6 +809,7 @@ static void suspend_request(BlockDriverState *bs, BlkdebugRule *rule)
}
}

/* Called with lock held. */
static void process_rule(BlockDriverState *bs, struct BlkdebugRule *rule,
int *action_count, int *new_state)
{
Expand Down Expand Up @@ -829,11 +849,13 @@ static void blkdebug_debug_event(BlockDriverState *bs, BlkdebugEvent event)

assert((int)event >= 0 && event < BLKDBG__MAX);

new_state = s->state;
QLIST_FOREACH_SAFE(rule, &s->rules[event], next, next) {
process_rule(bs, rule, actions_count, &new_state);
WITH_QEMU_LOCK_GUARD(&s->lock) {
new_state = s->state;
QLIST_FOREACH_SAFE(rule, &s->rules[event], next, next) {
process_rule(bs, rule, actions_count, &new_state);
}
s->state = new_state;
}
s->state = new_state;

while (actions_count[ACTION_SUSPEND] > 0) {
qemu_coroutine_yield();
Expand Down Expand Up @@ -861,11 +883,14 @@ static int blkdebug_debug_breakpoint(BlockDriverState *bs, const char *event,
.options.suspend.tag = g_strdup(tag),
};

qemu_mutex_lock(&s->lock);
QLIST_INSERT_HEAD(&s->rules[blkdebug_event], rule, next);
qemu_mutex_unlock(&s->lock);

return 0;
}

/* Called with lock held. May temporarily release lock. */
static int resume_req_by_tag(BDRVBlkdebugState *s, const char *tag, bool all)
{
BlkdebugSuspendedReq *r;
Expand All @@ -888,7 +913,9 @@ static int resume_req_by_tag(BDRVBlkdebugState *s, const char *tag, bool all)
g_free(r->tag);
g_free(r);

qemu_mutex_unlock(&s->lock);
qemu_coroutine_enter(co);
qemu_mutex_lock(&s->lock);

if (all) {
goto retry;
Expand All @@ -902,7 +929,7 @@ static int resume_req_by_tag(BDRVBlkdebugState *s, const char *tag, bool all)
static int blkdebug_debug_resume(BlockDriverState *bs, const char *tag)
{
BDRVBlkdebugState *s = bs->opaque;

QEMU_LOCK_GUARD(&s->lock);
return resume_req_by_tag(s, tag, false);
}

Expand All @@ -913,6 +940,7 @@ static int blkdebug_debug_remove_breakpoint(BlockDriverState *bs,
BlkdebugRule *rule, *next;
int i, ret = -ENOENT;

QEMU_LOCK_GUARD(&s->lock);
for (i = 0; i < BLKDBG__MAX; i++) {
QLIST_FOREACH_SAFE(rule, &s->rules[i], next, next) {
if (rule->action == ACTION_SUSPEND &&
Expand All @@ -933,6 +961,7 @@ static bool blkdebug_debug_is_suspended(BlockDriverState *bs, const char *tag)
BDRVBlkdebugState *s = bs->opaque;
BlkdebugSuspendedReq *r;

QEMU_LOCK_GUARD(&s->lock);
QLIST_FOREACH(r, &s->suspended_reqs, next) {
if (!strcmp(r->tag, tag)) {
return true;
Expand Down

0 comments on commit 36109bf

Please sign in to comment.