[Kimchi-devel] [PATCH] <Fix problems of edit a guest contains snapshots>

[Crístian Viana]

On 20-12-2014 05:49, gouzongmei at ourfuture.cn wrote:
> From: Zongmei Gou <gouzongmei at ourfuture.cn>
>
> ---
>   src/kimchi/control/base.py      |   13 ++++++++++---
>   src/kimchi/model/vms.py         |   24 ++++++++++++++++++------
>   src/kimchi/model/vmsnapshots.py |    8 ++++++--
>   src/kimchi/xmlutils/utils.py    |    5 +++++
>   4 files changed, 39 insertions(+), 11 deletions(-)

Thanks for the patch! It looks very nice written.

However, there are a few details which need to be fixed regarding 
snapshots (e.g. the order the snapshots need to be recreated because of 
parent/child relationship, setting back the current snapshot, among 
other things). Also, we need to make sure the commands "make 
check-local" (ensure formatting guidelines) and "make check" (run the 
local tests) complete successfully before applying any new patch. There 
are a few formatting issues with this patch which need to be updated, 
and it's always good to add a test case which exploits the problem that 
the patch fixes, so we don't hit it again in the future.

As I mentioned in a previous e-mail, I already have a patch which does 
the snapshot handling but fails when reverting to a snapshot which was 
created when the VM had a different name, due to the URL redirection 
approach. AFAIK, that was the only problem with it and I didn't have to 
look deeper to solve it. And your patch seems to fix that correctly (at 
src/kimchi/control/base.py). Please, take a look at my patch at 
https://github.com/cd1/kimchi/commit/20510e607e5f08ff6344fae3fbb0d84f324d16ce 
and let me know what you think about it. I haven't sent it yet to this 
mailing list because it's not completely finished, as I said.

Do you think we can fix this issue by submitting our patches separately 
(i.e. you send the changes to src/kimchi/control/base.py and I send the 
snapshot changes) or even by merging our patches together? It seems 
we've already fixed this issue but in separate patches.