Re: [libvirt] [PATCH v3 1/2] Storage: Introduce shadow vol for refresh while the main vol builds.

On Fri, Jun 26, 2015 at 05:05:11PM +0530, Prerna Saxena wrote: > Libvirt periodically refreshes all volumes in a storage pool, including > the volumes being cloned. > While cloning a storage volume from parent, we drop pool locks. Subsequent > volume refresh sometimes changes allocation for an ongoing copy, and leads > to corrupt images. > Fix: Introduce a shadow volume that isolates the volume object under refresh > from the base which has a copy ongoing. > > Signed-off-by: Prerna Saxena <prerna(a)linux.vnet.ibm.com&gt; > --- > src/storage/storage_driver.c | 14 ++++++++++++-- > 1 file changed, 12 insertions(+), 2 deletions(-) > ACK > diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c > index 57060ab..41de8df 100644 > --- a/src/storage/storage_driver.c > +++ b/src/storage/storage_driver.c > @@ -1898,7 +1898,7 @@ storageVolCreateXMLFrom(virStoragePoolPtr obj, > { > virStoragePoolObjPtr pool, origpool = NULL; > virStorageBackendPtr backend; > - virStorageVolDefPtr origvol = NULL, newvol = NULL; > + virStorageVolDefPtr origvol = NULL, newvol = NULL, shadowvol = NULL; > virStorageVolPtr ret = NULL, volobj = NULL; > unsigned long long allocation; > int buildret; > @@ -2010,6 +2010,15 @@ storageVolCreateXMLFrom(virStoragePoolPtr obj, > if (backend->createVol(obj->conn, pool, newvol) < 0) > goto cleanup; > > + /* Make a shallow copy of the 'defined' volume definition, since the > + * original allocation value will change as the user polls 'info', > + * but we only need the initial requested values > + */ > + if (VIR_ALLOC(shadowvol) < 0) > + goto cleanup; > + > + memcpy(shadowvol, newvol, sizeof(*newvol)); > + > pool->volumes.objs[pool->volumes.count++] = newvol; > volobj = virGetStorageVol(obj->conn, pool->def->name, newvol->name, > newvol->key, NULL, NULL); > @@ -2029,7 +2038,7 @@ storageVolCreateXMLFrom(virStoragePoolPtr obj, > virStoragePoolObjUnlock(origpool); > } > > - buildret = backend->buildVolFrom(obj->conn, pool, newvol, origvol, flags); > + buildret = backend->buildVolFrom(obj->conn, pool, shadowvol, origvol, flags); > newvol->target.allocation should also use the value from shadowvol. I have made the adjustment and pushed the patch. Jan