From patchwork Fri Apr 21 10:44:03 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Peter Lieven X-Patchwork-Id: 753278 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3w8XYL2R10z9ryv for ; Fri, 21 Apr 2017 20:49:26 +1000 (AEST) Received: from localhost ([::1]:58520 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1d1W7v-00018t-Nu for incoming@patchwork.ozlabs.org; Fri, 21 Apr 2017 06:49:23 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:56545) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1d1W2u-0005bj-7A for qemu-devel@nongnu.org; Fri, 21 Apr 2017 06:44:13 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1d1W2q-0000i7-4W for qemu-devel@nongnu.org; Fri, 21 Apr 2017 06:44:12 -0400 Received: from mx-v6.kamp.de ([2a02:248:0:51::16]:34581 helo=mx01.kamp.de) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1d1W2p-0000h2-RC for qemu-devel@nongnu.org; Fri, 21 Apr 2017 06:44:08 -0400 Received: (qmail 32012 invoked by uid 89); 21 Apr 2017 10:44:04 -0000 Received: from [195.62.97.28] by client-16-kamp (envelope-from , uid 89) with qmail-scanner-2010/03/19-MF (clamdscan: 0.99.2/23316. avast: 1.2.2/17010300. spamassassin: 3.4.1. Clear:RC:1(195.62.97.28):. Processed in 0.08195 secs); 21 Apr 2017 10:44:04 -0000 Received: from smtp.kamp.de (HELO submission.kamp.de) ([195.62.97.28]) by mx01.kamp.de with ESMTPS (DHE-RSA-AES256-GCM-SHA384 encrypted); 21 Apr 2017 10:44:03 -0000 X-GL_Whitelist: yes Received: (qmail 32487 invoked from network); 21 Apr 2017 10:44:03 -0000 Received: from ac9.vpn.kamp-intra.net (HELO ?172.20.250.9?) (pl@kamp.de@::ffff:172.20.250.9) by submission.kamp.de with ESMTPS (DHE-RSA-AES128-SHA encrypted) ESMTPA; 21 Apr 2017 10:44:03 -0000 To: Anton Nefedov , qemu-devel@nongnu.org References: <1492769076-64466-1-git-send-email-anton.nefedov@virtuozzo.com> From: Peter Lieven Message-ID: <98514b87-d2a3-254d-f03e-0c6d3cd73ff6@kamp.de> Date: Fri, 21 Apr 2017 12:44:03 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.5.1 MIME-Version: 1.0 In-Reply-To: <1492769076-64466-1-git-send-email-anton.nefedov@virtuozzo.com> X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 2a02:248:0:51::16 Subject: Re: [Qemu-devel] [PATCH v2 1/1] qemu-img: wait for convert coroutines to complete X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: kwolf@redhat.com, mreitz@redhat.com, den@virtuozzo.com, qemu-block@nongnu.org, qemu-stable@nongnu.org Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" Am 21.04.2017 um 12:04 schrieb Anton Nefedov: > On error path (like i/o error in one of the coroutines), it's required to > - wait for coroutines completion before cleaning the common structures > - reenter dependent coroutines so they ever finish > > Introduced in 2d9187bc65. > > Signed-off-by: Anton Nefedov > --- > qemu-img.c | 44 +++++++++++++++++++++++++++----------------- > 1 file changed, 27 insertions(+), 17 deletions(-) > > diff --git a/qemu-img.c b/qemu-img.c > index b220cf7..24950c1 100644 > --- a/qemu-img.c > +++ b/qemu-img.c > @@ -1735,6 +1735,27 @@ static int coroutine_fn convert_co_write(ImgConvertState *s, int64_t sector_num, > return 0; > } > > +static void coroutine_fn convert_reenter_waiting(ImgConvertState *s, > + uint64_t wr_offs) > +{ > + int i; > + if (s->wr_in_order) { > + /* reenter the coroutine that might have waited > + * for the write to complete */ > + for (i = 0; i < s->num_coroutines; i++) { > + if (s->co[i] && s->wait_sector_num[i] == wr_offs) { > + /* > + * A -> B -> A cannot occur because A has > + * s->wait_sector_num[i] == -1 during A -> B. Therefore > + * B will never enter A during this time window. > + */ > + qemu_coroutine_enter(s->co[i]); > + break; > + } > + } > + } > +} > + > static void coroutine_fn convert_co_do_copy(void *opaque) > { > ImgConvertState *s = opaque; > @@ -1792,6 +1813,7 @@ static void coroutine_fn convert_co_do_copy(void *opaque) > error_report("error while reading sector %" PRId64 > ": %s", sector_num, strerror(-ret)); > s->ret = ret; > + convert_reenter_waiting(s, sector_num + n); > goto out; > } > } else if (!s->min_sparse && status == BLK_ZERO) { > @@ -1803,6 +1825,7 @@ static void coroutine_fn convert_co_do_copy(void *opaque) > /* keep writes in order */ > while (s->wr_offs != sector_num) { > if (s->ret != -EINPROGRESS) { > + convert_reenter_waiting(s, sector_num + n); > goto out; > } > s->wait_sector_num[index] = sector_num; > @@ -1816,25 +1839,12 @@ static void coroutine_fn convert_co_do_copy(void *opaque) > error_report("error while writing sector %" PRId64 > ": %s", sector_num, strerror(-ret)); > s->ret = ret; > + convert_reenter_waiting(s, sector_num + n); > goto out; > } > > - if (s->wr_in_order) { > - /* reenter the coroutine that might have waited > - * for this write to complete */ > - s->wr_offs = sector_num + n; > - for (i = 0; i < s->num_coroutines; i++) { > - if (s->co[i] && s->wait_sector_num[i] == s->wr_offs) { > - /* > - * A -> B -> A cannot occur because A has > - * s->wait_sector_num[i] == -1 during A -> B. Therefore > - * B will never enter A during this time window. > - */ > - qemu_coroutine_enter(s->co[i]); > - break; > - } > - } > - } > + s->wr_offs = sector_num + n; > + convert_reenter_waiting(s, s->wr_offs); > } > > out: > @@ -1899,7 +1909,7 @@ static int convert_do_copy(ImgConvertState *s) > qemu_coroutine_enter(s->co[i]); > } > > - while (s->ret == -EINPROGRESS) { > + while (s->running_coroutines) { > main_loop_wait(false); > } > And what if we error out in the read path? Wouldn't be something like this easier? Peter diff --git a/qemu-img.c b/qemu-img.c index 22f559a..4ff1085 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -1903,6 +1903,16 @@ static int convert_do_copy(ImgConvertState *s) main_loop_wait(false); } + /* on error path we need to enter all coroutines that are still + * running before cleaning up common structures */ + if (s->ret) { + for (i = 0; i < s->num_coroutines; i++) { + if (s->co[i]) { + qemu_coroutine_enter(s->co[i]); + } + } + } + if (s->compressed && !s->ret) { /* signal EOF to align */ ret = blk_pwrite_compressed(s->target, 0, NULL, 0);