From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50867) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WLI95-000221-4c for qemu-devel@nongnu.org; Wed, 05 Mar 2014 15:10:33 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WLI8z-0006mt-4V for qemu-devel@nongnu.org; Wed, 05 Mar 2014 15:10:27 -0500 Received: from mx1.redhat.com ([209.132.183.28]:60452) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WLI8y-0006mZ-S6 for qemu-devel@nongnu.org; Wed, 05 Mar 2014 15:10:21 -0500 Message-ID: <5317849B.7030107@redhat.com> Date: Wed, 05 Mar 2014 21:10:03 +0100 From: Max Reitz MIME-Version: 1.0 References: <1393860533-2063-1-git-send-email-mreitz@redhat.com> <1393860533-2063-6-git-send-email-mreitz@redhat.com> <20140305161150.GG1709@irqsave.net> In-Reply-To: <20140305161150.GG1709@irqsave.net> Content-Type: text/plain; charset=ISO-8859-1; format=flowed 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: =?ISO-8859-1?Q?Beno=EEt_Canet?= Cc: Kevin Wolf , qemu-devel@nongnu.org, Stefan Hajnoczi 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_VAL= ID >> 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(Bloc= kDriverState *bs, >> return bdrv_co_write_zeroes(bs, sector_num, nb_sectors, flags); >> } >> =20 >> +static coroutine_fn int64_t json_co_get_block_status(BlockDriverState= *bs, >> + int64_t sector_n= um, >> + int nb_sectors, = int *pnum) >> +{ >> + return BDRV_BLOCK_RAW | BDRV_BLOCK_OFFSET_VALID | BDRV_BLOCK_DATA= | >> + (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,=20 this function does not pretend to provide the blocks itself but instead=20 tells the truth; that is, the blocks are provided by an underlying BDS=20 (bs->file). I wasn't really sure what to do myself. Generally, this driver is=20 actually meant to pretend that it provides the blocks itself. On the=20 other hand, I tried to imitate the behavior or "raw", since this is=20 something I can hope to be approximately correct. Also, as I've said=20 before, the value returned here is in fact at least technically correct. 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, >> =20 >> .bdrv_co_write_zeroes =3D json_co_write_zeroes, >> + .bdrv_co_get_block_status =3D json_co_get_block_status, >> =20 >> .bdrv_invalidate_cache =3D json_invalidate_cache, >> =20 >> --=20 >> 1.9.0 >> >>