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@huawei.com> Signed-off-by: Zhang Bo <oscar.zhangbo@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