From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:56339) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WLL8m-0002BP-AU for qemu-devel@nongnu.org; Wed, 05 Mar 2014 18:22:26 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WLL8d-0008MH-LR for qemu-devel@nongnu.org; Wed, 05 Mar 2014 18:22:17 -0500 Received: from lnantes-156-75-100-125.w80-12.abo.wanadoo.fr ([80.12.84.125]:55804 helo=paradis.irqsave.net) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WLL8d-0008M3-C0 for qemu-devel@nongnu.org; Wed, 05 Mar 2014 18:22:11 -0500 Date: Thu, 6 Mar 2014 00:22:10 +0100 From: =?iso-8859-1?Q?Beno=EEt?= Canet Message-ID: <20140305232210.GC5450@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> <20140305204122.GB4514@irqsave.net> <53178CC9.3070700@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline In-Reply-To: <53178CC9.3070700@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:44:57 (+0100), Max Reitz wrote : > On 05.03.2014 21:41, Beno=EEt Canet wrote: > >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: Acknowledg= e > >>>>that this is a passthrough driver (always return BDRV_BLOCK_OFFSET_= VALID > >>>>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(B= lockDriverState *bs, > >>>> return bdrv_co_write_zeroes(bs, sector_num, nb_sectors, flags= ); > >>>> } > >>>>+static coroutine_fn int64_t json_co_get_block_status(BlockDriverSt= ate *bs, > >>>>+ int64_t secto= r_num, > >>>>+ int nb_sector= s, int *pnum) > >>>>+{ > >>>>+ return BDRV_BLOCK_RAW | BDRV_BLOCK_OFFSET_VALID | BDRV_BLOCK_D= ATA | > >>>>+ (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. > >>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). > >> > >>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 beg= ining of the > >function. Did you left it on purpose ? >=20 > Oops, no, I did not. Okay, if I even fail at copying code, the > argument about "raw" being at least probably correct isn't worth > anything here, I guess. ;-) In fact I wonder i posix_raw act this way because one of the BDRV it does= wrap have simply no clue on how to answer to this request. Best regards Beno=EEt >=20 > Max >=20 > >Best regards > > > >Beno=EEt > >> > >>Max > >> > >>>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