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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxxxxxxxxxx>
> ---
>  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

Attachment: signature.asc
Description: Digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]