SYS-OSS/FRET-qemu
4
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Block patches for 6.1-rc0:

Browse Source 
- Make blkdebug's suspend/resume handling robust (and thread-safe)
 -----BEGIN PGP SIGNATURE-----
 
 iQFGBAABCAAwFiEEkb62CjDbPohX0Rgp9AfbAGHVz0AFAmD1tZwSHG1yZWl0ekBy
 ZWRoYXQuY29tAAoJEPQH2wBh1c9AS3wH/iQx1PsjnCGd/Hufpw9DrAIapmJKHZFw
 igHWkWMvysU6dpfkuZ+g8Zd2VdyDkZazf18qECp5xUqC6kAg/o4hc7Bmnh5nQwfQ
 YrL6E3h1WOuJ70Ee4T8whmaHAEbCWbhYJ5A6xCfUPLvZbsv0uBoBAVgF7MOOPHUR
 LuM2sFTgBIFOTO3vdFwA/A6Zf76u6/Fnc4pbpIala1PaOYDRRTLbC56qW7ZseMjF
 W+SwVAJBullUf3/QgmOcicvtkF/Mor/0HzSF6bujhkb7uLRNb5Ey+B3nVZwR+VzE
 ssCRnCxc7FF5dhB4eUSS+K518zehFybMFDq5HPnFN2rDHqkGP0RnbCA=
 =4WEx
 -----END PGP SIGNATURE-----

Merge remote-tracking branch 'remotes/maxreitz/tags/pull-block-2021-07-19' into staging

Block patches for 6.1-rc0:
- Make blkdebug's suspend/resume handling robust (and thread-safe)

# gpg: Signature made Mon 19 Jul 2021 18:25:48 BST
# gpg:                using RSA key 91BEB60A30DB3E8857D11829F407DB0061D5CF40
# gpg:                issuer "mreitz@redhat.com"
# gpg: Good signature from "Max Reitz <mreitz@redhat.com>" [full]
# Primary key fingerprint: 91BE B60A 30DB 3E88 57D1  1829 F407 DB00 61D5 CF40

* remotes/maxreitz/tags/pull-block-2021-07-19:
  blkdebug: protect rules and suspended_reqs with a lock
  block/blkdebug: remove new_state field and instead use a local variable
  blkdebug: do not suspend in the middle of QLIST_FOREACH_SAFE
  blkdebug: track all actions
  blkdebug: move post-resume handling to resume_req_by_tag
  blkdebug: refactor removal of a suspended request

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
This commit is contained in:
Peter Maydell 2021-07-19 19:06:05 +01:00
commit 143c2e0432
1 changed files with 92 additions and 44 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

136
block/blkdebug.c
View File

