[Qemu-commits] [qemu/qemu] 69d069: blkdebug: refactor removal of a suspended request

[Peter Maydell]

  Branch: refs/heads/master
  Home:   https://github.com/qemu/qemu
  Commit: 69d0690c10083f21c7daaac544a44589a3ee34fc
      
https://github.com/qemu/qemu/commit/69d0690c10083f21c7daaac544a44589a3ee34fc
  Author: Emanuele Giuseppe Esposito <eesposit@redhat.com>
  Date:   2021-07-19 (Mon, 19 Jul 2021)

  Changed paths:
    M block/blkdebug.c

  Log Message:
  -----------
  blkdebug: refactor removal of a suspended request

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 <pbonzini@redhat.com>
Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20210614082931.24925-2-eesposit@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>


  Commit: f48ff5af13eed0d2b39fdb91a37ed45fa3429e89
      
https://github.com/qemu/qemu/commit/f48ff5af13eed0d2b39fdb91a37ed45fa3429e89
  Author: Emanuele Giuseppe Esposito <eesposit@redhat.com>
  Date:   2021-07-19 (Mon, 19 Jul 2021)

  Changed paths:
    M block/blkdebug.c

  Log Message:
  -----------
  blkdebug: move post-resume handling to resume_req_by_tag

We want to move qemu_coroutine_yield() after the loop on rules,
because QLIST_FOREACH_SAFE is wrong if the rule list is modified
while the coroutine has yielded.  Therefore move the suspended
request to the heap and clean it up from the remove side.
All that is left is for blkdebug_debug_event to handle the
yielding.

Co-developed-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20210614082931.24925-3-eesposit@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>


  Commit: 51a463680d5620c15b8e88e73c75e4692553c3b5
      
https://github.com/qemu/qemu/commit/51a463680d5620c15b8e88e73c75e4692553c3b5
  Author: Emanuele Giuseppe Esposito <eesposit@redhat.com>
  Date:   2021-07-19 (Mon, 19 Jul 2021)

  Changed paths:
    M block/blkdebug.c

  Log Message:
  -----------
  blkdebug: track all actions

Add a counter for each action that a rule can trigger.
This is mainly used to keep track of how many coroutine_yield()
we need to perform after processing all rules in the list.

Co-developed-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20210614082931.24925-4-eesposit@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>


  Commit: 2196c341f7d0df161d412d3d7ea81545ab60ea2b
      
https://github.com/qemu/qemu/commit/2196c341f7d0df161d412d3d7ea81545ab60ea2b
  Author: Emanuele Giuseppe Esposito <eesposit@redhat.com>
  Date:   2021-07-19 (Mon, 19 Jul 2021)

  Changed paths:
    M block/blkdebug.c

  Log Message:
  -----------
  blkdebug: do not suspend in the middle of QLIST_FOREACH_SAFE

That would be unsafe in case a rule other than the current one
is removed while the coroutine has yielded.
Keep FOREACH_SAFE because suspend_request deletes the current rule.

After this patch, *all* matching rules are deleted before suspending
the coroutine, rather than just one.
This doesn't affect the existing testcases.

Use actions_count to see how many yield to issue.

Co-developed-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20210614082931.24925-5-eesposit@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>


  Commit: 4153b553bd81e5b270b816699184df5d74c46805
      
https://github.com/qemu/qemu/commit/4153b553bd81e5b270b816699184df5d74c46805
  Author: Emanuele Giuseppe Esposito <eesposit@redhat.com>
  Date:   2021-07-19 (Mon, 19 Jul 2021)

  Changed paths:
    M block/blkdebug.c

  Log Message:
  -----------
  block/blkdebug: remove new_state field and instead use a local variable

There seems to be no benefit in using a field. Replace it with a local
variable, and move the state update before the yields.

The state update has do be done before the yields because now using
a local variable does not allow the new updated state to be visible
by the other yields.

Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
Message-Id: <20210614082931.24925-6-eesposit@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>


  Commit: 36109bff171ba0811fa4c723cecdf6c3561fa318
      
https://github.com/qemu/qemu/commit/36109bff171ba0811fa4c723cecdf6c3561fa318
  Author: Emanuele Giuseppe Esposito <eesposit@redhat.com>
  Date:   2021-07-19 (Mon, 19 Jul 2021)

  Changed paths:
    M block/blkdebug.c

  Log Message:
  -----------
  blkdebug: protect rules and suspended_reqs with a lock

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>


  Commit: 143c2e0432859826c9e8d5b2baa307355f1a5332
      
https://github.com/qemu/qemu/commit/143c2e0432859826c9e8d5b2baa307355f1a5332
  Author: Peter Maydell <peter.maydell@linaro.org>
  Date:   2021-07-19 (Mon, 19 Jul 2021)

  Changed paths:
    M block/blkdebug.c

  Log Message:
  -----------
  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>


Compare: https://github.com/qemu/qemu/compare/7457b407edd6...143c2e043285