From patchwork Mon Oct 24 21:21:33 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eric Blake X-Patchwork-Id: 686201 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 3t2q4v4JKwz9sf9 for ; Tue, 25 Oct 2016 08:22:55 +1100 (AEDT) Received: from localhost ([::1]:50226 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bymho-0001ax-Ge for incoming@patchwork.ozlabs.org; Mon, 24 Oct 2016 17:22:52 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37164) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bymgg-00012g-Kv for qemu-devel@nongnu.org; Mon, 24 Oct 2016 17:21:43 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bymgf-0000fp-Gl for qemu-devel@nongnu.org; Mon, 24 Oct 2016 17:21:42 -0400 Received: from mx1.redhat.com ([209.132.183.28]:43910) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1bymgZ-0000dP-QN; Mon, 24 Oct 2016 17:21:35 -0400 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 04C0065840; Mon, 24 Oct 2016 21:21:35 +0000 (UTC) Received: from [10.3.116.197] (ovpn-116-197.phx2.redhat.com [10.3.116.197]) by int-mx13.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u9OLLYsI009439; Mon, 24 Oct 2016 17:21:34 -0400 To: Ed Swierk , qemu-block@nongnu.org, qemu-devel@nongnu.org References: From: Eric Blake Openpgp: url=http://people.redhat.com/eblake/eblake.gpg Organization: Red Hat, Inc. Message-ID: <8a8853e4-d1ce-66b2-5214-b508fb897386@redhat.com> Date: Mon, 24 Oct 2016 16:21:33 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: X-Scanned-By: MIMEDefang 2.68 on 10.5.11.26 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.38]); Mon, 24 Oct 2016 21:21:35 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 209.132.183.28 X-Content-Filtered-By: Mailman/MimeDel 2.1.21 Subject: Re: [Qemu-devel] Assertion failure on qcow2 disk with cluster_size != 64k 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: Kevin Wolf , "Denis V. Lunev" Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" On 10/20/2016 07:24 PM, Ed Swierk wrote: > Shortly after I start qemu 2.7.0 with a qcow2 disk image created with > -o cluster_size=1048576, it prints the following and dies: > > block/qcow2.c:2451: qcow2_co_pwrite_zeroes: Assertion `head + count <= > s->cluster_size' failed. > > I narrowed the problem to bdrv_co_do_pwrite_zeroes(), called by > bdrv_aligned_pwritev() with flags & BDRV_REQ_ZERO_WRITE set. > > On the first loop iteration, offset=8003584, count=2093056, > head=663552, tail=659456 and num=2093056. qcow2_co_pwrite_zeroes() is > called with offset=8003584 and count=385024 and finds that the head > portion is not already zero, so it returns -ENOTSUP. > bdrv_co_do_pwrite_zeroes() falls back to a normal write, with > max_transfer=65536. How are you getting max_transfer == 65536? I can't reproduce it with the following setup: $ qemu-img create -f qcow2 -o cluster_size=1M file 10M $ qemu-io -f qcow2 -c 'w 7m 1k' file $ qemu-io -f qcow2 -c 'w -z 8003584 2093056' file although I did confirm that the above sequence was enough to get the -ENOTSUP failure and fall into the code calculating max_transfer. I'm guessing that you are using something other than a file system as the backing protocol for your qcow2 image. But do you really have a protocol that takes AT MOST 64k per transaction, while still trying to a cluster size of 1M in the qcow2 format? That's rather awkward, as it means that you are required to do 16 transactions per cluster (the whole point of using larger clusters is usually to get fewer transactions). I think we need to get to a root cause of why you are seeing such a small max_transfer, before I can propose the right patch, since I haven't been able to reproduce it locally yet (although I admit I haven't tried to see if blkdebug could reliably introduce artificial limits to simulate your setup). And it may turn out that I just have to fix the bdrv_co_do_pwrite_zeroes() code to loop multiple times if the size of the unaligned head really does exceed the max_transfer size that the underlying protocol is able to support, rather than assuming that the unaligned head/tail always fit in a single fallback write. Can you also try this patch? If I'm right, you'll still fail, but the assertion will be slightly different. (Again, I'm passing locally, but that's because I'm using the file protocol, and my file system does not impose a puny 64k max transfer). head = offset % alignment; @@ -1197,6 +1199,8 @@ static int coroutine_fn bdrv_co_do_pwrite_zeroes(BlockDriverState *bs, /* Make a small request up to the first aligned sector. */ num = MIN(count, alignment - head); head = 0; + assert(num < max_write_zeroes); + assert(num < max_transfer); } else if (tail && num > alignment) { /* Shorten the request to the last aligned sector. */ num -= tail; @@ -1222,8 +1226,6 @@ static int coroutine_fn bdrv_co_do_pwrite_zeroes(BlockDriverState *bs, if (ret == -ENOTSUP) { /* Fall back to bounce buffer if write zeroes is unsupported */ - int max_transfer = MIN_NON_ZERO(bs->bl.max_transfer, - MAX_WRITE_ZEROES_BOUNCE_BUFFER); BdrvRequestFlags write_flags = flags & ~BDRV_REQ_ZERO_WRITE; if ((flags & BDRV_REQ_FUA) && diff --git i/block/io.c w/block/io.c index b136c89..8757063 100644 --- i/block/io.c +++ w/block/io.c @@ -1179,6 +1179,8 @@ static int coroutine_fn bdrv_co_do_pwrite_zeroes(BlockDriverState *bs, int max_write_zeroes = MIN_NON_ZERO(bs->bl.max_pwrite_zeroes, INT_MAX); int alignment = MAX(bs->bl.pwrite_zeroes_alignment, bs->bl.request_alignment); + int max_transfer = MIN_NON_ZERO(bs->bl.max_transfer, + MAX_WRITE_ZEROES_BOUNCE_BUFFER); assert(alignment % bs->bl.request_alignment == 0);