Re: [Qemu-devel] [RFC 3/5] block: Fall back to fallback truncate function

[Max Reitz <mreitz@redhat.com>]

[-- Attachment #1.1: Type: text/plain, Size: 2945 bytes --]

On 12.07.19 12:04, Kevin Wolf wrote:
> Am 11.07.2019 um 21:58 hat Max Reitz geschrieben:
>> file-posix does not need to basically duplicate our fallback truncate
>> implementation; and sheepdog can fall back to it for "shrinking" files.
>>
>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>> ---
>>  block/file-posix.c | 21 +--------------------
>>  block/sheepdog.c   |  2 +-
>>  2 files changed, 2 insertions(+), 21 deletions(-)
>>
>> diff --git a/block/file-posix.c b/block/file-posix.c
>> index ab05b51a66..bcddfc7fbe 100644
>> --- a/block/file-posix.c
>> +++ b/block/file-posix.c
>> @@ -2031,23 +2031,7 @@ static int coroutine_fn raw_co_truncate(BlockDriverState *bs, int64_t offset,
>>          return raw_regular_truncate(bs, s->fd, offset, prealloc, errp);
>>      }
> 
> The only thing that is left here is a fstat() check to see that we
> really have a regular file here. But since this function is for the
> 'file' driver, we can just assume this and the function can go away
> altogether.
> 
> In fact, 'file' with block/character devices has been deprecated since
> 3.0, so we can just remove support for it now and make it more than just
> an assumption.
> 
>> diff --git a/block/sheepdog.c b/block/sheepdog.c
>> index 6f402e5d4d..4af4961cb7 100644
>> --- a/block/sheepdog.c
>> +++ b/block/sheepdog.c
>> @@ -2301,7 +2301,7 @@ static int coroutine_fn sd_co_truncate(BlockDriverState *bs, int64_t offset,
>>      max_vdi_size = (UINT64_C(1) << s->inode.block_size_shift) * MAX_DATA_OBJS;
>>      if (offset < old_size) {
>>          error_setg(errp, "shrinking is not supported");
>> -        return -EINVAL;
>> +        return -ENOTSUP;
>>      } else if (offset > max_vdi_size) {
>>          error_setg(errp, "too big image size");
>>          return -EINVAL;
> 
> The image will be unchanged and the guest will keep seeing the old image
> size, so is there any use case where having the fallback here makes
> sense? The only expected case where an image is shrunk is that the user
> explicitly sent block_resize - and in that case returning success, but
> doing nothing achieves nothing except confusing the user.
> 
> file-posix has the same confusing case, but at least it also has cases
> where the fake truncate actually achieves something (such a allowing to
> create images on block devices).

Hm, yes, that’s right.  It is as confusing for block devices, but that
at least gives something in return.  For sheepdog (and others, like
ssh), there is nothing in return.

Explaining for every block driver why it needs to be a bit confusing and
fall back to the fixed-size device implementation (because it doesn’t
implement creation) seems a bit off, too.

Hm.  Maybe the protocol creation fallback should just ignore failures
when truncating an image and in such a case zero the first sector of the
image?  Maybe it should just ignore ENOTSUP errors?

Max


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