[libvirt] [PATCH 02/11] libxl: use job functions in libxlVmStart
Michal Privoznik
mprivozn at redhat.com
Tue Feb 11 14:36:27 UTC 2014
On 07.02.2014 04:53, Jim Fehlig wrote:
> Creating a large domain could potentially be time consuming. Use the
> recently added job functions and unlock the virDomainObj while
> the create operation is in progress.
>
> Signed-off-by: Jim Fehlig <jfehlig at suse.com>
> ---
> src/libxl/libxl_driver.c | 59 ++++++++++++++++++++++++++----------------------
> 1 file changed, 32 insertions(+), 27 deletions(-)
>
> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
> index 8e4242a..7c964c5 100644
> --- a/src/libxl/libxl_driver.c
> +++ b/src/libxl/libxl_driver.c
> @@ -593,7 +593,7 @@ libxlVmStart(libxlDriverPrivatePtr driver, virDomainObjPtr vm,
> virDomainDefPtr def = NULL;
> virObjectEventPtr event = NULL;
> libxlSavefileHeader hdr;
> - int ret;
> + int ret = -1;
> uint32_t domid = 0;
> char *dom_xml = NULL;
> char *managed_save_path = NULL;
> @@ -605,14 +605,17 @@ libxlVmStart(libxlDriverPrivatePtr driver, virDomainObjPtr vm,
> #endif
>
> if (libxlDomainObjPrivateInitCtx(vm) < 0)
> - goto error;
> + goto cleanup;
> +
> + if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY) < 0)
> + goto cleanup;
>
> /* If there is a managed saved state restore it instead of starting
> * from scratch. The old state is removed once the restoring succeeded. */
> if (restore_fd < 0) {
> managed_save_path = libxlDomainManagedSavePath(driver, vm);
> if (managed_save_path == NULL)
> - goto error;
> + goto endjob;
>
> if (virFileExists(managed_save_path)) {
>
> @@ -620,7 +623,7 @@ libxlVmStart(libxlDriverPrivatePtr driver, virDomainObjPtr vm,
> managed_save_path,
> &def, &hdr);
> if (managed_save_fd < 0)
> - goto error;
> + goto endjob;
>
> restore_fd = managed_save_fd;
>
> @@ -634,7 +637,7 @@ libxlVmStart(libxlDriverPrivatePtr driver, virDomainObjPtr vm,
> _("cannot restore domain '%s' uuid %s from a file"
> " which belongs to domain '%s' uuid %s"),
> vm->def->name, vm_uuidstr, def->name, def_uuidstr);
> - goto error;
> + goto endjob;
> }
>
> virDomainObjAssignDef(vm, def, true, NULL);
> @@ -652,17 +655,17 @@ libxlVmStart(libxlDriverPrivatePtr driver, virDomainObjPtr vm,
> libxl_domain_config_init(&d_config);
>
> if (libxlBuildDomainConfig(driver, vm, &d_config) < 0)
> - goto error;
> + goto endjob;
>
> if (cfg->autoballoon && libxlFreeMem(priv, &d_config) < 0) {
> virReportError(VIR_ERR_INTERNAL_ERROR,
> _("libxenlight failed to get free memory for domain '%s'"),
> d_config.c_info.name);
> - goto error;
> + goto endjob;
> }
>
> - /* use as synchronous operations => ao_how = NULL and no intermediate reports => ao_progress = NULL */
> -
> + /* Unlock virDomainObj while creating the domain */
> + virObjectUnlock(vm);
> if (restore_fd < 0) {
> ret = libxl_domain_create_new(priv->ctx, &d_config,
> &domid, NULL, NULL);
> @@ -676,6 +679,7 @@ libxlVmStart(libxlDriverPrivatePtr driver, virDomainObjPtr vm,
> restore_fd, NULL, NULL);
> #endif
> }
> + virObjectLock(vm);
>
> if (ret) {
> if (restore_fd < 0)
> @@ -686,25 +690,25 @@ libxlVmStart(libxlDriverPrivatePtr driver, virDomainObjPtr vm,
> virReportError(VIR_ERR_INTERNAL_ERROR,
> _("libxenlight failed to restore domain '%s'"),
> d_config.c_info.name);
> - goto error;
> + goto endjob;
> }
>
> vm->def->id = domid;
> if (libxlDomEventsRegister(vm) < 0)
> - goto error;
> + goto endjob;
>
> if ((dom_xml = virDomainDefFormat(vm->def, 0)) == NULL)
> - goto error;
> + goto endjob;
>
> if (libxl_userdata_store(priv->ctx, domid, "libvirt-xml",
> (uint8_t *)dom_xml, strlen(dom_xml) + 1)) {
> virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> _("libxenlight failed to store userdata"));
> - goto error;
> + goto endjob;
> }
>
> if (libxlDomainSetVcpuAffinities(driver, vm) < 0)
> - goto error;
> + goto endjob;
>
> if (!start_paused) {
> libxl_domain_unpause(priv->ctx, domid);
> @@ -715,7 +719,7 @@ libxlVmStart(libxlDriverPrivatePtr driver, virDomainObjPtr vm,
>
>
> if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0)
> - goto error;
> + goto endjob;
>
> if (virAtomicIntInc(&driver->nactive) == 1 && driver->inhibitCallback)
> driver->inhibitCallback(true, driver->inhibitOpaque);
> @@ -727,25 +731,26 @@ libxlVmStart(libxlDriverPrivatePtr driver, virDomainObjPtr vm,
> if (event)
> libxlDomainEventQueue(driver, event);
>
> - libxl_domain_config_dispose(&d_config);
> - VIR_FREE(dom_xml);
> - VIR_FORCE_CLOSE(managed_save_fd);
> - virObjectUnref(cfg);
> - return 0;
> + ret = 0;
>
> -error:
> - if (domid > 0) {
> - libxl_domain_destroy(priv->ctx, domid, NULL);
> - vm->def->id = -1;
> - virDomainObjSetState(vm, VIR_DOMAIN_SHUTOFF, VIR_DOMAIN_SHUTOFF_FAILED);
> +endjob:
> + libxlDomainObjEndJob(driver, vm);
See my comment in the next patch where my concern about this pattern is
more visible IMO.
> +
> +cleanup:
> + if (ret) {
s/ret/ret < 0/ please
> + if (domid > 0) {
> + libxl_domain_destroy(priv->ctx, domid, NULL);
> + vm->def->id = -1;
> + virDomainObjSetState(vm, VIR_DOMAIN_SHUTOFF, VIR_DOMAIN_SHUTOFF_FAILED);
> + }
> + virDomainDefFree(def);
> }
> libxl_domain_config_dispose(&d_config);
> VIR_FREE(dom_xml);
> VIR_FREE(managed_save_path);
> - virDomainDefFree(def);
> VIR_FORCE_CLOSE(managed_save_fd);
> virObjectUnref(cfg);
> - return -1;
> + return ret;
> }
>
>
>
ACK
Michal
More information about the libvir-list
mailing list