Re: [Xen-devel] [libvirt] [PATCH 1/3] libxl: Move job acquisition in libxlDomainStart to callers

[Martin Kletzander <mkletzan@xxxxxxxxxx>]

On Wed, Mar 25, 2015 at 02:08:34PM -0600, Jim Fehlig wrote:
> Let callers of libxlDomainStart decide when it is appropriate to
> acquire a job on the associated virDomainObj.
This makes sense, I see many bugs this fixes, but how come
libxlDomainShutdownThread(), libxlDomainRestoreFlags() and
libxlDoMigrateReceive() don't need to start the job when they all call
libxlDomainStart()?

> Signed-off-by: Jim Fehlig <jfehlig@xxxxxxxx>
> ---
> src/libxl/libxl_domain.c | 24 ++++++++--------------
> src/libxl/libxl_driver.c | 53 +++++++++++++++++++++++++++++++++++++++---------
> 2 files changed, 52 insertions(+), 25 deletions(-)
>
> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
> index 05f6eb1..da4c1c7 100644
> --- a/src/libxl/libxl_driver.c
> +++ b/src/libxl/libxl_driver.c
> @@ -885,17 +893,26 @@ libxlDomainCreateXML(virConnectPtr conn, const char *xml,
>         goto cleanup;
>     def = NULL;
>
> +    if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY) < 0) {
> +        virDomainObjListRemove(driver->domains, vm);
> +        vm = NULL;
> +        goto cleanup;
> +    }
> +
This should be acquired before virDomainObjListAdd() since you need to
check whether it's active after creating the job.  If I'm wrong, then
virDomainObjListRemove() should be only called if the vm is not
persistent (as CreateXML can be called on persistent ones as well),
shouldn't it?

[...]
> @@ -1695,11 +1712,20 @@ libxlDomainRestoreFlags(virConnectPtr conn, const char 
> *from,
>
>     def = NULL;
>
> +    if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY) < 0) {
> +        if (!vm->persistent) {
> +            virDomainObjListRemove(driver->domains, vm);
> +            vm = NULL;
> +        }
> +        goto cleanup;
> +    }
> +
Same here, I guess.

>     ret = libxlDomainStart(driver, vm, (flags & VIR_DOMAIN_SAVE_PAUSED) != 0, 
> fd);
> -    if (ret < 0 && !vm->persistent) {
> +    if (ret < 0 && !vm->persistent)
>         virDomainObjListRemove(driver->domains, vm);
> +
> +    if (!libxlDomainObjEndJob(driver, vm))
>         vm = NULL;
> -    }
>
>  cleanup:
>     if (VIR_CLOSE(fd) < 0)
> @@ -2567,17 +2593,24 @@ libxlDomainCreateWithFlags(virDomainPtr dom,
>     if (virDomainCreateWithFlagsEnsureACL(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 already running"));
> -        goto cleanup;
> +        goto endjob;
>     }
>
>     ret = libxlDomainStart(driver, vm, (flags & VIR_DOMAIN_START_PAUSED) != 0, 
> -1);
>     if (ret < 0)
> -        goto cleanup;
> +        goto endjob;
>     dom->id = vm->def->id;
>
> + endjob:
> +    if (!libxlDomainObjEndJob(driver, vm))
> +        vm = NULL;
> +
>  cleanup:
>     if (vm)
>         virObjectUnlock(vm);
> --
> 1.8.4.5
>
> --
> libvir-list mailing list
> libvir-list@xxxxxxxxxx
> https://www.redhat.com/mailman/listinfo/libvir-list

Attachment: pgpn8c39zfTOY.pgp
Description: PGP signature

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel