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@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,