Re: [Kimchi-devel] [PATCH] Re: Solve the snapshot revert problem, relate to issue #526

Reviewed-by: Crístian Viana <vianac(a)linux.vnet.ibm.com&gt; A few comments below: On 20-01-2015 10:10, gouzongmei(a)ourfuture.cn wrote: > From: Zongmei Gou <gouzongmei(a)ourfuture.cn&gt; > > Hi, Cristian > > Thanks for your time on this, I've modified my patch according to > your opinion, > some problems may still exist. > > 1. The 'revert' function need return vm name recorded in the snapshot > xml, not the snapshot > name, so the "vir_snap.getName()" won't help. You're right, we need the VM name there, not the snapshot name. So I guess you could call "vir_dom.name()" instead of "vir_snap.getXMLDesc() + xpath_get_text". I'm expecting that libvirt will return the updated value when it changes. But you don't need to send another revision of this patch just because of that, it already works as it is. > 4. For "test_rest", the snapshot revert function in mockmodel.py is > different with the real model, > which causes the response status different. So I just kept it, and > all related test cases passed > by now in my environment. Now I see that we cannot create that specific test scenario in our mockmodel, given its current implementation. Our MockSnapshot is too simple, it isn't aware of the original VM's name (and many more things), so unless we change our class MockSnapshot - which I don't think it's worth it now - we cannot reproduce that issue. So we can keep it as it is now, as you proposed.

_______________________________________________ Kimchi-devel mailing list Kimchi-devel(a)ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel