On 27.11.19 19:08, Vladimir Sementsov-Ogievskiy wrote: > Split block_copy_find_inflight_req to be used in seprate.
*separate, but separate what?
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com>
> ---
> block/block-copy.c | 31 +++++++++++++++++++------------
> 1 file changed, 19 insertions(+), 12 deletions(-)
>
> diff --git a/block/block-copy.c b/block/block-copy.c
> index 74295d93d5..94e7e855ef 100644
> --- a/block/block-copy.c
> +++ b/block/block-copy.c
> @@ -24,23 +24,30 @@
> #define BLOCK_COPY_MAX_BUFFER (1 * MiB)
> #define BLOCK_COPY_MAX_MEM (128 * MiB)
>
> +static BlockCopyInFlightReq *block_copy_find_inflight_req(BlockCopyState *s,
Hm. Long already, but the name begs the question what in-flight
requests this is about. Maybe drop the block_copy prefix (unless you
plan to make this function global) and call it “find_inflight_conflict”?
(Or “find_conflicting_inflight_req” to be more verbose)
> + int64_t start,
> + int64_t end)
> +{
> + BlockCopyInFlightReq *req;
> +
> + QLIST_FOREACH(req, &s->inflight_reqs, list) {
> + if (end > req->start_byte && start < req->end_byte) {
> + return req;
> + }
> + }
> +
> + return NULL;
> +}
> +
> static void coroutine_fn block_copy_wait_inflight_reqs(BlockCopyState *s,
> int64_t start,
> int64_t end)
> {
> BlockCopyInFlightReq *req;
> - bool waited;
> -
> - do {
> - waited = false;
> - QLIST_FOREACH(req, &s->inflight_reqs, list) {
> - if (end > req->start_byte && start < req->end_byte) {
> - qemu_co_queue_wait(&req->wait_queue, NULL);
> - waited = true;
> - break;
> - }
> - }
> - } while (waited);
> +
> + while ((req = block_copy_find_inflight_req(s, start, end))) {
> + qemu_co_queue_wait(&req->wait_queue, NULL);
> + }
> }
The change itself looks rather nice to me. Even without other users of
this new function, it turns what’s happening into a more obvious pattern.
Max
signature.asc
Description: OpenPGP digital signature
