Transient state means Starting state

"Transient" state was used by clients to indicate an impermanent state,
while on the server, to indicate that Qrexec should be running but is
not. One of the issues is that this is not a libvirt state,

The state aren't always as transient (short duration) as we think it is,
clients were never never notified of state changes from
"Running/Paused/Suspended" to "Transient", which can happen on "qrexec"
feature change and on qube's qrexec agent stopping.

A replacement for "Transient" state that clients will be satisfied is
"Starting" and "Halting". I judged introducing these new states,
although not libvirt states, as a better replacement because they can be
dealt with events already received by the client
"domain-(pre-)?(start|shutdown)(-failed)?", and don't need to learn a
new argument for "domain-(resumed|unpaused)" to set the state from
"Paused/Suspended" to "Transient".

In case the old "Transient" meaning of qrexec not being ready, is needed
in the future, add it somewhere else, with another more insightful name,
and not as a power state.

Note that "Transient" is a temporary name and it will be replaced by
"Starting" on R5.0.

For: QubesOS/qubes-issues#10966

Author: ben-grande

qubes/vm/qubesvm.py

@@ -1454,6 +1454,12 @@ async def _ensure_shutdown_handled(self):
                         self._lifecycle_waiter = None
                     raise
 
+    async def notify_failed_startup(self, exc: Exception):
+        self.log.error("Start failed: %s", str(exc))
+        self._power_state = "Halted"
+        # let anyone receiving domain-pre-start know that startup failed
+        await self.fire_event_async("domain-start-failed", reason=str(exc))
+
     async def start(
         self, start_guid=True, notify_function=None, mem_required=None
     ):
@@ -1487,6 +1493,7 @@ async def start(
                 )
 
             self.log.info("Starting qube {}".format(self.name))
