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@xxxxxxxxxx> >> --- >> 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 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
