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

[Stefano Garzarella]

On Tue, Apr 30, 2019 at 10:41:02AM -0500, Eric Blake wrote:
> 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 <address@hidden>
> 
> >>>> -    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.
> 

That sounds good to me!

Thanks,
Stefano