From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44897) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1V4Aha-0001bv-UB for qemu-devel@nongnu.org; Tue, 30 Jul 2013 10:15:09 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1V4AhU-0004Mf-DV for qemu-devel@nongnu.org; Tue, 30 Jul 2013 10:15:02 -0400 Received: from mx1.redhat.com ([209.132.183.28]:26175) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1V4AhU-0004Lm-4y for qemu-devel@nongnu.org; Tue, 30 Jul 2013 10:14:56 -0400 Date: Tue, 30 Jul 2013 16:14:47 +0200 From: Kevin Wolf Message-ID: <20130730141447.GC2475@dhcp-200-207.str.redhat.com> References: <1374762197-7261-1-git-send-email-pbonzini@redhat.com> <1374762197-7261-12-git-send-email-pbonzini@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1374762197-7261-12-git-send-email-pbonzini@redhat.com> Subject: Re: [Qemu-devel] [PATCH v3 11/19] block: define get_block_status return value List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: pl@kamp.de, qemu-devel@nongnu.org, stefanha@redhat.com Am 25.07.2013 um 16:23 hat Paolo Bonzini geschrieben: > Define the return value of get_block_status. Bits 0, 1, 2 and 9-62 > are valid; bit 63 (the sign bit) is reserved for errors. Bits 3-8 > are left for future extensions. > > The return code is compatible with the old is_allocated API: if a driver > only returns 0 or 1 (aka BDRV_BLOCK_DATA) like is_allocated used to, > clients of is_allocated will not have any change in behavior. Still, > we will return more precise information in the next patches and the > new definition of bdrv_is_allocated is already prepared for this. > > Reviewed-by: Eric Blake > Signed-off-by: Paolo Bonzini > --- > block.c | 10 ++++++++-- > include/block/block.h | 26 ++++++++++++++++++++++++++ > 2 files changed, 34 insertions(+), 2 deletions(-) > > diff --git a/block.c b/block.c > index f533c36..7cfbf71 100644 > --- a/block.c > +++ b/block.c > @@ -3004,7 +3004,7 @@ static int64_t coroutine_fn bdrv_co_get_block_status(BlockDriverState *bs, > > if (!bs->drv->bdrv_co_get_block_status) { > *pnum = nb_sectors; > - return 1; > + return BDRV_BLOCK_DATA; > } > > return bs->drv->bdrv_co_get_block_status(bs, sector_num, nb_sectors, pnum); > @@ -3054,7 +3054,13 @@ int64_t bdrv_get_block_status(BlockDriverState *bs, int64_t sector_num, > int coroutine_fn bdrv_is_allocated(BlockDriverState *bs, int64_t sector_num, > int nb_sectors, int *pnum) > { > - return bdrv_get_block_status(bs, sector_num, nb_sectors, pnum); > + int64_t ret = bdrv_get_block_status(bs, sector_num, nb_sectors, pnum); > + if (ret < 0) { > + return ret; > + } > + return > + (ret & BDRV_BLOCK_DATA) || > + ((ret & BDRV_BLOCK_ZERO) && !bdrv_has_zero_init(bs)); > } > > /* > diff --git a/include/block/block.h b/include/block/block.h > index e41854e..d044b31 100644 > --- a/include/block/block.h > +++ b/include/block/block.h > @@ -81,6 +81,32 @@ typedef struct BlockDevOps { > #define BDRV_SECTOR_SIZE (1ULL << BDRV_SECTOR_BITS) > #define BDRV_SECTOR_MASK ~(BDRV_SECTOR_SIZE - 1) > > +/* BDRV_BLOCK_DATA: data is read from bs->file or another file > + * BDRV_BLOCK_ZERO: sectors read as zero > + * BDRV_BLOCK_OFFSET_VALID: sector stored in bs->file as raw data > + * > + * If BDRV_BLOCK_OFFSET_VALID is set, bits 9-62 represent the offset in > + * bs->file where sector data can be read from as raw data. > + * > + * DATA == 0 && ZERO == 0 means that data is read from backing_hd if present. > + * > + * DATA ZERO OFFSET_VALID > + * t t t sectors read as zero, bs->file is zero at offset > + * t f t sectors read as valid from bs->file at offset > + * f t t sectors preallocated, read as zero, bs->file not > + * necessarily zero at offset > + * f f t sectors preallocated but read from backing_hd, > + * bs->file contains garbage at offset > + * t t f sectors preallocated, read as zero, unknown offset > + * t f f sectors read from unknown file or offset > + * f t f not allocated or unknown offset, read as zero > + * f f f not allocated or unknown offset, read from backing_hd > + */ > +#define BDRV_BLOCK_DATA 1 > +#define BDRV_BLOCK_ZERO 2 > +#define BDRV_BLOCK_OFFSET_VALID 4 > +#define BDRV_BLOCK_OFFSET_MASK BDRV_SECTOR_MASK When are block driver supposed to set the BDRV_BLOCK_OFFSET_VALID flag? For example, qcow2 could in theory set the flag, it has all of the information already in memory. But with a fragmented image this might mean that it returns only one cluster instead of a large area with one bdrv_get_block_status() call. Should the caller pass a flag that tells whether he is interested in the offset or not? Kevin