[{"id":1770779,"web_url":"http://patchwork.ozlabs.org/comment/1770779/","msgid":"<20170919091719.GE11534@lemon.lan>","list_archive_url":null,"date":"2017-09-19T09:17:19","subject":"Re: [Qemu-devel] [PATCH v8 05/20] dirty-bitmap: Avoid size query\n\tfailure during truncate","submitter":{"id":24872,"url":"http://patchwork.ozlabs.org/api/people/24872/","name":"Fam Zheng","email":"famz@redhat.com"},"content":"On Mon, 09/18 13:58, Eric Blake wrote:\n> We've previously fixed several places where we failed to account\n> for possible errors from bdrv_nb_sectors(). Fix another one by\n> making bdrv_dirty_bitmap_truncate() take the new size from the\n> caller instead of querying itself; then adjust the sole caller\n> bdrv_truncate() to pass the size just determined by a successful\n> resize, or to skip the bitmap resize on failure, thus avoiding\n> sizing the bitmaps to -1.\n> \n> Signed-off-by: Eric Blake \n> \n> ---\n> v8: retitle and rework to avoid possibility of secondary failure [John]\n> v7: new patch [Kevin]\n> ---\n> include/block/dirty-bitmap.h | 2 +-\n> block.c | 15 ++++++++++-----\n> block/dirty-bitmap.c | 6 +++---\n> 3 files changed, 14 insertions(+), 9 deletions(-)\n> \n> diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h\n> index 8fd842eac9..7a27590047 100644\n> --- a/include/block/dirty-bitmap.h\n> +++ b/include/block/dirty-bitmap.h\n> @@ -83,7 +83,7 @@ int64_t bdrv_dirty_iter_next(BdrvDirtyBitmapIter *iter);\n> void bdrv_set_dirty_iter(BdrvDirtyBitmapIter *hbi, int64_t sector_num);\n> int64_t bdrv_get_dirty_count(BdrvDirtyBitmap *bitmap);\n> int64_t bdrv_get_meta_dirty_count(BdrvDirtyBitmap *bitmap);\n> -void bdrv_dirty_bitmap_truncate(BlockDriverState *bs);\n> +void bdrv_dirty_bitmap_truncate(BlockDriverState *bs, int64_t bytes);\n> bool bdrv_dirty_bitmap_readonly(const BdrvDirtyBitmap *bitmap);\n> bool bdrv_has_readonly_bitmaps(BlockDriverState *bs);\n> bool bdrv_dirty_bitmap_get_autoload(const BdrvDirtyBitmap *bitmap);\n> diff --git a/block.c b/block.c\n> index ee6a48976e..61ee9d4b83 100644\n> --- a/block.c\n> +++ b/block.c\n> @@ -3450,12 +3450,17 @@ int bdrv_truncate(BdrvChild *child, int64_t offset, PreallocMode prealloc,\n> assert(!(bs->open_flags & BDRV_O_INACTIVE));\n> \n> ret = drv->bdrv_truncate(bs, offset, prealloc, errp);\n> - if (ret == 0) {\n> - ret = refresh_total_sectors(bs, offset >> BDRV_SECTOR_BITS);\n> - bdrv_dirty_bitmap_truncate(bs);\n> - bdrv_parent_cb_resize(bs);\n> - atomic_inc(&bs->write_gen);\n> + if (ret < 0) {\n> + return ret;\n> }\n> + ret = refresh_total_sectors(bs, offset >> BDRV_SECTOR_BITS);\n> + if (ret < 0) {\n\nUgh, if we get here the situation is a bit embarrassing, because...\n\n> + error_setg_errno(errp, -ret, \"Could not refresh total sector count\");\n> + return ret;\n> + }\n> + bdrv_dirty_bitmap_truncate(bs, bs->total_sectors * BDRV_SECTOR_SIZE);\n> + bdrv_parent_cb_resize(bs);\n> + atomic_inc(&bs->write_gen);\n\nI think we still want to inc write_gen even if refresh_total_sectors failed, if\ndrv->bdrv_truncate has succeeded? That way the next bdrv_co_flush will actually\nflush the metadata change to disk.\n\nMaybe similarly call bdrv_parent_cb_resize() as long as drv->bdrv_truncate()\nsucceeded? The effect is the virtual devices notify guest about this \"resized\"\nevent, which I think is correct.\n\nFam","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=nongnu.org\n\t(client-ip=2001:4830:134:3::11; helo=lists.gnu.org;\n\tenvelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n\treceiver=)","ext-mx08.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx08.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=famz@redhat.com"],"Received":["from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11])\n\t(using TLSv1 with cipher AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xxHN63qn1z9sBZ\n\tfor ;\n\tTue, 19 Sep 2017 19:17:57 +1000 (AEST)","from localhost ([::1]:40949 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t)\n\tid 1duEfD-0004WH-6d\n\tfor incoming@patchwork.ozlabs.org; Tue, 19 Sep 2017 05:17:55 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:49784)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1duEer-0004VL-4u\n\tfor qemu-devel@nongnu.org; Tue, 19 Sep 2017 05:17:36 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from ) id 1duEen-0000wp-1G\n\tfor qemu-devel@nongnu.org; Tue, 19 Sep 2017 05:17:33 -0400","from mx1.redhat.com ([209.132.183.28]:60480)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from )\n\tid 1duEei-0000va-CL; Tue, 19 Sep 2017 05:17:24 -0400","from smtp.corp.redhat.com\n\t(int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16])\n\t(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby mx1.redhat.com (Postfix) with ESMTPS id 753C3C057FAF;\n\tTue, 19 Sep 2017 09:17:23 +0000 (UTC)","from localhost (ovpn-12-162.pek2.redhat.com [10.72.12.162])\n\tby smtp.corp.redhat.com (Postfix) with ESMTP id AF8CD5C541;\n\tTue, 19 Sep 2017 09:17:20 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 753C3C057FAF","Date":"Tue, 19 Sep 2017 17:17:19 +0800","From":"Fam Zheng ","To":"Eric Blake ","Message-ID":"<20170919091719.GE11534@lemon.lan>","References":"<20170918185819.5984-1-eblake@redhat.com>\n\t<20170918185819.5984-6-eblake@redhat.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20170918185819.5984-6-eblake@redhat.com>","User-Agent":"Mutt/1.8.3 (2017-05-23)","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.16","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.32]);\n\tTue, 19 Sep 2017 09:17:23 +0000 (UTC)","X-detected-operating-system":"by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic]\n\t[fuzzy]","X-Received-From":"209.132.183.28","Subject":"Re: [Qemu-devel] [PATCH v8 05/20] dirty-bitmap: Avoid size query\n\tfailure during truncate","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"kwolf@redhat.com, vsementsov@virtuozzo.com, qemu-block@nongnu.org,\n\tqemu-devel@nongnu.org, Max Reitz , jsnow@redhat.com","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t"}}]