Re: [Xen-devel] [Qemu-devel] [PATCH] xen_disk: convert discard input to byte ranges

2016-11-22 Thread Eric Blake
On 11/22/2016 11:00 AM, Olaf Hering wrote: > On Tue, Nov 22, Eric Blake wrote: > >> if (sec_start + sec_count < sec_count || >> sec_start > (INT64_MAX >> BDRV_SECTOR_BITS) - sec_count) { >> return false; >> } > > My point was: how does this handle sec_start=0,sec_count=UINT64_MAX or >

Re: [Xen-devel] [Qemu-devel] [PATCH] xen_disk: convert discard input to byte ranges

2016-11-22 Thread Olaf Hering
On Tue, Nov 22, Eric Blake wrote: > if (sec_start + sec_count < sec_count || > sec_start > (INT64_MAX >> BDRV_SECTOR_BITS) - sec_count) { > return false; > } My point was: how does this handle sec_start=0,sec_count=UINT64_MAX or sec_start=INT64_MAX,sec_count=INT64_MAX? For me this gets

Re: [Xen-devel] [Qemu-devel] [PATCH] xen_disk: convert discard input to byte ranges

2016-11-22 Thread Eric Blake
On 11/22/2016 10:12 AM, Olaf Hering wrote: > On Fri, Nov 18, Eric Blake wrote: > >> if (sec_start > (INT64_MAX >> BDRV_SECTOR_BITS) - sec_count) > > I have looked at this for a while now and cant spot how this would cover > all cases. Are you saying there should be just a single overflow check,

Re: [Xen-devel] [Qemu-devel] [PATCH] xen_disk: convert discard input to byte ranges

2016-11-22 Thread Olaf Hering
On Fri, Nov 18, Eric Blake wrote: > if (sec_start > (INT64_MAX >> BDRV_SECTOR_BITS) - sec_count) I have looked at this for a while now and cant spot how this would cover all cases. Are you saying there should be just a single overflow check, yours? My change has two: one to check for wrap around

Re: [Xen-devel] [Qemu-devel] [PATCH] xen_disk: convert discard input to byte ranges

2016-11-18 Thread Eric Blake
On 11/18/2016 11:41 AM, Olaf Hering wrote: > On Fri, Nov 18, Eric Blake wrote: > >> On 11/18/2016 04:24 AM, Olaf Hering wrote: >>> +/* Overflowing byte limit? */ >>> +if ((sec_start + sec_count) > ((INT64_MAX + INT_MAX) >> >>> BDRV_SECTOR_BITS)) { >> This is undefined. INT64_MAX +

Re: [Xen-devel] [Qemu-devel] [PATCH] xen_disk: convert discard input to byte ranges

2016-11-18 Thread Olaf Hering
On Fri, Nov 18, Eric Blake wrote: > On 11/18/2016 04:24 AM, Olaf Hering wrote: > > +/* Overflowing byte limit? */ > > +if ((sec_start + sec_count) > ((INT64_MAX + INT_MAX) >> > > BDRV_SECTOR_BITS)) { > This is undefined. INT64_MAX + anything non-negative overflows int64, The expanded

Re: [Xen-devel] [Qemu-devel] [PATCH] xen_disk: convert discard input to byte ranges

2016-11-18 Thread Eric Blake
On 11/18/2016 04:24 AM, Olaf Hering wrote: > The guest sends discard requests as u64 sector/count pairs, but the > block layer operates internally with s64/s32 pairs. The conversion > leads to IO errors in the guest, the discard request is not processed. > > domU.cfg: > 'vdev=xvda,

Re: [Xen-devel] [Qemu-devel] [PATCH] xen_disk: convert discard input to byte ranges

2016-11-18 Thread Kevin Wolf
Am 18.11.2016 um 15:35 hat Eric Blake geschrieben: > On 11/18/2016 08:19 AM, Olaf Hering wrote: > > Am 18. November 2016 14:43:18 MEZ, schrieb Eric Blake : > >> On 11/18/2016 04:24 AM, Olaf Hering wrote: > >>> The guest sends discard requests as u64 sector/count pairs, but the >

Re: [Xen-devel] [Qemu-devel] [PATCH] xen_disk: convert discard input to byte ranges

2016-11-18 Thread Eric Blake
On 11/18/2016 08:19 AM, Olaf Hering wrote: > Am 18. November 2016 14:43:18 MEZ, schrieb Eric Blake : >> On 11/18/2016 04:24 AM, Olaf Hering wrote: >>> The guest sends discard requests as u64 sector/count pairs, but the >>> block layer operates internally with s64/s32 pairs. The

Re: [Xen-devel] [Qemu-devel] [PATCH] xen_disk: convert discard input to byte ranges

2016-11-18 Thread Olaf Hering
Am 18. November 2016 14:43:18 MEZ, schrieb Eric Blake : >On 11/18/2016 04:24 AM, Olaf Hering wrote: >> The guest sends discard requests as u64 sector/count pairs, but the >> block layer operates internally with s64/s32 pairs. The conversion >> leads to IO errors in the guest,

Re: [Xen-devel] [Qemu-devel] [PATCH] xen_disk: convert discard input to byte ranges

2016-11-18 Thread Eric Blake
On 11/18/2016 04:24 AM, Olaf Hering wrote: > The guest sends discard requests as u64 sector/count pairs, but the > block layer operates internally with s64/s32 pairs. The conversion > leads to IO errors in the guest, the discard request is not processed. Doesn't the block layer already split