| Message ID | 1465395011-26088-4-git-send-email-kwolf@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 06/08/2016 08:10 AM, Kevin Wolf wrote: > Signed-off-by: Kevin Wolf <kwolf@redhat.com> > --- > block/io.c | 13 +++++-------- > 1 file changed, 5 insertions(+), 8 deletions(-) > > diff --git a/block/io.c b/block/io.c > index 2fd88cb..4af9c59 100644 > --- a/block/io.c > +++ b/block/io.c > @@ -1241,11 +1241,9 @@ static int coroutine_fn bdrv_aligned_pwritev(BlockDriverState *bs, > bool waited; > int ret; > > - int64_t sector_num = offset >> BDRV_SECTOR_BITS; > - unsigned int nb_sectors = bytes >> BDRV_SECTOR_BITS; > + int64_t start_sector = offset >> BDRV_SECTOR_BITS; > + int64_t end_sector = DIV_ROUND_UP(offset + bytes, BDRV_SECTOR_SIZE); > > - assert((offset & (BDRV_SECTOR_SIZE - 1)) == 0); > - assert((bytes & (BDRV_SECTOR_SIZE - 1)) == 0); I wonder if we should add an 'unsigned int align' parameter to this function, to mirror bdrv_aligned_preadv() - that way, we can assert that any divisions we do based on 'align' are indeed aligned. If we do that, then just as in the previous patch, I think we would want to keep 'assert((offset & (align - 1)) == 0)'. But at the moment, I don't see any divisions based on align, so I think you are okay. > if (ret >= 0) { > - bs->total_sectors = MAX(bs->total_sectors, sector_num + nb_sectors); > + bs->total_sectors = MAX(bs->total_sectors, end_sector); Someday, we may want bs->total_bytes instead of total_sectors, but unrelated to this patch. Looks clean: Reviewed-by: Eric Blake <eblake@redhat.com>
On Wed, Jun 08, 2016 at 04:10:08PM +0200, Kevin Wolf wrote: > Signed-off-by: Kevin Wolf <kwolf@redhat.com> > --- > block/io.c | 13 +++++-------- > 1 file changed, 5 insertions(+), 8 deletions(-) Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
diff --git a/block/io.c b/block/io.c index 2fd88cb..4af9c59 100644 --- a/block/io.c +++ b/block/io.c @@ -1241,11 +1241,9 @@ static int coroutine_fn bdrv_aligned_pwritev(BlockDriverState *bs, bool waited; int ret; - int64_t sector_num = offset >> BDRV_SECTOR_BITS; - unsigned int nb_sectors = bytes >> BDRV_SECTOR_BITS; + int64_t start_sector = offset >> BDRV_SECTOR_BITS; + int64_t end_sector = DIV_ROUND_UP(offset + bytes, BDRV_SECTOR_SIZE); - assert((offset & (BDRV_SECTOR_SIZE - 1)) == 0); - assert((bytes & (BDRV_SECTOR_SIZE - 1)) == 0); assert(!qiov || bytes == qiov->size); assert((bs->open_flags & BDRV_O_NO_IO) == 0); @@ -1269,22 +1267,21 @@ static int coroutine_fn bdrv_aligned_pwritev(BlockDriverState *bs, /* Do nothing, write notifier decided to fail this request */ } else if (flags & BDRV_REQ_ZERO_WRITE) { bdrv_debug_event(bs, BLKDBG_PWRITEV_ZERO); - ret = bdrv_co_do_pwrite_zeroes(bs, sector_num << BDRV_SECTOR_BITS, - nb_sectors << BDRV_SECTOR_BITS, flags); + ret = bdrv_co_do_pwrite_zeroes(bs, offset, bytes, flags); } else { bdrv_debug_event(bs, BLKDBG_PWRITEV); ret = bdrv_driver_pwritev(bs, offset, bytes, qiov, flags); } bdrv_debug_event(bs, BLKDBG_PWRITEV_DONE); - bdrv_set_dirty(bs, sector_num, nb_sectors); + bdrv_set_dirty(bs, start_sector, end_sector - start_sector); if (bs->wr_highest_offset < offset + bytes) { bs->wr_highest_offset = offset + bytes; } if (ret >= 0) { - bs->total_sectors = MAX(bs->total_sectors, sector_num + nb_sectors); + bs->total_sectors = MAX(bs->total_sectors, end_sector); } return ret;
Signed-off-by: Kevin Wolf <kwolf@redhat.com> --- block/io.c | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-)