[sheepdog] [PATCH v2 06/17] block/io: support int64_t bytes in bdrv_aligned_pwritev()
vsementsov at virtuozzo.com
Thu Apr 30 11:26:18 CEST 2020
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.
> bdrv_aligned_pwritev() - changed in this patch to 'int64_t', analyzed below 
> 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
>  except I hit the end of my work day, so my analysis will have to continue tomorrow...
More information about the sheepdog