Re: [libvirt] [PATCH 5/5] snapshot: Add tests of virsh -c test:///default snapshot*
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(a)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