On 07/28/14 15:24, John Ferlan wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1072653
Upon successful upload of a volume, the target volume and storage pool
were not updated to reflect any changes as a result of the upload. Make
use of the existing stream close callback mechanism to force a backend
pool refresh to occur once the stream closes.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/libvirt.c | 8 +++++
src/libvirt_private.syms | 1 +
src/storage/storage_driver.c | 78 ++++++++++++++++++++++++++++++++++++++++++++
tools/virsh.pod | 3 ++
4 files changed, 90 insertions(+)
diff --git a/src/libvirt.c b/src/libvirt.c
index 143d319..992e4f2 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -13960,6 +13960,14 @@ virStorageVolDownload(virStorageVolPtr vol,
* detect any errors. The results will be unpredictable if
* another active stream is writing to the storage volume.
*
+ * When the data stream is closed whether the upload is successful
+ * or not the target storage pool will be refreshed to reflect pool
+ * and volume changes as a result of the upload. Depending on
+ * the target volume storage backend and the source stream type
+ * for a successful upload, the target volume may take on the
+ * characteristics from the source stream such as format type,
+ * capacity, and allocation.
+ *
* Returns 0, or -1 upon error.
*/
int
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index b1fb7c9..7ef68a1 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -825,6 +825,7 @@ virFDStreamCreateFile;
virFDStreamOpen;
virFDStreamOpenFile;
virFDStreamOpenPTY;
+virFDStreamSetInternalCloseCb;
# libvirt_internal.h
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index efbe5ff..5fd5514 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -59,6 +59,12 @@ static virStorageDriverStatePtr driverState;
static int storageStateCleanup(void);
+typedef struct _virStorageVolStreamInfo virStorageVolStreamInfo;
+typedef virStorageVolStreamInfo *virStorageVolStreamInfoPtr;
+struct _virStorageVolStreamInfo {
+ char *pool_name;
+};
+
static void storageDriverLock(virStorageDriverStatePtr driver)
{
virMutexLock(&driver->lock);
@@ -1956,6 +1962,52 @@ storageVolDownload(virStorageVolPtr obj,
}
+/**
+ * Frees opaque data provided for the stream closing callback
+ *
+ * @opaque Data to be freed.
+ */
+static void virStorageVolFDStreamCloseCbFree(void *opaque)
+{
+ virStorageVolStreamInfoPtr cbdata = opaque;
+
+ VIR_FREE(cbdata->pool_name);
+ VIR_FREE(cbdata);
+}
+
+/**
+ * Callback being called if a FDstream is closed. Frees device entries
+ * from data structures and removes lockfiles.
+ *
+ * @st Pointer to stream being closed.
+ * @opaque Domain's device information structure.
+ */
+static void virStorageVolFDStreamCloseCb(virStreamPtr st ATTRIBUTE_UNUSED,
+ void *opaque)
+{
+
+ virStorageVolStreamInfoPtr cbdata = opaque;
+ virStoragePoolObjPtr pool = NULL;
+ virStorageBackendPtr backend;
+
+ storageDriverLock(driverState);
I'm a bit concerned about locking the storage driver here. This code
will be called by the stream close callback which is handled from the
event loop. While I don't currently see any problem, this instance could
bite us in the future.
+ if (!(pool =
virStoragePoolObjFindByName(&driverState->pools,
+ cbdata->pool_name)))
+ goto cleanup;
+
+ if (!(backend = virStorageBackendForType(pool->def->type)))
+ goto cleanup;
+
+ virStoragePoolObjClearVols(pool);
Hmmm, having a single volume update func would be nice here.
+ if (backend->refreshPool(NULL, pool) < 0)
+ VIR_DEBUG("Failed to refresh storage pool");
+
+ cleanup:
+ if (pool)
+ virStoragePoolObjUnlock(pool);
+ storageDriverUnlock(driverState);
+}
+
static int
storageVolUpload(virStorageVolPtr obj,
virStreamPtr stream,
ACK, but please push this after the release so that we can give it a
cycle of testing before releasing it.
Peter