On Tuesday 30 June 2015 06:20 PM, Ján Tomko wrote:
> 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
Hi Jan,
Thanks.
Could you also take a look at the second patch in this series ?
Regards,
--
Prerna Saxena
Linux Technology Centre,
IBM Systems and Technology Lab,
Bangalore, India
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list