Re: [Qemu-devel] [PATCH v5 2/3] block/io: bdrv_pdiscard: support int64_t bytes parameter

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1778 bytes --]

On 4/30/19 6:09 AM, Kevin Wolf wrote:
> Am 30.04.2019 um 12:03 hat Vladimir Sementsov-Ogievskiy geschrieben:
>> 30.04.2019 12:24, Stefano Garzarella wrote:
>>> On Tue, Apr 23, 2019 at 03:57:05PM +0300, Vladimir Sementsov-Ogievskiy wrote:
>>>> This fixes at least one overflow in qcow2_process_discards, which
>>>> passes 64bit region length to bdrv_pdiscard where bytes (or sectors in
>>>> the past) parameter is int since its introduction in 0b919fae.
>>>>
>>>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

>>>> -    if (!bs || !bs->drv) {
>>>> +    if (!bs || !bs->drv || !bdrv_is_inserted(bs)) {
>>>
>>> Should we describe this change in the commit message?
>>
>> Honestly, don't want to resend the series for this.
> 
> I haven't reviewed the patches yet, but if this remains the only thing
> to change, it can be updated while applying the series if we have a
> specific proposal for a new commit message.

How about:

This fixes at least one overflow in qcow2_process_discards, which was
inadvertently truncating a 64-bit region length to the bdrv_pdiscard
'int bytes' parameter (previously bdrv_discard's 'int sectors') since
its introduction in 0b919fae.

By inlining the remaining portions of bdrv_check_byte_request() that are
still independent from the code previously validating a 32-bit request,
we can drop the call to that function. A request larger than 31 bits (or
the driver's max discard limit, if specified) is still split into
smaller chunks by the block layer before reaching the driver.


At any rate, you can add:
Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org


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