[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] qemu-img: Check post-truncation size
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH] qemu-img: Check post-truncation size |
Date: |
Sat, 21 Apr 2018 10:35:41 -0500 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.7.0 |
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 <address@hidden>
> ---
> 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
signature.asc
Description: OpenPGP digital signature