[PATCH rfcv3 07/11] qemu: add hard reboot in QEMU driver

Zhenzhong Duan posted 11 patches 2 years, 2 months ago
There is a newer version of this series
[PATCH rfcv3 07/11] qemu: add hard reboot in QEMU driver
Posted by Zhenzhong Duan 2 years, 2 months ago
From: Chenyi Qiang <chenyi.qiang@intel.com>

Add the new flag VIR_DOMAIN_REBOOT_HARD/VIR_DOMAIN_SHUTDOWN_HARD to
carry out a hard reboot, which kills the QEMU process and creates a new
one with the same definition.

Hard reboot will be the highest priority to check. If succeed, other
reboot policy (i.e. agent and acpi) would be skipped. Otherwise, use the
traditional way.

To make the shutdown graceful, the workflow of hard reboot is similar
to the existing fakeReboot procedure.

 - In qemuDomainObjPrivatePtr we have a bool hardReboot flag.
 - The virDomainReboot method sets this flag and then triggers a normal
   "system_powerdown" if we specify the reboot mode "--mode hard"
 - The QEMU process is started with '-no-shutdown' so that the guest
   CPUs pause when it powers off the guest.
 - When we receive the "SHUTDOWN" event from QEMU monitor, if hardReboot
   is not set, then check fakeReboot or the last choice to invoke
   qemuProcessKill command to shutdown normally.
 - If hardReboot was set, we spawn a background thread, which issues
   qemuProcessStop to kill the QEMU process and cleanup the working
   environment. Then issue qemuProcessStart to create a new QEMU process
   with the same domain definition. At last, issues 'cont' to start the
   CPUs again.

The state transfer during the hardReboot is
RUNNING->SHUTDOWN->SHUTOFF->PAUSED->RUNNING, This patch doesn't hide
it as the states is not transient.

Signed-off-by: Chenyi Qiang <chenyi.qiang@intel.com>
---
 include/libvirt/libvirt-domain.h |  2 +
 src/qemu/qemu_domain.c           | 18 +++++++
 src/qemu/qemu_domain.h           |  4 ++
 src/qemu/qemu_driver.c           | 76 +++++++++++++++++++++------
 src/qemu/qemu_process.c          | 88 +++++++++++++++++++++++++++++++-
 tools/virsh-domain.c             |  8 ++-
 6 files changed, 177 insertions(+), 19 deletions(-)

diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
index a1902546bb..5cedbbd1fc 100644
--- a/include/libvirt/libvirt-domain.h
+++ b/include/libvirt/libvirt-domain.h
@@ -1520,6 +1520,7 @@ typedef enum {
     VIR_DOMAIN_SHUTDOWN_INITCTL        = (1 << 2), /* Use initctl (Since: 1.0.1) */
     VIR_DOMAIN_SHUTDOWN_SIGNAL         = (1 << 3), /* Send a signal (Since: 1.0.1) */
     VIR_DOMAIN_SHUTDOWN_PARAVIRT       = (1 << 4), /* Use paravirt guest control (Since: 1.2.5) */
+    VIR_DOMAIN_SHUTDOWN_HARD           = (1 << 5), /* Powercycle control (Since: 8.6.0) */
 } virDomainShutdownFlagValues;
 
 int                     virDomainShutdown       (virDomainPtr domain);
