On 08.05.20 20:03, Eric Blake wrote: > Upcoming patches will enhance bitmap support in qemu-img, but in doing > so, it turns out to be nice to suppress output when bitmaps make no > sense (such as on a qcow2 v2 image). Add a hook to make this easier > to query. > > In the future, when we improve the ability to look up bitmaps through > a filter, we will probably also want to teach the block layer to > automatically let filters pass this request on through. > > Signed-off-by: Eric Blake <ebl...@redhat.com> > --- > block/qcow2.h | 1 + > include/block/block_int.h | 1 + > include/block/dirty-bitmap.h | 1 + > block/dirty-bitmap.c | 9 +++++++++ > block/qcow2-bitmap.c | 7 +++++++ > block/qcow2.c | 1 + > 6 files changed, 20 insertions(+) > > diff --git a/block/qcow2.h b/block/qcow2.h > index f4de0a27d5c3..fb2b2b5a7b4d 100644 > --- a/block/qcow2.h > +++ b/block/qcow2.h > @@ -764,6 +764,7 @@ bool qcow2_co_can_store_new_dirty_bitmap(BlockDriverState > *bs, > int qcow2_co_remove_persistent_dirty_bitmap(BlockDriverState *bs, > const char *name, > Error **errp); > +bool qcow2_dirty_bitmap_supported(BlockDriverState *bs); > > ssize_t coroutine_fn > qcow2_co_compress(BlockDriverState *bs, void *dest, size_t dest_size, > diff --git a/include/block/block_int.h b/include/block/block_int.h > index df6d0273d679..cb1082da4c43 100644 > --- a/include/block/block_int.h > +++ b/include/block/block_int.h > @@ -560,6 +560,7 @@ struct BlockDriver { > uint64_t parent_perm, uint64_t parent_shared, > uint64_t *nperm, uint64_t *nshared); > > + bool (*bdrv_dirty_bitmap_supported)(BlockDriverState *bs);
All BDSs support bitmaps, but only some support persistent dirty bitmaps, so I think the name should reflect that. Conceptually, this looks reasonable. This information might indeed be nice to have, and I’m not sure whether we should extend any existing interface to return it. (The interfaces that come to my mind are: (1) bdrv_can_store_new_dirty_bitmap() below, which we could make accept a NULL @name to return basically the same information. But it’s still a bit different, because I’d expect that function to return whether any bitmap can be stored then, not whether the node supports bitmaps at all. So e.g. if there are already too many bitmaps, it should return false, even though the node itself does support bitmaps. (2) bdrv_get_info()/BlockDriverInfo: This information would fit in very nicely here, but do we have to put it here just because it does? I don’t think so. This patch adds 20 lines of code, that shows that it’s very simple to add a dedicated method, and it’s certainly a bit easier to use than to invoke bdrv_get_info() and throw away all the other information. Perhaps this patch only shows that BlockDriverInfo doesn’t make much sense in the first place, and most of its fields should have been scalar return values from dedicated functions.) Max
signature.asc
Description: OpenPGP digital signature