[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [PATCH] block/rbd: implement .bdrv_get_allocated_file_s
From: |
Stefano Garzarella |
Subject: |
Re: [Qemu-block] [PATCH] block/rbd: implement .bdrv_get_allocated_file_size callback |
Date: |
Fri, 3 May 2019 14:10:41 +0200 |
User-agent: |
NeoMutt/20180716 |
On Fri, May 03, 2019 at 07:55:01AM -0400, Jason Dillaman wrote:
> On Fri, May 3, 2019 at 7:02 AM Stefano Garzarella <address@hidden> 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 <address@hidden>
> > ---
> > 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