Re: [Qemu-devel] [PATCH v2 0/3] block: Fix is_allocated() for truncated images

[Max Reitz <mreitz@redhat.com>]

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
>