[libvirt] [PATCH 3/3] qemuDomainPMSuspendForDuration: check for QEMU_CAPS_WAKEUP_SUSPEND_SUPPORT

[Peter Krempa]

On Mon, Apr 01, 2019 at 18:18:26 -0300, 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_WAKEUP_SUSPEND_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>
> ---
> 
> I am not sure if Libvirt uses Launchpad for bug tracking like QEMU
> does. If it's not the case, I believe the 'Fixes:' line up above can
> be ignored/deleted.
> 
> 
>  src/qemu/qemu_driver.c | 24 ++++++++++++++++++++++++
>  1 file changed, 24 insertions(+)
> 
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 62d8d977c5..9f1f170dfc 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -19209,6 +19209,8 @@ qemuDomainPMSuspendForDuration(virDomainPtr dom,
>      virQEMUDriverPtr driver = dom->conn->privateData;
>      virDomainObjPtr vm;
>      qemuAgentPtr agent;
> +    qemuDomainObjPrivatePtr priv;
> +    bool hasQueryMachineAPI = false;
>      int ret = -1;
>  
>      virCheckFlags(0, -1);
> @@ -19231,6 +19233,28 @@ 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.
> +     */
> +    hasQueryMachineAPI = virQEMUCapsGet(priv->qemuCaps,
> +                                        QEMU_CAPS_QUERY_CURRENT_MACHINE);

I don't think you need the extra variable, just put it in the single
condition.

> +    if (hasQueryMachineAPI &&
> +        !virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_WAKEUP_SUSPEND_SUPPORT)) {
> +
> +        virReportError(VIR_ERR_NO_SUPPORT, "%s",

You want to use VIR_ERR_OPERATION_UNSUPPORTED. VIR_ERR_NO_SUPPORT is
reserved for hypervisor drivers notifying that the API call is not
supported

> +                       _("Domain does not have suspend support"));
> +        goto cleanup;
> +    }
> +
>      if (virDomainPMSuspendForDurationEnsureACL(dom->conn, vm->def) < 0)
>          goto cleanup;
>  
> -- 
> 2.20.1
> 
> --
> libvir-list mailing list
> libvir-list at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20190402/aef73080/attachment-0001.sig>