[Kimchi-devel] [PATCH] UI: Guest Snapshot

[Crístian Viana]

On 18-11-2014 09:17, Yu Xin Huo wrote:
>> - When I open the dialog "Edit" on a VM without snapshots, the error 
>> KCHSNAP0007E* is shown. That's because the VM doesn't have snapshots 
>> but the UI should handle that and do nothing. Not having snapshots is 
>> not an error from the UI perspective.
> Let us discuss why it returns a 404, anyway, the uri has nothing 
> wrong, to me, 404 means I am requesting a wrong thing.

Actually, 404 doesn't mean there's something wrong with the URL, it 
means the resource on the requested URL could not be found, which, in my 
opinion, is the case here. But I discussed this also with Aline and she 
agrees that the request should return an empty dict in when there's no 
current snapshot, so I'll send a patch to change this.

> why sometimes it returns 500.
>

Can you paste the stack trace of this error as well? I have no idea of 
what might have happened just by looking at the code 500, that's a 
generic status code. Only then I can understand and fix this issue.

>> - I cannot create a snapshot at all. Whenever I click on the button 
>> '+', the error "KCHAPI0006E: Unable to parse JSON request" is shown.
> Can you make the backend code more robust, I have to submit an empty 
> json below. If submit nothing, then error above.

OK, I'll change the backend so it can receive nothing as input.

>>
>> - I cannot revert to the current snapshot. The button "Revert" 
>> becomes disabled whenever the current snapshot is selected. And 
>> that's a very important use case. Suppose I just created a snapshot. 
>> Then I mistakenly removed some files on my guest system and I want to 
>> revert to that snapshot. With the current UI, I'm not able to do that.
> Corrected since it can revert to current snapshot. By this way, radio 
> button add no value, so change back to original UI to make revert an 
> item level action.

Good.

>>
>> - There's no visual feedback when I click on the button "Revert". 
>> When I click on it and the revert operation starts, nothing happens. 
>> The user might click on it again thinking that the button wasn't 
>> clicked on the first time. Also there's no way to know if the revert 
>> operation finished.

Good. But I guess the loading Kimchi icon won't appear again if the user 
closes the dialog and reopens it. That should be easier to implement if 
the revert operation returns a task, which is not the case now - but 
will be in the future. So this looks good for now.
>>
>> - If the user clicks on the button "Revert", closes the dialog "Edit" 
>> and reopens it while the revert operation is still happening, the 
>> error KCHSNAP0007E* is shown and the tab "Snapshot" is rendered 
>> empty, even if there are snapshots to be listed on the VM. The same 
>> problem also happens when the user clicks on the button with the 
>> trash can, instead of "Revert", and closes the dialog and reopens it 
>> while the operation hasn't finished.
> Seems like 'Revert', 'Create snapshot', 'delete', 'list snapshots' are 
> all exclusive, disable all buttons when any action is ongoing, and 
> only list existing snapshots when no snapshot creation is ongoing.
> Revert is not a long task, it should response within 5 secs, then user 
> should wait there for it to complete, or else make it a long task also.

Revert can be a very long task, depending on the type of snapshot and 
the VM memory size. But as Kimchi only supports snapshot with shutoff 
VMs and those snapshots are usually fast to be created and reverted, we 
can keep the non-task operation. But that should change soon.

>> - Why isn't there a button "Revert" on each snapshot line? The button 
>> with the trash can is there, the button "Revert" should also be 
>> there. That's an action that applies to only one snapshot at a time, 
>> just like delete. Other actions in other tabs, like "Edit", are also 
>> shown on the relevant line instead of at the top of the dialog. The 
>> button "+" is different, though, because it doesn't act in one entry 
>> alone, it acts on the whole set of items. So that makes sense for me 
>> to be at the top.
> Changed it back as it can revert back to current snapshot and radio 
> button has no value here.

Good.

> Please correct all those 500 error above and below
>

I'll look into this issue and fix it.

>
>
I don't know what happened there. Please paste the stack trace so I can 
identify and fix the issue.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.ovirt.org/pipermail/kimchi-devel/attachments/20141118/96e5704c/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: image/png
Size: 6341 bytes
Desc: not available
URL: <http://lists.ovirt.org/pipermail/kimchi-devel/attachments/20141118/96e5704c/attachment.png>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: image/png
Size: 4328 bytes
Desc: not available
URL: <http://lists.ovirt.org/pipermail/kimchi-devel/attachments/20141118/96e5704c/attachment-0001.png>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: image/png
Size: 2735 bytes
Desc: not available
URL: <http://lists.ovirt.org/pipermail/kimchi-devel/attachments/20141118/96e5704c/attachment-0002.png>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: image/png
Size: 12947 bytes
Desc: not available
URL: <http://lists.ovirt.org/pipermail/kimchi-devel/attachments/20141118/96e5704c/attachment-0003.png>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: image/png
Size: 8213 bytes
Desc: not available
URL: <http://lists.ovirt.org/pipermail/kimchi-devel/attachments/20141118/96e5704c/attachment-0004.png>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: image/png
Size: 6595 bytes
Desc: not available
URL: <http://lists.ovirt.org/pipermail/kimchi-devel/attachments/20141118/96e5704c/attachment-0005.png>