feat(instances): external snapshots first, internal ones a fallback

The Snapshots tab had three sub-tabs: internal, external and the
lists, with the two kinds side by side as equals. The owner set the
direction: external snapshots are the way; internal ones pause the VM
while its memory is saved and can take long, so they stay only as a
fallback for a VM that cannot take an external one.

The tab now has the external snapshot form on top, its name and
description on one line, with its memory and freeze options; the
internal snapshot under a folded "fallback" line with its warning; and
one list of all snapshots, newest first, each on one line with its
name, description, kind and date, and Revert and Delete.

Codex reviewed snapshots end to end; these are fixed:

- The pause and memory lines follow the VM's state and the kind.
- An internal snapshot is reverted to on a shut-off VM only, on the
  server too; the tab alone held that rule.
- A paused VM's revert says it comes back paused, not that it starts.
- An external snapshot waits, with the reason, while snapshots newer
  than the one reverted to exist; the server refused it unexplained.
- Deleting an external snapshot says its files are merged with the
  disks; "into the next one" was wrong for the last one.
- The description has no 45-character limit only the browser kept.
- Freezing is not shown ticked where it cannot be offered, and
  unticking memory gives back the earlier freeze choice.
- Internal create, revert and delete refuse a saved state or a running
  disk job, as external ones do.

vrtManager/tests/test_external_snapshots.py

@@ -236,6 +236,27 @@ class SnapshotStateTestCase(unittest.TestCase):
     def test_a_clean_vm_passes(self):
         proxy()._check_snapshot_state()
 
+    def test_internal_snapshots_are_refused_alike(self):
+        # a saved state or a running job refuses internal create, revert and delete too
+        for state in ({"managed_save": True}, {"jobs": {"vda": {"cur": 1, "end": 2}}}):
+            for name, act in (("create", lambda p: p.create_snapshot("s")),
+                              ("revert", lambda p: p.snapshot_revert("i")),
+                              ("delete", lambda p: p.snapshot_delete("i"))):
+                with self.subTest(name, **{key: True for key in state}):
+                    p = proxy(status=5, internal=[fake_snap("i")], **state)
+                    with self.assertRaises(util.OperationError):
+                        act(p)
+                    p.instance.revertToSnapshot.assert_not_called()
+                    p._snapshotCreateXML.assert_not_called()
+
+    def test_an_internal_snapshot_is_reverted_to_on_a_shut_off_vm(self):
+        for status in (1, 3):
+            with self.subTest(status=status), self.assertRaisesRegex(util.OperationError, "Shut the VM down"):
+                proxy(status=status, internal=[fake_snap("i")]).snapshot_revert("i")
+        p = proxy(status=5, internal=[fake_snap("i")])
+        p.snapshot_revert("i")
+        p.instance.revertToSnapshot.assert_called_once()
+
 
 DISK_ONLY_ATOMIC = libvirt.VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY | libvirt.VIR_DOMAIN_SNAPSHOT_CREATE_ATOMIC
 

vrtManager/tests/test_xml_safety.py

@@ -57,7 +57,8 @@ class TestXmlSafety(unittest.TestCase):
 
     def test_snapshot_xml_payload_is_escaped(self):
         inst = wvmInstance.__new__(wvmInstance)
-        inst.instance = MagicMock(**{"snapshotListNames.return_value": []})  # no snapshots yet
+        inst.instance = MagicMock(**{"snapshotListNames.return_value": [], "hasManagedSaveImage.return_value": 0})  # no snapshots yet
+        inst.get_snapshot_jobs = MagicMock(return_value=[])
         inst.get_status = MagicMock(return_value=5)  # shutoff
         inst._XMLDesc = MagicMock(return_value="<domain></domain>")
         inst._snapshotCreateXML = MagicMock(return_value=None)
@@ -76,7 +77,8 @@ class TestXmlSafety(unittest.TestCase):
 
     def test_original_rom_loader_preserved(self):
         inst = wvmInstance.__new__(wvmInstance)
-        inst.instance = MagicMock(**{"snapshotListNames.return_value": []})  # no snapshots yet
+        inst.instance = MagicMock(**{"snapshotListNames.return_value": [], "hasManagedSaveImage.return_value": 0})  # no snapshots yet
+        inst.get_snapshot_jobs = MagicMock(return_value=[])
         inst._XMLDesc = MagicMock(return_value="<domain><os><loader type='rom'>/usr/share/rom.bin</loader></os></domain>")
         inst._defineXML = MagicMock()
         inst._snapshotCreateXML = MagicMock(return_value=None)
@@ -87,7 +89,9 @@ class TestXmlSafety(unittest.TestCase):
 
     def test_snapshot_revert_rom_loader_preserved(self):
         inst = wvmInstance.__new__(wvmInstance)
-        inst.instance = MagicMock(**{"snapshotListNames.return_value": ["snap_rom"]})
+        inst.instance = MagicMock(**{"snapshotListNames.return_value": ["snap_rom"], "hasManagedSaveImage.return_value": 0})
+        inst.get_snapshot_jobs = MagicMock(return_value=[])
+        inst.get_status = MagicMock(return_value=5)  # an internal snapshot is reverted to on a shut-off VM
         mock_snap = MagicMock()
         inst.instance.snapshotLookupByName = MagicMock(return_value=mock_snap)
         inst.instance.revertToSnapshot = MagicMock()
@@ -102,7 +106,8 @@ class TestXmlSafety(unittest.TestCase):
     def test_internal_snapshot_never_rewrites_a_pflash_loader(self):
         # libvirt decides; the definition is not switched to rom.
         inst = wvmInstance.__new__(wvmInstance)
-        inst.instance = MagicMock(**{"snapshotListNames.return_value": []})  # no snapshots yet
+        inst.instance = MagicMock(**{"snapshotListNames.return_value": [], "hasManagedSaveImage.return_value": 0})  # no snapshots yet
+        inst.get_snapshot_jobs = MagicMock(return_value=[])
         inst.get_status = MagicMock(return_value=5)
         inst._XMLDesc = MagicMock(
             return_value="<domain><os><loader readonly='yes' type='pflash'>/usr/share/OVMF/OVMF.fd</loader></os></domain>"