[virt-tools-list] [virt-manager][PATCH ] Add basic snapshot support.

Tue Aug 6 21:27:33 UTC 2013

On 08/05/2013 06:51 PM, Cole Robinson wrote:
> On 08/02/2013 03:27 PM, Leonardo Garcia wrote:
>> From: Leonardo Garcia <lagarcia at br.ibm.com>
>>
> Thanks for the patch. Would you believe I was working on the same thing just
> last friday?
Hehe... I knew it was on Fedora 20 plans, but was not sure if anyone was 
already working on it. Anyway, the idea here is just to share our 
thoughts and our needs.
>   I'm trying to get snapshot support ready as part of my Fedora work:
>
> https://fedoraproject.org/wiki/Changes/Virt_Manager_Snapshots
>
> We had the exact same idea about how to expose it in the details view :)
That's a good start! :)
> However the rest is a bit different: I went with a view similar to how the
> virtual network, storage, and interface pages look, with a list on the left
> side and a details pane on the right. While there isn't too much interesting
> in the snapshot XML besides name and timestamp, I also want to leave open the
> option of showing a screenshot on that page.
The UI suggested in our patch is really simple. I am OK with any other 
proposals. Two things I have in mind, though: a) We can also add the 
snapshot description (if the user input any), as this is supported in 
the snapshot XML; and b) I would like to make a better representation of 
the snapshots in the TreeView, such that we can show the dependency 
chain that might occur between different snapshot images. None of these 
things are implemented in my patches, however.
>
> My work also splits the snapshot UI logic out into a snapshot.py file, similar
> to how console.py is split out. details.py is large enough as is.
Yes, agree.
>
> However mine doesn't do much of anything useful yet :) Because there's a few
> details here that take some care.
>
> - Not all hypervisors support snapshots, so we need to de-sensitize the UI
> here and set clear tooltips. Also, depending on the snapshot type, not all
> guest configurations are supported, and not all libvirt versions are
> supported. Getting this clear is important to save us from future bug reports.
Agree. One doubt that I have is how further should we check for the 
support in virt-manager. I mean, libvirt probably have all the logic on 
what is supported and what isn't supported for each hypervisor. Although 
we can call libvirt and catch any exceptions, it would be useful to do a 
pre-check before and only allow the user to do what is supported in 
libvirt. However, in this case, we would be duplicating a good quantity 
of code from what libvirt is already doing today...
>
> - There's several different types of snapshots. qcow2 internal snapshots have
> been available the longest and are certainly simplest WRT the desktop usecase.
Yes, I can say that this is my major interest. I, unfortunately, don't 
have ways to test other hypervisors, but I would be open to test other 
snapshot types supported by KVM/QEMU, and, eventually, adopt something 
different than qcow2 internal snapshots as default.
> However they have historically been frowned upon by some QEMU developers
> because they are non-live, and slow (although apparently recent qemu 1.6 is
> better). And most recent upstream qemu activity is based around external
> snapshots.
Is any of these already supported? I mean, I agree there is a lot of 
activity around external snapshots, but I was not aware anything on that 
regard is already included in a QEMU release. But I might be missing 
something here...
>   Punting the decision to the user with a checkbox in the UI or
> similar is not too friendly, since most people won't know which one they want,
> and we will have to choose a default anyways. So my thought was to only allow
> creating these new style external snapshots. We could still list any
> pre-existing internal snapshots and allow reverting to them.
Ok, I agree with this approach.
>   So there's a lot
> of variables here.
>
> I'd like to work together though, but I think it's better to start with my UI
> as base.
Ok, np.
>   I'll clean it up a bit and push it to a separate branch upstream,
> probably in a couple days, and report back here. Then please submit patches
> against that.
Ok, I'll wait for that branch to be ready.
>   When things are ready, I'll rebase things to clean up the
> history, preserving proper attribution, and merge it into the master branch.
> Sound fair?
Sure!

Best regards,

Leonardo Garcia
>
> Thanks,
> Cole
>
>
>
>> Signed-off-by: Leonardo Garcia <lagarcia at br.ibm.com>
>> Signed-off-by: Eduardo Elias Ferreira <edusf at linux.vnet.ibm.com>
>> ---
>>   ui/vmm-details.ui      |  179 ++++++++++++++++++++++++++++++++++++++++++++++++
>>   virtManager/details.py |  150 ++++++++++++++++++++++++++++++++++-------
>>   virtManager/domain.py  |   34 +++++++++
>>   virtManager/engine.py  |   33 +++++++++
>>   virtManager/error.py   |   23 ++++--
>>   5 files changed, 388 insertions(+), 31 deletions(-)
>>