Re: [PATCH 06/11] libxl: use job functions in domain save operations

[Michal Privoznik <mprivozn@xxxxxxxxxx>]

On 07.02.2014 04:53, Jim Fehlig wrote:
> Saving domain memory and cpu state can take considerable time.
> Use the recently added job functions and unlock the virDomainObj
> while saving the domain.
>
> Signed-off-by: Jim Fehlig <jfehlig@xxxxxxxx>
> ---
>   src/libxl/libxl_driver.c | 54 ++++++++++++++++++++++++++++++++++--------------
>   1 file changed, 39 insertions(+), 15 deletions(-)
>
> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
> index bb574bc..a804b45 100644
> --- a/src/libxl/libxl_driver.c
> +++ b/src/libxl/libxl_driver.c
> @@ -1900,10 +1900,16 @@ libxlDoDomainSave(libxlDriverPrivatePtr driver, virDomainObjPtr vm,
>           goto cleanup;
>       }
>
> -    if (libxl_domain_suspend(priv->ctx, vm->def->id, fd, 0, NULL) != 0) {
> +    /* Unlock virDomainObj while saving domain */
> +    virObjectUnlock(vm);
> +    ret = libxl_domain_suspend(priv->ctx, vm->def->id, fd, 0, NULL);
> +    virObjectLock(vm);
> +
> +    if (ret != 0) {
>           virReportError(VIR_ERR_INTERNAL_ERROR,
>                          _("Failed to save domain '%d' with libxenlight"),
>                          vm->def->id);
> +        ret = -1;
>           goto cleanup;
>       }
>
> @@ -1935,6 +1941,7 @@ libxlDomainSaveFlags(virDomainPtr dom, const char *to, const char *dxml,
>       libxlDriverPrivatePtr driver = dom->conn->privateData;
>       virDomainObjPtr vm;
>       int ret = -1;
> +    bool remove_dom = false;
>
>       virCheckFlags(0, -1);
>       if (dxml) {
> @@ -1949,22 +1956,30 @@ libxlDomainSaveFlags(virDomainPtr dom, const char *to, const char *dxml,
>       if (virDomainSaveFlagsEnsureACL(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 cleanup;
> +        goto endjob;
>       }
>
>       if (libxlDoDomainSave(driver, vm, to) < 0)
> -        goto cleanup;
> +        goto endjob;
>
> -    if (!vm->persistent) {
> -        virDomainObjListRemove(driver->domains, vm);
> -        vm = NULL;
> -    }
> +    if (!vm->persistent)
> +        remove_dom = true;
>
>       ret = 0;
>
> +endjob:
> +    libxlDomainObjEndJob(driver, vm);
> +
>   cleanup:
> +    if (remove_dom) {
> +        virDomainObjListRemove(driver->domains, vm);
> +        vm = NULL;
> +    }
>       if (vm)
>           virObjectUnlock(vm);
>       return ret;
> @@ -2125,6 +2140,7 @@ libxlDomainManagedSave(virDomainPtr dom, unsigned int flags)
>       virDomainObjPtr vm = NULL;
>       char *name = NULL;
>       int ret = -1;
> +    bool remove_dom = false;
>
>       virCheckFlags(0, -1);
>
> @@ -2134,33 +2150,41 @@ libxlDomainManagedSave(virDomainPtr dom, unsigned int flags)
>       if (virDomainManagedSaveEnsureACL(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 cleanup;
> +        goto endjob;
>       }
>       if (!vm->persistent) {
>           virReportError(VIR_ERR_OPERATION_INVALID, "%s",
>                          _("cannot do managed save for transient domain"));
> -        goto cleanup;
> +        goto endjob;
>       }
>
>       name = libxlDomainManagedSavePath(driver, vm);
>       if (name == NULL)
> -        goto cleanup;
> +        goto endjob;
>
>       VIR_INFO("Saving state to %s", name);
>
>       if (libxlDoDomainSave(driver, vm, name) < 0)
> -        goto cleanup;
> +        goto endjob;
>
> -    if (!vm->persistent) {
> -        virDomainObjListRemove(driver->domains, vm);
> -        vm = NULL;
> -    }
> +    if (!vm->persistent)
> +        remove_dom = true;
>
>       ret = 0;
>
> +endjob:
> +    libxlDomainObjEndJob(driver, vm);
> +
>   cleanup:
> +    if (remove_dom) {
> +        virDomainObjListRemove(driver->domains, vm);
> +        vm = NULL;
> +    }
>       if (vm)
>           virObjectUnlock(vm);
>       VIR_FREE(name);

Conditional ACK

Michal

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