From: Jason Dillaman <dillaman@redhat.com>
For pools with numerous volumes or very large volumes, the disk-usage
calculation can take a non-trivial amount of time even with the
fast-diff feature enabled (especially since the the usage is calculated
serially for each image in the pool).
The "rbd:config_opts" node now supports a new libvirt-internal
"libvirt_calculate_disk_usage" option which can be set to false to
disable this calculation as needed.
Signed-off-by: Jason Dillaman <dillaman@redhat.com>
---
docs/formatstorage.html.in | 13 ++++++++--
src/storage/storage_backend_rbd.c | 41 ++++++++++++++++++++++++++++---
2 files changed, 48 insertions(+), 6 deletions(-)
diff --git a/docs/formatstorage.html.in b/docs/formatstorage.html.in
index 968651330f..eb65edc4d4 100644
--- a/docs/formatstorage.html.in
+++ b/docs/formatstorage.html.in
@@ -613,12 +613,21 @@
<rbd:option name='client_mount_timeout' value='45'/>
<rbd:option name='rados_mon_op_timeout' value='20'/>
<rbd:option name='rados_osd_op_timeout' value='10'/>
+ <rbd:option name='libvirt_calculate_disk_usage' value='false'/>
</rbd:config_opts>
</pool>
</pre>
- <span class="since">Since 5.1.0.</span></dd>
-
+ <span class="since">Since 5.1.0.</span>
+ <dl>
+ <dt><code>libvirt_calculate_disk_usage</code></dt>
+ <dd>Disable calculating the actual disk usage of RBD volumes when
+ set to <code>false</code>. This will speed up pool and volume
+ refresh operations for pools with many volumes or pools with
+ large images.
+ </dd>
+ </dl>
+ </dd>
</dl>
<h2><a id="StorageVol">Storage volume XML</a></h2>
diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backend_rbd.c
index e67911f928..07ab72f97c 100644
--- a/src/storage/storage_backend_rbd.c
+++ b/src/storage/storage_backend_rbd.c
@@ -61,6 +61,9 @@ struct _virStoragePoolRBDConfigOptionsDef {
#define STORAGE_POOL_RBD_NAMESPACE_HREF "http://libvirt.org/schemas/storagepool/rbd/1.0"
+#define CONFIG_OPTION_INTERNAL_PREFIX "libvirt_"
+#define CONFIG_OPTION_CALCULATE_DISK_USAGE CONFIG_OPTION_INTERNAL_PREFIX "calculate_disk_usage"
+
static void
virStoragePoolDefRBDNamespaceFree(void *nsdata)
{
@@ -321,6 +324,9 @@ virStorageBackendRBDOpenRADOSConn(virStorageBackendRBDStatePtr ptr,
char uuidstr[VIR_UUID_STRING_BUFLEN];
for (i = 0; i < cmdopts->noptions; i++) {
+ if (STRPREFIX(cmdopts->names[i], CONFIG_OPTION_INTERNAL_PREFIX))
+ continue;
+
if (virStorageBackendRBDRADOSConfSet(ptr->cluster,
cmdopts->names[i],
cmdopts->values[i]) < 0)
@@ -527,10 +533,30 @@ virStorageBackendRBDSetAllocation(virStorageVolDefPtr vol ATTRIBUTE_UNUSED,
}
#endif
+static bool
+virStorageBackendRBDCalculateDiskUsage(virStoragePoolDefPtr def)
+{
+ size_t i;
+ bool calculate_disk_usage = true;
+
+ if (def->namespaceData) {
+ virStoragePoolRBDConfigOptionsDefPtr cmdopts = def->namespaceData;
+
+ for (i = 0; i < cmdopts->noptions; i++) {
+ if (STREQ(cmdopts->names[i], CONFIG_OPTION_CALCULATE_DISK_USAGE)) {
+ calculate_disk_usage = STRCASEEQ(cmdopts->values[i], "true");
+ break;
+ }
+ }
+ }
+ return calculate_disk_usage;
+}
+
static int
volStorageBackendRBDRefreshVolInfo(virStorageVolDefPtr vol,
virStoragePoolObjPtr pool,
- virStorageBackendRBDStatePtr ptr)
+ virStorageBackendRBDStatePtr ptr,
+ bool calculate_disk_usage)
{
int ret = -1;
virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool);
@@ -564,7 +590,8 @@ volStorageBackendRBDRefreshVolInfo(virStorageVolDefPtr vol,
vol->type = VIR_STORAGE_VOL_NETWORK;
vol->target.format = VIR_STORAGE_FILE_RAW;
- if (volStorageBackendRBDUseFastDiff(features, flags)) {
+ if (calculate_disk_usage &&
+ volStorageBackendRBDUseFastDiff(features, flags)) {
VIR_DEBUG("RBD image %s/%s has fast-diff feature enabled. "
"Querying for actual allocation",
def->source.name, vol->name);
@@ -610,8 +637,10 @@ virStorageBackendRBDRefreshPool(virStoragePoolObjPtr pool)
virStorageBackendRBDStatePtr ptr = NULL;
struct rados_cluster_stat_t clusterstat;
struct rados_pool_stat_t poolstat;
+ bool calculate_disk_usage = virStorageBackendRBDCalculateDiskUsage(def);
VIR_AUTOFREE(char *) names = NULL;
+
if (!(ptr = virStorageBackendRBDNewState(pool)))
goto cleanup;
@@ -663,7 +692,8 @@ virStorageBackendRBDRefreshPool(virStoragePoolObjPtr pool)
name += strlen(name) + 1;
- r = volStorageBackendRBDRefreshVolInfo(vol, pool, ptr);
+ r = volStorageBackendRBDRefreshVolInfo(vol, pool, ptr,
+ calculate_disk_usage);
/* It could be that a volume has been deleted through a different route
* then libvirt and that will cause a -ENOENT to be returned.
@@ -1238,12 +1268,15 @@ virStorageBackendRBDRefreshVol(virStoragePoolObjPtr pool,
virStorageVolDefPtr vol)
{
virStorageBackendRBDStatePtr ptr = NULL;
+ virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool);
+ bool calculate_disk_usage = virStorageBackendRBDCalculateDiskUsage(def);
int ret = -1;
if (!(ptr = virStorageBackendRBDNewState(pool)))
goto cleanup;
- if (volStorageBackendRBDRefreshVolInfo(vol, pool, ptr) < 0)
+ if (volStorageBackendRBDRefreshVolInfo(vol, pool, ptr,
+ calculate_disk_usage) < 0)
goto cleanup;
ret = 0;
--
2.20.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Mon, Mar 11, 2019 at 11:39:55AM -0400, jdillama@redhat.com wrote: > From: Jason Dillaman <dillaman@redhat.com> > > For pools with numerous volumes or very large volumes, the disk-usage > calculation can take a non-trivial amount of time even with the > fast-diff feature enabled (especially since the the usage is calculated > serially for each image in the pool). > > The "rbd:config_opts" node now supports a new libvirt-internal > "libvirt_calculate_disk_usage" option which can be set to false to > disable this calculation as needed. The RBD namespace is not something we want to encourage applications to use. It is supposed to be for hacks that are only used in testing or QA, never in production. Essentially any usage is considered unsupported for production. This scalability problem sounds like something most likely to hit in production envs, and as such we need a fully supportable solution. This probably suggests making it a core part of the storage pool schema, and extending its impl to other drivers too. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Mon, Mar 11, 2019 at 11:50 AM Daniel P. Berrangé <berrange@redhat.com> wrote: > > On Mon, Mar 11, 2019 at 11:39:55AM -0400, jdillama@redhat.com wrote: > > From: Jason Dillaman <dillaman@redhat.com> > > > > For pools with numerous volumes or very large volumes, the disk-usage > > calculation can take a non-trivial amount of time even with the > > fast-diff feature enabled (especially since the the usage is calculated > > serially for each image in the pool). > > > > The "rbd:config_opts" node now supports a new libvirt-internal > > "libvirt_calculate_disk_usage" option which can be set to false to > > disable this calculation as needed. > > The RBD namespace is not something we want to encourage applications > to use. It is supposed to be for hacks that are only used in testing > or QA, never in production. Essentially any usage is considered > unsupported for production. Gotcha. Honestly, in that case, since starting w/ the Mimic release of Ceph all configuration options can be stored in the Ceph monitors and starting with the Nautilus release of Ceph all RBD configuration options can be overridden in the pools and individual images, I actually wonder the value of this XML section even for non-production settings. Also, should the documentation be updated to indicate that this is only for non-production environments (assuming support for this isn't just yanked)? > This scalability problem sounds like something most likely to hit in > production envs, and as such we need a fully supportable solution. > This probably suggests making it a core part of the storage pool > schema, and extending its impl to other drivers too. >From a quick scan, it only looked like RBD was using a costly "stat"-like method to calculate actual volume usage which is why I implemented it as an RBD-only feature. If a new "volume_usage=[physical|allocated]"-like option is acceptable in the root storage pool XML, I can add it and repost. > Regards, > Daniel > -- > |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| > |: https://libvirt.org -o- https://fstop138.berrange.com :| > |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :| Thanks, -- Jason -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2024 Red Hat, Inc.