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 <sgarzare@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);
+    if (r < 0) {
+        return r;
+    }
+
+    return alloc_size;
+}
+
 static int coroutine_fn qemu_rbd_co_truncate(BlockDriverState *bs,
                                              int64_t offset,
                                              PreallocMode prealloc,
@@ -1254,6 +1286,7 @@ static BlockDriver bdrv_rbd = {
     .bdrv_get_info          = qemu_rbd_getinfo,
     .create_opts            = &qemu_rbd_create_opts,
     .bdrv_getlength         = qemu_rbd_getlength,
+    .bdrv_get_allocated_file_size = qemu_rbd_get_allocated_file_size,
     .bdrv_co_truncate       = qemu_rbd_co_truncate,
     .protocol_name          = "rbd",
 
-- 
2.20.1

On Fri, May 3, 2019 at 7:02 AM Stefano Garzarella <sgarzare@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 <sgarzare@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.

> +    if (r < 0) {
> +        return r;
> +    }
> +
> +    return alloc_size;
> +}
> +
>  static int coroutine_fn qemu_rbd_co_truncate(BlockDriverState *bs,
>                                               int64_t offset,
>                                               PreallocMode prealloc,
> @@ -1254,6 +1286,7 @@ static BlockDriver bdrv_rbd = {
>      .bdrv_get_info          = qemu_rbd_getinfo,
>      .create_opts            = &qemu_rbd_create_opts,
>      .bdrv_getlength         = qemu_rbd_getlength,
> +    .bdrv_get_allocated_file_size = qemu_rbd_get_allocated_file_size,
>      .bdrv_co_truncate       = qemu_rbd_co_truncate,
>      .protocol_name          = "rbd",
>
> --
> 2.20.1
>


-- 
Jason

On Fri, May 03, 2019 at 07:55:01AM -0400, Jason Dillaman wrote:
> On Fri, May 3, 2019 at 7:02 AM Stefano Garzarella <sgarzare@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 <sgarzare@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