Re: [Qemu-devel] [PATCH 07/19] block: Really pause block jobs on drain

[Paolo Bonzini]

On 12/04/2018 15:27, Kevin Wolf wrote:
> Not sure I follow. Let's look at an example. Say, we have a block job
> BlockBackend as the root (because that uses proper layering, unlike
> devices which use aio_disable_external()), connected to a qcow2 node
> over file.
> 
> 1. The block job issues a request to the qcow2 node
> 
> 2. In order to process that request, the qcow2 node internally issues
>    one or more requests to the file node
> 
> 3. Someone calls bdrv_drained_begin(qcow2_node)
> 
> 4. We call BlockDriver.bdrv_drained_begin() for qcow2_node and
>    file_node, and BdrvChildRole.drained_begin() for the block job.
> 
>    Now the block nodes don't create any new original requests any more
>    (qcow2 and file don't do that anyway; qcow2 only creates requests to
>    its children in the context of parent requests). The job potentially
>    continues sending requests until it reaches the next pause point. The
>    previously issued requests are still in flight.
>
>    Is this step what you meant by X->drv->bdrv_drain(X)? I don't see why
>    pending requests can only be in X's children. Why can't the request
>    be pending in X itself, say waiting for a thread pool worker
>    decrypting a buffer?

No, that step is ->bdrv_co_drain_begin in BlockDriver.  It's where the
"last" requests are sent to file_node after we know that qcow2_node
won't get any more requests.

>    Also, note that after this series, the driver callbacks are called
>    asynchronously, but I don't think it makes a big difference here.
> 
> 5. The file_node request completes. file_node doesn't have any requests
>    in flight any more, but in theory it could still get new requests
>    from qcow2_node. Anyway, let's say this is the last request, so I
>    think we'd call its requests concluded?

No, if it can still get more requests they're not concluded.  That's why
we need to first ensure qcow2_node is quiescent, and before then we need
to ensure that the BlockBackends are quiescent (in this case meaning the
job has reached its pause point).  Only then we can look at file_node.
In this case we'll see that we have nothing to do---file_node is already
quiescent---and bdrv_drained_begin() can return.

> 6. qcow2 still has a request in flight, but doesn't need to access
>    file_node for it. It finishes the work and therefore concludes its
>    requests as well. Note that qcow2_node (the parent) concludes after
>    file_node (the child).
> 
> 7. We'll keep the example simple, so after completion of its request,
>    the job reaches a pause point without sending a new request. Again,
>    this happens after qcow2_node has concluded.
> 
> 8. Only when neither file_node nor qcow2_node have a request in flight
>    and the job has reached a pause point, bdrv_drained_begin() can
>    return.
> 
> So completing the last request and reaching an actually quiescent state
> looks very much like a process in child-to-parent order to me?