[libvirt] [PATCH] rbd: Use RBD fast-diff for querying actual volume allocation

[John Ferlan]

On 02/10/2016 04:21 AM, Wido den Hollander wrote:
> Since Ceph version Infernalis (9.2.0) the new fast-diff mechanism
> of RBD allows for querying actual volume usage.
> 
> Prior to this version there was no easy and fast way to query how
> much allocation a RBD volume had inside a Ceph cluster.
> 
> To use the fast-diff feature it needs to be enabled per RBD image
> and is only supported by Ceph cluster running version Infernalis
> (9.2.0) or newer.
> 
> Without the fast-diff feature enabled libvirt will report an allocation
> identical to the image capacity. This is how libvirt behaves currently.
> 
> 'virsh vol-info rbd/image2' might output for example:
> 
>   Name:           image2
>   Type:           network
>   Capacity:       1,00 GiB
>   Allocation:     124,00 MiB
> 
> Newly created volumes will have the fast-diff feature enabled if the
> backing Ceph cluster supports it.
> 
> Signed-off-by: Wido den Hollander <wido at widodh.nl>
> ---
>  src/storage/storage_backend_rbd.c | 48 +++++++++++++++++++++++++++++++++++----
>  1 file changed, 44 insertions(+), 4 deletions(-)
> 
> diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backend_rbd.c
> index 5d73370..cdcfa48 100644
> --- a/src/storage/storage_backend_rbd.c
> +++ b/src/storage/storage_backend_rbd.c
> @@ -279,6 +279,20 @@ virStorageBackendRBDCloseRADOSConn(virStorageBackendRBDStatePtr ptr)
>      return ret;
>  }
>  
> +#if LIBRBD_VERSION_CODE > 265
> +static int
> +virStorageBackendRBDRefreshVolInfoCb(uint64_t offset ATTRIBUTE_UNUSED,
> +                                     size_t len,
> +                                     int exists,
> +                                     void *arg) {
> +    uint64_t *used_size = (uint64_t *)(arg);
> +    if (exists)
> +        (*used_size) += len;
> +
> +    return 0;
> +}
> +#endif
> +

The above could move into my suggestions below:

How about perhaps a

/*
 * Returns 0 on success, -1 on failure and messages
 */
static int
volStorageBackendRBDGetFeatures(rbd_image_t image,
                                const char *volname,
                                uint64_t *features)
{
    int r, ret = -1;

    if ((r = rbd_get_features(image, features)) < 0) {
        virReportSystemError(-r,
                             _("failed to get the features of RBD image
%s"),
                             volname);
        goto cleanup;
    }
    ret = 0;

 cleanup:
    return ret;
}

[the above can be used by virStorageBackendRBDImageInfo]... So what I
would do is create a patch 1 that replaces the current usage with this
methodology.  Then patch 2 can be these changes.


#if LIBRBD_VERSION_CODE > 265
static bool
volStorageBackendRBDUseFastDiff(uint64_t features)
{
    return features & RBD_FEATURE_FAST_DIFF;
}

... The virStorageBackendRBDRefreshVolInfoCb function ...

static int
virStorageBackendRBDSetAllocation(virStorageVolDefPtr,
                                  rbd_image_t *image,
                                  rbd_image_info_t *info)
{
    int r, ret = -1;
    uint64_t used_size;

    if ((r = rbd_diff_iterate2(image, NULL, 0, info->size, 0, 1,
                               &virStorageBackendRBDRefreshVolInfoCb,
                               &used_size)) < 0) {
        virReportSystemError(-r, _("failed to iterate RBD image '%s'"),
                             vol->name);
        goto cleanup;
    }
    vol->target.allocation = used_size
    ret = 0;

 cleanup:
    return ret;
}


static int
#else
volStorageBackendRBDUseFastDiff(uint64_t features ATTRIBUTE_UNUSED)
{
    return false;
}
#endif

>  static int
>  volStorageBackendRBDRefreshVolInfo(virStorageVolDefPtr vol,
>                                     virStoragePoolObjPtr pool,
> @@ -288,6 +302,8 @@ volStorageBackendRBDRefreshVolInfo(virStorageVolDefPtr vol,
>      int r = 0;
>      rbd_image_t image = NULL;
>      rbd_image_info_t info;
> +    uint64_t features ATTRIBUTE_UNUSED;

This will remove the ATTRIBUTE_UNUSED

> +    uint64_t used_size ATTRIBUTE_UNUSED = 0;

This isn't needed here anymore.

>  
>      if ((r = rbd_open_read_only(ptr->ioctx, vol->name, &image, NULL)) < 0) {
>          ret = -r;
> @@ -303,15 +319,39 @@ volStorageBackendRBDRefreshVolInfo(virStorageVolDefPtr vol,
>          goto cleanup;
>      }
>  
> -    VIR_DEBUG("Refreshed RBD image %s/%s (size: %zu obj_size: %zu num_objs: %zu)",
> -              pool->def->source.name, vol->name, info.size, info.obj_size,
> -              info.num_objs);
> -

    if (volStorageBackendRBDGetFeatures(image, vol->name,
                                        &features) < 0)
        goto cleanup;

>      vol->target.capacity = info.size;
>      vol->target.allocation = info.obj_size * info.num_objs;

Since you're just changing allocation...

    if (volStorageBackendRBDUseFastDiff(features) {
        if (virStorageBackendRBDSetAllocation(vol, image, &info) < 0)
            goto cleanup;
    } else {
        vol->target.allocation = info.obj_size * info.num_objs;
    }

>      vol->type = VIR_STORAGE_VOL_NETWORK;
>      vol->target.format = VIR_STORAGE_FILE_RAW;
>  
> +#if LIBRBD_VERSION_CODE > 265
> +    if ((r = rbd_get_features(image, &features)) < 0) {

^^
A call to rbd_get_features already exists, so it doesn't seem to need to
be placed inside that #if

> +        virReportSystemError(-r, _("failed to get flags of RBD image '%s'"),
> +                             vol->name);
> +        goto cleanup;
> +    }
> +
> +    if (features & RBD_FEATURE_FAST_DIFF) {
> +        VIR_DEBUG("RBD image %s/%s has fast-diff enabled. Querying allocation",
> +                  pool->def->source.name, vol->name);
> +
> +        if ((r = rbd_diff_iterate2(image, NULL, 0, info.size, 0, 1,

^^
Calling this is already in the code with:

#if LIBRBD_VERSION_CODE > 266

but this is > 265

does it matter? see my dilemma?


Hope this makes sense...

John

> +                                   &virStorageBackendRBDRefreshVolInfoCb,
> +                                   &used_size)) < 0) {
> +            virReportSystemError(-r, _("failed to iterate RBD image '%s'"),
> +                                 vol->name);
> +            goto cleanup;
> +        }
> +
> +        vol->target.allocation = used_size;
> +    }
> +#endif
> +
> +    VIR_DEBUG("Refreshed RBD image %s/%s (capacity: %llu allocation: %llu "
> +                      "obj_size: %zu num_objs: %zu)",
> +              pool->def->source.name, vol->name, vol->target.capacity,
> +              vol->target.allocation, info.obj_size, info.num_objs);
> +
>      VIR_FREE(vol->target.path);
>      if (virAsprintf(&vol->target.path, "%s/%s",
>                      pool->def->source.name,
>