[libvirt] [PATCH 2/3] libxl: implement virDomainPM* functions

Jim Fehlig jfehlig at suse.com
Thu Aug 9 22:13:49 UTC 2018


On 08/05/2018 05:01 PM, Marek Marczykowski-Górecki wrote:
> Signed-off-by: Marek Marczykowski-Górecki <marmarek at invisiblethingslab.com>
> ---
>   src/libxl/libxl_driver.c | 126 ++++++++++++++++++++++++++++++++++++++++-
>   1 file changed, 126 insertions(+)
> 
> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
> index 5a5e792..10c7aab 100644
> --- a/src/libxl/libxl_driver.c
> +++ b/src/libxl/libxl_driver.c
> @@ -1403,6 +1403,128 @@ libxlDomainDestroy(virDomainPtr dom)
>       return libxlDomainDestroyFlags(dom, 0);
>   }
>   
> +#ifdef LIBXL_HAVE_DOMAIN_SUSPEND_ONLY
> +static int
> +libxlDomainPMSuspendForDuration(virDomainPtr dom,
> +                                unsigned int target,
> +                                unsigned long long duration,
> +                                unsigned int flags)
> +{
> +    virDomainObjPtr vm;
> +    int ret = -1;
> +    libxlDriverPrivatePtr driver = dom->conn->privateData;
> +    libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver);
> +
> +    virCheckFlags(0, -1);
> +    if (target != VIR_NODE_SUSPEND_TARGET_MEM) {
> +        virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED,
> +                _("PMSuspend type %d not supported by libxenlight driver"),
> +                target);
> +        return -1;
> +    }
> +
> +    if (duration != 0) {
> +        virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
> +                _("libxenlight driver supports only duration=0"));
> +        return -1;
> +    }
> +
> +    if (!(vm = libxlDomObjFromDomain(dom)))
> +        goto cleanup;
> +
> +    if (virDomainPMSuspendForDurationEnsureACL(dom->conn, vm->def) < 0)
> +        goto cleanup;
> +
> +    if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY) < 0)
> +        goto cleanup;
> +
> +    if (!virDomainObjIsActive(vm)) {
> +        virReportError(VIR_ERR_OPERATION_INVALID,
> +                       "%s", _("Domain is not running"));
> +        goto endjob;
> +    }

Error is now reported in virDomainObjIsActive, so this can be simplified to

     if (virDomainObjCheckActive(vm) < 0)
         goto endjob;

> +
> +    /* Unlock virDomainObjPtr to not deadlock with even handler, which will try
> +     * to send lifecycle event
> +     */
> +    virObjectUnlock(vm);
> +    ret = libxl_domain_suspend_only(cfg->ctx, vm->def->id, NULL);
> +    virObjectLock(vm);
> +
> +    if (ret < 0) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR,
> +                       _("Failed to suspend domain '%d'"), vm->def->id);
> +        goto endjob;
> +    }
> +
> +    ret = 0;
> +
> + endjob:
> +    libxlDomainObjEndJob(driver, vm);
> +
> + cleanup:
> +    if (vm)
> +        virObjectUnlock(vm);

We now use virDomainObjEndAPI(&vm).

> +    return ret;
> +}
> +#endif
> +
> +static int
> +libxlDomainPMWakeup(virDomainPtr dom,
> +                        unsigned int flags)

Whitespace off a bit. Could probably be moved to previous line.

> +{
> +    libxlDriverPrivatePtr driver = dom->conn->privateData;
> +    virDomainObjPtr vm;
> +    int ret = -1;
> +    virObjectEventPtr event = NULL;
> +    libxlDomainObjPrivatePtr priv;
> +    libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver);
> +
> +    virCheckFlags(0, -1);
> +
> +    if (!(vm = libxlDomObjFromDomain(dom)))
> +        goto cleanup;
> +
> +    if (virDomainPMWakeupEnsureACL(dom->conn, vm->def) < 0)
> +        goto cleanup;
> +
> +    if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY) < 0)
> +        goto cleanup;
> +
> +    if (virDomainObjGetState(vm, NULL) != VIR_DOMAIN_PMSUSPENDED) {
> +        virReportError(VIR_ERR_OPERATION_INVALID,
> +                       "%s", _("Domain is not suspended"));
> +        goto endjob;
> +    }
> +
> +    event = virDomainEventLifecycleNewFromObj(vm, VIR_DOMAIN_EVENT_STARTED,
> +                                     VIR_DOMAIN_EVENT_STARTED_WAKEUP);
> +
> +    priv = vm->privateData;
> +    if (libxl_domain_resume(cfg->ctx, vm->def->id, 1, NULL) < 0) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR,
> +                       _("Failed to resume domain '%d'"), vm->def->id);
> +        goto endjob;
> +    }
> +    virDomainObjSetState(vm, VIR_DOMAIN_RUNNING, VIR_DOMAIN_RUNNING_WAKEUP);
> +    /* reenable death event - libxl reports it only once */
> +    if (priv->deathW)
> +        libxl_evdisable_domain_death(cfg->ctx, priv->deathW);
> +    if (libxl_evenable_domain_death(cfg->ctx, vm->def->id, 0, &priv->deathW))
> +        goto endjob;

Is returning failure the right thing to do here? Should we kill off the domain 
first? In libxlDomainStart we destroy the domain if enabling death events fails.

> +
> +    ret = 0;
> +
> + endjob:
> +    libxlDomainObjEndJob(driver, vm);
> +
> + cleanup:
> +    if (vm)
> +        virObjectUnlock(vm);

virDomainObjEndAPI(&vm);

> +    virObjectEventStateQueue(driver->domainEventState, event);
> +    return ret;
> +}
> +
>   static char *
>   libxlDomainGetOSType(virDomainPtr dom)
>   {
> @@ -6385,6 +6507,10 @@ static virHypervisorDriver libxlHypervisorDriver = {
>       .domainReboot = libxlDomainReboot, /* 0.9.0 */
>       .domainDestroy = libxlDomainDestroy, /* 0.9.0 */
>       .domainDestroyFlags = libxlDomainDestroyFlags, /* 0.9.4 */
> +#ifdef LIBXL_HAVE_DOMAIN_SUSPEND_ONLY
> +    .domainPMSuspendForDuration = libxlDomainPMSuspendForDuration, /* 4.7.0 */
> +#endif
> +    .domainPMWakeup = libxlDomainPMWakeup, /* 4.7.0 */

Is there a reason to have domainPMWakeup without domainPMSuspendForDuration? I'd 
think both of these should be conditional on LIBXL_HAVE_DOMAIN_SUSPEND_ONLY.

Regards,
Jim

>       .domainGetOSType = libxlDomainGetOSType, /* 0.9.0 */
>       .domainGetMaxMemory = libxlDomainGetMaxMemory, /* 0.9.0 */
>       .domainSetMaxMemory = libxlDomainSetMaxMemory, /* 0.9.2 */
> 




More information about the libvir-list mailing list