[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 02/11] libxl: use job functions in libxlVmStart



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 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


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]