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

[John Ferlan]

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 at redhat.com>
---
 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