From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:52137) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YG9hz-0007r6-Nm for qemu-devel@nongnu.org; Tue, 27 Jan 2015 12:13:48 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YG9hv-0006a2-DC for qemu-devel@nongnu.org; Tue, 27 Jan 2015 12:13:47 -0500 Received: from mx1.redhat.com ([209.132.183.28]:41424) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YG9hv-0006Zy-6h for qemu-devel@nongnu.org; Tue, 27 Jan 2015 12:13:43 -0500 Message-ID: <54C7C73E.3070000@redhat.com> Date: Tue, 27 Jan 2015 12:13:34 -0500 From: Max Reitz MIME-Version: 1.0 References: <1422366699-17473-1-git-send-email-den@openvz.org> <1422366699-17473-4-git-send-email-den@openvz.org> In-Reply-To: <1422366699-17473-4-git-send-email-den@openvz.org> Content-Type: text/plain; charset=iso-8859-15; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 3/7] block/raw-posix: refactor handle_aiocb_write_zeroes a bit List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Denis V. Lunev" Cc: Kevin Wolf , Peter Lieven , Fam Zheng , qemu-devel@nongnu.org, Stefan Hajnoczi On 2015-01-27 at 08:51, Denis V. Lunev wrote: > move code dealing with a block device to a separate function. This will > allow to implement additional processing for ordinary files. > > Pls note, that xfs_code has been moved before checking for Please use "Please". :-) > s->has_write_zeroes as xfs_write_zeroes does not touch this flag inside. > This makes code a bit more consistent. > > Signed-off-by: Denis V. Lunev > CC: Kevin Wolf > CC: Stefan Hajnoczi > CC: Peter Lieven > CC: Fam Zheng > --- > block/raw-posix.c | 48 +++++++++++++++++++++++++++++------------------- > 1 file changed, 29 insertions(+), 19 deletions(-) > > diff --git a/block/raw-posix.c b/block/raw-posix.c > index 2aa268a..24e1fab 100644 > --- a/block/raw-posix.c > +++ b/block/raw-posix.c > @@ -914,41 +914,51 @@ static int do_fallocate(int fd, int mode, off_t offset, off_t len) > } > #endif > > -static ssize_t handle_aiocb_write_zeroes(RawPosixAIOData *aiocb) > +static ssize_t handle_aiocb_write_zeroes_block(RawPosixAIOData *aiocb) > { > - int ret = -EOPNOTSUPP; > + int ret = -ENOTSUP; > BDRVRawState *s = aiocb->bs->opaque; > > - if (s->has_write_zeroes == 0) { > + if (!s->has_write_zeroes) { > return -ENOTSUP; > } > > - if (aiocb->aio_type & QEMU_AIO_BLKDEV) { > #ifdef BLKZEROOUT > - do { > - uint64_t range[2] = { aiocb->aio_offset, aiocb->aio_nbytes }; > - if (ioctl(aiocb->aio_fildes, BLKZEROOUT, range) == 0) { > - return 0; > - } > - } while (errno == EINTR); > - > - ret = -errno; > -#endif > - } else { > -#ifdef CONFIG_XFS > - if (s->is_xfs) { > - return xfs_write_zeroes(s, aiocb->aio_offset, aiocb->aio_nbytes); > + do { > + uint64_t range[2] = { aiocb->aio_offset, aiocb->aio_nbytes }; > + if (ioctl(aiocb->aio_fildes, BLKZEROOUT, range) == 0) { > + return 0; > } > + } while (errno == EINTR); > + > + ret = translate_err(-errno); > #endif > - } > > - ret = translate_err(ret); > if (ret == -ENOTSUP) { > s->has_write_zeroes = false; > } > return ret; > } > > +static ssize_t handle_aiocb_write_zeroes(RawPosixAIOData *aiocb) > +{ > + int ret = -ENOTSUP; > + BDRVRawState *s = aiocb->bs->opaque; > + > + if (aiocb->aio_type & QEMU_AIO_BLKDEV) { > + return handle_aiocb_write_zeroes_block(aiocb); > + } > + > +#ifdef CONFIG_XFS > + if (s->is_xfs) { > + return xfs_write_zeroes(s, aiocb->aio_offset, aiocb->aio_nbytes); > + } > +#endif > + > + s->has_write_zeroes = false; > + return ret; > +} > + It'll probably look nicer if you remove the "ret" variable from this function completely and just "return -ENOTSUP" at the end. With s/Pls/Please/ in the commit message and with or without "ret" removed from this function: Reviewed-by: Max Reitz