Re: [PATCH v2 3/3] qemuDomainPMSuspendForDuration: check for QEMU_CAPS_PM_WAKEUP_SUPPORT

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 4/9/19 6:18 PM, Daniel Henrique Barboza wrote:
If the current QEMU guest can't wake up from suspend properly,
avoid suspending the guest at all. This is done by checking the
QEMU_CAPS_PM_WAKEUP_SUPPORT cap.

The absence of the cap indicates that we're dealing with a QEMU
version older than 4.0 (which implements the required QMP API).
In this case, proceed as usual with the suspend logic since
we can't assume whether the guest has support or not.

This is the output of dompmsuspend in a guest that does not
have wake-up support declared in the query-current-machine:

$ sudo ./run tools/virsh dompmsuspend ub1810-noACPI3 mem
error: Domain ub1810-noACPI3 could not be suspended
error: this function is not supported by the connection driver: Domain does not have suspend support

Fixes: https://bugs.launchpad.net/ubuntu/+source/qemu/+bug/1759509
Reported-by: Balamuruhan S <bala24@xxxxxxxxxxxxxxxxxx>
Signed-off-by: Daniel Henrique Barboza <danielhb413@xxxxxxxxx>
---
  src/qemu/qemu_driver.c | 21 +++++++++++++++++++++
  1 file changed, 21 insertions(+)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 7e5bbc3cc9..6ee1247c7b 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -19152,6 +19152,7 @@ qemuDomainPMSuspendForDuration(virDomainPtr dom,
      virQEMUDriverPtr driver = dom->conn->privateData;
      virDomainObjPtr vm;
      qemuAgentPtr agent;
+    qemuDomainObjPrivatePtr priv;
      int ret = -1;
virCheckFlags(0, -1);
@@ -19174,6 +19175,26 @@ qemuDomainPMSuspendForDuration(virDomainPtr dom,
      if (!(vm = qemuDomObjFromDomain(dom)))
          goto cleanup;
+ priv = vm->privateData;
+
+    /*
+     * We can't check just for QEMU_CAPS_WAKEUP_SUSPEND_SUPPORT because,
+     * in case this cap is disabled, it is not possible to tell if the guest
+     * does not have wake-up from suspend support or if the current QEMU
+     * instance does not have the API.
+     *
+     * The case we want to handle here is when QEMU has the API and
+     * QEMU_CAPS_WAKEUP_SUSPEND_SUPPORT cap is disabled. Otherwise, do
+     * not interfere with the suspend process.
+     */
+    if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_QUERY_CURRENT_MACHINE) &&
+        !virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_PM_WAKEUP_SUPPORT)) {
+
+        virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
+                       _("Domain does not have suspend support"));
+        goto cleanup;
+    }
+
      if (virDomainPMSuspendForDurationEnsureACL(dom->conn, vm->def) < 0)
          goto cleanup;

See this EnsureACL() call? It has to be done before this caps check you're introducing. The reason is that if there is an ACL rule that prohibits access to a domain, then this would leak info on it. For instance, instead of "no such domain" or "no perms for this domain" a malicious user would see "domain does not have suspend support" so he/she would know the domain is there and that it doesn't have suspend support.

Long story short, this check of yours needs to be placed after the ACL check.

Michal

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux