Re: [Qemu-devel] [PATCH] qemu-img: Check post-truncation size

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2414 bytes --]

On 04/20/2018 05:53 PM, Max Reitz wrote:
> Some block drivers (iscsi and file-posix when dealing with device files)
> do not actually support truncation, even though they provide a
> .bdrv_truncate() method and will happily return success when providing a
> new size that does not exceed the current size.  This is because these
> drivers expect the user to resize the image outside of qemu and then
> provide qemu with that information through the block_resize command
> (compare cb1b83e740384b4e0d950f3d7c81c02b8ce86c2e).
> 
> Of course, anyone using qemu-img resize will find that behavior useless.
> So we should check the actual size of the image after the supposedly
> successful truncation took place, emit an error if nothing changed and
> emit a warning if the target size was not met.
> 
> Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1523065
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
> Testing this is not quite trivial.  Or, well, it is, but you need either
> an iscsi test server or root access.

Or, you need NBD to document and implement NBD_CMD_RESIZE, and then the
nbd driver will support .bdrv_truncate() but fail when talking to a
server that doesn't actually resize after all.

> 
> Because in my opinion iotests that require root access are never run, I
> decided against writing such a test case.

So maybe when I get around to adding NBD resize support, I should add
such a test ;)


> +    if (new_size != total_size && new_size == current_size) {
> +        error_report("Image was not resized. Resizing may not be supported "
> +                     "for this image.");

error_report() generally does not have trailing dot, and generally has a
single sentence.  Would this be better as:

Image was not resized; resizing may not be supported for this image

> +        ret = -1;
> +        goto out;
> +    }
> +
> +    if (new_size != total_size) {
> +        warn_report("Image should have been resized to %" PRIi64
> +                    " bytes, but was resized to %" PRIi64 " bytes.",
> +                    total_size, new_size);

Trailing dot again.  Also, PRId64 is much more common than PRIi64, even
though the two are identical in behavior.

But the idea makes sense to me.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]