qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Kevin Wolf <kwolf@redhat.com>
To: aurelien@aurel32.net
Cc: kwolf@redhat.com, qemu-devel@nongnu.org
Subject: [Qemu-devel] [STABLE][PATCH 13/14] block: Fix early failure in multiwrite
Date: Wed, 14 Jul 2010 13:24:12 +0200	[thread overview]
Message-ID: <1279106653-24351-14-git-send-email-kwolf@redhat.com> (raw)
In-Reply-To: <1279106653-24351-1-git-send-email-kwolf@redhat.com>

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 <kwolf@redhat.com>
(cherry picked from commit 453f9a1652629e5805995b165be2e634c8487139)

Conflicts:

	block.c

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block.c |   37 ++++++++++++++++++++++++++++++-------
 1 files changed, 30 insertions(+), 7 deletions(-)

diff --git a/block.c b/block.c
index e3b3a55..80f2fae 100644
--- a/block.c
+++ b/block.c
@@ -1802,8 +1802,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);
 
@@ -1811,23 +1832,25 @@ 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);
                 break;
             }
-        } else {
-            mcb->num_requests++;
         }
     }
 
+    /* Complete the dummy request */
+    multiwrite_cb(mcb, 0);
+
     return 0;
 
 fail:
-    free(mcb);
+    for (i = 0; i < mcb->num_callbacks; i++) {
+        reqs[i].error = -EIO;
+    }
+    qemu_free(mcb);
     return -1;
 }
 
-- 
1.7.1.1

  parent reply	other threads:[~2010-07-14 11:24 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-07-14 11:23 [Qemu-devel] [STABLE][PULL 00/14] Block patches for 0.12.5 Kevin Wolf
2010-07-14 11:24 ` [Qemu-devel] [STABLE][PATCH 01/14] vmdk: fix double free Kevin Wolf
2010-07-14 11:24 ` [Qemu-devel] [STABLE][PATCH 02/14] qcow2: Fix creation of large images Kevin Wolf
2010-07-14 11:24 ` [Qemu-devel] [STABLE][PATCH 03/14] vmdk: Fix COW Kevin Wolf
2010-07-14 11:24 ` [Qemu-devel] [STABLE][PATCH 04/14] qcow2: Remove abort on free_clusters failure Kevin Wolf
2010-07-14 11:24 ` [Qemu-devel] [STABLE][PATCH 05/14] block/vpc: Fix conversion from size to disk geometry Kevin Wolf
2010-07-14 11:24 ` [Qemu-devel] [STABLE][PATCH 06/14] block/vdi: Fix image opening and creation for odd disk sizes Kevin Wolf
2010-07-14 11:24 ` [Qemu-devel] [STABLE][PATCH 07/14] qcow2: Restore L1 entry on l2_allocate failure Kevin Wolf
2010-07-14 11:24 ` [Qemu-devel] [STABLE][PATCH 08/14] block: Add bdrv_(p)write_sync Kevin Wolf
2010-07-14 11:24 ` [Qemu-devel] [STABLE][PATCH 09/14] qcow: Use bdrv_(p)write_sync for metadata writes Kevin Wolf
2010-07-14 11:24 ` [Qemu-devel] [STABLE][PATCH 10/14] qcow2: " Kevin Wolf
2010-07-14 11:24 ` [Qemu-devel] [STABLE][PATCH 11/14] vmdk: " Kevin Wolf
2010-07-14 11:24 ` [Qemu-devel] [STABLE][PATCH 12/14] vpc: " Kevin Wolf
2010-07-14 11:24 ` Kevin Wolf [this message]
2010-07-14 11:24 ` [Qemu-devel] [STABLE][PATCH 14/14] block: Handle multiwrite errors only when all requests have completed Kevin Wolf
2010-07-14 12:50 ` [Qemu-devel] [STABLE][PULL 00/14] Block patches for 0.12.5 Aurelien Jarno

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1279106653-24351-14-git-send-email-kwolf@redhat.com \
    --to=kwolf@redhat.com \
    --cc=aurelien@aurel32.net \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).