On Fri, Jun 04, 2021 at 12:07:36PM +0200, Emanuele Giuseppe Esposito wrote: > Extract to a separate function. Do not rely on FOREACH_SAFE, which is > only "safe" if the *current* node is removed---not if another node is > removed. Instead, just walk the entire list from the beginning when > asked to resume all suspended requests with a given tag. > > Co-developed-by: Paolo Bonzini <pbonz...@redhat.com> > Signed-off-by: Emanuele Giuseppe Esposito <eespo...@redhat.com> > Reviewed-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com> > --- > block/blkdebug.c | 28 +++++++++++++++++----------- > 1 file changed, 17 insertions(+), 11 deletions(-) > > diff --git a/block/blkdebug.c b/block/blkdebug.c > index 2c0b9b0ee8..8f19d991fa 100644 > --- a/block/blkdebug.c
> -static int blkdebug_debug_resume(BlockDriverState *bs, const char *tag) > +static int resume_req_by_tag(BDRVBlkdebugState *s, const char *tag, bool all) > { > - BDRVBlkdebugState *s = bs->opaque; > - BlkdebugSuspendedReq *r, *next; > + BlkdebugSuspendedReq *r; > > - QLIST_FOREACH_SAFE(r, &s->suspended_reqs, next, next) { > +retry: > + QLIST_FOREACH(r, &s->suspended_reqs, next) { > if (!strcmp(r->tag, tag)) { > + QLIST_REMOVE(r, next); Isn't the whole point of using QLIST_FOREACH_SAFE the ability to call QLIST_REMOVE on an element in that list while still iterating? > qemu_coroutine_enter(r->co); > + if (all) { > + goto retry; > + } > return 0; Oh, I see - you abandon the iteration in all control flow paths, so the simpler loop is still okay. Still, this confused me enough on first read that it may be worth a comment, maybe: /* No need for _SAFE, because iteration stops on first hit */ Reviewed-by: Eric Blake <ebl...@redhat.com> -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org