From 6043a88a0cccd95e029947a31a2362dc2f3c3557 Mon Sep 17 00:00:00 2001 From: Cole Robinson Date: Mon, 30 Sep 2013 17:53:55 -0400 Subject: [PATCH] snapshots: Add some specific UI for external snapshots We sort them separately in the snapshot list, explicitly mention that they are 'external', and add a UI field listing the memory/disk details. In general mixing internal and external snapshots is a recipe for confusion and disaster, so I think the best thing to do is at least acknowledge their presence in the UI but not make any attempt to predict what will or will not work. --- tests/testdriver.xml | 76 +++++++++++++++++++++- tests/xmlparse-xml/change-snapshot-out.xml | 4 +- tests/xmlparse.py | 5 ++ ui/snapshots.ui | 34 +++++++++- virtManager/domain.py | 10 ++- virtManager/snapshots.py | 52 ++++++++++++--- virtinst/snapshot.py | 16 +++-- 7 files changed, 174 insertions(+), 23 deletions(-) diff --git a/tests/testdriver.xml b/tests/testdriver.xml index 44924e710..e68ee4219 100644 --- a/tests/testdriver.xml +++ b/tests/testdriver.xml @@ -593,7 +593,7 @@ test-many-devices, like an alternate RNG. - test-internal-snapshots + test-snapshots 409600 12345678-1234-FDDF-1234-12345678FFFF /tmp/bootfoo @@ -612,6 +612,9 @@ test-many-devices, like an alternate RNG. + FOO +desc line #2 +ba test-internal-snapshots 12345678-1234-fddf-1234-12345678ffff @@ -851,6 +854,77 @@ test-many-devices, like an alternate RNG. 0 + + aaa-external-mem + running + 1365903080 + + + + + + + + test-internal-snapshots + 12345678-1234-fddf-1234-12345678ffff + 409600 + 409600 + 1 + /tmp/bootfoo + + xen + + + destroy + restart + destroy + + + + +
+ + + + + 0 + + + + zzz-external-diskonly + shutoff + 1365903080 + + + + + + + test-internal-snapshots + 12345678-1234-fddf-1234-12345678ffff + 409600 + 409600 + 1 + /tmp/bootfoo + + xen + + + destroy + restart + destroy + + + + +
+ + + + + 0 + + diff --git a/tests/xmlparse-xml/change-snapshot-out.xml b/tests/xmlparse-xml/change-snapshot-out.xml index 2c55a7f37..124c698e7 100644 --- a/tests/xmlparse-xml/change-snapshot-out.xml +++ b/tests/xmlparse-xml/change-snapshot-out.xml @@ -8,9 +8,9 @@ newline newparent 1234 - + - + test-internal-snapshots diff --git a/tests/xmlparse.py b/tests/xmlparse.py index 250f84a1a..c3751cd22 100644 --- a/tests/xmlparse.py +++ b/tests/xmlparse.py @@ -833,6 +833,11 @@ class XMLParseTest(unittest.TestCase): check("description", "offline desk", "foo\nnewline\n indent") check("parent", "offline-root", "newparent") check("creationTime", 1375905916, 1234) + check("memory_type", "no", "internal") + + check = self._make_checker(snap.disks[0]) + check("name", "hda", "hdb") + check("snapshot", "internal", "no") utils.diff_compare(snap.get_xml_config(), outfile) diff --git a/ui/snapshots.ui b/ui/snapshots.ui index cfc1114f0..a7541d96e 100644 --- a/ui/snapshots.ui +++ b/ui/snapshots.ui @@ -1,5 +1,5 @@ - + @@ -343,7 +343,7 @@ 0 - 3 + 4 1 1 @@ -364,7 +364,7 @@ 1 - 3 + 4 1 1 @@ -464,6 +464,34 @@ 1 + + + True + False + 0 + Snapshot Mode: + + + 0 + 3 + 1 + 1 + + + + + True + False + 0 + label + + + 1 + 3 + 1 + 1 + + diff --git a/virtManager/domain.py b/virtManager/domain.py index c7643806b..70fd3e144 100644 --- a/virtManager/domain.py +++ b/virtManager/domain.py @@ -159,8 +159,6 @@ class vmmDomainSnapshot(vmmLibvirtObject): def _XMLDesc(self, flags): return self._backend.getXMLDesc(flags=flags) - def is_current(self): - return self._backend.isCurrent() def delete(self, force=True): ignore = force self._backend.delete() @@ -175,6 +173,14 @@ class vmmDomainSnapshot(vmmLibvirtObject): status = libvirt.VIR_DOMAIN_NOSTATE return uihelpers.vm_status_icons[status] + def is_external(self): + if self.get_xmlobj().memory_type == "external": + return True + for disk in self.get_xmlobj().disks: + if disk.snapshot == "external": + return True + return False + class vmmDomain(vmmLibvirtObject): """ diff --git a/virtManager/snapshots.py b/virtManager/snapshots.py index 38e1bbe3d..c045e88aa 100644 --- a/virtManager/snapshots.py +++ b/virtManager/snapshots.py @@ -92,9 +92,9 @@ class vmmSnapshotPage(vmmGObjectUI): buf = Gtk.TextBuffer() self.widget("snapshot-new-description").set_buffer(buf) - # [snap object, row label, tooltip, icon name] - model = Gtk.ListStore(object, str, str, str) - model.set_sort_column_id(1, Gtk.SortType.ASCENDING) + # [snap object, row label, tooltip, icon name, sortname] + model = Gtk.ListStore(object, str, str, str, str) + model.set_sort_column_id(4, Gtk.SortType.ASCENDING) col = Gtk.TreeViewColumn("") col.set_min_width(150) @@ -108,10 +108,14 @@ class vmmSnapshotPage(vmmGObjectUI): col.add_attribute(txt, 'markup', 1) col.add_attribute(img, 'icon-name', 3) + def _sep_cb(_model, _iter, ignore): + return not bool(_model[_iter][0]) + slist = self.widget("snapshot-list") slist.set_model(model) slist.set_tooltip_column(2) slist.append_column(col) + slist.set_row_separator_func(_sep_cb, None) ################### @@ -156,6 +160,8 @@ class vmmSnapshotPage(vmmGObjectUI): str(e)) return + has_external = False + has_internal = False for snap in snapshots: desc = snap.get_xmlobj().description if not uihelpers.can_set_row_none: @@ -163,9 +169,22 @@ class vmmSnapshotPage(vmmGObjectUI): name = snap.get_name() state = util.xml_escape(snap.run_status()) - label = "%s\n%s: %s" % ( - (name, _("State"), state)) - model.append([snap, label, desc, snap.run_status_icon_name()]) + if snap.is_external(): + has_external = True + sortname = "3%s" % name + external = " (%s)" % _("External") + else: + has_internal = True + external = "" + sortname = "1%s" % name + + label = "%s\n%s: %s%s" % ( + (name, _("State"), state, external)) + model.append([snap, label, desc, snap.run_status_icon_name(), + sortname]) + + if has_internal and has_external: + model.append([None, None, None, None, "2"]) if len(model): self.widget("snapshot-list").get_selection().select_iter( @@ -180,6 +199,8 @@ class vmmSnapshotPage(vmmGObjectUI): desc = snap and xmlobj.description or "" state = snap and snap.run_status() or "" icon = snap and snap.run_status_icon_name() or None + is_external = snap and snap.is_external() or False + timestamp = "" if snap: timestamp = str(datetime.datetime.fromtimestamp( @@ -198,6 +219,19 @@ class vmmSnapshotPage(vmmGObjectUI): self.widget("snapshot-status-icon").set_from_icon_name( icon, Gtk.IconSize.MENU) + uihelpers.set_grid_row_visible(self.widget("snapshot-mode"), + is_external) + if is_external: + is_mem = xmlobj.memory_type == "external" + is_disk = [d.snapshot == "external" for d in xmlobj.disks] + if is_mem and is_disk: + mode = _("External disk and memory") + elif is_mem: + mode = _("External memory only") + else: + mode = _("External disk only") + self.widget("snapshot-mode").set_text(mode) + self.widget("snapshot-add").set_sensitive(True) self.widget("snapshot-delete").set_sensitive(bool(snap)) self.widget("snapshot-start").set_sensitive(bool(snap)) @@ -314,12 +348,12 @@ class vmmSnapshotPage(vmmGObjectUI): snap = self._get_selected_snapshot() result = self.err.yes_no(_("Are you sure you want to revert to " "snapshot '%s'? All disk changes since " - "the last snapshot will be discarded.") % - snap.get_name()) + "the last snapshot was created will be " + "discarded.") % snap.get_name()) if not result: return - logging.debug("Revertin to snapshot '%s'", snap.get_name()) + logging.debug("Reverting to snapshot '%s'", snap.get_name()) vmmAsyncJob.simple_async_noshow(self.vm.revert_to_snapshot, [snap], self, _("Error reverting to snapshot '%s'") % diff --git a/virtinst/snapshot.py b/virtinst/snapshot.py index 387e620f9..44d8588db 100644 --- a/virtinst/snapshot.py +++ b/virtinst/snapshot.py @@ -20,7 +20,13 @@ import libvirt from virtinst import util -from virtinst.xmlbuilder import XMLBuilder, XMLProperty +from virtinst.xmlbuilder import XMLBuilder, XMLChildProperty, XMLProperty + + +class _SnapshotDisk(XMLBuilder): + _XML_ROOT_NAME = "disk" + name = XMLProperty("./@name") + snapshot = XMLProperty("./@snapshot") class DomainSnapshot(XMLBuilder): @@ -57,11 +63,9 @@ class DomainSnapshot(XMLBuilder): creationTime = XMLProperty("./creationTime", is_int=True) parent = XMLProperty("./parent/name") - # Missing bits: - # @type and @file - # block which has a psuedo VM disk device - # block which tracks the snapshot guest XML - # which should list active status for an internal snapshot + memory_type = XMLProperty("./memory/@snapshot") + + disks = XMLChildProperty(_SnapshotDisk, relative_xpath="./disks") ##################