@@ -1538,6 +1539,7 @@ typedef enum {
     VIR_DOMAIN_REBOOT_INITCTL        = (1 << 2), /* Use initctl (Since: 1.0.1) */
     VIR_DOMAIN_REBOOT_SIGNAL         = (1 << 3), /* Send a signal (Since: 1.0.1) */
     VIR_DOMAIN_REBOOT_PARAVIRT       = (1 << 4), /* Use paravirt guest control (Since: 1.2.5) */
+    VIR_DOMAIN_REBOOT_HARD           = (1 << 5), /* Powercycle control (Since: 8.6.0) */
 } virDomainRebootFlagValues;
 
 int                     virDomainReboot         (virDomainPtr domain,
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index ae19ce884b..b65d85f91b 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -2614,6 +2614,9 @@ qemuDomainObjPrivateXMLFormat(virBuffer *buf,
     if (priv->fakeReboot)
         virBufferAddLit(buf, "<fakereboot/>\n");
 
+    if (priv->hardReboot)
+        virBufferAddLit(buf, "<hardreboot/>\n");
+
     if (priv->qemuDevices && *priv->qemuDevices) {
         char **tmp = priv->qemuDevices;
         virBufferAddLit(buf, "<devices>\n");
@@ -3305,6 +3308,8 @@ qemuDomainObjPrivateXMLParse(xmlXPathContextPtr ctxt,
 
     priv->fakeReboot = virXPathBoolean("boolean(./fakereboot)", ctxt) == 1;
 
+    priv->hardReboot = virXPathBoolean("boolean(./hardreboot)", ctxt) == 1;
+
     if ((n = virXPathNodeSet("./devices/device", ctxt, &nodes)) < 0) {
         virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
                        _("failed to parse qemu device list"));
@@ -7445,6 +7450,19 @@ qemuDomainSetFakeReboot(virDomainObj *vm,
     qemuDomainSaveStatus(vm);
 }
 
+void
+qemuDomainSetHardReboot(virDomainObj *vm,
+                        bool value)
+{
+    qemuDomainObjPrivate *priv = vm->privateData;
+
+    if (priv->hardReboot == value)
+        return;
+
+    priv->hardReboot = value;
+    qemuDomainSaveStatus(vm);
+}
+
 static void
 qemuDomainCheckRemoveOptionalDisk(virQEMUDriver *driver,
                                   virDomainObj *vm,
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 1e56e50672..fd6058c5bc 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -137,6 +137,7 @@ struct _qemuDomainObjPrivate {
      * -no-reboot instead.
      */
     virTristateBool allowReboot;
+    bool hardReboot;
 
     unsigned long migMaxBandwidth;
     char *origname;
@@ -700,6 +701,9 @@ qemuDomainRemoveInactiveLocked(virQEMUDriver *driver,
 void qemuDomainSetFakeReboot(virDomainObj *vm,
                              bool value);
 
+void qemuDomainSetHardReboot(virDomainObj *vm,
+                             bool value);
+
 int qemuDomainCheckDiskStartupPolicy(virQEMUDriver *driver,
                                      virDomainObj *vm,
                                      size_t diskIndex,
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index d00d2a27c6..86e8efbfcb 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -1788,6 +1788,7 @@ qemuDomainShutdownFlagsAgent(virDomainObj *vm,
         goto endjob;
 
     qemuDomainSetFakeReboot(vm, false);
+    qemuDomainSetHardReboot(vm, false);
     agent = qemuDomainObjEnterAgent(vm);
     ret = qemuAgentShutdown(agent, agentFlag);
     qemuDomainObjExitAgent(vm, agent);
@@ -1800,7 +1801,8 @@ qemuDomainShutdownFlagsAgent(virDomainObj *vm,
 
 static int
 qemuDomainShutdownFlagsMonitor(virDomainObj *vm,
-                               bool isReboot)
+                               bool isReboot,
+                               bool hard)
 {
     int ret = -1;
     qemuDomainObjPrivate *priv;
@@ -1816,7 +1818,17 @@ qemuDomainShutdownFlagsMonitor(virDomainObj *vm,
         goto endjob;
     }
 
-    qemuDomainSetFakeReboot(vm, isReboot);
+    if (hard) {
+        /* hard reboot control the reboot */
+        VIR_DEBUG("Set hard reboot %d in shutdown monitor", isReboot);
+        qemuDomainSetHardReboot(vm, isReboot);
+        qemuDomainSetFakeReboot(vm, false);
+    } else {
+        /* fake reboot control the reboot */
+        VIR_DEBUG("Set fake reboot %d in shutdown monitor", isReboot);
+        qemuDomainSetFakeReboot(vm, isReboot);
+        qemuDomainSetHardReboot(vm, false);
+    }
     qemuDomainObjEnterMonitor(vm);
     ret = qemuMonitorSystemPowerdown(priv->mon);
     qemuDomainObjExitMonitor(vm);
@@ -1832,12 +1844,13 @@ static int qemuDomainShutdownFlags(virDomainPtr dom, unsigned int flags)
     virDomainObj *vm;
     int ret = -1;
     qemuDomainObjPrivate *priv;
-    bool useAgent = false, agentRequested, acpiRequested;
+    bool useAgent = false, agentRequested, acpiRequested, hardRequested;
     bool isReboot = false;
     bool agentForced;
 
     virCheckFlags(VIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN |
-                  VIR_DOMAIN_SHUTDOWN_GUEST_AGENT, -1);
+                  VIR_DOMAIN_SHUTDOWN_GUEST_AGENT |
+                  VIR_DOMAIN_SHUTDOWN_HARD, -1);
 
     if (!(vm = qemuDomainObjFromDomain(dom)))
         goto cleanup;
@@ -1851,14 +1864,23 @@ static int qemuDomainShutdownFlags(virDomainPtr dom, unsigned int flags)
     priv = vm->privateData;
     agentRequested = flags & VIR_DOMAIN_SHUTDOWN_GUEST_AGENT;
     acpiRequested  = flags & VIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN;
+    hardRequested  = flags & VIR_DOMAIN_SHUTDOWN_HARD;
+
+    if (virDomainShutdownFlagsEnsureACL(dom->conn, vm->def, flags) < 0)
+        goto cleanup;
+
+    /* Take hard reboot as the highest priority.
+     * if failed, consider the agent and acpi */
+    if (hardRequested)
+        ret = qemuDomainShutdownFlagsMonitor(vm, isReboot, true);
+
+    if (!ret)
+        goto cleanup;
 
     /* Prefer agent unless we were requested to not to. */
     if (agentRequested || (!flags && priv->agent))
         useAgent = true;
 
-    if (virDomainShutdownFlagsEnsureACL(dom->conn, vm->def, flags) < 0)
-        goto cleanup;
-
     agentForced = agentRequested && !acpiRequested;
     if (useAgent) {
         ret = qemuDomainShutdownFlagsAgent(vm, isReboot, agentForced);
@@ -1877,7 +1899,7 @@ static int qemuDomainShutdownFlags(virDomainPtr dom, unsigned int flags)
             goto cleanup;
         }
 
-        ret = qemuDomainShutdownFlagsMonitor(vm, isReboot);
+        ret = qemuDomainShutdownFlagsMonitor(vm, isReboot, false);
     }
 
  cleanup:
@@ -1913,6 +1935,7 @@ qemuDomainRebootAgent(virDomainObj *vm,
         goto endjob;
 
     qemuDomainSetFakeReboot(vm, false);
+    qemuDomainSetHardReboot(vm, false);
     agent = qemuDomainObjEnterAgent(vm);
     ret = qemuAgentShutdown(agent, agentFlag);
     qemuDomainObjExitAgent(vm, agent);
@@ -1925,7 +1948,8 @@ qemuDomainRebootAgent(virDomainObj *vm,
 
 static int
 qemuDomainRebootMonitor(virDomainObj *vm,
-                        bool isReboot)
+                        bool isReboot,
+                        bool hard)
 {
     qemuDomainObjPrivate *priv = vm->privateData;
     int ret = -1;
@@ -1936,7 +1960,15 @@ qemuDomainRebootMonitor(virDomainObj *vm,
     if (virDomainObjCheckActive(vm) < 0)
         goto endjob;
 
-    qemuDomainSetFakeReboot(vm, isReboot);
+    if (hard) {
+        VIR_DEBUG("Set hard reboot %d in reboot monitor", isReboot);
+        qemuDomainSetHardReboot(vm, isReboot);
+        qemuDomainSetFakeReboot(vm, false);
+    } else {
+        VIR_DEBUG("Set fake reboot %d in reboot monitor", isReboot);
+        qemuDomainSetFakeReboot(vm, isReboot);
+        qemuDomainSetHardReboot(vm, false);
+    }
     qemuDomainObjEnterMonitor(vm);
     ret = qemuMonitorSystemPowerdown(priv->mon);
     qemuDomainObjExitMonitor(vm);
@@ -1953,12 +1985,13 @@ qemuDomainReboot(virDomainPtr dom, unsigned int flags)
     virDomainObj *vm;
     int ret = -1;
     qemuDomainObjPrivate *priv;
-    bool useAgent = false, agentRequested, acpiRequested;
+    bool useAgent = false, agentRequested, acpiRequested, hardRequested;
     bool isReboot = true;
     bool agentForced;
 
     virCheckFlags(VIR_DOMAIN_REBOOT_ACPI_POWER_BTN |
-                  VIR_DOMAIN_REBOOT_GUEST_AGENT, -1);
+                  VIR_DOMAIN_REBOOT_GUEST_AGENT |
+                  VIR_DOMAIN_REBOOT_HARD, -1);
 
     if (!(vm = qemuDomainObjFromDomain(dom)))
         goto cleanup;
@@ -1972,14 +2005,24 @@ qemuDomainReboot(virDomainPtr dom, unsigned int flags)
     priv = vm->privateData;
     agentRequested = flags & VIR_DOMAIN_REBOOT_GUEST_AGENT;
     acpiRequested  = flags & VIR_DOMAIN_REBOOT_ACPI_POWER_BTN;
+    hardRequested = flags & VIR_DOMAIN_REBOOT_HARD;
+
+    if (virDomainRebootEnsureACL(dom->conn, vm->def, flags) < 0)
+        goto cleanup;
+
+    /* Take hard reboot as the highest priority.
+     * This is for TDX which is not allowed to warm reboot.
+     */
+    if (hardRequested)
+        ret = qemuDomainRebootMonitor(vm, isReboot, true);
+
+    if (!ret)
+        goto cleanup;
 
     /* Prefer agent unless we were requested to not to. */
     if (agentRequested || (!flags && priv->agent))
         useAgent = true;
 
-    if (virDomainRebootEnsureACL(dom->conn, vm->def, flags) < 0)
-        goto cleanup;
-
     agentForced = agentRequested && !acpiRequested;
     if (useAgent)
         ret = qemuDomainRebootAgent(vm, isReboot, agentForced);
@@ -1992,7 +2035,7 @@ qemuDomainReboot(virDomainPtr dom, unsigned int flags)
      */
     if ((!useAgent) ||
         (ret < 0 && (acpiRequested || !flags))) {
-        ret = qemuDomainRebootMonitor(vm, isReboot);
+        ret = qemuDomainRebootMonitor(vm, isReboot, false);
     }
 
  cleanup:
@@ -2095,6 +2138,7 @@ qemuDomainDestroyFlags(virDomainPtr dom,
     }
 
     qemuDomainSetFakeReboot(vm, false);
+    qemuDomainSetHardReboot(vm, false);
 
     if (vm->job->asyncJob == VIR_ASYNC_JOB_MIGRATION_IN)
         stopFlags |= VIR_QEMU_PROCESS_STOP_MIGRATED;
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index f27f6653f5..9fa679e408 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -502,13 +502,98 @@ qemuProcessFakeReboot(void *opaque)
     virDomainObjEndAPI(&vm);
 }
 
+static void
+qemuProcessHardReboot(void *opaque)
+{
+    virDomainObj *vm = opaque;
+    qemuDomainObjPrivate *priv = vm->privateData;
+    virQEMUDriver *driver = priv->driver;
+    unsigned int stopFlags = 0;
+    virObjectEvent *event = NULL;
+    virObjectEvent * event2 = NULL;
+
+    VIR_DEBUG("Handle hard reboot: destroy phase");
+
+    virObjectLock(vm);
+    if (virDomainObjCheckActive(vm) < 0)
+        goto cleanup;
+
+    if (qemuProcessBeginStopJob(vm, VIR_JOB_DESTROY, 0) < 0)
+        goto cleanup;
+
+    if (!virDomainObjIsActive(vm)) {
+        virReportError(VIR_ERR_OPERATION_INVALID,
+                       "%s", _("domain is not running"));
+        virDomainObjEndJob(vm);
+        goto cleanup;
+    }
+
+    if (vm->job->asyncJob == VIR_ASYNC_JOB_MIGRATION_IN)
+        stopFlags |= VIR_QEMU_PROCESS_STOP_MIGRATED;
+
+    qemuProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_DESTROYED,
+                    VIR_ASYNC_JOB_NONE, stopFlags);
+    virDomainAuditStop(vm, "destroyed");
+
+    event = virDomainEventLifecycleNewFromObj(vm,
+                                     VIR_DOMAIN_EVENT_STOPPED,
+                                     VIR_DOMAIN_EVENT_STOPPED_DESTROYED);
+
+    virObjectEventStateQueue(driver->domainEventState, event);
+    /* skip remove inactive domain from active list */
+    virDomainObjEndJob(vm);
+
+    VIR_DEBUG("Handle hard reboot: boot phase");
+
+    if (qemuProcessBeginJob(vm, VIR_DOMAIN_JOB_OPERATION_START,
+                            0) < 0) {
+        qemuDomainRemoveInactive(driver, vm, 0, false);
+        goto cleanup;
+    }
+
+    if (qemuProcessStart(NULL, driver, vm, NULL, VIR_ASYNC_JOB_START,
+                         NULL, -1, NULL, NULL,
+                         VIR_NETDEV_VPORT_PROFILE_OP_CREATE,
+                         0) < 0) {
+        virDomainAuditStart(vm, "booted", false);
+        qemuDomainRemoveInactive(driver, vm, 0, false);
+        qemuProcessEndJob(vm);
+        goto cleanup;
+    }
+
+    virDomainAuditStart(vm, "booted", true);
+    event2 = virDomainEventLifecycleNewFromObj(vm,
+                                     VIR_DOMAIN_EVENT_STARTED,
+                                     VIR_DOMAIN_EVENT_STARTED_BOOTED);
+    virObjectEventStateQueue(driver->domainEventState, event2);
+
+    qemuProcessEndJob(vm);
+
+ cleanup:
+    virDomainObjEndAPI(&vm);
+}
 
 void
 qemuProcessShutdownOrReboot(virDomainObj *vm)
 {
     qemuDomainObjPrivate *priv = vm->privateData;
 
-    if (priv->fakeReboot ||
+    if (priv->hardReboot) {
+        g_autofree char *name = g_strdup_printf("hard reboot-%s", vm->def->name);
+        virThread th;
+
+        virObjectRef(vm);
+        if (virThreadCreateFull(&th,
+                                false,
+                                qemuProcessHardReboot,
+                                name,
+                                false,
+                                vm) < 0) {
+            VIR_ERROR(_("Failed to create hard reboot thread, killing domain"));
+            ignore_value(qemuProcessKill(vm, VIR_QEMU_PROCESS_KILL_NOWAIT));
+            virObjectUnref(vm);
+        }
+    } else if (priv->fakeReboot ||
         vm->def->onPoweroff == VIR_DOMAIN_LIFECYCLE_ACTION_RESTART) {
         g_autofree char *name = g_strdup_printf("reboot-%s", vm->def->name);
         virThread th;
@@ -5673,6 +5758,7 @@ qemuProcessInit(virQEMUDriver *driver,
     } else {
         vm->def->id = qemuDriverAllocateID(driver);
         qemuDomainSetFakeReboot(vm, false);
+        qemuDomainSetFakeReboot(vm, false);
         virDomainObjSetState(vm, VIR_DOMAIN_PAUSED, VIR_DOMAIN_PAUSED_STARTING_UP);
 
         if (g_atomic_int_add(&driver->nactive, 1) == 0 && driver->inhibitCallback)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 66f933dead..21864c92cb 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -5915,7 +5915,7 @@ static const vshCmdOptDef opts_shutdown[] = {
     {.name = "mode",
      .type = VSH_OT_STRING,
      .completer = virshDomainShutdownModeCompleter,
-     .help = N_("shutdown mode: acpi|agent|initctl|signal|paravirt")
+     .help = N_("shutdown mode: acpi|agent|initctl|signal|paravirt|hard")
     },
     {.name = NULL}
 };
@@ -5952,6 +5952,8 @@ cmdShutdown(vshControl *ctl, const vshCmd *cmd)
             flags |= VIR_DOMAIN_SHUTDOWN_SIGNAL;
         } else if (STREQ(mode, "paravirt")) {
             flags |= VIR_DOMAIN_SHUTDOWN_PARAVIRT;
+        } else if (STREQ(mode, "hard")) {
+            flags |= VIR_DOMAIN_SHUTDOWN_HARD;
         } else {
             vshError(ctl, _("Unknown mode %1$s value, expecting 'acpi', 'agent', 'initctl', 'signal', or 'paravirt'"),
                      mode);
@@ -5995,7 +5997,7 @@ static const vshCmdOptDef opts_reboot[] = {
     {.name = "mode",
      .type = VSH_OT_STRING,
      .completer = virshDomainShutdownModeCompleter,
-     .help = N_("shutdown mode: acpi|agent|initctl|signal|paravirt")
+     .help = N_("shutdown mode: acpi|agent|initctl|signal|paravirt|hard")
     },
     {.name = NULL}
 };
@@ -6031,6 +6033,8 @@ cmdReboot(vshControl *ctl, const vshCmd *cmd)
             flags |= VIR_DOMAIN_REBOOT_SIGNAL;
         } else if (STREQ(mode, "paravirt")) {
             flags |= VIR_DOMAIN_REBOOT_PARAVIRT;
+        } else if (STREQ(mode, "hard")) {
+            flags |= VIR_DOMAIN_REBOOT_HARD;
         } else {
             vshError(ctl, _("Unknown mode %1$s value, expecting 'acpi', 'agent', 'initctl', 'signal' or 'paravirt'"),
                      mode);
-- 
2.34.1
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org
Re: [PATCH rfcv3 07/11] qemu: add hard reboot in QEMU driver
Posted by Daniel P. Berrangé 2 years, 1 month ago
On Mon, Nov 27, 2023 at 04:55:17PM +0800, Zhenzhong Duan wrote:
> From: Chenyi Qiang <chenyi.qiang@intel.com>
> 
> Add the new flag VIR_DOMAIN_REBOOT_HARD/VIR_DOMAIN_SHUTDOWN_HARD to
> carry out a hard reboot, which kills the QEMU process and creates a new
> one with the same definition.

AFAICT, the _HARD flags cause it to issue a QEMU monitor
command todo an ACPI shutdown and then re-create QEMU.

IOW, it seems like this is just the same as the existing
_ACPI flags, and so I'm not sure why we need any new
functionality at all.

What is the existing ACPI shutdown & fake reboot not
able to achieve ?

> 
> Hard reboot will be the highest priority to check. If succeed, other
> reboot policy (i.e. agent and acpi) would be skipped. Otherwise, use the
> traditional way.
> 
> To make the shutdown graceful, the workflow of hard reboot is similar
> to the existing fakeReboot procedure.
> 
>  - In qemuDomainObjPrivatePtr we have a bool hardReboot flag.
>  - The virDomainReboot method sets this flag and then triggers a normal
>    "system_powerdown" if we specify the reboot mode "--mode hard"
>  - The QEMU process is started with '-no-shutdown' so that the guest
>    CPUs pause when it powers off the guest.
>  - When we receive the "SHUTDOWN" event from QEMU monitor, if hardReboot
>    is not set, then check fakeReboot or the last choice to invoke
>    qemuProcessKill command to shutdown normally.
>  - If hardReboot was set, we spawn a background thread, which issues
>    qemuProcessStop to kill the QEMU process and cleanup the working
>    environment. Then issue qemuProcessStart to create a new QEMU process
>    with the same domain definition. At last, issues 'cont' to start the
>    CPUs again.
> 
> The state transfer during the hardReboot is
> RUNNING->SHUTDOWN->SHUTOFF->PAUSED->RUNNING, This patch doesn't hide
> it as the states is not transient.
> 
> Signed-off-by: Chenyi Qiang <chenyi.qiang@intel.com>
> ---
>  include/libvirt/libvirt-domain.h |  2 +
>  src/qemu/qemu_domain.c           | 18 +++++++
>  src/qemu/qemu_domain.h           |  4 ++
>  src/qemu/qemu_driver.c           | 76 +++++++++++++++++++++------
>  src/qemu/qemu_process.c          | 88 +++++++++++++++++++++++++++++++-
>  tools/virsh-domain.c             |  8 ++-
>  6 files changed, 177 insertions(+), 19 deletions(-)
> 
> diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
> index a1902546bb..5cedbbd1fc 100644
> --- a/include/libvirt/libvirt-domain.h
> +++ b/include/libvirt/libvirt-domain.h
> @@ -1520,6 +1520,7 @@ typedef enum {
>      VIR_DOMAIN_SHUTDOWN_INITCTL        = (1 << 2), /* Use initctl (Since: 1.0.1) */
>      VIR_DOMAIN_SHUTDOWN_SIGNAL         = (1 << 3), /* Send a signal (Since: 1.0.1) */
>      VIR_DOMAIN_SHUTDOWN_PARAVIRT       = (1 << 4), /* Use paravirt guest control (Since: 1.2.5) */
> +    VIR_DOMAIN_SHUTDOWN_HARD           = (1 << 5), /* Powercycle control (Since: 8.6.0) */
>  } virDomainShutdownFlagValues;
>  
>  int                     virDomainShutdown       (virDomainPtr domain);
> @@ -1538,6 +1539,7 @@ typedef enum {
>      VIR_DOMAIN_REBOOT_INITCTL        = (1 << 2), /* Use initctl (Since: 1.0.1) */
>      VIR_DOMAIN_REBOOT_SIGNAL         = (1 << 3), /* Send a signal (Since: 1.0.1) */
>      VIR_DOMAIN_REBOOT_PARAVIRT       = (1 << 4), /* Use paravirt guest control (Since: 1.2.5) */
> +    VIR_DOMAIN_REBOOT_HARD           = (1 << 5), /* Powercycle control (Since: 8.6.0) */
>  } virDomainRebootFlagValues;
>  
>  int                     virDomainReboot         (virDomainPtr domain,
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index ae19ce884b..b65d85f91b 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -2614,6 +2614,9 @@ qemuDomainObjPrivateXMLFormat(virBuffer *buf,
>      if (priv->fakeReboot)
>          virBufferAddLit(buf, "<fakereboot/>\n");
>  
> +    if (priv->hardReboot)
> +        virBufferAddLit(buf, "<hardreboot/>\n");
> +
>      if (priv->qemuDevices && *priv->qemuDevices) {
>          char **tmp = priv->qemuDevices;
>          virBufferAddLit(buf, "<devices>\n");
> @@ -3305,6 +3308,8 @@ qemuDomainObjPrivateXMLParse(xmlXPathContextPtr ctxt,
>  
>      priv->fakeReboot = virXPathBoolean("boolean(./fakereboot)", ctxt) == 1;
>  
> +    priv->hardReboot = virXPathBoolean("boolean(./hardreboot)", ctxt) == 1;
> +
>      if ((n = virXPathNodeSet("./devices/device", ctxt, &nodes)) < 0) {
>          virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>                         _("failed to parse qemu device list"));
> @@ -7445,6 +7450,19 @@ qemuDomainSetFakeReboot(virDomainObj *vm,
>      qemuDomainSaveStatus(vm);
>  }
>  
> +void
> +qemuDomainSetHardReboot(virDomainObj *vm,
> +                        bool value)
> +{
> +    qemuDomainObjPrivate *priv = vm->privateData;
> +
> +    if (priv->hardReboot == value)
> +        return;
> +
> +    priv->hardReboot = value;
> +    qemuDomainSaveStatus(vm);
> +}
> +
>  static void
>  qemuDomainCheckRemoveOptionalDisk(virQEMUDriver *driver,
>                                    virDomainObj *vm,
> diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
> index 1e56e50672..fd6058c5bc 100644
> --- a/src/qemu/qemu_domain.h
> +++ b/src/qemu/qemu_domain.h
> @@ -137,6 +137,7 @@ struct _qemuDomainObjPrivate {
>       * -no-reboot instead.
>       */
>      virTristateBool allowReboot;
> +    bool hardReboot;
>  
>      unsigned long migMaxBandwidth;
>      char *origname;
> @@ -700,6 +701,9 @@ qemuDomainRemoveInactiveLocked(virQEMUDriver *driver,
>  void qemuDomainSetFakeReboot(virDomainObj *vm,
>                               bool value);
>  
> +void qemuDomainSetHardReboot(virDomainObj *vm,
> +                             bool value);
> +
>  int qemuDomainCheckDiskStartupPolicy(virQEMUDriver *driver,
>                                       virDomainObj *vm,
>                                       size_t diskIndex,
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index d00d2a27c6..86e8efbfcb 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -1788,6 +1788,7 @@ qemuDomainShutdownFlagsAgent(virDomainObj *vm,
>          goto endjob;
>  
>      qemuDomainSetFakeReboot(vm, false);
> +    qemuDomainSetHardReboot(vm, false);
>      agent = qemuDomainObjEnterAgent(vm);
>      ret = qemuAgentShutdown(agent, agentFlag);
>      qemuDomainObjExitAgent(vm, agent);
> @@ -1800,7 +1801,8 @@ qemuDomainShutdownFlagsAgent(virDomainObj *vm,
>  
>  static int
>  qemuDomainShutdownFlagsMonitor(virDomainObj *vm,
> -                               bool isReboot)
> +                               bool isReboot,
> +                               bool hard)
>  {
>      int ret = -1;
>      qemuDomainObjPrivate *priv;
> @@ -1816,7 +1818,17 @@ qemuDomainShutdownFlagsMonitor(virDomainObj *vm,
>          goto endjob;
>      }
>  
> -    qemuDomainSetFakeReboot(vm, isReboot);
> +    if (hard) {
> +        /* hard reboot control the reboot */
> +        VIR_DEBUG("Set hard reboot %d in shutdown monitor", isReboot);
> +        qemuDomainSetHardReboot(vm, isReboot);
> +        qemuDomainSetFakeReboot(vm, false);
> +    } else {
> +        /* fake reboot control the reboot */
> +        VIR_DEBUG("Set fake reboot %d in shutdown monitor", isReboot);
> +        qemuDomainSetFakeReboot(vm, isReboot);
> +        qemuDomainSetHardReboot(vm, false);
> +    }
>      qemuDomainObjEnterMonitor(vm);
>      ret = qemuMonitorSystemPowerdown(priv->mon);
>      qemuDomainObjExitMonitor(vm);
> @@ -1832,12 +1844,13 @@ static int qemuDomainShutdownFlags(virDomainPtr dom, unsigned int flags)
>      virDomainObj *vm;
>      int ret = -1;
>      qemuDomainObjPrivate *priv;
> -    bool useAgent = false, agentRequested, acpiRequested;
> +    bool useAgent = false, agentRequested, acpiRequested, hardRequested;
>      bool isReboot = false;
>      bool agentForced;
>  
>      virCheckFlags(VIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN |
> -                  VIR_DOMAIN_SHUTDOWN_GUEST_AGENT, -1);
> +                  VIR_DOMAIN_SHUTDOWN_GUEST_AGENT |
> +                  VIR_DOMAIN_SHUTDOWN_HARD, -1);
>  
>      if (!(vm = qemuDomainObjFromDomain(dom)))
>          goto cleanup;
> @@ -1851,14 +1864,23 @@ static int qemuDomainShutdownFlags(virDomainPtr dom, unsigned int flags)
>      priv = vm->privateData;
>      agentRequested = flags & VIR_DOMAIN_SHUTDOWN_GUEST_AGENT;
>      acpiRequested  = flags & VIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN;
> +    hardRequested  = flags & VIR_DOMAIN_SHUTDOWN_HARD;
> +
> +    if (virDomainShutdownFlagsEnsureACL(dom->conn, vm->def, flags) < 0)
> +        goto cleanup;
> +
> +    /* Take hard reboot as the highest priority.
> +     * if failed, consider the agent and acpi */
> +    if (hardRequested)
> +        ret = qemuDomainShutdownFlagsMonitor(vm, isReboot, true);
> +
> +    if (!ret)
> +        goto cleanup;
>  
>      /* Prefer agent unless we were requested to not to. */
>      if (agentRequested || (!flags && priv->agent))
>          useAgent = true;
>  
> -    if (virDomainShutdownFlagsEnsureACL(dom->conn, vm->def, flags) < 0)
> -        goto cleanup;
> -
>      agentForced = agentRequested && !acpiRequested;
>      if (useAgent) {
>          ret = qemuDomainShutdownFlagsAgent(vm, isReboot, agentForced);
> @@ -1877,7 +1899,7 @@ static int qemuDomainShutdownFlags(virDomainPtr dom, unsigned int flags)
>              goto cleanup;
>          }
>  
> -        ret = qemuDomainShutdownFlagsMonitor(vm, isReboot);
> +        ret = qemuDomainShutdownFlagsMonitor(vm, isReboot, false);
>      }
>  
>   cleanup:
> @@ -1913,6 +1935,7 @@ qemuDomainRebootAgent(virDomainObj *vm,
>          goto endjob;
>  
>      qemuDomainSetFakeReboot(vm, false);
> +    qemuDomainSetHardReboot(vm, false);
>      agent = qemuDomainObjEnterAgent(vm);
>      ret = qemuAgentShutdown(agent, agentFlag);
>      qemuDomainObjExitAgent(vm, agent);
> @@ -1925,7 +1948,8 @@ qemuDomainRebootAgent(virDomainObj *vm,
>  
>  static int
>  qemuDomainRebootMonitor(virDomainObj *vm,
> -                        bool isReboot)
> +                        bool isReboot,
> +                        bool hard)
>  {
>      qemuDomainObjPrivate *priv = vm->privateData;
>      int ret = -1;
> @@ -1936,7 +1960,15 @@ qemuDomainRebootMonitor(virDomainObj *vm,
>      if (virDomainObjCheckActive(vm) < 0)
>          goto endjob;
>  
> -    qemuDomainSetFakeReboot(vm, isReboot);
> +    if (hard) {
> +        VIR_DEBUG("Set hard reboot %d in reboot monitor", isReboot);
> +        qemuDomainSetHardReboot(vm, isReboot);
> +        qemuDomainSetFakeReboot(vm, false);
> +    } else {
> +        VIR_DEBUG("Set fake reboot %d in reboot monitor", isReboot);
> +        qemuDomainSetFakeReboot(vm, isReboot);
> +        qemuDomainSetHardReboot(vm, false);
> +    }
>      qemuDomainObjEnterMonitor(vm);
>      ret = qemuMonitorSystemPowerdown(priv->mon);
>      qemuDomainObjExitMonitor(vm);
> @@ -1953,12 +1985,13 @@ qemuDomainReboot(virDomainPtr dom, unsigned int flags)
>      virDomainObj *vm;
>      int ret = -1;
>      qemuDomainObjPrivate *priv;
> -    bool useAgent = false, agentRequested, acpiRequested;
> +    bool useAgent = false, agentRequested, acpiRequested, hardRequested;
>      bool isReboot = true;
>      bool agentForced;
>  
>      virCheckFlags(VIR_DOMAIN_REBOOT_ACPI_POWER_BTN |
> -                  VIR_DOMAIN_REBOOT_GUEST_AGENT, -1);
> +                  VIR_DOMAIN_REBOOT_GUEST_AGENT |
> +                  VIR_DOMAIN_REBOOT_HARD, -1);
>  
>      if (!(vm = qemuDomainObjFromDomain(dom)))
>          goto cleanup;
> @@ -1972,14 +2005,24 @@ qemuDomainReboot(virDomainPtr dom, unsigned int flags)
>      priv = vm->privateData;
>      agentRequested = flags & VIR_DOMAIN_REBOOT_GUEST_AGENT;
>      acpiRequested  = flags & VIR_DOMAIN_REBOOT_ACPI_POWER_BTN;
> +    hardRequested = flags & VIR_DOMAIN_REBOOT_HARD;
> +
> +    if (virDomainRebootEnsureACL(dom->conn, vm->def, flags) < 0)
> +        goto cleanup;
> +
> +    /* Take hard reboot as the highest priority.
> +     * This is for TDX which is not allowed to warm reboot.
> +     */
> +    if (hardRequested)
> +        ret = qemuDomainRebootMonitor(vm, isReboot, true);
> +
> +    if (!ret)
> +        goto cleanup;
>  
>      /* Prefer agent unless we were requested to not to. */
>      if (agentRequested || (!flags && priv->agent))
>          useAgent = true;
>  
> -    if (virDomainRebootEnsureACL(dom->conn, vm->def, flags) < 0)
> -        goto cleanup;
> -
>      agentForced = agentRequested && !acpiRequested;
>      if (useAgent)
>          ret = qemuDomainRebootAgent(vm, isReboot, agentForced);
> @@ -1992,7 +2035,7 @@ qemuDomainReboot(virDomainPtr dom, unsigned int flags)
>       */
>      if ((!useAgent) ||
>          (ret < 0 && (acpiRequested || !flags))) {
> -        ret = qemuDomainRebootMonitor(vm, isReboot);
> +        ret = qemuDomainRebootMonitor(vm, isReboot, false);
>      }
>  
>   cleanup:
> @@ -2095,6 +2138,7 @@ qemuDomainDestroyFlags(virDomainPtr dom,
>      }
>  
>      qemuDomainSetFakeReboot(vm, false);
> +    qemuDomainSetHardReboot(vm, false);
>  
>      if (vm->job->asyncJob == VIR_ASYNC_JOB_MIGRATION_IN)
>          stopFlags |= VIR_QEMU_PROCESS_STOP_MIGRATED;
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index f27f6653f5..9fa679e408 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -502,13 +502,98 @@ qemuProcessFakeReboot(void *opaque)
>      virDomainObjEndAPI(&vm);
>  }
>  
> +static void
> +qemuProcessHardReboot(void *opaque)
> +{
> +    virDomainObj *vm = opaque;
> +    qemuDomainObjPrivate *priv = vm->privateData;
> +    virQEMUDriver *driver = priv->driver;
> +    unsigned int stopFlags = 0;
> +    virObjectEvent *event = NULL;
> +    virObjectEvent * event2 = NULL;
> +
> +    VIR_DEBUG("Handle hard reboot: destroy phase");
> +
> +    virObjectLock(vm);
> +    if (virDomainObjCheckActive(vm) < 0)
> +        goto cleanup;
> +
> +    if (qemuProcessBeginStopJob(vm, VIR_JOB_DESTROY, 0) < 0)
> +        goto cleanup;
> +
> +    if (!virDomainObjIsActive(vm)) {
> +        virReportError(VIR_ERR_OPERATION_INVALID,
> +                       "%s", _("domain is not running"));
> +        virDomainObjEndJob(vm);
> +        goto cleanup;
> +    }
> +
> +    if (vm->job->asyncJob == VIR_ASYNC_JOB_MIGRATION_IN)
> +        stopFlags |= VIR_QEMU_PROCESS_STOP_MIGRATED;
> +
> +    qemuProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_DESTROYED,
> +                    VIR_ASYNC_JOB_NONE, stopFlags);
> +    virDomainAuditStop(vm, "destroyed");
> +
> +    event = virDomainEventLifecycleNewFromObj(vm,
> +                                     VIR_DOMAIN_EVENT_STOPPED,
> +                                     VIR_DOMAIN_EVENT_STOPPED_DESTROYED);
> +
> +    virObjectEventStateQueue(driver->domainEventState, event);
> +    /* skip remove inactive domain from active list */
> +    virDomainObjEndJob(vm);
> +
> +    VIR_DEBUG("Handle hard reboot: boot phase");
> +
> +    if (qemuProcessBeginJob(vm, VIR_DOMAIN_JOB_OPERATION_START,
> +                            0) < 0) {
> +        qemuDomainRemoveInactive(driver, vm, 0, false);
> +        goto cleanup;
> +    }
> +
> +    if (qemuProcessStart(NULL, driver, vm, NULL, VIR_ASYNC_JOB_START,
> +                         NULL, -1, NULL, NULL,
> +                         VIR_NETDEV_VPORT_PROFILE_OP_CREATE,
> +                         0) < 0) {
> +        virDomainAuditStart(vm, "booted", false);
> +        qemuDomainRemoveInactive(driver, vm, 0, false);
> +        qemuProcessEndJob(vm);
> +        goto cleanup;
> +    }
> +
> +    virDomainAuditStart(vm, "booted", true);
> +    event2 = virDomainEventLifecycleNewFromObj(vm,
> +                                     VIR_DOMAIN_EVENT_STARTED,
> +                                     VIR_DOMAIN_EVENT_STARTED_BOOTED);
> +    virObjectEventStateQueue(driver->domainEventState, event2);
> +
> +    qemuProcessEndJob(vm);
> +
> + cleanup:
> +    virDomainObjEndAPI(&vm);
> +}
>  
>  void
>  qemuProcessShutdownOrReboot(virDomainObj *vm)
>  {
>      qemuDomainObjPrivate *priv = vm->privateData;
>  
> -    if (priv->fakeReboot ||
> +    if (priv->hardReboot) {
> +        g_autofree char *name = g_strdup_printf("hard reboot-%s", vm->def->name);
> +        virThread th;
> +
> +        virObjectRef(vm);
> +        if (virThreadCreateFull(&th,
> +                                false,
> +                                qemuProcessHardReboot,
> +                                name,
> +                                false,
> +                                vm) < 0) {
> +            VIR_ERROR(_("Failed to create hard reboot thread, killing domain"));
> +            ignore_value(qemuProcessKill(vm, VIR_QEMU_PROCESS_KILL_NOWAIT));
> +            virObjectUnref(vm);
> +        }
> +    } else if (priv->fakeReboot ||
>          vm->def->onPoweroff == VIR_DOMAIN_LIFECYCLE_ACTION_RESTART) {
>          g_autofree char *name = g_strdup_printf("reboot-%s", vm->def->name);
>          virThread th;
> @@ -5673,6 +5758,7 @@ qemuProcessInit(virQEMUDriver *driver,
>      } else {
>          vm->def->id = qemuDriverAllocateID(driver);
>          qemuDomainSetFakeReboot(vm, false);
> +        qemuDomainSetFakeReboot(vm, false);
>          virDomainObjSetState(vm, VIR_DOMAIN_PAUSED, VIR_DOMAIN_PAUSED_STARTING_UP);
>  
>          if (g_atomic_int_add(&driver->nactive, 1) == 0 && driver->inhibitCallback)
> diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
> index 66f933dead..21864c92cb 100644
> --- a/tools/virsh-domain.c
> +++ b/tools/virsh-domain.c
> @@ -5915,7 +5915,7 @@ static const vshCmdOptDef opts_shutdown[] = {
>      {.name = "mode",
>       .type = VSH_OT_STRING,
>       .completer = virshDomainShutdownModeCompleter,
> -     .help = N_("shutdown mode: acpi|agent|initctl|signal|paravirt")
> +     .help = N_("shutdown mode: acpi|agent|initctl|signal|paravirt|hard")
>      },
>      {.name = NULL}
>  };
> @@ -5952,6 +5952,8 @@ cmdShutdown(vshControl *ctl, const vshCmd *cmd)
>              flags |= VIR_DOMAIN_SHUTDOWN_SIGNAL;
>          } else if (STREQ(mode, "paravirt")) {
>              flags |= VIR_DOMAIN_SHUTDOWN_PARAVIRT;
> +        } else if (STREQ(mode, "hard")) {
> +            flags |= VIR_DOMAIN_SHUTDOWN_HARD;
>          } else {
>              vshError(ctl, _("Unknown mode %1$s value, expecting 'acpi', 'agent', 'initctl', 'signal', or 'paravirt'"),
>                       mode);
> @@ -5995,7 +5997,7 @@ static const vshCmdOptDef opts_reboot[] = {
>      {.name = "mode",
>       .type = VSH_OT_STRING,
>       .completer = virshDomainShutdownModeCompleter,
> -     .help = N_("shutdown mode: acpi|agent|initctl|signal|paravirt")
> +     .help = N_("shutdown mode: acpi|agent|initctl|signal|paravirt|hard")
>      },
>      {.name = NULL}
>  };
> @@ -6031,6 +6033,8 @@ cmdReboot(vshControl *ctl, const vshCmd *cmd)
>              flags |= VIR_DOMAIN_REBOOT_SIGNAL;
>          } else if (STREQ(mode, "paravirt")) {
>              flags |= VIR_DOMAIN_REBOOT_PARAVIRT;
> +        } else if (STREQ(mode, "hard")) {
> +            flags |= VIR_DOMAIN_REBOOT_HARD;
>          } else {
>              vshError(ctl, _("Unknown mode %1$s value, expecting 'acpi', 'agent', 'initctl', 'signal' or 'paravirt'"),
>                       mode);
> -- 
> 2.34.1
> 

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org
RE: [PATCH rfcv3 07/11] qemu: add hard reboot in QEMU driver
Posted by Duan, Zhenzhong 2 years ago

>-----Original Message-----
>From: Daniel P. Berrangé <berrange@redhat.com>
>Subject: Re: [PATCH rfcv3 07/11] qemu: add hard reboot in QEMU driver
>
>On Mon, Nov 27, 2023 at 04:55:17PM +0800, Zhenzhong Duan wrote:
>> From: Chenyi Qiang <chenyi.qiang@intel.com>
>>
>> Add the new flag
>VIR_DOMAIN_REBOOT_HARD/VIR_DOMAIN_SHUTDOWN_HARD to
>> carry out a hard reboot, which kills the QEMU process and creates a new
>> one with the same definition.
>
>AFAICT, the _HARD flags cause it to issue a QEMU monitor
>command todo an ACPI shutdown and then re-create QEMU.
>
>IOW, it seems like this is just the same as the existing
>_ACPI flags, and so I'm not sure why we need any new
>functionality at all.
>
>What is the existing ACPI shutdown & fake reboot not
>able to achieve ?

ACPI shutdown & fake reboot send below QMP command in sequence:

"system_powerdown", "system_reset", "cont".

"system_reset" QMP command isn't supported by TDX for security reasons.
So we have to kill old QEMU and re-create new one.

Thanks
Zhenzhong
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org
Re: [PATCH rfcv3 07/11] qemu: add hard reboot in QEMU driver
Posted by Daniel P. Berrangé 2 years ago
On Fri, Jan 12, 2024 at 09:20:19AM +0000, Duan, Zhenzhong wrote:
> 
> 
> >-----Original Message-----
> >From: Daniel P. Berrangé <berrange@redhat.com>
> >Subject: Re: [PATCH rfcv3 07/11] qemu: add hard reboot in QEMU driver
> >
> >On Mon, Nov 27, 2023 at 04:55:17PM +0800, Zhenzhong Duan wrote:
> >> From: Chenyi Qiang <chenyi.qiang@intel.com>
> >>
> >> Add the new flag
> >VIR_DOMAIN_REBOOT_HARD/VIR_DOMAIN_SHUTDOWN_HARD to
> >> carry out a hard reboot, which kills the QEMU process and creates a new
> >> one with the same definition.
> >
> >AFAICT, the _HARD flags cause it to issue a QEMU monitor
> >command todo an ACPI shutdown and then re-create QEMU.
> >
> >IOW, it seems like this is just the same as the existing
> >_ACPI flags, and so I'm not sure why we need any new
> >functionality at all.
> >
> >What is the existing ACPI shutdown & fake reboot not
> >able to achieve ?
> 
> ACPI shutdown & fake reboot send below QMP command in sequence:
> 
> "system_powerdown", "system_reset", "cont".
> 
> "system_reset" QMP command isn't supported by TDX for security reasons.
> So we have to kill old QEMU and re-create new one.

We are still doing the 'system_powerdown' step first though, to do a
graceful shutdown, before re-creating QEMU, and the powerdown is what
we refer to as "ACPI".

So I think we don't need new public API constants. We should still use
the _ACPI flag, and just "do the right thing" with choosing between
system_reset vs re-creating QEMU depending on whether TDX/SEV are
active.

That way the public API user experience is unchanged, only the internal
impl varies.

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org
RE: [PATCH rfcv3 07/11] qemu: add hard reboot in QEMU driver
Posted by Duan, Zhenzhong 2 years ago

>-----Original Message-----
>From: Daniel P. Berrangé <berrange@redhat.com>
>Subject: Re: [PATCH rfcv3 07/11] qemu: add hard reboot in QEMU driver
>
>On Fri, Jan 12, 2024 at 09:20:19AM +0000, Duan, Zhenzhong wrote:
>>
>>
>> >-----Original Message-----
>> >From: Daniel P. Berrangé <berrange@redhat.com>
>> >Subject: Re: [PATCH rfcv3 07/11] qemu: add hard reboot in QEMU driver
>> >
>> >On Mon, Nov 27, 2023 at 04:55:17PM +0800, Zhenzhong Duan wrote:
>> >> From: Chenyi Qiang <chenyi.qiang@intel.com>
>> >>
>> >> Add the new flag
>> >VIR_DOMAIN_REBOOT_HARD/VIR_DOMAIN_SHUTDOWN_HARD to
>> >> carry out a hard reboot, which kills the QEMU process and creates a
>new
>> >> one with the same definition.
>> >
>> >AFAICT, the _HARD flags cause it to issue a QEMU monitor
>> >command todo an ACPI shutdown and then re-create QEMU.
>> >
>> >IOW, it seems like this is just the same as the existing
>> >_ACPI flags, and so I'm not sure why we need any new
>> >functionality at all.
>> >
>> >What is the existing ACPI shutdown & fake reboot not
>> >able to achieve ?
>>
>> ACPI shutdown & fake reboot send below QMP command in sequence:
>>
>> "system_powerdown", "system_reset", "cont".
>>
>> "system_reset" QMP command isn't supported by TDX for security
>reasons.
>> So we have to kill old QEMU and re-create new one.
>
>We are still doing the 'system_powerdown' step first though, to do a
>graceful shutdown, before re-creating QEMU, and the powerdown is what
>we refer to as "ACPI".
>
>So I think we don't need new public API constants. We should still use
>the _ACPI flag, and just "do the right thing" with choosing between
>system_reset vs re-creating QEMU depending on whether TDX/SEV are
>active.

Get your point.

>
>That way the public API user experience is unchanged, only the internal
>impl varies.

Yes, really good suggestion, I'll follow this way in next version.

Thanks
Zhenzhong
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org