From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:36066) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Xgvx0-0002OJ-1M for qemu-devel@nongnu.org; Wed, 22 Oct 2014 09:27:46 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Xgvwv-0000OZ-2r for qemu-devel@nongnu.org; Wed, 22 Oct 2014 09:27:41 -0400 Received: from mx1.redhat.com ([209.132.183.28]:53660) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Xgvwu-0000OV-Ri for qemu-devel@nongnu.org; Wed, 22 Oct 2014 09:27:37 -0400 Message-ID: <5447B0C2.8010204@redhat.com> Date: Wed, 22 Oct 2014 15:27:30 +0200 From: Max Reitz MIME-Version: 1.0 References: <1413984271-15471-1-git-send-email-mreitz@redhat.com> In-Reply-To: <1413984271-15471-1-git-send-email-mreitz@redhat.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v2 0/3] block: Fix is_allocated() for truncated images List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org Cc: Kevin Wolf , =?UTF-8?B?QmVub8OudCBDYW5ldA==?= , Stefan Hajnoczi On 2014-10-22 at 15:24, Max Reitz wrote: > Patch 2: > The bdrv_is_allocated() functions may return a number of zero sectors > e.g. if a sector beyond the image end has been queried. Respect this > case in qemu-io's map implementation so it doesn't run into an infinite > loop (https://bugs.launchpad.net/qemu/+bug/1356969). > > Patch 1: > In that bug report, bdrv_co_get_block_status() fell through to the > underlying file to get additional information (whether the sectors are > zeroed). However, the offset reported by the image format (qcow2) was > beyond the size of the underlying file and so this second query returned > only zero clusters which replaced the actual number of sectors queried > in the "formatted" (on qcow2 level) image. This replacement is generally > fine because it allows bdrv_co_get_block_status() to follow the > granularity (holes and non-holes) of the underlying file, but not if > only zero clusters could be queried (which indicates a request beyond > the image end). > This patch makes bdrv_co_get_block_status() respect the underlying > file's EOF and act accordingly. Sorry, disregard the following paragraph (which I deliberately highlighted so it's easier to disregard). vvv > But because errors from this > second call are ignored, the number of sector queried successfully > should be ignored as well, which makes qemu-io -c map actually work for > that bug report. ^^^ > Patch 3 adds a test for patch 1; I could not conceive a test for patch 2 > which patch 1 would not already catch, but patch 2 should be simple > enough not to require a test anyway. > > > v2: > - In v1, patch 1 completely ignored the number of queried sectors > returned by the recursive call; but it should only be ignored in case > of EOF (which is indicated by returning 0 as the queried clusters > count) > - Thanks to fixing patch 1, the area queried in the test added in patch > 3 is now marked unallocated (which is correct because it's beyond the > EOF). Therefore, this test does no longer require > _filter_qemu_img_map() (because there are no mappings shown) and this > series no longer depends on the qemu-img/QMP commit series. > > > Max Reitz (3): > block: Respect underlying file's EOF > qemu-io: Respect early image end for map > iotests: Add test for map commands > > block.c | 16 ++++++++++-- > qemu-io-cmds.c | 5 +++- > tests/qemu-iotests/102 | 64 ++++++++++++++++++++++++++++++++++++++++++++++ > tests/qemu-iotests/102.out | 10 ++++++++ > tests/qemu-iotests/group | 1 + > 5 files changed, 93 insertions(+), 3 deletions(-) > create mode 100755 tests/qemu-iotests/102 > create mode 100644 tests/qemu-iotests/102.out >