Re: [PATCH 08/16] snapshot: Access snapshot def directly when needed

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

 



On 3/20/19 4:37 PM, John Ferlan wrote:
> 
> 
> On 3/20/19 1:40 AM, Eric Blake wrote:
>> An upcoming patch will rework virDomainSnapshotObjList to be generic
>> for both snapshots and checkpoints; reduce the churn by adding a new
>> accessor virDomainSnapshotObjGetDef() which returns the
>> snapshot-specific definition even when the list is rewritten to
>> operate only on a base class, then using it at sites that that are
>> specific to snapshots.
>>
>> Signed-off-by: Eric Blake <eblake@xxxxxxxxxx>
>> ---
>>  src/conf/virdomainsnapshotobj.h     |  6 +++++
>>  src/conf/snapshot_conf.c            | 41 +++++++++++++++++------------
>>  src/conf/virdomainsnapshotobjlist.c | 17 +++++++-----
>>  src/qemu/qemu_domain.c              |  6 ++---
>>  4 files changed, 43 insertions(+), 27 deletions(-)
>>
> 

>> @@ -977,8 +984,8 @@ virDomainSnapshotRedefinePrep(virDomainPtr domain,
>>          /* Drop and rebuild the parent relationship, but keep all
>>           * child relations by reusing snap. */
>>          virDomainSnapshotDropParent(other);
>> -        virDomainSnapshotDefFree(other->def);
>> -        other->def = def;
>> +        virDomainSnapshotDefFree(otherdef);
>> +        otherdef = def;
> 
> Too much substitution ;-/... leave the old code other->def = def; Or
> maybe we'll need the corresponding virDomainSnapshotObjSetDef...

Indeed. (And proves I don't have as much testsuite coverage automated as
I would like)

> 
> Reviewed-by: John Ferlan <jferlan@xxxxxxxxxx>
> 
> John
> 
> 
>>          *defptr = NULL;
>>          *snap = other;
>>      }
> 
> [...]
> 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | 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]

  Powered by Linux