+            self._power_state = "Transient"
 
             try:
                 await self.fire_event_async(
@@ -1496,10 +1503,7 @@ async def start(
                     mem_required=mem_required,
                 )
             except Exception as exc:
-                self.log.error("Start failed: %s", str(exc))
-                await self.fire_event_async(
-                    "domain-start-failed", reason=str(exc)
-                )
+                await self.notify_failed_startup(exc=exc)
                 raise
 
             qmemman_client = None
@@ -1537,11 +1541,7 @@ async def start(
                 await self.storage.start()
 
             except Exception as exc:
-                self.log.error("Start failed: %s", str(exc))
-                # let anyone receiving domain-pre-start know that startup failed
-                await self.fire_event_async(
-                    "domain-start-failed", reason=str(exc)
-                )
+                await self.notify_failed_startup(exc=exc)
                 if qmemman_client:
                     qmemman_client.close()
                 raise
@@ -1583,18 +1583,11 @@ async def start(
                         "Failed to start an HVM qube with PCI devices assigned "
                         "- hardware does not support IOMMU/VT-d/AMD-Vi"
                     )
-                self.log.error("Start failed: %s", str(exc))
-                await self.fire_event_async(
-                    "domain-start-failed", reason=str(exc)
-                )
+                await self.notify_failed_startup(exc=exc)
                 await self.storage.stop()
                 raise exc
             except Exception as exc:
-                self.log.error("Start failed: %s", str(exc))
-                # let anyone receiving domain-pre-start know that startup failed
-                await self.fire_event_async(
-                    "domain-start-failed", reason=str(exc)
-                )
+                await self.notify_failed_startup(exc=exc)
                 await self.storage.stop()
                 raise
 
@@ -1633,19 +1626,14 @@ async def start(
                 ):
                     await self.start_qrexec_daemon(stubdom=True)
                 await self.start_qrexec_daemon()
-                if self._power_state == "Transient" and self.is_fully_usable():
-                    self._power_state = "Running"
 
+                self._power_state = "Running"
                 await self.fire_event_async(
                     "domain-start", start_guid=start_guid
                 )
 
             except Exception as exc:  # pylint: disable=bare-except
-                self.log.error("Start failed: %s", str(exc))
-                # let anyone receiving domain-pre-start know that startup failed
-                await self.fire_event_async(
-                    "domain-start-failed", reason=str(exc)
-                )
+                await self.notify_failed_startup(exc=exc)
                 # This avoids losing the exception if an exception is
                 # raised in self.kill(), because the vm is not
                 # running or paused
@@ -1718,18 +1706,22 @@ def on_libvirt_domain_defined(self):
 
         This is not a Qubes event handler.
         """
-        self._is_running = False
-        self._power_state = "Halted"
+        if self._is_running is None:
+            self._is_running = False
+        if self._power_state is None:
+            self._power_state = "Halted"
         self.end_lifecycle_waiter()
 
-    def on_libvirt_domain_started(self, detail: int):
+    def on_libvirt_domain_started(self, detail):
         """Handle VIR_DOMAIN_EVENT_STARTED event from libvirt when booted.
 
         This is not a Qubes event handler.
         """
         self._is_running = True
         if detail == libvirt.VIR_DOMAIN_EVENT_STARTED_BOOTED:
-            self._power_state = "Paused"
+            # Let power state be "Transient/Starting" if set.
+            if self._power_state is None:
+                self._power_state = "Running"
         elif detail == libvirt.VIR_DOMAIN_EVENT_STARTED_WAKEUP:
             self._power_state = "Running"
         self.end_lifecycle_waiter()
@@ -2588,87 +2580,167 @@ def set_power_state(self):
         =============== ========================================================
         return value    meaning
         =============== ========================================================
-        ``'Halted'``    Machine is not active.
-        ``'Transient'`` Machine is running, but does not have :program:`guid`
-                        or :program:`qrexec` available.
-        ``'Running'``   Machine is ready and running.
-        ``'Paused'``    Machine is paused.
-        ``'Suspended'`` Machine is S3-suspended.
-        ``'Halting'``   Machine is in process of shutting down.
-        ``'Crashed'``   Machine crashed and is unusable, probably because of
+        ``"Halted"``    Machine is not active.
+        ``"Transient"`` Machine is in process of starting. This state will be
+                        deprecated in the future in favor of ``"Starting"``.
+        ``"Running"``   Machine is ready and running.
+        ``"Paused"``    Machine is paused.
+        ``"Suspended"`` Machine is S3-suspended.
+        ``"Halting"``   Machine is in process of shutting down.
+        ``"Crashed"``   Machine crashed and is unusable, probably because of
                         bug in dom0.
-        ``'NA'``        Machine is in unknown state (most likely libvirt domain
+        ``"NA"``        Machine is in unknown state (most likely libvirt domain
                         is undefined).
         =============== ========================================================
 
-        FIXME: graph below may be incomplete and wrong. Click on method name to
-        see its documentation.
+        Click on method name to see its documentation.
 
         .. graphviz::
 
             digraph {
-                node [fontname="sans-serif"];
-                edge [fontname="mono"];
-
-
-                Halted;
-                NA;
-                Dying;
-                Crashed;
-                Transient;
-                Halting;
-                Running;
-                Paused [color=gray75 fontcolor=gray75];
-                Suspended;
-
+                rankdir=LR;
+
+                node [
+                    fontname="sans-serif",
+                    shape=box,
+                    style="rounded,filled",
+                    color="darkslategray",
+                    fillcolor="aliceblue",
+                ];
+
+                edge [
+                    fontname="mono",
+                    color="lightslategray",
+                ];
+
+                NA        [fillcolor="lightgray", color="dimgray"];
+                Halted    [fillcolor="lightgray", color="black"];
+                Transient [fillcolor="lightblue", color="blue"];
+                Running   [fillcolor="palegreen", color="forestgreen"];
+                Paused    [fillcolor="lightyellow", color="yellow"];
+                Suspended [fillcolor="lavenderblush", color="purple"];
+                Crashed   [fillcolor="mistyrose", color="red"];
+                Halting   [fillcolor="lightcoral", color="red"];
+
+                // From NA/Halted/Transient
                 NA -> Halted;
                 Halted -> NA [constraint=false];
-
-                Halted -> Transient
-                    [xlabel="start()" URL="#qubes.vm.qubesvm.QubesVM.start"];
-                Transient -> Running;
-
-                Running -> Halting
-                    [xlabel="shutdown()"
-                        URL="#qubes.vm.qubesvm.QubesVM.shutdown"
-                        constraint=false];
-                Halting -> Dying -> Halted [constraint=false];
-
-                /* cosmetic, invisible edges to put rank constraint */
-                Dying -> Halting [style="invis"];
-                Halting -> Transient [style="invis"];
-
-                Running -> Halted
-                    [label="kill()"
-                        URL="#qubes.vm.qubesvm.QubesVM.kill"
-                        constraint=false];
-
-                Running -> Crashed [constraint=false];
-                Crashed -> Halted [constraint=false];
-
-                Running -> Paused
-                    [label="pause()" URL="#qubes.vm.qubesvm.QubesVM.pause"
-                        color=gray75 fontcolor=gray75];
-                Running -> Suspended
-                    [label="suspend()" URL="#qubes.vm.qubesvm.QubesVM.suspend"
-                        color=gray50 fontcolor=gray50];
-                Paused -> Running
-                    [label="unpause()" URL="#qubes.vm.qubesvm.QubesVM.unpause"
-                        color=gray75 fontcolor=gray75];
-                Suspended -> Running
-                    [label="resume()" URL="#qubes.vm.qubesvm.QubesVM.resume"
-                        color=gray50 fontcolor=gray50];
-
-                Running -> Suspended
-                    [label="suspend()" URL="#qubes.vm.qubesvm.QubesVM.suspend"];
-                Suspended -> Running
-                    [label="resume()" URL="#qubes.vm.qubesvm.QubesVM.resume"];
-
-
-                { rank=source; Halted NA };
-                { rank=same; Transient Halting };
-                { rank=same; Crashed Dying };
-                { rank=sink; Paused Suspended };
+                Halted -> Transient -> Running [
+                    label="start()",
+                    URL="#qubes.vm.qubesvm.QubesVM.start",
+                    color="blue",
+                    constraint=true,
+                ];
+                Transient -> Halted [
+                    label="domain-start-failed",
+                    URL="#event-domain-start-failed",
+                    color="black",
+                    constraint=true,
+                ];
+
+                // From Running
+                Running -> Halting [
+                    label="shutdown()",
+                    URL="#qubes.vm.qubesvm.QubesVM.shutdown",
+                    constraint=false,
+                    color="red",
+                ];
+                Running -> Halting [
+                    label="kill()",
+                    URL="#qubes.vm.qubesvm.QubesVM.kill",
+                    color="red",
+                    constraint=false,
+                ];
+                Running -> Paused [
+                    label="pause()",
+                    URL="#qubes.vm.qubesvm.QubesVM.pause",
+                    color="yellow",
+                    constraint=true,
+                ];
+                Running -> Suspended [
+                    label="suspend()",
+                    URL="#qubes.vm.qubesvm.QubesVM.suspend",
+                    color="purple",
+                    constraint=true,
+                ];
+                Running -> Crashed [
+                    color="black",
+                    constraint=false,
+                ];
+                Running -> Halted [
+                    label="shutdown without API",
+                    color="black",
+                    constraint=false,
+                ];
+
+                // From Paused/Suspended
+                Paused -> Running [
+                    label="unpause()",
+                    URL="#qubes.vm.qubesvm.QubesVM.unpause",
+                    color="yellow",
+                    constraint=false,
+                ];
+                Paused -> Halting [
+                    label="kill()",
+                    URL="#qubes.vm.qubesvm.QubesVM.kill",
+                    color="red4",
+                    constraint=true,
+                ];
+                Suspended -> Running [
+                    label="resume()",
+                    URL="#qubes.vm.qubesvm.QubesVM.resume",
+                    color="purple",
+                    constraint=false,
+                ];
+                Suspended -> Halting [
+                    label="kill()",
+                    URL="#qubes.vm.qubesvm.QubesVM.kill",
+                    color="red4",
+                    constraint=true,
+                ];
+
+                // From Halting
+                Halting -> Halted [
+                    color="black",
+                    constraint=true,
+                ];
+                Halting -> Running [
+                    label="domain-shutdown-failed",
+                    URL="#event-domain-shutdown-failed",
+                    color="blue",
+                    constraint=true,
+                ];
+
+                // From Crashed
+                Crashed -> Halted [
+                    color="black",
+                    constraint=false,
+                ];
+
+
+                subgraph cluster_inactive {
+                    label="Inactive (no ID)"
+                    color="gray"
+                    { rank=""; Halted; NA; }
+                }
+
+                subgraph cluster_active {
+                    label="Active (has ID)"
+                    color="dodgerblue"
+                    { rank="same"; Running; }
+                    subgraph cluster_transition {
+                        label="Transition"
+                        style="dashed"
+                        color="lightskyblue"
+                        { rank="sink"; Transient; Halting; Crashed; }
+                    }
+                    subgraph cluster_suspended {
+                        label="Suspended"
+                        style="rounded"
+                        color="plum"
+                        { rank="sink"; Paused; Suspended; }
+                    }
+                }
             }
 
         .. seealso::
@@ -2706,10 +2778,6 @@ def set_power_state(self):
             }
             if state in state_dict:
                 self._power_state = state_dict[state]
-            elif not self.is_fully_usable():
-                # libvirt.VIR_DOMAIN_RUNNING # 0x1
-                # libvirt.VIR_DOMAIN_BLOCKED # 0x2
-                self._power_state = "Transient"
         except libvirt.libvirtError as e:
             if e.get_error_code() != libvirt.VIR_ERR_NO_DOMAIN:
                 raise
@@ -2724,7 +2792,8 @@ def is_halted(self):
         return self.get_power_state() == "Halted"
 
     def is_running(self):
-        """Check whether this domain is running.
+        """Check whether this domain is active. This is not the same as
+        ``Running`` power state, it only means the qube has a libvirt ID.
 
         :returns: :py:obj:`True` if this domain is started, \
             :py:obj:`False` otherwise.