On Fri, May 03, 2019 at 07:55:01AM -0400, Jason Dillaman wrote: > On Fri, May 3, 2019 at 7:02 AM Stefano Garzarella <sgarz...@redhat.com> wrote: > > > > This patch allows 'qemu-img info' to show the 'disk size' for > > rbd images. We use the rbd_diff_iterate2() API to calculate the > > allocated size for the image. > > > > Signed-off-by: Stefano Garzarella <sgarz...@redhat.com> > > --- > > block/rbd.c | 33 +++++++++++++++++++++++++++++++++ > > 1 file changed, 33 insertions(+) > > > > diff --git a/block/rbd.c b/block/rbd.c > > index 0c549c9935..61447bc0cb 100644 > > --- a/block/rbd.c > > +++ b/block/rbd.c > > @@ -1046,6 +1046,38 @@ static int64_t qemu_rbd_getlength(BlockDriverState > > *bs) > > return info.size; > > } > > > > +static int rbd_allocated_size_cb(uint64_t offset, size_t len, int exists, > > + void *arg) > > +{ > > + int64_t *alloc_size = (int64_t *) arg; > > + > > + if (exists) { > > + (*alloc_size) += len; > > + } > > + > > + return 0; > > +} > > + > > +static int64_t qemu_rbd_get_allocated_file_size(BlockDriverState *bs) > > +{ > > + BDRVRBDState *s = bs->opaque; > > + int64_t alloc_size = 0; > > + int r; > > + > > + /* > > + * rbd_diff_iterate2(), if the source snapshot name is NULL, invokes > > + * the callback on all allocated regions of the image. > > + */ > > + r = rbd_diff_iterate2(s->image, NULL, 0, > > + bs->total_sectors * BDRV_SECTOR_SIZE, 0, 1, > > + &rbd_allocated_size_cb, &alloc_size); > > Is there any concern that running this on very large images will take > a very long time since it needs to iterate through each individual > 4MiB (by default) backing object in the image? In libvirt, it only > attempts to calculate the actual usage if the fast-diff feature is > enabled, and recently it also got a new control to optionally disable > the functionality entirely since even with fast-diff it's can be very > slow to compute over hundreds of images in a libvirt storage pool. >
Thank you for pointing that out to me. I'll add check on fast-diff feature on v2. Since we only have one image here, do you think it would be reasonable to add this feature or is it useless? Thanks, Stefano