On 2018-04-21 17:35, Eric Blake wrote: > 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 <[email protected]> >> --- >> 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.
I suppose the NBD client would recognize that, though, and return an
error code (and set *errp). The issue in this case is that the drivers
in question pretend that everything went according to plan (they return
success) when actually nothing was resized at 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
Yes, it would. I just made this a qprintf() in the first version and
forgot to change it when making it an error_report().
>> + 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.
Same here, yes.
> Also, PRId64 is much more common than PRIi64, even
> though the two are identical in behavior.
:-(
But I like my %i!
> But the idea makes sense to me.
OK, thanks.
Max
signature.asc
Description: OpenPGP digital signature
