On 05.06.2019 12:09, Michal Privoznik wrote:
> The reasoning here is the same as in qemu driver fixed in
> previous commit. Long story short, changing an XML of a domain
> requires modify job to be acquired.
>
> Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx>
> ---
> src/libxl/libxl_domain.c | 57 +++++++++++++++++++++++++++++++++++++
> src/libxl/libxl_domain.h | 7 +++++
> src/libxl/libxl_driver.c | 24 ++++------------
> src/libxl/libxl_migration.c | 14 +++------
> 4 files changed, 73 insertions(+), 29 deletions(-)
>
> diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
> index 2d8569e592..f2e1af52e5 100644
> --- a/src/libxl/libxl_domain.c
> +++ b/src/libxl/libxl_domain.c
> @@ -65,6 +65,63 @@ libxlDomainObjPrivateOnceInit(void)
>
> VIR_ONCE_GLOBAL_INIT(libxlDomainObjPrivate);
>
> +
> +/**
> + * libxlDomainObjListAdd:
> + * @driver: libxl driver
> + * @def: domain definition
> + * @oldDef: previous domain definition
> + * @live: whether @def is live definition
> + * @flags: an bitwise-OR of virDomainObjListAdd flags
> + *
> + * Add a domain onto the list of domain object and sets its
> + * definition. If @oldDef is not NULL and there was pre-existing
> + * definition it's returned in @oldDef.
> + *
> + * In addition to that, if definition of an existing domain is
> + * changed a MODIFY job is acquired prior to that.
> + *
> + * Returns: domain object pointer on success,
> + * NULL otherwise.
> + */
> +virDomainObjPtr
> +libxlDomainObjListAdd(libxlDriverPrivatePtr driver,
> + virDomainDefPtr def,
> + virDomainDefPtr *oldDef,
> + bool live,
> + unsigned int flags)
> +{
> + virDomainObjPtr vm = NULL;
> +
> + if (!(vm = virDomainObjListAdd(driver->domains, def, driver->xmlopt, flags)))
> + return NULL;
> +
> + /* At this point, @vm is locked. If it doesn't have any
> + * definition set, then the call above just added it and
> + * there can't be anybody else using the object. It's safe to
> + * just set the definition without acquiring job. */
> + if (!vm->def) {
> + virDomainObjAssignDef(vm, def, live, oldDef);
> + VIR_RETURN_PTR(vm);
> + }
> +
> + /* Bad luck. Domain was pre-existing and this call is trying
> + * to update its definition. Modify job MUST be acquired. */
> + if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY) < 0) {
> + if (!vm->persistent)
> + virDomainObjListRemove(driver->domains, vm);
> + virDomainObjEndAPI(&vm);
> + return NULL;
> + }
> +
> + virDomainObjAssignDef(vm, def, live, oldDef);
> +
> + libxlDomainObjEndJob(driver, vm);
> +
> + return vm;
> +}
> +
> +
> static int
> libxlDomainObjInitJob(libxlDomainObjPrivatePtr priv)
> {
> diff --git a/src/libxl/libxl_domain.h b/src/libxl/libxl_domain.h
> index 769ee8ec4d..b5d5127e91 100644
> --- a/src/libxl/libxl_domain.h
> +++ b/src/libxl/libxl_domain.h
> @@ -83,6 +83,13 @@ extern const struct libxl_event_hooks ev_hooks;
> int
> libxlDomainObjPrivateInitCtx(virDomainObjPtr vm);
>
> +virDomainObjPtr
> +libxlDomainObjListAdd(libxlDriverPrivatePtr driver,
> + virDomainDefPtr def,
> + virDomainDefPtr *oldDef,
> + bool live,
> + unsigned int flags);
> +
> int
> libxlDomainObjBeginJob(libxlDriverPrivatePtr driver,
> virDomainObjPtr obj,
> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
> index 809d298ac1..9c9a30bb90 100644
> --- a/src/libxl/libxl_driver.c
> +++ b/src/libxl/libxl_driver.c
> @@ -601,12 +601,8 @@ libxlAddDom0(libxlDriverPrivatePtr driver)
> if (virUUIDParse("00000000-0000-0000-0000-000000000000", def->uuid) < 0)
> goto cleanup;
>
> - if (!(vm = virDomainObjListAdd(driver->domains, def,
> - driver->xmlopt,
> - 0)))
> + if (!(vm = libxlDomainObjListAdd(driver, def, &oldDef, false, 0)))
> goto cleanup;
> -
> - virDomainObjAssignDef(vm, def, false, &oldDef);
> def = NULL;
>
> vm->persistent = 1;
> @@ -1030,12 +1026,9 @@ libxlDomainCreateXML(virConnectPtr conn, const char *xml,
> if (virDomainCreateXMLEnsureACL(conn, def) < 0)
> goto cleanup;
>
> - if (!(vm = virDomainObjListAdd(driver->domains, def,
> - driver->xmlopt,
> - VIR_DOMAIN_OBJ_LIST_ADD_CHECK_LIVE)))
> + if (!(vm = libxlDomainObjListAdd(driver, def, NULL, true,
> + VIR_DOMAIN_OBJ_LIST_ADD_CHECK_LIVE)))
> goto cleanup;
> -
> - virDomainObjAssignDef(vm, def, true, NULL);
> def = NULL;
>
> if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY) < 0) {
> @@ -1950,12 +1943,9 @@ libxlDomainRestoreFlags(virConnectPtr conn, const char *from,
> if (virDomainRestoreFlagsEnsureACL(conn, def) < 0)
> goto cleanup;
>
> - if (!(vm = virDomainObjListAdd(driver->domains, def,
> - driver->xmlopt,
> + if (!(vm = libxlDomainObjListAdd(driver, def, NULL, true,
> VIR_DOMAIN_OBJ_LIST_ADD_CHECK_LIVE)))
just indentation, so
Reviewed-by: Nikolay Shirokovskiy <nshirokovskiy@xxxxxxxxxxxxx>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
