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

Re: [libvirt] [PATCH 2/3] libxl: implement virDomainPM* functions



On 08/05/2018 05:01 PM, Marek Marczykowski-Górecki wrote:
Signed-off-by: Marek Marczykowski-Górecki <marmarek invisiblethingslab com>
---
  src/libxl/libxl_driver.c | 126 ++++++++++++++++++++++++++++++++++++++++-
  1 file changed, 126 insertions(+)

diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 5a5e792..10c7aab 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -1403,6 +1403,128 @@ libxlDomainDestroy(virDomainPtr dom)
      return libxlDomainDestroyFlags(dom, 0);
  }
+#ifdef LIBXL_HAVE_DOMAIN_SUSPEND_ONLY
+static int
+libxlDomainPMSuspendForDuration(virDomainPtr dom,
+                                unsigned int target,
+                                unsigned long long duration,
+                                unsigned int flags)
+{
+    virDomainObjPtr vm;
+    int ret = -1;
+    libxlDriverPrivatePtr driver = dom->conn->privateData;
+    libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver);
+
+    virCheckFlags(0, -1);
+    if (target != VIR_NODE_SUSPEND_TARGET_MEM) {
+        virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED,
+                _("PMSuspend type %d not supported by libxenlight driver"),
+                target);
+        return -1;
+    }
+
+    if (duration != 0) {
+        virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
+                _("libxenlight driver supports only duration=0"));
+        return -1;
+    }
+
+    if (!(vm = libxlDomObjFromDomain(dom)))
+        goto cleanup;
+
+    if (virDomainPMSuspendForDurationEnsureACL(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 endjob;
+    }

Error is now reported in virDomainObjIsActive, so this can be simplified to

    if (virDomainObjCheckActive(vm) < 0)
        goto endjob;

+
+    /* Unlock virDomainObjPtr to not deadlock with even handler, which will try
+     * to send lifecycle event
+     */
+    virObjectUnlock(vm);
+    ret = libxl_domain_suspend_only(cfg->ctx, vm->def->id, NULL);
+    virObjectLock(vm);
+
+    if (ret < 0) {
+        virReportError(VIR_ERR_INTERNAL_ERROR,
+                       _("Failed to suspend domain '%d'"), vm->def->id);
+        goto endjob;
+    }
+
+    ret = 0;
+
+ endjob:
+    libxlDomainObjEndJob(driver, vm);
+
+ cleanup:
+    if (vm)
+        virObjectUnlock(vm);

We now use virDomainObjEndAPI(&vm).

+    return ret;
+}
+#endif
+
+static int
+libxlDomainPMWakeup(virDomainPtr dom,
+                        unsigned int flags)

Whitespace off a bit. Could probably be moved to previous line.

+{
+    libxlDriverPrivatePtr driver = dom->conn->privateData;
+    virDomainObjPtr vm;
+    int ret = -1;
+    virObjectEventPtr event = NULL;
+    libxlDomainObjPrivatePtr priv;
+    libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver);
+
+    virCheckFlags(0, -1);
+
+    if (!(vm = libxlDomObjFromDomain(dom)))
+        goto cleanup;
+
+    if (virDomainPMWakeupEnsureACL(dom->conn, vm->def) < 0)
+        goto cleanup;
+
+    if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY) < 0)
+        goto cleanup;
+
+    if (virDomainObjGetState(vm, NULL) != VIR_DOMAIN_PMSUSPENDED) {
+        virReportError(VIR_ERR_OPERATION_INVALID,
+                       "%s", _("Domain is not suspended"));
+        goto endjob;
+    }
+
+    event = virDomainEventLifecycleNewFromObj(vm, VIR_DOMAIN_EVENT_STARTED,
+                                     VIR_DOMAIN_EVENT_STARTED_WAKEUP);
+
+    priv = vm->privateData;
+    if (libxl_domain_resume(cfg->ctx, vm->def->id, 1, NULL) < 0) {
+        virReportError(VIR_ERR_INTERNAL_ERROR,
+                       _("Failed to resume domain '%d'"), vm->def->id);
+        goto endjob;
+    }
+    virDomainObjSetState(vm, VIR_DOMAIN_RUNNING, VIR_DOMAIN_RUNNING_WAKEUP);
+    /* reenable death event - libxl reports it only once */
+    if (priv->deathW)
+        libxl_evdisable_domain_death(cfg->ctx, priv->deathW);
+    if (libxl_evenable_domain_death(cfg->ctx, vm->def->id, 0, &priv->deathW))
+        goto endjob;

Is returning failure the right thing to do here? Should we kill off the domain first? In libxlDomainStart we destroy the domain if enabling death events fails.

+
+    ret = 0;
+
+ endjob:
+    libxlDomainObjEndJob(driver, vm);
+
+ cleanup:
+    if (vm)
+        virObjectUnlock(vm);

virDomainObjEndAPI(&vm);

+    virObjectEventStateQueue(driver->domainEventState, event);
+    return ret;
+}
+
  static char *
  libxlDomainGetOSType(virDomainPtr dom)
  {
@@ -6385,6 +6507,10 @@ static virHypervisorDriver libxlHypervisorDriver = {
      .domainReboot = libxlDomainReboot, /* 0.9.0 */
      .domainDestroy = libxlDomainDestroy, /* 0.9.0 */
      .domainDestroyFlags = libxlDomainDestroyFlags, /* 0.9.4 */
+#ifdef LIBXL_HAVE_DOMAIN_SUSPEND_ONLY
+    .domainPMSuspendForDuration = libxlDomainPMSuspendForDuration, /* 4.7.0 */
+#endif
+    .domainPMWakeup = libxlDomainPMWakeup, /* 4.7.0 */

Is there a reason to have domainPMWakeup without domainPMSuspendForDuration? I'd think both of these should be conditional on LIBXL_HAVE_DOMAIN_SUSPEND_ONLY.

Regards,
Jim

      .domainGetOSType = libxlDomainGetOSType, /* 0.9.0 */
      .domainGetMaxMemory = libxlDomainGetMaxMemory, /* 0.9.0 */
      .domainSetMaxMemory = libxlDomainSetMaxMemory, /* 0.9.2 */



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