Re: [libvirt] [PATCH] storage: Fix problem with disk backend pool allocation calculation

https://bugzilla.redhat.com/show_bug.cgi?id=1224018 The disk pool recalculates the pool allocation, capacity, and available values each time through processing a newly created disk partition. However, there were two issues with doing so. The process generally is to read the partition table via virStorageBackendDiskReadPartitions and process the volume. This is called during create and refresh backend API's with the only difference being create passes a specific volume to be processed while refresh processes all volumes (passing NULL for the volume value). The first issue via virStorageBackendDiskCreateVol was that the pool's available value was cleared, so when virStorageBackendDiskMakeVol processes the (new) volume it will ignore any other partitions on the disk, thus after returning the pool allocation would only include the newly added volume. The second is while processing a partition, the adjustment of the available value depends on the type of partition. For "primary" and "logical" partitions, the available value was adjusted. However, for "extended" partitions, the available value wasn't adjusted. Since the calling function(s) storageVolCreateXML[From] will only adjust the pool available value when they determine that the backend code didn't this resulted in the available value being incorrectly adjusted by that code. If a pool refresh was executed, the "correct" value showed up. To fix the first issue, only clear the available value when we're being processed from the refresh path (vol will be NULL). To fix the second issue, increment available by 1 so that the calling function won't adjust after we're done. That could leave the appearance of a single byte being used by a pool that only has an extended partition, but that's better than having it show up as the size of the partition until someone refreshes.

The refresh path will also nick by the same value so it'll be consistent.

Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; --- src/storage/storage_backend_disk.c | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/src/storage/storage_backend_disk.c b/src/storage/storage_backend_disk.c index 6394dac..bc14aab 100644 --- a/src/storage/storage_backend_disk.c +++ b/src/storage/storage_backend_disk.c @@ -164,8 +164,18 @@ virStorageBackendDiskMakeDataVol(virStoragePoolObjPtr pool, return -1; } + /* For "data", adjust the pool's allocation by the size of the volume + * For "metadata" (aka "extended" volume), we haven't yet used the space; + * however, the calling createVol path will adjust the pool allocation + * by the volume allocation if it determines a storage backend doesn't + * adjust the pool's allocation value. So bump the allocation to avoid + * having the calling code misrepresent the value. The refresh path + * would resolve/change the value.

+ */ if (STRNEQ(groups[2], "metadata")) pool->def->allocation += vol->target.allocation; + else + pool->def->allocation++; if (vol->source.extents[0].end > pool->def->capacity) pool->def->capacity = vol->source.extents[0].end; @@ -309,7 +319,13 @@ virStorageBackendDiskReadPartitions(virStoragePoolObjPtr pool, pool->def->source.devices[0].path, NULL); - pool->def->allocation = pool->def->capacity = pool->def->available = 0; + /* The reload/restart/refresh path will recalculate everything; + * otherwise, keep allocation as is as capacity and available + * will be adjusted + */

+ if (!vol) + pool->def->allocation = 0; + pool->def->capacity = pool->def->available = 0; ret = virCommandRunNul(cmd, 6, -- 2.1.0 -- libvir-list mailing list libvir-list(a)redhat.com https://www.redhat.com/mailman/listinfo/libvir-list