| Message ID | 1466414680-18383-1-git-send-email-pl@kamp.de |
|---|---|
| State | New |
| Headers | show |
On 06/20/2016 03:24 AM, Peter Lieven wrote: > Commit 94d047a added an assertion the the request alignment check. > This introduced 2 issues: > a) A off-by-one error since a request of BDRV_REQUEST_MAX_SECTORS > is actually allowed. > b) The bdrv_get_block_status call in the read path to check the allocation > status requests up to INT_MAX sectors which triggers the assertion. > > Fixes: 94d047a35bf663e28f8fef137544d8ea78165add > Signed-off-by: Peter Lieven <pl@kamp.de> > --- > block/iscsi.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > Reviewed-by: Eric Blake <eblake@redhat.com> > diff --git a/block/iscsi.c b/block/iscsi.c > index 7e78ade..9bb5ff6 100644 > --- a/block/iscsi.c > +++ b/block/iscsi.c > @@ -417,7 +417,7 @@ static bool is_byte_request_lun_aligned(int64_t offset, int count, > static bool is_sector_request_lun_aligned(int64_t sector_num, int nb_sectors, > IscsiLun *iscsilun) > { > - assert(nb_sectors < BDRV_REQUEST_MAX_SECTORS); > + assert(nb_sectors <= BDRV_REQUEST_MAX_SECTORS); > return is_byte_request_lun_aligned(sector_num << BDRV_SECTOR_BITS, > nb_sectors << BDRV_SECTOR_BITS, > iscsilun); > @@ -661,7 +661,8 @@ static int coroutine_fn iscsi_co_readv(BlockDriverState *bs, > int64_t ret; > int pnum; > BlockDriverState *file; > - ret = iscsi_co_get_block_status(bs, sector_num, INT_MAX, &pnum, &file); > + ret = iscsi_co_get_block_status(bs, sector_num, > + BDRV_REQUEST_MAX_SECTORS, &pnum, &file); > if (ret < 0) { > return ret; > } >
On 20/06/2016 11:24, Peter Lieven wrote: > Commit 94d047a added an assertion the the request alignment check. > This introduced 2 issues: > a) A off-by-one error since a request of BDRV_REQUEST_MAX_SECTORS > is actually allowed. > b) The bdrv_get_block_status call in the read path to check the allocation > status requests up to INT_MAX sectors which triggers the assertion. > > Fixes: 94d047a35bf663e28f8fef137544d8ea78165add > Signed-off-by: Peter Lieven <pl@kamp.de> > --- > block/iscsi.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/block/iscsi.c b/block/iscsi.c > index 7e78ade..9bb5ff6 100644 > --- a/block/iscsi.c > +++ b/block/iscsi.c > @@ -417,7 +417,7 @@ static bool is_byte_request_lun_aligned(int64_t offset, int count, > static bool is_sector_request_lun_aligned(int64_t sector_num, int nb_sectors, > IscsiLun *iscsilun) > { > - assert(nb_sectors < BDRV_REQUEST_MAX_SECTORS); > + assert(nb_sectors <= BDRV_REQUEST_MAX_SECTORS); > return is_byte_request_lun_aligned(sector_num << BDRV_SECTOR_BITS, > nb_sectors << BDRV_SECTOR_BITS, > iscsilun); > @@ -661,7 +661,8 @@ static int coroutine_fn iscsi_co_readv(BlockDriverState *bs, > int64_t ret; > int pnum; > BlockDriverState *file; > - ret = iscsi_co_get_block_status(bs, sector_num, INT_MAX, &pnum, &file); > + ret = iscsi_co_get_block_status(bs, sector_num, > + BDRV_REQUEST_MAX_SECTORS, &pnum, &file); > if (ret < 0) { > return ret; > } > Hi, I've queued this patch. Can you rebase the allocation map patch on top of master + this one? Thanks, Paolo
Am 23.06.2016 um 17:50 schrieb Paolo Bonzini: > On 20/06/2016 11:24, Peter Lieven wrote: >> Commit 94d047a added an assertion the the request alignment check. >> This introduced 2 issues: >> a) A off-by-one error since a request of BDRV_REQUEST_MAX_SECTORS >> is actually allowed. >> b) The bdrv_get_block_status call in the read path to check the allocation >> status requests up to INT_MAX sectors which triggers the assertion. >> >> Fixes: 94d047a35bf663e28f8fef137544d8ea78165add >> Signed-off-by: Peter Lieven <pl@kamp.de> >> --- >> block/iscsi.c | 5 +++-- >> 1 file changed, 3 insertions(+), 2 deletions(-) >> >> diff --git a/block/iscsi.c b/block/iscsi.c >> index 7e78ade..9bb5ff6 100644 >> --- a/block/iscsi.c >> +++ b/block/iscsi.c >> @@ -417,7 +417,7 @@ static bool is_byte_request_lun_aligned(int64_t offset, int count, >> static bool is_sector_request_lun_aligned(int64_t sector_num, int nb_sectors, >> IscsiLun *iscsilun) >> { >> - assert(nb_sectors < BDRV_REQUEST_MAX_SECTORS); >> + assert(nb_sectors <= BDRV_REQUEST_MAX_SECTORS); >> return is_byte_request_lun_aligned(sector_num << BDRV_SECTOR_BITS, >> nb_sectors << BDRV_SECTOR_BITS, >> iscsilun); >> @@ -661,7 +661,8 @@ static int coroutine_fn iscsi_co_readv(BlockDriverState *bs, >> int64_t ret; >> int pnum; >> BlockDriverState *file; >> - ret = iscsi_co_get_block_status(bs, sector_num, INT_MAX, &pnum, &file); >> + ret = iscsi_co_get_block_status(bs, sector_num, >> + BDRV_REQUEST_MAX_SECTORS, &pnum, &file); >> if (ret < 0) { >> return ret; >> } >> > Hi, I've queued this patch. Can you rebase the allocation map patch on > top of master + this one? will do. Peter
diff --git a/block/iscsi.c b/block/iscsi.c index 7e78ade..9bb5ff6 100644 --- a/block/iscsi.c +++ b/block/iscsi.c @@ -417,7 +417,7 @@ static bool is_byte_request_lun_aligned(int64_t offset, int count, static bool is_sector_request_lun_aligned(int64_t sector_num, int nb_sectors, IscsiLun *iscsilun) { - assert(nb_sectors < BDRV_REQUEST_MAX_SECTORS); + assert(nb_sectors <= BDRV_REQUEST_MAX_SECTORS); return is_byte_request_lun_aligned(sector_num << BDRV_SECTOR_BITS, nb_sectors << BDRV_SECTOR_BITS, iscsilun); @@ -661,7 +661,8 @@ static int coroutine_fn iscsi_co_readv(BlockDriverState *bs, int64_t ret; int pnum; BlockDriverState *file; - ret = iscsi_co_get_block_status(bs, sector_num, INT_MAX, &pnum, &file); + ret = iscsi_co_get_block_status(bs, sector_num, + BDRV_REQUEST_MAX_SECTORS, &pnum, &file); if (ret < 0) { return ret; }
Commit 94d047a added an assertion the the request alignment check. This introduced 2 issues: a) A off-by-one error since a request of BDRV_REQUEST_MAX_SECTORS is actually allowed. b) The bdrv_get_block_status call in the read path to check the allocation status requests up to INT_MAX sectors which triggers the assertion. Fixes: 94d047a35bf663e28f8fef137544d8ea78165add Signed-off-by: Peter Lieven <pl@kamp.de> --- block/iscsi.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)