@ -38,25 +38,27 @@
#include "qapi/qobject-input-visitor.h" #include "qapi/qobject-input-visitor.h"
#include "sysemu/qtest.h" #include "sysemu/qtest.h"
/* All APIs are thread-safe */
typedef struct BDRVBlkdebugState { typedef struct BDRVBlkdebugState {
int state; /* IN: initialized in blkdebug_open() and never changed */
int new_state;
uint64_t align; uint64_t align;
uint64_t max_transfer; uint64_t max_transfer;
uint64_t opt_write_zero; uint64_t opt_write_zero;
uint64_t max_write_zero; uint64_t max_write_zero;
uint64_t opt_discard; uint64_t opt_discard;
uint64_t max_discard; uint64_t max_discard;
char *config_file; /* For blkdebug_refresh_filename() */
/* initialized in blkdebug_parse_perms() */
uint64_t take_child_perms; uint64_t take_child_perms;
uint64_t unshare_child_perms; uint64_t unshare_child_perms;
/* For blkdebug_refresh_filename() */ /* State. Protected by lock */
char *config_file; int state;
QLIST_HEAD(, BlkdebugRule) rules[BLKDBG__MAX]; QLIST_HEAD(, BlkdebugRule) rules[BLKDBG__MAX];
QSIMPLEQ_HEAD(, BlkdebugRule) active_rules; QSIMPLEQ_HEAD(, BlkdebugRule) active_rules;
QLIST_HEAD(, BlkdebugSuspendedReq) suspended_reqs; QLIST_HEAD(, BlkdebugSuspendedReq) suspended_reqs;
QemuMutex lock;
} BDRVBlkdebugState; } BDRVBlkdebugState;
typedef struct BlkdebugAIOCB { typedef struct BlkdebugAIOCB {
@ -65,8 +67,11 @@ typedef struct BlkdebugAIOCB {
} BlkdebugAIOCB; } BlkdebugAIOCB;
typedef struct BlkdebugSuspendedReq { typedef struct BlkdebugSuspendedReq {
/* IN: initialized in suspend_request() */
Coroutine *co; Coroutine *co;
char *tag; char *tag;
/* List entry protected BDRVBlkdebugState's lock */
QLIST_ENTRY(BlkdebugSuspendedReq) next; QLIST_ENTRY(BlkdebugSuspendedReq) next;
} BlkdebugSuspendedReq; } BlkdebugSuspendedReq;
@ -74,9 +79,11 @@ enum {
ACTION_INJECT_ERROR, ACTION_INJECT_ERROR,
ACTION_SET_STATE, ACTION_SET_STATE,
ACTION_SUSPEND, ACTION_SUSPEND,
ACTION__MAX,
}; };
typedef struct BlkdebugRule { typedef struct BlkdebugRule {
/* IN: initialized in add_rule() or blkdebug_debug_breakpoint() */
BlkdebugEvent event; BlkdebugEvent event;
int action; int action;
int state; int state;
@ -95,6 +102,8 @@ typedef struct BlkdebugRule {
char *tag; char *tag;
} suspend; } suspend;
} options; } options;
/* List entries protected BDRVBlkdebugState's lock */
QLIST_ENTRY(BlkdebugRule) next; QLIST_ENTRY(BlkdebugRule) next;
QSIMPLEQ_ENTRY(BlkdebugRule) active_next; QSIMPLEQ_ENTRY(BlkdebugRule) active_next;
} BlkdebugRule; } BlkdebugRule;
@ -244,11 +253,14 @@ static int add_rule(void *opaque, QemuOpts *opts, Error **errp)
}; };
/* Add the rule */ /* Add the rule */
qemu_mutex_lock(&s->lock);
QLIST_INSERT_HEAD(&s->rules[event], rule, next); QLIST_INSERT_HEAD(&s->rules[event], rule, next);
qemu_mutex_unlock(&s->lock);
return 0; return 0;
} }
/* Called with lock held or from .bdrv_close */
static void remove_rule(BlkdebugRule *rule) static void remove_rule(BlkdebugRule *rule)
{ {
switch (rule->action) { switch (rule->action) {
@ -467,6 +479,7 @@ static int blkdebug_open(BlockDriverState *bs, QDict *options, int flags,
int ret; int ret;
uint64_t align; uint64_t align;
qemu_mutex_init(&s->lock);
opts = qemu_opts_create(&runtime_opts, NULL, 0, &error_abort); opts = qemu_opts_create(&runtime_opts, NULL, 0, &error_abort);
if (!qemu_opts_absorb_qdict(opts, options, errp)) { if (!qemu_opts_absorb_qdict(opts, options, errp)) {
ret = -EINVAL; ret = -EINVAL;
@ -567,6 +580,7 @@ static int blkdebug_open(BlockDriverState *bs, QDict *options, int flags,
ret = 0; ret = 0;
out: out:
if (ret < 0) { if (ret < 0) {
qemu_mutex_destroy(&s->lock);
g_free(s->config_file); g_free(s->config_file);
} }
qemu_opts_del(opts); qemu_opts_del(opts);
@ -581,6 +595,7 @@ static int rule_check(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
int error; int error;
bool immediately; bool immediately;
qemu_mutex_lock(&s->lock);
QSIMPLEQ_FOREACH(rule, &s->active_rules, active_next) { QSIMPLEQ_FOREACH(rule, &s->active_rules, active_next) {
uint64_t inject_offset = rule->options.inject.offset; uint64_t inject_offset = rule->options.inject.offset;
@ -594,6 +609,7 @@ static int rule_check(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
} }
if (!rule || !rule->options.inject.error) { if (!rule || !rule->options.inject.error) {
qemu_mutex_unlock(&s->lock);
return 0; return 0;
} }
@ -605,6 +621,7 @@ static int rule_check(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
remove_rule(rule); remove_rule(rule);
} }
qemu_mutex_unlock(&s->lock);
if (!immediately) { if (!immediately) {
aio_co_schedule(qemu_get_current_aio_context(), qemu_coroutine_self()); aio_co_schedule(qemu_get_current_aio_context(), qemu_coroutine_self());
qemu_coroutine_yield(); qemu_coroutine_yield();
@ -770,78 +787,80 @@ static void blkdebug_close(BlockDriverState *bs)
} }
g_free(s->config_file); g_free(s->config_file);
qemu_mutex_destroy(&s->lock);
} }
/* Called with lock held. */
static void suspend_request(BlockDriverState *bs, BlkdebugRule *rule) static void suspend_request(BlockDriverState *bs, BlkdebugRule *rule)
{ {
BDRVBlkdebugState *s = bs->opaque; BDRVBlkdebugState *s = bs->opaque;
BlkdebugSuspendedReq r; BlkdebugSuspendedReq *r;
r = (BlkdebugSuspendedReq) { r = g_new(BlkdebugSuspendedReq, 1);
.co = qemu_coroutine_self(),
.tag = g_strdup(rule->options.suspend.tag), r->co = qemu_coroutine_self();
}; r->tag = g_strdup(rule->options.suspend.tag);
remove_rule(rule); remove_rule(rule);
QLIST_INSERT_HEAD(&s->suspended_reqs, &r, next); QLIST_INSERT_HEAD(&s->suspended_reqs, r, next);
if (!qtest_enabled()) { if (!qtest_enabled()) {
printf("blkdebug: Suspended request '%s'\n", r.tag); printf("blkdebug: Suspended request '%s'\n", r->tag);
} }
qemu_coroutine_yield();
if (!qtest_enabled()) {
printf("blkdebug: Resuming request '%s'\n", r.tag);
}
QLIST_REMOVE(&r, next);
g_free(r.tag);
} }
static bool process_rule(BlockDriverState *bs, struct BlkdebugRule *rule, /* Called with lock held. */
bool injected) static void process_rule(BlockDriverState *bs, struct BlkdebugRule *rule,
int *action_count, int *new_state)
{ {
BDRVBlkdebugState *s = bs->opaque; BDRVBlkdebugState *s = bs->opaque;
/* Only process rules for the current state */ /* Only process rules for the current state */
if (rule->state && rule->state != s->state) { if (rule->state && rule->state != s->state) {
return injected; return;
} }
/* Take the action */ /* Take the action */
action_count[rule->action]++;
switch (rule->action) { switch (rule->action) {
case ACTION_INJECT_ERROR: case ACTION_INJECT_ERROR:
if (!injected) { if (action_count[ACTION_INJECT_ERROR] == 1) {
QSIMPLEQ_INIT(&s->active_rules); QSIMPLEQ_INIT(&s->active_rules);
injected = true;
} }
QSIMPLEQ_INSERT_HEAD(&s->active_rules, rule, active_next); QSIMPLEQ_INSERT_HEAD(&s->active_rules, rule, active_next);
break; break;
case ACTION_SET_STATE: case ACTION_SET_STATE:
s->new_state = rule->options.set_state.new_state; *new_state = rule->options.set_state.new_state;
break; break;
case ACTION_SUSPEND: case ACTION_SUSPEND:
suspend_request(bs, rule); suspend_request(bs, rule);
break; break;
} }
return injected;
} }
static void blkdebug_debug_event(BlockDriverState *bs, BlkdebugEvent event) static void blkdebug_debug_event(BlockDriverState *bs, BlkdebugEvent event)
{ {
BDRVBlkdebugState *s = bs->opaque; BDRVBlkdebugState *s = bs->opaque;
struct BlkdebugRule *rule, *next; struct BlkdebugRule *rule, *next;
bool injected; int new_state;
int actions_count[ACTION__MAX] = { 0 };
assert((int)event >= 0 && event < BLKDBG__MAX); assert((int)event >= 0 && event < BLKDBG__MAX);
injected = false; WITH_QEMU_LOCK_GUARD(&s->lock) {
s->new_state = s->state; new_state = s->state;
QLIST_FOREACH_SAFE(rule, &s->rules[event], next, next) { QLIST_FOREACH_SAFE(rule, &s->rules[event], next, next) {
injected = process_rule(bs, rule, injected); process_rule(bs, rule, actions_count, &new_state);
}
s->state = new_state;
}
while (actions_count[ACTION_SUSPEND] > 0) {
qemu_coroutine_yield();
actions_count[ACTION_SUSPEND]--;
} }
s->state = s->new_state;
} }
static int blkdebug_debug_breakpoint(BlockDriverState *bs, const char *event, static int blkdebug_debug_breakpoint(BlockDriverState *bs, const char *event,
@ -864,33 +883,64 @@ static int blkdebug_debug_breakpoint(BlockDriverState *bs, const char *event,
.options.suspend.tag = g_strdup(tag), .options.suspend.tag = g_strdup(tag),
}; };
qemu_mutex_lock(&s->lock);
QLIST_INSERT_HEAD(&s->rules[blkdebug_event], rule, next); QLIST_INSERT_HEAD(&s->rules[blkdebug_event], rule, next);
qemu_mutex_unlock(&s->lock);
return 0; return 0;
} }
static int blkdebug_debug_resume(BlockDriverState *bs, const char *tag) /* Called with lock held. May temporarily release lock. */
static int resume_req_by_tag(BDRVBlkdebugState *s, const char *tag, bool all)
{ {
BDRVBlkdebugState *s = bs->opaque; BlkdebugSuspendedReq *r;
BlkdebugSuspendedReq *r, *next;
QLIST_FOREACH_SAFE(r, &s->suspended_reqs, next, next) { retry:
/*
* No need for _SAFE, since a different coroutine can remove another node
* (not the current one) in this list, and when the current one is removed
* the iteration starts back from beginning anyways.
*/
QLIST_FOREACH(r, &s->suspended_reqs, next) {
if (!strcmp(r->tag, tag)) { if (!strcmp(r->tag, tag)) {
qemu_coroutine_enter(r->co); Coroutine *co = r->co;
if (!qtest_enabled()) {
printf("blkdebug: Resuming request '%s'\n", r->tag);
}
QLIST_REMOVE(r, next);
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;
}
return 0; return 0;
} }
} }
return -ENOENT; return -ENOENT;
} }
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);
}
static int blkdebug_debug_remove_breakpoint(BlockDriverState *bs, static int blkdebug_debug_remove_breakpoint(BlockDriverState *bs,
const char *tag) const char *tag)
{ {
BDRVBlkdebugState *s = bs->opaque; BDRVBlkdebugState *s = bs->opaque;
BlkdebugSuspendedReq *r, *r_next;
BlkdebugRule *rule, *next; BlkdebugRule *rule, *next;
int i, ret = -ENOENT; int i, ret = -ENOENT;
QEMU_LOCK_GUARD(&s->lock);
for (i = 0; i < BLKDBG__MAX; i++) { for (i = 0; i < BLKDBG__MAX; i++) {
QLIST_FOREACH_SAFE(rule, &s->rules[i], next, next) { QLIST_FOREACH_SAFE(rule, &s->rules[i], next, next) {
if (rule->action == ACTION_SUSPEND && if (rule->action == ACTION_SUSPEND &&
@ -900,11 +950,8 @@ static int blkdebug_debug_remove_breakpoint(BlockDriverState *bs,
} }
} }
} }
QLIST_FOREACH_SAFE(r, &s->suspended_reqs, next, r_next) { if (resume_req_by_tag(s, tag, true) == 0) {
if (!strcmp(r->tag, tag)) { ret = 0;
qemu_coroutine_enter(r->co);
ret = 0;
}
} }
return ret; return ret;
} }
@ -914,6 +961,7 @@ static bool blkdebug_debug_is_suspended(BlockDriverState *bs, const char *tag)
BDRVBlkdebugState *s = bs->opaque; BDRVBlkdebugState *s = bs->opaque;
BlkdebugSuspendedReq *r; BlkdebugSuspendedReq *r;
QEMU_LOCK_GUARD(&s->lock);
QLIST_FOREACH(r, &s->suspended_reqs, next) { QLIST_FOREACH(r, &s->suspended_reqs, next) {
if (!strcmp(r->tag, tag)) { if (!strcmp(r->tag, tag)) {
return true; return true;