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@xxxxxxxxx>
> ---
> 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,
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list