<div><font size="3">Hi,Crístian<br><br>I've read your patch, which has solved the snapshots restore problems more clearly. I agree with you that we've already fixed this issue in separate patches, and I need to make some changes for the formatting issues and test case.<br><br>My patch will be responsible for the url redirect problem, which modifies base.py and function 'revert' in vmsnapshots.py.<br><br>Besides, I found there are two points need to pay attention to:<br><br>1. <span class="pl-en">snap.listAllChildren() in _backup_snapshots.py </span>need flag in older libvirt versions, as I used "0.10.2", it raised "listAllChildren() takes exactly 2 arguments (1 given)" <br><br>2. UI transfer param 'name' no matter you update vm name or not (of course while you edit a vm), so do we only need to bakup and restore the snapshots while vm name is really updated ? </font><br><br><font size="3"><span lang="EN-US">Best regards,<br>Zongmei Gou<br></span></font><br><quark_editor_sign></quark_editor_sign><quark_editor_card></quark_editor_card>-------- Original message --------<br><b>From: </b>Cr�stian Viana <vianac@linux.vnet.ibm.com><br><b>Date: </b>2015-01-07 02:46<br><b>To: </b>gouzongmei@ourfuture.cn;Kimchi Devel <kimchi-devel@ovirt.org><br><b>Subject: </b>Re:Re: [Kimchi-devel] [PATCH] <fix problems="" of="" edit="" a="" guest="" containssnapshots=""><br><br>
<br>On 20-12-2014 05:49, gouzongmei@ourfuture.cn wrote:<br>> From: Zongmei Gou <gouzongmei@ourfuture.cn><br>><br>> ---<br>> src/kimchi/control/base.py | 13 ++++++++++---<br>> src/kimchi/model/vms.py | 24 ++++++++++++++++++------<br>> src/kimchi/model/vmsnapshots.py | 8 ++++++--<br>> src/kimchi/xmlutils/utils.py | 5 +++++<br>> 4 files changed, 39 insertions(+), 11 deletions(-)<br><br>Thanks for the patch! It looks very nice written.<br><br>However, there are a few details which need to be fixed regarding <br>snapshots (e.g. the order the snapshots need to be recreated because of <br>parent/child relationship, setting back the current snapshot, among <br>other things). Also, we need to make sure the commands "make <br>check-local" (ensure formatting guidelines) and "make check" (run the <br>local tests) complete successfully before applying any new patch. There <br>are a few formatting issues with this patch which need to be updated, <br>and it's always good to add a test case which exploits the problem that <br>the patch fixes, so we don't hit it again in the future.<br><br>As I mentioned in a previous e-mail, I already have a patch which does <br>the snapshot handling but fails when reverting to a snapshot which was <br>created when the VM had a different name, due to the URL redirection <br>approach. AFAIK, that was the only problem with it and I didn't have to <br>look deeper to solve it. And your patch seems to fix that correctly (at <br>src/kimchi/control/base.py). Please, take a look at my patch at <br>https://github.com/cd1/kimchi/commit/20510e607e5f08ff6344fae3fbb0d84f324d16ce <br>and let me know what you think about it. I haven't sent it yet to this <br>mailing list because it's not completely finished, as I said.<br><br>Do you think we can fix this issue by submitting our patches separately <br>(i.e. you send the changes to src/kimchi/control/base.py and I send the <br>snapshot changes) or even by merging our patches together? It seems <br>we've already fixed this issue but in separate patches.<br><br>
</fix></div>