11:55 a.m.
Currently, qemuDomainShutdownFlags() chooses the agent method of
shutdown whenever the agent is configured. However, this
assumption is not enough as the guest agent may be unresponsive
at the moment. So unless guest agent method has been explicitly
requested, we should fall back to the ACPI method.
---
src/qemu/qemu_driver.c | 34 ++++++++++++++++++++++++----------
1 file changed, 24 insertions(+), 10 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 435c37c..06b14ae 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -1702,7 +1702,7 @@ static int qemuDomainShutdownFlags(virDomainPtr dom, unsigned int
flags) {
virDomainObjPtr vm;
int ret = -1;
qemuDomainObjPrivatePtr priv;
- bool useAgent = false;
+ bool useAgent = false, agentRequested;
virCheckFlags(VIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN |
VIR_DOMAIN_SHUTDOWN_GUEST_AGENT, -1);
@@ -1719,23 +1719,32 @@ static int qemuDomainShutdownFlags(virDomainPtr dom, unsigned int
flags) {
goto cleanup;
priv = vm->privateData;
+ agentRequested = flags & VIR_DOMAIN_SHUTDOWN_GUEST_AGENT;
- if ((flags & VIR_DOMAIN_SHUTDOWN_GUEST_AGENT) ||
+ if (agentRequested ||
(!(flags & VIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN) &&
priv->agent))
useAgent = true;
if (useAgent) {
if (priv->agentError) {
- virReportError(VIR_ERR_AGENT_UNRESPONSIVE, "%s",
- _("QEMU guest agent is not "
- "available due to an error"));
- goto cleanup;
+ if (agentRequested) {
+ virReportError(VIR_ERR_AGENT_UNRESPONSIVE, "%s",
+ _("QEMU guest agent is not "
+ "available due to an error"));
+ goto cleanup;
+ } else {
+ useAgent = false;
+ }
}
if (!priv->agent) {
- virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
- _("QEMU guest agent is not configured"));
- goto cleanup;
+ if (agentRequested) {
+ virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
+ _("QEMU guest agent is not configured"));
+ goto cleanup;
+ } else {
+ useAgent = false;
+ }
}
}
@@ -1752,7 +1761,12 @@ static int qemuDomainShutdownFlags(virDomainPtr dom, unsigned int
flags) {
qemuDomainObjEnterAgent(vm);
ret = qemuAgentShutdown(priv->agent, QEMU_AGENT_SHUTDOWN_POWERDOWN);
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) && !agentRequested)) {
qemuDomainSetFakeReboot(driver, vm, false);
qemuDomainObjEnterMonitor(driver, vm);
--
1.8.1.4
12:33 p.m.
On 02/25/2013 10:55 AM, Michal Privoznik wrote:
Currently, qemuDomainShutdownFlags() chooses the agent method of
shutdown whenever the agent is configured. However, this
assumption is not enough as the guest agent may be unresponsive
at the moment. So unless guest agent method has been explicitly
requested, we should fall back to the ACPI method.
---
src/qemu/qemu_driver.c | 34 ++++++++++++++++++++++++----------
1 file changed, 24 insertions(+), 10 deletions(-)
When Daniel added two new shutdown methods for LXC, I had the question
on whether shutdown methods should be mutually exclusive, or a bitmask
of all permissible things to attempt (where passing 0 leaves the
attempts up the hypervisor). The consensus was that allowing the user
to pass more than one method makes sense (although the hypervisor then
gets to choose method priorities). I'm not sure your logic matches the
goal of allowing the user to request both agent and acpi at the same time.
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 435c37c..06b14ae 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -1702,7 +1702,7 @@ static int qemuDomainShutdownFlags(virDomainPtr dom, unsigned int
flags) {
virDomainObjPtr vm;
int ret = -1;
qemuDomainObjPrivatePtr priv;
- bool useAgent = false;
+ bool useAgent = false, agentRequested;
virCheckFlags(VIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN |
VIR_DOMAIN_SHUTDOWN_GUEST_AGENT, -1);
@@ -1719,23 +1719,32 @@ static int qemuDomainShutdownFlags(virDomainPtr dom, unsigned int
flags) {
goto cleanup;
priv = vm->privateData;
+ agentRequested = flags & VIR_DOMAIN_SHUTDOWN_GUEST_AGENT;
- if ((flags & VIR_DOMAIN_SHUTDOWN_GUEST_AGENT) ||
+ if (agentRequested ||
(!(flags & VIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN) &&
priv->agent))
useAgent = true;
A better logic might be:
/* Explicitly requested agent, or no requests made and agent seems
possible */
if (agentRequested ||
(!flags && priv->agent))
if (useAgent) {
if (priv->agentError) {
- virReportError(VIR_ERR_AGENT_UNRESPONSIVE, "%s",
- _("QEMU guest agent is not "
- "available due to an error"));
- goto cleanup;
+ if (agentRequested) {
+ virReportError(VIR_ERR_AGENT_UNRESPONSIVE, "%s",
+ _("QEMU guest agent is not "
+ "available due to an error"));
+ goto cleanup;
+ } else {
+ useAgent = false;
+ }
}
Here, if there is an agent error, but the user requested both agent and
acpi flags at the same time, then you want to fall back to acpi instead
of erroring out completely.
if (!priv->agent) {
- virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
- _("QEMU guest agent is not configured"));
- goto cleanup;
+ if (agentRequested) {
+ virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
+ _("QEMU guest agent is not configured"));
+ goto cleanup;
+ } else {
+ useAgent = false;
+ }
}
}
@@ -1752,7 +1761,12 @@ static int qemuDomainShutdownFlags(virDomainPtr dom, unsigned int
flags) {
qemuDomainObjEnterAgent(vm);
ret = qemuAgentShutdown(priv->agent, QEMU_AGENT_SHUTDOWN_POWERDOWN);
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) && !agentRequested)) {
Redundant () around ret<0.
Here, the fallback seems like it should be:
/* Get here if agent request failed or was not attempted. Try acpi
unless it was excluded from an explicit request */
if (ret < 0 &&
(!flags || (flags & VIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN))) {
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
1:54 a.m.
On 25.02.2013 19:33, Eric Blake wrote:
On 02/25/2013 10:55 AM, Michal Privoznik wrote:
> Currently, qemuDomainShutdownFlags() chooses the agent method of
> shutdown whenever the agent is configured. However, this
> assumption is not enough as the guest agent may be unresponsive
> at the moment. So unless guest agent method has been explicitly
> requested, we should fall back to the ACPI method.
> ---
> src/qemu/qemu_driver.c | 34 ++++++++++++++++++++++++----------
> 1 file changed, 24 insertions(+), 10 deletions(-)
When Daniel added two new shutdown methods for LXC, I had the question
on whether shutdown methods should be mutually exclusive, or a bitmask
of all permissible things to attempt (where passing 0 leaves the
attempts up the hypervisor). The consensus was that allowing the user
to pass more than one method makes sense (although the hypervisor then
gets to choose method priorities). I'm not sure your logic matches the
goal of allowing the user to request both agent and acpi at the same time.
Aah. Okay; I thought they were considered as mutually exclusive. Hence
my code is not correct. Let me re-spin v2.
Michal
4287
days inactive
4288
days old
2 comments
2 participants
participants (2)
-
Eric Blake -
Michal Privoznik