From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58170) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WLIgh-0005mg-G6 for qemu-devel@nongnu.org; Wed, 05 Mar 2014 15:45:17 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WLIgb-0001PX-GI for qemu-devel@nongnu.org; Wed, 05 Mar 2014 15:45:11 -0500 Received: from mx1.redhat.com ([209.132.183.28]:64415) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WLIgb-0001M2-6y for qemu-devel@nongnu.org; Wed, 05 Mar 2014 15:45:05 -0500 Message-ID: <53178CC9.3070700@redhat.com> Date: Wed, 05 Mar 2014 21:44:57 +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> <5317849B.7030107@redhat.com> <20140305204122.GB4514@irqsave.net> In-Reply-To: <20140305204122.GB4514@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 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: Acknowledge >>>> that this is a passthrough driver (always return BDRV_BLOCK_OFFSET_V= ALID >>>> 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(Bl= ockDriverState *bs, >>>> return bdrv_co_write_zeroes(bs, sector_num, nb_sectors, flags)= ; >>>> } >>>> +static coroutine_fn int64_t json_co_get_block_status(BlockDriverSta= te *bs, >>>> + int64_t sector= _num, >>>> + int nb_sectors= , int *pnum) >>>> +{ >>>> + return BDRV_BLOCK_RAW | BDRV_BLOCK_OFFSET_VALID | BDRV_BLOCK_DA= TA | >>>> + (sector_num << BDRV_SECTOR_BITS); >>>> +} >>> I don't understand what is the selling point of this method instead o= f 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 begi= ning of the > function. Did you left it on purpose ? Oops, no, I did not. Okay, if I even fail at copying code, the argument=20 about "raw" being at least probably correct isn't worth anything here, I=20 guess. ;-) Max > 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 >>>> >>>>