fix(instances): reverting a running VM to an internal snapshot fails before any effect

The internal revert's shut-off check ran in the job's run, after its
intent was recorded, so the refusal went to recovery: the job ended
uncertain and kept the VM's claim until a superuser resolved it. The check
(check_internal_revert) now runs in precheck too, so a page older than the
VM's start, or a direct POST, fails the job with the reason.

instances/jobs.py

@@ -449,6 +449,8 @@ class RevertSnapshot(Kind):
         proxy = _checked_proxy(op)
         if op.params["external"]:
             proxy.check_external_revert(op.params["name"])
+        else:
+            proxy.check_internal_revert()
         # a revert gives the VM the host devices it had then: not one given since to another VM
         has_hostdevs, definitions = _snapshot_definitions(proxy, op.params)
         if has_hostdevs:

instances/tests/test_snapshot_jobs.py

@@ -92,6 +92,14 @@ class SnapshotJobsTestCase(TransactionTestCase):
             self.queue_and_run("revert_external_snapshot", {"name": "a"})
         self.assertFalse(Operation.objects.get().effects_started)
 
+    def test_an_internal_revert_of_a_running_vm_is_refused_before_effects(self):
+        # the page disables the button, but it may be older than the VM's start
+        self.proxy.check_internal_revert.side_effect = OperationError("Shut the VM down to revert to an internal snapshot")
+        with self.assertRaises(OperationError):
+            self.queue_and_run("revert_snapshot", {"name": "b"})
+        self.assertFalse(Operation.objects.get().effects_started)
+        self.proxy.snapshot_revert.assert_not_called()
+
     def test_an_internal_revert_is_made_by_the_worker(self):
         self.queue_and_run("revert_snapshot", {"name": "b"})
         self.proxy.snapshot_revert.assert_called_once_with("b")

vrtManager/instance.py

@@ -2301,12 +2301,17 @@ class wvmInstance(wvmConnect):
         self._check_snapshot_state()
         self._snapshot(snapshot, external=False).delete(0)
 
-    def snapshot_revert(self, snapshot):
-        """An internal snapshot is reverted to on a shut-off VM; one of a
-        running VM starts it again as it was then."""
+    def check_internal_revert(self):
+        """Refuses a revert to an internal snapshot, before anything changes,
+        unless the VM is shut off."""
         if self.get_status() != 5:
             raise util.OperationError(gettext_noop("Shut the VM down to revert to an internal snapshot"))
         self._check_snapshot_state()
+
+    def snapshot_revert(self, snapshot):
+        """An internal snapshot is reverted to on a shut-off VM; one of a
+        running VM starts it again as it was then."""
+        self.check_internal_revert()
         self.instance.revertToSnapshot(self._snapshot(snapshot, external=False), 0)
 
     def get_managed_save_image(self):