On 4/7/19 6:16 AM, Roman Bogorodskiy wrote:
Eric Blake wrote:
Had this been in place earlier, I would have avoided the bugs in commit 0baf6945 and 55c2ab3e. Writing the test required me to extend the power of virsh - creating enough snapshots to cause fanout requires enough input in a single session that adding comments and markers makes it easier to check that output is correct. It's still a bit odd that with test:///default, reverting to a snapshot changes the domain from running to paused (possibly a bug in how the test driver copied from the qemu driver) - but the important part is that the test is reproducible, and any future tweaks we make to snapshot code have less chance of breaking successful command sequences.
Signed-off-by: Eric Blake <eblake@redhat.com> --- tests/Makefile.am | 3 +- tests/virsh-snapshot | 212 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 214 insertions(+), 1 deletion(-) create mode 100755 tests/virsh-snapshot
Hi,
I noticed this test is failing for me:
Shoot, running under valgrind shows the same problem: +==54014== Invalid read of size 8 +==54014== at 0x540C802: virDomainMomentForEachChild (virdomainmomentobjlist.c:59) +==54014== by 0x540C914: virDomainMomentForEachDescendant (virdomainmomentobjlist.c:105) +==54014== by 0x540C8AB: virDomainMomentActOnDescendant (virdomainmomentobjlist.c:85) +==54014== by 0x540C832: virDomainMomentForEachChild (virdomainmomentobjlist.c:63) +==54014== by 0x540C914: virDomainMomentForEachDescendant (virdomainmomentobjlist.c:105) +==54014== by 0x54C7242: testDomainSnapshotDelete (test_driver.c:6506) +==54014== by 0x55D29BE: virDomainSnapshotDelete (libvirt-domain-snapshot.c:1047) +==54014== by 0x177CBE: cmdSnapshotDelete (virsh-snapshot.c:1921) +==54014== by 0x17EC2F: vshCommandRun (vsh.c:1335) +==54014== by 0x1347F6: main (virsh.c:920) +==54014== Address 0x102e1000 is 32 bytes inside a block of size 40 free'd +==54014== at 0x4C3013B: free (vg_replace_malloc.c:530) +==54014== by 0x52D0CB8: virFree (viralloc.c:581) +==54014== by 0x540CC62: virDomainMomentObjFree (virdomainmomentobjlist.c:210) +==54014== by 0x540CD94: virDomainMomentObjListDataFree (virdomainmomentobjlist.c:247) +==54014== by 0x53142CB: virHashRemoveEntry (virhash.c:533) +==54014== by 0x540D2E6: virDomainMomentObjListRemove (virdomainmomentobjlist.c:437) +==54014== by 0x540D95A: virDomainSnapshotObjListRemove (virdomainsnapshotobjlist.c:204) +==54014== by 0x54C702F: testDomainSnapshotDiscardAll (test_driver.c:6449) +==54014== by 0x540C88C: virDomainMomentActOnDescendant (virdomainmomentobjlist.c:84) +==54014== by 0x540C832: virDomainMomentForEachChild (virdomainmomentobjlist.c:63) +==54014== by 0x540C914: virDomainMomentForEachDescendant (virdomainmomentobjlist.c:105) +==54014== by 0x54C7242: testDomainSnapshotDelete (test_driver.c:6506) +==54014== Block was alloc'd at +==54014== at 0x4C30F26: calloc (vg_replace_malloc.c:711) +==54014== by 0x52D0483: virAlloc (viralloc.c:143) +==54014== by 0x540CB84: virDomainMomentObjNew (virdomainmomentobjlist.c:191) +==54014== by 0x540CCFF: virDomainMomentAssignDef (virdomainmomentobjlist.c:228) +==54014== by 0x540D577: virDomainSnapshotAssignDef (virdomainsnapshotobjlist.c:46) +==54014== by 0x54C6E3F: testDomainSnapshotCreateXML (test_driver.c:6396) +==54014== by 0x55D0952: virDomainSnapshotCreateXML (libvirt-domain-snapshot.c:241) +==54014== by 0x173680: virshSnapshotCreate (virsh-snapshot.c:51) +==54014== by 0x1743BC: cmdSnapshotCreateAs (virsh-snapshot.c:437) +==54014== by 0x17EC2F: vshCommandRun (vsh.c:1335) +==54014== by 0x1347F6: main (virsh.c:920) Looks like the problem is that while iterating over list we remove some elements and then call (some different) iterator over them. Eric? Michal