From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53486) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b8QcT-0003SO-WF for qemu-devel@nongnu.org; Thu, 02 Jun 2016 07:16:59 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1b8QcR-0003Cb-Tq for qemu-devel@nongnu.org; Thu, 02 Jun 2016 07:16:56 -0400 Date: Thu, 2 Jun 2016 13:16:47 +0200 From: Kevin Wolf Message-ID: <20160602111647.GD6867@noname.redhat.com> References: <1464815413-613-1-git-send-email-eblake@redhat.com> <1464815413-613-10-git-send-email-eblake@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1464815413-613-10-git-send-email-eblake@redhat.com> Subject: Re: [Qemu-devel] [PATCH v2 09/13] qed: Convert to bdrv_co_pwrite_zeroes() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake Cc: qemu-devel@nongnu.org, qemu-block@nongnu.org, Stefan Hajnoczi , Max Reitz Am 01.06.2016 um 23:10 hat Eric Blake geschrieben: > Another step on our continuing quest to switch to byte-based > interfaces. > > Kill an abuse of the comma operator while at it (fortunately, > the semantics were still right). Also, the test for requests > not aligned to clusters should be applied always, not just > when a backing file is present. > > Signed-off-by: Eric Blake > --- > block/qed.c | 33 +++++++++++++++------------------ > 1 file changed, 15 insertions(+), 18 deletions(-) > > diff --git a/block/qed.c b/block/qed.c > index 0ab5b40..45ec13d 100644 > --- a/block/qed.c > +++ b/block/qed.c > @@ -1419,7 +1419,7 @@ typedef struct { > bool done; > } QEDWriteZeroesCB; > > -static void coroutine_fn qed_co_write_zeroes_cb(void *opaque, int ret) > +static void coroutine_fn qed_co_pwrite_zeroes_cb(void *opaque, int ret) > { > QEDWriteZeroesCB *cb = opaque; > > @@ -1430,10 +1430,10 @@ static void coroutine_fn qed_co_write_zeroes_cb(void *opaque, int ret) > } > } > > -static int coroutine_fn bdrv_qed_co_write_zeroes(BlockDriverState *bs, > - int64_t sector_num, > - int nb_sectors, > - BdrvRequestFlags flags) > +static int coroutine_fn bdrv_qed_co_pwrite_zeroes(BlockDriverState *bs, > + int64_t offset, > + int count, > + BdrvRequestFlags flags) > { > BlockAIOCB *blockacb; > BDRVQEDState *s = bs->opaque; > @@ -1441,25 +1441,22 @@ static int coroutine_fn bdrv_qed_co_write_zeroes(BlockDriverState *bs, > QEMUIOVector qiov; > struct iovec iov; > > - /* Refuse if there are untouched backing file sectors */ > - if (bs->backing) { > - if (qed_offset_into_cluster(s, sector_num * BDRV_SECTOR_SIZE) != 0) { > - return -ENOTSUP; > - } > - if (qed_offset_into_cluster(s, nb_sectors * BDRV_SECTOR_SIZE) != 0) { > - return -ENOTSUP; > - } > + /* Fall back if the request is not */ ...aligned? > + if (qed_offset_into_cluster(s, offset) || > + qed_offset_into_cluster(s, count)) { > + return -ENOTSUP; > } This is obviously correct and almost as obviously suboptimal compared to the original version (we need cluster alignment with a backing file, but without a backing file, sector alignment would be enough). But as this is QED, which is only supported for compatibility these days, simpler if slightly suboptimal code is okay with me. Kevin