]> xenbits.xensource.com Git - libvirt.git/commitdiff
storage: Resolve storage driver crash
authorJohn Ferlan <jferlan@redhat.com>
Mon, 6 Nov 2017 20:22:07 +0000 (15:22 -0500)
committerJohn Ferlan <jferlan@redhat.com>
Thu, 16 Nov 2017 16:34:26 +0000 (11:34 -0500)
Resolve a storage driver crash as a result of a long running
storageVolCreateXML when the virStorageVolPoolRefreshThread is
run as a result of when a storageVolUpload completed and ran the
virStoragePoolObjClearVols without checking if the creation
code was currently processing a buildVol after incrementing
the driver->asyncjob count.

The refreshThread will now check the pool asyncjob count before
attempting to pursue the pool refresh. Adjust the documentation
to describe the condition.

Crash from valgrind is as follows (with a bit of editing):

==21309== Invalid read of size 8
==21309==    at 0x153E47AF: storageBackendUpdateVolTargetInfo
==21309==    by 0x153E4C30: virStorageBackendUpdateVolInfo
==21309==    by 0x153E52DE: virStorageBackendVolRefreshLocal
==21309==    by 0x153DE29E: storageVolCreateXML
==21309==    by 0x562035B: virStorageVolCreateXML
==21309==    by 0x147366: remoteDispatchStorageVolCreateXML
...
==21309==  Address 0x2590a720 is 64 bytes inside a block of size 336 free'd
==21309==    at 0x4C2F2BB: free
==21309==    by 0x54CB9FA: virFree
==21309==    by 0x55BC800: virStorageVolDefFree
==21309==    by 0x55BF1D8: virStoragePoolObjClearVols
==21309==    by 0x153D967E: virStorageVolPoolRefreshThread
...
==21309==  Block was alloc'd at
==21309==    at 0x4C300A5: calloc
==21309==    by 0x54CB483: virAlloc
==21309==    by 0x55BDC1F: virStorageVolDefParseXML
==21309==    by 0x55BDC1F: virStorageVolDefParseNode
==21309==    by 0x55BE5A4: virStorageVolDefParse
==21309==    by 0x153DDFF1: storageVolCreateXML
==21309==    by 0x562035B: virStorageVolCreateXML
==21309==    by 0x147366: remoteDispatchStorageVolCreateXML
...

src/libvirt-storage.c
src/storage/storage_driver.c

index 9ab0fe28ea12b9b960c65c793ac14c9345f73b7f..e4646cb80ff97cdd07d0994ac4aec9de1e8ecf47 100644 (file)
@@ -1632,7 +1632,8 @@ virStorageVolDownload(virStorageVolPtr vol,
  * 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
+ * or not an attempt will be made to refresh the target storage pool
+ * if an asynchronous build is not running in order 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
index 2b7a299706a0f3875ef3e08e3dfc789fe7565962..d209f5afb8c60943602f081947c738a0261e1005 100644 (file)
@@ -2267,6 +2267,13 @@ virStorageVolPoolRefreshThread(void *opaque)
         goto cleanup;
     def = virStoragePoolObjGetDef(obj);
 
+    /* If some thread is building a new volume in the pool, then we cannot
+     * clear out all vols and refresh the pool. So we'll just pass. */
+    if (virStoragePoolObjGetAsyncjobs(obj) > 0) {
+        VIR_DEBUG("Asyncjob in process, cannot refresh storage pool");
+        goto cleanup;
+    }
+
     if (!(backend = virStorageBackendForType(def->type)))
         goto cleanup;