Re: [Qemu-block] [PATCH] block/io.c: fix for the allocation failure

[Andrey Shinkevich]

On 06/04/2019 01:50, John Snow wrote:
> 
> 
> On 4/5/19 10:24 AM, Andrey Shinkevich wrote:
>> On a file system used by the customer, fallocate() returns an error
>> if the block is not properly aligned. So, bdrv_co_pwrite_zeroes()
>> fails. We can handle that case the same way as it is done for the
>> unsupported cases, namely, call to bdrv_driver_pwritev() that writes
>> zeroes to an image for the unaligned chunk of the block.
>>
>> Suggested-by: Denis V. Lunev <address@hidden>
>> Signed-off-by: Andrey Shinkevich <address@hidden>
>> ---
>>   block/io.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/block/io.c b/block/io.c
>> index dfc153b..0412a51 100644
>> --- a/block/io.c
>> +++ b/block/io.c
>> @@ -1516,7 +1516,7 @@ static int coroutine_fn 
>> bdrv_co_do_pwrite_zeroes(BlockDriverState *bs,
>>               assert(!bs->supported_zero_flags);
>>           }
>>   
>> -        if (ret == -ENOTSUP && !(flags & BDRV_REQ_NO_FALLBACK)) {
>> +        if (ret < 0 && !(flags & BDRV_REQ_NO_FALLBACK)) {
>>               /* Fall back to bounce buffer if write zeroes is unsupported */
>>               BdrvRequestFlags write_flags = flags & ~BDRV_REQ_ZERO_WRITE;
>>   
>>
> 
> I suppose that if fallocate fails for any reason and we're allowing
> fallback, we're either going to succeed ... or fail again very soon
> thereafter.
> 
> Are there any cases where it is vital to not ignore the first fallocate
> failure? I'm a little wary of ignoring the return code from
> bdrv_co_pwrite_zeroes, but I am assuming that if there is a "real"
> failure here that the following bounce writes will also fail "safely."
> 
> I'm not completely confident, but I have no tangible objections:
> Reviewed-by: John Snow <address@hidden>
> 

Thank you for your review, John!

Let me clarify the circumstances and quote the bug report:
"Customer had Win-2012 VM with 50GB system disk which was later resized 
to 256GB without resizing the partition inside VM.
Now, while trying to resize to 50G, the following error will appear
'Failed to reduce the number of L2 tables: Invalid argument'
It was found that it is possible to shrink the disk to 128G and any size 
above that number, but size below 128G will bring the mentioned error."

The fallocate() returns no error on that file system if the offset and
the (offset + bytes) parameters of the bdrv_co_do_pwrite_zeroes() both
are aligned to 4K.
-- 
With the best regards,
Andrey Shinkevich