Message ID | 1385124001-3576-2-git-send-email-pbonzini@redhat.com |
---|---|
State | New |
Headers | show |
On 22.11.2013 13:39, Paolo Bonzini wrote: > bdrv_co_discard is only covering drivers which have a .bdrv_co_discard() > implementation, but not those with .bdrv_aio_discard(). Not very nice, > and easy to avoid. > > Suggested-by: Kevin Wolf <kwolf@redhat.com> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > --- > block.c | 80 +++++++++++++++++++++++++++++++++-------------------------------- > 1 file changed, 41 insertions(+), 39 deletions(-) > > diff --git a/block.c b/block.c > index e22b55f..1b3e8b2 100644 > --- a/block.c > +++ b/block.c > @@ -4288,6 +4288,8 @@ static void coroutine_fn bdrv_discard_co_entry(void *opaque) > int coroutine_fn bdrv_co_discard(BlockDriverState *bs, int64_t sector_num, > int nb_sectors) > { > + int max_discard; > + > if (!bs->drv) { > return -ENOMEDIUM; > } else if (bdrv_check_request(bs, sector_num, nb_sectors)) { > @@ -4305,55 +4307,55 @@ int coroutine_fn bdrv_co_discard(BlockDriverState *bs, int64_t sector_num, > return 0; > } > > - if (bs->drv->bdrv_co_discard) { > - int max_discard = bs->bl.max_discard ? > - bs->bl.max_discard : MAX_DISCARD_DEFAULT; > + if (!bs->drv->bdrv_co_discard && !bs->drv->bdrv_aio_discard) { > + return 0; > + } > > - while (nb_sectors > 0) { > - int ret; > - int num = nb_sectors; > + max_discard = bs->bl.max_discard ? bs->bl.max_discard : MAX_DISCARD_DEFAULT; > + while (nb_sectors > 0) { > + int ret; > + int num = nb_sectors; > > - /* align request */ > - if (bs->bl.discard_alignment && > - num >= bs->bl.discard_alignment && > - sector_num % bs->bl.discard_alignment) { > - if (num > bs->bl.discard_alignment) { > - num = bs->bl.discard_alignment; > - } > - num -= sector_num % bs->bl.discard_alignment; > + /* align request */ > + if (bs->bl.discard_alignment && > + num >= bs->bl.discard_alignment && > + sector_num % bs->bl.discard_alignment) { > + if (num > bs->bl.discard_alignment) { > + num = bs->bl.discard_alignment; > } > + num -= sector_num % bs->bl.discard_alignment; > + } > > - /* limit request size */ > - if (num > max_discard) { > - num = max_discard; > - } > + /* limit request size */ > + if (num > max_discard) { > + num = max_discard; > + } > > + if (bs->drv->bdrv_co_discard) { > ret = bs->drv->bdrv_co_discard(bs, sector_num, num); > - if (ret) { > - return ret; > + } else { > + BlockDriverAIOCB *acb; > + CoroutineIOCompletion co = { > + .coroutine = qemu_coroutine_self(), > + }; > + > + acb = bs->drv->bdrv_aio_discard(bs, sector_num, nb_sectors, > + bdrv_co_io_em_complete, &co); > + if (acb == NULL) { > + return -EIO; > + } else { > + qemu_coroutine_yield(); > + ret = co.ret; > } > - > - sector_num += num; > - nb_sectors -= num; > } > - return 0; > - } else if (bs->drv->bdrv_aio_discard) { > - BlockDriverAIOCB *acb; > - CoroutineIOCompletion co = { > - .coroutine = qemu_coroutine_self(), > - }; > - > - acb = bs->drv->bdrv_aio_discard(bs, sector_num, nb_sectors, > - bdrv_co_io_em_complete, &co); > - if (acb == NULL) { > - return -EIO; > - } else { > - qemu_coroutine_yield(); > - return co.ret; > + if (ret) { > + return ret; > } > - } else { > - return 0; > + > + sector_num += num; > + nb_sectors -= num; > } > + return 0; > } > > int bdrv_discard(BlockDriverState *bs, int64_t sector_num, int nb_sectors) Reviewed-by: Peter Lieven <pl@kamp.de>
diff --git a/block.c b/block.c index e22b55f..1b3e8b2 100644 --- a/block.c +++ b/block.c @@ -4288,6 +4288,8 @@ static void coroutine_fn bdrv_discard_co_entry(void *opaque) int coroutine_fn bdrv_co_discard(BlockDriverState *bs, int64_t sector_num, int nb_sectors) { + int max_discard; + if (!bs->drv) { return -ENOMEDIUM; } else if (bdrv_check_request(bs, sector_num, nb_sectors)) { @@ -4305,55 +4307,55 @@ int coroutine_fn bdrv_co_discard(BlockDriverState *bs, int64_t sector_num, return 0; } - if (bs->drv->bdrv_co_discard) { - int max_discard = bs->bl.max_discard ? - bs->bl.max_discard : MAX_DISCARD_DEFAULT; + if (!bs->drv->bdrv_co_discard && !bs->drv->bdrv_aio_discard) { + return 0; + } - while (nb_sectors > 0) { - int ret; - int num = nb_sectors; + max_discard = bs->bl.max_discard ? bs->bl.max_discard : MAX_DISCARD_DEFAULT; + while (nb_sectors > 0) { + int ret; + int num = nb_sectors; - /* align request */ - if (bs->bl.discard_alignment && - num >= bs->bl.discard_alignment && - sector_num % bs->bl.discard_alignment) { - if (num > bs->bl.discard_alignment) { - num = bs->bl.discard_alignment; - } - num -= sector_num % bs->bl.discard_alignment; + /* align request */ + if (bs->bl.discard_alignment && + num >= bs->bl.discard_alignment && + sector_num % bs->bl.discard_alignment) { + if (num > bs->bl.discard_alignment) { + num = bs->bl.discard_alignment; } + num -= sector_num % bs->bl.discard_alignment; + } - /* limit request size */ - if (num > max_discard) { - num = max_discard; - } + /* limit request size */ + if (num > max_discard) { + num = max_discard; + } + if (bs->drv->bdrv_co_discard) { ret = bs->drv->bdrv_co_discard(bs, sector_num, num); - if (ret) { - return ret; + } else { + BlockDriverAIOCB *acb; + CoroutineIOCompletion co = { + .coroutine = qemu_coroutine_self(), + }; + + acb = bs->drv->bdrv_aio_discard(bs, sector_num, nb_sectors, + bdrv_co_io_em_complete, &co); + if (acb == NULL) { + return -EIO; + } else { + qemu_coroutine_yield(); + ret = co.ret; } - - sector_num += num; - nb_sectors -= num; } - return 0; - } else if (bs->drv->bdrv_aio_discard) { - BlockDriverAIOCB *acb; - CoroutineIOCompletion co = { - .coroutine = qemu_coroutine_self(), - }; - - acb = bs->drv->bdrv_aio_discard(bs, sector_num, nb_sectors, - bdrv_co_io_em_complete, &co); - if (acb == NULL) { - return -EIO; - } else { - qemu_coroutine_yield(); - return co.ret; + if (ret) { + return ret; } - } else { - return 0; + + sector_num += num; + nb_sectors -= num; } + return 0; } int bdrv_discard(BlockDriverState *bs, int64_t sector_num, int nb_sectors)
bdrv_co_discard is only covering drivers which have a .bdrv_co_discard() implementation, but not those with .bdrv_aio_discard(). Not very nice, and easy to avoid. Suggested-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- block.c | 80 +++++++++++++++++++++++++++++++++-------------------------------- 1 file changed, 41 insertions(+), 39 deletions(-)