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

[Michal Privoznik]

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 at linux.vnet.ibm.com>
> Signed-off-by: Daniel Henrique Barboza <danielhb413 at gmail.com>
> ---
>   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