Re: [PATCH v2 7/8] blockcopy: add qemu implementation of new API

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

 



On 08/26/2014 09:46 AM, Peter Krempa wrote:
> On 08/26/14 13:21, Eric Blake wrote:
>> The hard part of managing the disk copy is already coded; all
>> this had to do was convert the XML and virTypedParameters into
>> the internal representation.
>>
>> With this patch, all blockcopy operations that used the old
>> API should also work via the new API.  Additional extensions,
>> such as supporting the granularity tunable or a network rather
>> than file destination, will be added as later patches.
>>

>> +    ret = qemuDomainBlockCopyCommon(&vm, dom->conn, disk, dest,
>> +                                    bandwidth, flags);
> 
> I don't think you need to do the dance with passing vm as an pointer and
> having it returned. Insted you can unlock/free it in
> qemuDomainBlockCopyCommon.
> 
> This will also clean the messy part in 5/8.

But it was the fact that there was transfer semantics that confused me.
 qemuDomainBlockCopyCommon cannot return early if it MUST free vm on all
exit paths, and I found it easier to reason about the code if the
function that allocated vm also freed it.  I'll think about whether to
change anything for v3.

> 
>> +
>> + cleanup:
>> +    virStorageSourceFree(dest);
>> +    if (vm)
>> +        virObjectUnlock(vm);
>> +    return ret;
>> +}
>> +
>> +
>>  static int
>>
> 
> ACK
> 

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP 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]