| Message ID | 1385124001-3576-6-git-send-email-pbonzini@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 22.11.2013 13:39, Paolo Bonzini wrote: > Similar to write_zeroes, let the generic code receive a ENOTSUP for > discard operations. Since bdrv_discard has advisory semantics, > we can just swallow the error. > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > --- > block.c | 2 +- > block/raw-posix.c | 12 ++++++------ > 2 files changed, 7 insertions(+), 7 deletions(-) > > diff --git a/block.c b/block.c > index f9674d9..b18ee6b 100644 > --- a/block.c > +++ b/block.c > @@ -4364,7 +4364,7 @@ int coroutine_fn bdrv_co_discard(BlockDriverState *bs, int64_t sector_num, > ret = co.ret; > } > } > - if (ret) { > + if (ret && ret != -ENOTSUP) { > return ret; > } > > diff --git a/block/raw-posix.c b/block/raw-posix.c > index f836c8e..cfa3162 100644 > --- a/block/raw-posix.c > +++ b/block/raw-posix.c > @@ -323,10 +323,10 @@ static int raw_open_common(BlockDriverState *bs, QDict *options, > } > #endif > > - s->has_discard = 1; > + s->has_discard = true; > #ifdef CONFIG_XFS > if (platform_test_xfs_fd(s->fd)) { > - s->is_xfs = 1; > + s->is_xfs = true; > } > #endif > > @@ -698,8 +698,8 @@ static ssize_t handle_aiocb_discard(RawPosixAIOData *aiocb) > int ret = -EOPNOTSUPP; > BDRVRawState *s = aiocb->bs->opaque; > > - if (s->has_discard == 0) { > - return 0; > + if (!s->has_discard) { > + return -ENOTSUP; > } > > if (aiocb->aio_type & QEMU_AIO_BLKDEV) { > @@ -734,8 +734,8 @@ static ssize_t handle_aiocb_discard(RawPosixAIOData *aiocb) > > if (ret == -ENODEV || ret == -ENOSYS || ret == -EOPNOTSUPP || > ret == -ENOTTY) { > - s->has_discard = 0; > - ret = 0; > + s->has_discard = false; > + ret = -ENOTSUP; > } > return ret; > } wouldn't it make sense to return -ENOTSUP in other drivers a well if the operation is not supported and not return 0? Otherwise: Reviewed-by: Peter Lieven <pl@kamp.de>
Il 25/11/2013 11:06, Peter Lieven ha scritto: > wouldn't it make sense to return -ENOTSUP in other drivers a well if the > operation is not supported and not return 0? Yes; patches are welcome. :) (I have a patch for block/iscsi.c that handles LBPU=1/LBPWS=0, and one of the things it does is "return 0" -> "return -ENOTSUP" for consistency between iscsi_co_discard and iscsi_co_write_zeroes). Paolo
diff --git a/block.c b/block.c index f9674d9..b18ee6b 100644 --- a/block.c +++ b/block.c @@ -4364,7 +4364,7 @@ int coroutine_fn bdrv_co_discard(BlockDriverState *bs, int64_t sector_num, ret = co.ret; } } - if (ret) { + if (ret && ret != -ENOTSUP) { return ret; } diff --git a/block/raw-posix.c b/block/raw-posix.c index f836c8e..cfa3162 100644 --- a/block/raw-posix.c +++ b/block/raw-posix.c @@ -323,10 +323,10 @@ static int raw_open_common(BlockDriverState *bs, QDict *options, } #endif - s->has_discard = 1; + s->has_discard = true; #ifdef CONFIG_XFS if (platform_test_xfs_fd(s->fd)) { - s->is_xfs = 1; + s->is_xfs = true; } #endif @@ -698,8 +698,8 @@ static ssize_t handle_aiocb_discard(RawPosixAIOData *aiocb) int ret = -EOPNOTSUPP; BDRVRawState *s = aiocb->bs->opaque; - if (s->has_discard == 0) { - return 0; + if (!s->has_discard) { + return -ENOTSUP; } if (aiocb->aio_type & QEMU_AIO_BLKDEV) { @@ -734,8 +734,8 @@ static ssize_t handle_aiocb_discard(RawPosixAIOData *aiocb) if (ret == -ENODEV || ret == -ENOSYS || ret == -EOPNOTSUPP || ret == -ENOTTY) { - s->has_discard = 0; - ret = 0; + s->has_discard = false; + ret = -ENOTSUP; } return ret; }
Similar to write_zeroes, let the generic code receive a ENOTSUP for discard operations. Since bdrv_discard has advisory semantics, we can just swallow the error. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- block.c | 2 +- block/raw-posix.c | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-)