From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:36607) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b2gVH-0007CP-4v for qemu-devel@nongnu.org; Tue, 17 May 2016 11:01:50 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1b2gVA-0003e1-PR for qemu-devel@nongnu.org; Tue, 17 May 2016 11:01:45 -0400 Received: from mail-am1on0108.outbound.protection.outlook.com ([157.56.112.108]:29792 helo=emea01-am1-obe.outbound.protection.outlook.com) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b2gV9-0003dP-O6 for qemu-devel@nongnu.org; Tue, 17 May 2016 11:01:40 -0400 References: <1463229957-14253-1-git-send-email-den@openvz.org> <1463229957-14253-2-git-send-email-den@openvz.org> <5739FAEB.9080702@redhat.com> From: Pavel Butsykin Message-ID: <573B324D.4060003@virtuozzo.com> Date: Tue, 17 May 2016 18:01:33 +0300 MIME-Version: 1.0 In-Reply-To: <5739FAEB.9080702@redhat.com> Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 01/10] block/io: add bdrv_co_write_compressed List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake , "Denis V. Lunev" , qemu-devel@nongnu.org, Kevin Wolf Cc: Jeff Cody , Markus Armbruster , John Snow , Stefan Hajnoczi On 16.05.2016 19:52, Eric Blake wrote: > On 05/14/2016 06:45 AM, Denis V. Lunev wrote: >> From: Pavel Butsykin >> >> This patch just adds the interface to the bdrv_co_write_compressed, which >> is currently not used but will be useful for safe implementation of the >> bdrv_co_write_compressed callback in format drivers. >> >> Signed-off-by: Pavel Butsykin >> Signed-off-by: Denis V. Lunev >> CC: Jeff Cody >> CC: Markus Armbruster >> CC: Eric Blake >> CC: John Snow >> CC: Stefan Hajnoczi >> CC: Kevin Wolf >> --- > >> +++ b/block/io.c >> @@ -1828,8 +1828,8 @@ int bdrv_is_allocated_above(BlockDriverState *top, >> return 0; >> } >> >> -int bdrv_write_compressed(BlockDriverState *bs, int64_t sector_num, >> - const uint8_t *buf, int nb_sectors) >> +int bdrv_co_write_compressed(BlockDriverState *bs, int64_t sector_num, >> + int nb_sectors, QEMUIOVector *qiov) > > As long as we're adding a new public interface, I'd really like us to > make it byte-based. int64_t sector_num might be better represented as a > byte offset, and int nb_sectors seems redundant with qiov->size. > >> { >> BlockDriver *drv = bs->drv; >> int ret; >> @@ -1837,7 +1837,7 @@ int bdrv_write_compressed(BlockDriverState *bs, int64_t sector_num, >> if (!drv) { >> return -ENOMEDIUM; >> } >> - if (!drv->bdrv_write_compressed) { >> + if (!drv->bdrv_co_write_compressed) { >> return -ENOTSUP; >> } >> ret = bdrv_check_request(bs, sector_num, nb_sectors); >> @@ -1846,8 +1846,71 @@ int bdrv_write_compressed(BlockDriverState *bs, int64_t sector_num, >> } >> >> assert(QLIST_EMPTY(&bs->dirty_bitmaps)); >> + assert(qemu_in_coroutine()); >> + >> + return drv->bdrv_co_write_compressed(bs, sector_num, nb_sectors, qiov); > > Of course, if you make the public interface byte-based, then calling > into the back end will have to scale back to sectors (after first > asserting that we aren't violating the scaling); see how Kevin did it in > commit 166fe9605. > >> +} >> + >> +typedef struct BdrvWriteCompressedCo { >> + BlockDriverState *bs; >> + int64_t sector_num; > > Again, I think a byte offset is smarter than a sector number. > Kevin used the byte offset for functions bdrv_driver_pread/_pwrite(It looks like just an additional interface), which is not the same thing. Here the bdrv_co/bdrv_write_compressed functions are analogues of the bdrv_co/bdrv_write functions that still use sectors in the arguments. So I'm not sure that the interface there needs to be some other. Kevin, what do you think about this?