storage: Fix regression cloning volume into a logical pool

https://bugzilla.redhat.com/show_bug.cgi?id=1318993

Commit id 'dd519a294' caused a regression cloning a volume into a
logical pool by removing just the 'allocation' adjustment during
storageVolCreateXMLFrom. Combined with the change to not require the
new volume input XML to have a capacity listed (commit id 'e3f1d2a8')
left the possibility that a zero allocation value (e.g., not provided)
would create a thin/sparse logical volume. When a thin lv becomes fully
populated, then LVM sets the partition 'inactive' and the subsequent
fdatasync() fails.

Add a new 'has_allocation' flag to be set at XML parse time to indicate
that allocation was provided. This is done so that if it's not provided
the create-from code uses the capacity value since we document that if
omitted, the volume will be fully allocated at time of creation.

For a logical backend, that creation time is 'createVol', while for a
file backend, creation doesn't set the size, but the 'createRaw' called
during buildVolFrom will decide whether the file is sparse or not based
on the provided capacity and allocation value.

For volume clones that provide different allocation and capacity values
to allow for sparse files, there is no change.

diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index 0b91956c4ae2519b4b00b0e6976b30ee3550d0e8..69321952f09aa08e8a21c94c45ca9a24b505339a 100644 (file)
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -1370,6 +1370,7 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool,
         unit = virXPathString("string(./allocation/@unit)", ctxt);
         if (virStorageSize(unit, allocation, &ret->target.allocation) < 0)
             goto error;
+        ret->target.has_allocation = true;
     } else {
         ret->target.allocation = ret->target.capacity;
     }

diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index 1d42f24618179405be799a7652fd71986c2d6e9b..ae6a511a132d1930afd81025f448de3149d33f2f 100644 (file)
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -2039,6 +2039,13 @@ storageVolCreateXMLFrom(virStoragePoolPtr obj,
     if (newvol->target.capacity < origvol->target.capacity)
         newvol->target.capacity = origvol->target.capacity;
 
+    /* If the allocation was not provided in the XML, then use capacity
+     * as it's specifically documented "If omitted when creating a volume,
+     * the  volume will be fully allocated at time of creation.". This
+     * is especially important for logical volume creation. */
+    if (!newvol->target.has_allocation)
+        newvol->target.allocation = newvol->target.capacity;
+
     if (!backend->buildVolFrom) {
         virReportError(VIR_ERR_NO_SUPPORT,
                        "%s", _("storage pool does not support"

diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index 4f44e05de5d2bc5514f8dbab0d0819d83f0a64fe..d4e61ca6aad3274476a45febd0369af6b78b1833 100644 (file)
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -1835,6 +1835,7 @@ virStorageSourceCopy(const virStorageSource *src,
     ret->format = src->format;
     ret->capacity = src->capacity;
     ret->allocation = src->allocation;
+    ret->has_allocation = src->has_allocation;
     ret->physical = src->physical;
     ret->readonly = src->readonly;
     ret->shared = src->shared;

diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h
index 17e12779c119de56c279e92eaad5b6e130205083..b88e71564a23df9ef1e75d7b4cd6b6d9dc5f85b7 100644 (file)
--- a/src/util/virstoragefile.h
+++ b/src/util/virstoragefile.h
@@ -260,6 +260,8 @@ struct _virStorageSource {
     unsigned long long capacity; /* in bytes, 0 if unknown */
     unsigned long long allocation; /* in bytes, 0 if unknown */
     unsigned long long physical; /* in bytes, 0 if unknown */
+    bool has_allocation; /* Set to true when provided in XML */
+
     size_t nseclabels;
     virSecurityDeviceLabelDefPtr *seclabels;