From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57508) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WLId6-00020j-WE for qemu-devel@nongnu.org; Wed, 05 Mar 2014 15:41:34 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WLId0-0000LO-LM for qemu-devel@nongnu.org; Wed, 05 Mar 2014 15:41:28 -0500 Received: from lnantes-156-75-100-125.w80-12.abo.wanadoo.fr ([80.12.84.125]:55787 helo=paradis.irqsave.net) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WLId0-0000LI-GJ for qemu-devel@nongnu.org; Wed, 05 Mar 2014 15:41:22 -0500 Date: Wed, 5 Mar 2014 21:41:22 +0100 From: =?iso-8859-1?Q?Beno=EEt?= Canet Message-ID: <20140305204122.GB4514@irqsave.net> References: <1393860533-2063-1-git-send-email-mreitz@redhat.com> <1393860533-2063-6-git-send-email-mreitz@redhat.com> <20140305161150.GG1709@irqsave.net> <5317849B.7030107@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline In-Reply-To: <5317849B.7030107@redhat.com> Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 05/10] block/json: Add bdrv_co_get_block_status() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Max Reitz Cc: =?iso-8859-1?Q?Beno=EEt?= Canet , Kevin Wolf , qemu-devel@nongnu.org, Stefan Hajnoczi The Wednesday 05 Mar 2014 =E0 21:10:03 (+0100), Max Reitz wrote : > On 05.03.2014 17:11, Beno=EEt Canet wrote: > >The Monday 03 Mar 2014 =E0 16:28:48 (+0100), Max Reitz wrote : > >>Implement this function in the same way as raw_bsd does: Acknowledge > >>that this is a passthrough driver (always return BDRV_BLOCK_OFFSET_VA= LID > >>and BDRV_BLOCK_DATA and derive the offset directly from the sector > >>index) and add BDRV_BLOCK_RAW to the returned value. > >> > >>Signed-off-by: Max Reitz > >>--- > >> block/json.c | 9 +++++++++ > >> 1 file changed, 9 insertions(+) > >> > >>diff --git a/block/json.c b/block/json.c > >>index a2f4691..7392802 100644 > >>--- a/block/json.c > >>+++ b/block/json.c > >>@@ -113,6 +113,14 @@ static coroutine_fn int json_co_write_zeroes(Blo= ckDriverState *bs, > >> return bdrv_co_write_zeroes(bs, sector_num, nb_sectors, flags); > >> } > >>+static coroutine_fn int64_t json_co_get_block_status(BlockDriverStat= e *bs, > >>+ int64_t sector_= num, > >>+ int nb_sectors,= int *pnum) > >>+{ > >>+ return BDRV_BLOCK_RAW | BDRV_BLOCK_OFFSET_VALID | BDRV_BLOCK_DAT= A | > >>+ (sector_num << BDRV_SECTOR_BITS); > >>+} > >I don't understand what is the selling point of this method instead of= calling > >bdrv_co_get_block_status on bs->file. > >Some information risk to be lost and it does look like magic. >=20 > This is the same what "raw" does. It just is more meaningful: This > way, this function does not pretend to provide the blocks itself but > instead tells the truth; that is, the blocks are provided by an > underlying BDS (bs->file). >=20 > I wasn't really sure what to do myself. Generally, this driver is > actually meant to pretend that it provides the blocks itself. On the > other hand, I tried to imitate the behavior or "raw", since this is > something I can hope to be approximately correct. Also, as I've said > before, the value returned here is in fact at least technically > correct. The raw_bsd driver have an additional *pnum =3D nb_sectors; at the begini= ng of the function. Did you left it on purpose ? Best regards Beno=EEt=20 >=20 >=20 > Max >=20 > >Best regards > > > >Beno=EEt > > > >>+ > >> static void json_invalidate_cache(BlockDriverState *bs) > >> { > >> return bdrv_invalidate_cache(bs->file); > >>@@ -159,6 +167,7 @@ static BlockDriver bdrv_json =3D { > >> .bdrv_aio_discard =3D json_aio_discard, > >> .bdrv_co_write_zeroes =3D json_co_write_zeroes, > >>+ .bdrv_co_get_block_status =3D json_co_get_block_status, > >> .bdrv_invalidate_cache =3D json_invalidate_cache, > >>--=20 > >>1.9.0 > >> > >> >=20