[sheepdog] [PATCH v2 06/17] block/io: support int64_t bytes in bdrv_aligned_pwritev()

[Vladimir Sementsov-Ogievskiy]

30.04.2020 1:04, Eric Blake wrote:
> On 4/27/20 3:23 AM, Vladimir Sementsov-Ogievskiy wrote:
>> We are generally moving to int64_t for both offset and bytes parameters
>> on all io paths. Prepare bdrv_aligned_pwritev() now (and convert the
>> dependencies: bdrv_co_write_req_prepare() and
>> bdrv_co_write_req_finish() to signed type bytes)
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov at virtuozzo.com>
>> ---
>>   block/io.c | 12 +++++++-----
>>   1 file changed, 7 insertions(+), 5 deletions(-)
>>
>> diff --git a/block/io.c b/block/io.c
>> index c8c30e3699..fe19e09034 100644
>> --- a/block/io.c
>> +++ b/block/io.c
>> @@ -1854,7 +1854,7 @@ fail:
>>   }
>>   static inline int coroutine_fn
>> -bdrv_co_write_req_prepare(BdrvChild *child, int64_t offset, uint64_t bytes,
>> +bdrv_co_write_req_prepare(BdrvChild *child, int64_t offset, int64_t bytes,
>>                             BdrvTrackedRequest *req, int flags)
>>   {
> 
> No change in size.  First, check usage within function:
>      int64_t end_sector = DIV_ROUND_UP(offset + bytes, BDRV_SECTOR_SIZE);

this variable used only for assertion, it's simple to refactor to avoid the problem.

> Changes computation from uint64_t to int64_t.  This causes a borderline bug on images between INT64_MAX-511 and INT64_MAX (nbdkit can produce such images over NBD, although they are atypical on disk), where DIV_ROUND_UP() would give the right answer as uint64_t but a negative answer with int64_t.  As those images are not sector-aligned, maybe we don't need to care?
> all other uses appear to be within asserts related to offset+bytes being positive, so that's what we should check for.
> 
> Callers:
> bdrv_aligned_pwritev() - changed in this patch to 'int64_t', analyzed below [1]
> bdrv_co_pdiscard() - already passes 'int64_t', also checks for offset+bytes overflow - safe
> bdrv_co_copy_range_internal() - 'uint64_t', but already analyzed for 3/17 how it was capped < 2M - safe
> bdrv_co_truncate() - already passes 'int64_t', passes new_bytes computed by subtracting from a positive 'int64_t offset' - safe
> 
> 
> [1] except I hit the end of my work day, so my analysis will have to continue tomorrow...
> 
> 


-- 
Best regards,
Vladimir