Re: [libvirt] [PATCH v2] qemu: Don't fail to shutdown domains with unresponsive agent
On 28.02.2013 02:13, Eric Blake wrote: On 02/27/2013 03:30 AM, Michal Privoznik wrote: On 27.02.2013 00:22, Eric Blake wrote: On 02/26/2013 04:02 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. --- diff to v1: - Rework some conditions as Eric suggested in v1 src/qemu/qemu_driver.c | 38 ++ 1 file changed, 22 insertions(+), 16 deletions(-) ACK. Do you think this one is safe to push now? It is a bug fix and I think the code is well understood. However, the issue it's fixing doesn't seem to be so usual to hit. Otherwise there would be a bug report much sooner than 2012-12-22, right? https://bugzilla.redhat.com/show_bug.cgi?id=889635 I've hit the bug myself, but never filed a BZ - it's quite annoying that the shutdown button in virt-manager fails to work if you have the guest agent wired up in the host XML, but not installed and running in the guest, at which point you can no longer shut down the guest using virt-manager. Yes, I think this is safe to push for 1.0.3. Okay. Pushed. Thanks. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] qemu: Don't fail to shutdown domains with unresponsive agent
On 27.02.2013 00:22, Eric Blake wrote: On 02/26/2013 04:02 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. --- diff to v1: - Rework some conditions as Eric suggested in v1 src/qemu/qemu_driver.c | 38 ++ 1 file changed, 22 insertions(+), 16 deletions(-) ACK. Do you think this one is safe to push now? It is a bug fix and I think the code is well understood. However, the issue it's fixing doesn't seem to be so usual to hit. Otherwise there would be a bug report much sooner than 2012-12-22, right? https://bugzilla.redhat.com/show_bug.cgi?id=889635 Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] qemu: Don't fail to shutdown domains with unresponsive agent
On 02/27/2013 03:30 AM, Michal Privoznik wrote: On 27.02.2013 00:22, Eric Blake wrote: On 02/26/2013 04:02 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. --- diff to v1: - Rework some conditions as Eric suggested in v1 src/qemu/qemu_driver.c | 38 ++ 1 file changed, 22 insertions(+), 16 deletions(-) ACK. Do you think this one is safe to push now? It is a bug fix and I think the code is well understood. However, the issue it's fixing doesn't seem to be so usual to hit. Otherwise there would be a bug report much sooner than 2012-12-22, right? https://bugzilla.redhat.com/show_bug.cgi?id=889635 I've hit the bug myself, but never filed a BZ - it's quite annoying that the shutdown button in virt-manager fails to work if you have the guest agent wired up in the host XML, but not installed and running in the guest, at which point you can no longer shut down the guest using virt-manager. Yes, I think this is safe to push for 1.0.3. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2] qemu: Don't fail to shutdown domains with unresponsive agent
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.
---
diff to v1:
- Rework some conditions as Eric suggested in v1
src/qemu/qemu_driver.c | 38 ++
1 file changed, 22 insertions(+), 16 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 1e96915..1e96aa4 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -1702,40 +1702,40 @@ static int qemuDomainShutdownFlags(virDomainPtr dom,
unsigned int flags) {
virDomainObjPtr vm;
int ret = -1;
qemuDomainObjPrivatePtr priv;
-bool useAgent = false;
+bool useAgent = false, agentRequested, acpiRequested;
virCheckFlags(VIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN |
VIR_DOMAIN_SHUTDOWN_GUEST_AGENT, -1);
-/* At most one of these two flags should be set. */
-if ((flags VIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN)
-(flags VIR_DOMAIN_SHUTDOWN_GUEST_AGENT)) {
-virReportInvalidArg(flags, %s,
-_(flags for acpi power button and guest agent are
mutually exclusive));
-return -1;
-}
-
if (!(vm = qemuDomObjFromDomain(dom)))
goto cleanup;
priv = vm-privateData;
+agentRequested = flags VIR_DOMAIN_SHUTDOWN_GUEST_AGENT;
+acpiRequested = flags VIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN;
-if ((flags VIR_DOMAIN_SHUTDOWN_GUEST_AGENT) ||
-(!(flags VIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN)
- priv-agent))
+/* Prefer agent unless we were requested to not to. */
+if (agentRequested || (!flags priv-agent))
useAgent = true;
-if (useAgent) {
-if (priv-agentError) {
+if (priv-agentError) {
+if (agentRequested !acpiRequested) {
virReportError(VIR_ERR_AGENT_UNRESPONSIVE, %s,
_(QEMU guest agent is not
available due to an error));
goto cleanup;
+} else {
+useAgent = false;
}
-if (!priv-agent) {
+}
+
+if (!priv-agent) {
+if (agentRequested !acpiRequested) {
virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, %s,
_(QEMU guest agent is not configured));
goto cleanup;
+} else {
+useAgent = false;
}
}
@@ -1752,7 +1752,13 @@ 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 (acpiRequested || !flags))) {
qemuDomainSetFakeReboot(driver, vm, false);
qemuDomainObjEnterMonitor(driver, vm);
--
1.8.1.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] qemu: Don't fail to shutdown domains with unresponsive agent
On 02/26/2013 04:02 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. --- diff to v1: - Rework some conditions as Eric suggested in v1 src/qemu/qemu_driver.c | 38 ++ 1 file changed, 22 insertions(+), 16 deletions(-) ACK. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
