On Fri, Jul 2, 2010 at 1:07 PM, Kevin Wolf <kw...@redhat.com> wrote:
> bdrv_aio_writev may call the callback immediately (and it will commonly do so
> in error cases). Current code doesn't consider this. For details see the
> comment added by this patch.
>
> Signed-off-by: Kevin Wolf <kw...@redhat.com>
> ---
> block.c | 35 +++++++++++++++++++++++++++++------
> 1 files changed, 29 insertions(+), 6 deletions(-)
>
> diff --git a/block.c b/block.c
> index 9176dec..e65971c 100644
> --- a/block.c
> +++ b/block.c
> @@ -2183,8 +2183,29 @@ int bdrv_aio_multiwrite(BlockDriverState *bs,
> BlockRequest *reqs, int num_reqs)
> // Check for mergable requests
> num_reqs = multiwrite_merge(bs, reqs, num_reqs, mcb);
>
> - // Run the aio requests
> + /*
> + * Run the aio requests. As soon as one request can't be submitted
> + * successfully, fail all requests that are not yet submitted (we must
> + * return failure for all requests anyway)
> + *
> + * num_requests cannot be set to the right value immediately: If
> + * bdrv_aio_writev fails for some request, num_requests would be too high
> + * and therefore multiwrite_cb() would never recognize the multiwrite
> + * request as completed. We also cannot use the loop variable i to set it
> + * when the first request fails because the callback may already have
> been
> + * called for previously submitted requests. Thus, num_requests must be
> + * incremented for each request that is submitted.
> + *
> + * The problem that callbacks may be called early also means that we need
> + * to take care that num_requests doesn't become 0 before all requests
> are
> + * submitted - multiwrite_cb() would consider the multiwrite request
> + * completed. A dummy request that is "completed" by a manual call to
> + * multiwrite_cb() takes care of this.
> + */
> + mcb->num_requests = 1;
> +
> for (i = 0; i < num_reqs; i++) {
> + mcb->num_requests++;
> acb = bdrv_aio_writev(bs, reqs[i].sector, reqs[i].qiov,
> reqs[i].nb_sectors, multiwrite_cb, mcb);
>
> @@ -2192,22 +2213,24 @@ int bdrv_aio_multiwrite(BlockDriverState *bs,
> BlockRequest *reqs, int num_reqs)
> // We can only fail the whole thing if no request has been
> // submitted yet. Otherwise we'll wait for the submitted AIOs to
> // complete and report the error in the callback.
> - if (mcb->num_requests == 0) {
> - reqs[i].error = -EIO;
> + if (i == 0) {
> goto fail;
> } else {
> - mcb->num_requests++;
> multiwrite_cb(mcb, -EIO);
When bdrv_aio_writev() fails we don't know if the callback has been
invoked by the block driver. Qcow2 will invoke the callback in some
cases. This is a problem because num_requests will be decremented
twice if we unconditionally call it here.
Stefan
