[libvirt] [PATCH] qemu: Don't fail to reboot domains with unresponsive agent

[Michal Privoznik]

On 23.04.2015 05:40, zhang bo wrote:
> just as what b8e25c35d7f80a2fadc0e51e95318e39db3d1687 did,
> we fall back to the ACPI method when the guest agent is unresponsive in qemuDomainReboot.
> 
> Signed-off-by: YueWenyuan <yuewenyuan at huawei.com>
> Signed-off-by: Zhang Bo <oscar.zhangbo at huawei.com>
> 
> ---
>  src/qemu/qemu_driver.c | 67 +++++++++++++++++++++++++-------------------------
>  1 file changed, 34 insertions(+), 33 deletions(-)
> 
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 6fc9696..964a9c5 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -2002,21 +2002,14 @@ qemuDomainReboot(virDomainPtr dom, unsigned int flags)
>      virDomainObjPtr vm;
>      int ret = -1;
>      qemuDomainObjPrivatePtr priv;
> -    bool useAgent = false;
> +    bool useAgent = false, agentRequested, acpiRequested;
>      bool isReboot = true;
> +    bool agentForced;
>      int agentFlag = QEMU_AGENT_SHUTDOWN_REBOOT;
> 
>      virCheckFlags(VIR_DOMAIN_REBOOT_ACPI_POWER_BTN |
>                    VIR_DOMAIN_REBOOT_GUEST_AGENT, -1);
> 
> -    /* At most one of these two flags should be set.  */
> -    if ((flags & VIR_DOMAIN_REBOOT_ACPI_POWER_BTN) &&
> -        (flags & VIR_DOMAIN_REBOOT_GUEST_AGENT)) {
> -        virReportInvalidArg(flags, "%s",
> -                            _("flags for acpi power button and guest agent are mutually exclusive"));
> -        return -1;
> -    }
> -
>      if (!(vm = qemuDomObjFromDomain(dom)))
>          goto cleanup;
> 
> @@ -2028,38 +2021,25 @@ qemuDomainReboot(virDomainPtr dom, unsigned int flags)
>      }
> 
>      priv = vm->privateData;
> +    agentRequested = flags & VIR_DOMAIN_REBOOT_GUEST_AGENT;
> +    acpiRequested  = flags & VIR_DOMAIN_REBOOT_ACPI_POWER_BTN;
> 
>      if (virDomainRebootEnsureACL(dom->conn, vm->def, flags) < 0)
>          goto cleanup;
> 
> -    if ((flags & VIR_DOMAIN_REBOOT_GUEST_AGENT) ||
> -        (!(flags & VIR_DOMAIN_REBOOT_ACPI_POWER_BTN) &&
> -         priv->agent))
> +    /* Prefer agent unless we were requested to not to. */
> +    if (agentRequested || (!flags && priv->agent))
>          useAgent = true;

Not that it would harm or anything, but this check can be made just before checkACL() like in shutdown.

> 
> -    if (!useAgent) {
> -#if WITH_YAJL
> -        if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_MONITOR_JSON)) {
> -            if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_NO_SHUTDOWN)) {
> -                virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> -                               _("Reboot is not supported with this QEMU binary"));
> -                goto cleanup;
> -            }
> -        } else {
> -#endif
> -            virReportError(VIR_ERR_OPERATION_INVALID, "%s",
> -                           _("Reboot is not supported without the JSON monitor"));
> -            goto cleanup;
> -#if WITH_YAJL
> -        }
> -#endif
> -    }
> -
>      if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
>          goto cleanup;
> 
> -    if (useAgent && !qemuDomainAgentAvailable(vm, true))
> -        goto endjob;
> +    agentForced = agentRequested && !acpiRequested;
> +    if (useAgent && !qemuDomainAgentAvailable(vm, true)) {

Or if (!qemuDomainAgentAvailable(vm, agentForced)) {}

> +        if (agentForced)
> +            goto endjob;
> +        useAgent = false;
> +    }
> 
>      if (!virDomainObjIsActive(vm)) {
>          virReportError(VIR_ERR_OPERATION_INVALID,

Not to be seen in the context, but this is the correct place to set fakeReboot.

> @@ -2071,7 +2051,28 @@ qemuDomainReboot(virDomainPtr dom, unsigned int flags)
>          qemuDomainObjEnterAgent(vm);
>          ret = qemuAgentShutdown(priv->agent, agentFlag);
>          qemuDomainObjExitAgent(vm);
> -    } else {
> +    }
> +
> +    /* If we are not enforced to use just an agent, try ACPI
> +     * shutdown as well in case agent did not succeed.
> +     */
> +    if ((!useAgent) ||
> +        (ret < 0 && (acpiRequested || !flags))) {
> +#if WITH_YAJL
> +        if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_MONITOR_JSON)) {
> +            if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_NO_SHUTDOWN)) {
> +                virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +                               _("ACPI reboot is not supported with this QEMU binary"));
> +                goto endjob;
> +            }
> +        } else {
> +#endif
> +            virReportError(VIR_ERR_OPERATION_INVALID, "%s",
> +                           _("ACPI reboot is not supported without the JSON monitor"));
> +            goto endjob;
> +#if WITH_YAJL
> +        }
> +#endif
>          qemuDomainObjEnterMonitor(driver, vm);
>          ret = qemuMonitorSystemPowerdown(priv->mon);
>          if (qemuDomainObjExitMonitor(driver, vm) < 0)
> 

So, I'm squashing this in:

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 509ba94..82f34ec 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -2024,18 +2024,18 @@ qemuDomainReboot(virDomainPtr dom, unsigned int flags)
     agentRequested = flags & VIR_DOMAIN_REBOOT_GUEST_AGENT;
     acpiRequested  = flags & VIR_DOMAIN_REBOOT_ACPI_POWER_BTN;
 
+    /* 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;
 
-    /* Prefer agent unless we were requested to not to. */
-    if (agentRequested || (!flags && priv->agent))
-        useAgent = true;
-
     if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
         goto cleanup;
 
     agentForced = agentRequested && !acpiRequested;
-    if (useAgent && !qemuDomainAgentAvailable(vm, true)) {
+    if (!qemuDomainAgentAvailable(vm, agentForced)) {
         if (agentForced)
             goto endjob;
         useAgent = false;
@@ -2047,6 +2047,8 @@ qemuDomainReboot(virDomainPtr dom, unsigned int flags)
         goto endjob;
     }
 
+    qemuDomainSetFakeReboot(driver, vm, isReboot);
+
     if (useAgent) {
         qemuDomainObjEnterAgent(vm);
         ret = qemuAgentShutdown(priv->agent, agentFlag);
@@ -2077,9 +2079,6 @@ qemuDomainReboot(virDomainPtr dom, unsigned int flags)
         ret = qemuMonitorSystemPowerdown(priv->mon);
         if (qemuDomainObjExitMonitor(driver, vm) < 0)
             ret = -1;
-
-        if (ret == 0)
-            qemuDomainSetFakeReboot(driver, vm, isReboot);
     }
 
  endjob:

ACKing and pushing.

Michal