On 09/03/2018 04:09 PM, Marek Marczykowski-Górecki wrote:
Signed-off-by: Marek Marczykowski-Górecki <marmarek@xxxxxxxxxxxxxxxxxxxxxx>
---
Changes in v2:
- use virDomainObjEndAPI
- drop duplicated error reporting on virDomainObjIsActive
- bump version comment to 4.8.0
You missed some other comments from V1. I'll repeat them here.
---
src/libxl/libxl_driver.c | 121 ++++++++++++++++++++++++++++++++++++++++-
1 file changed, 121 insertions(+)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 5a5e792..0a4e716 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -1403,6 +1403,123 @@ 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))
+ goto endjob;
You still need to report the error if domain is inactive. To do that use
virDomainObjCheckActive(). E.g.
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:
+ virDomainObjEndAPI(&vm);
+ return ret;
+}
+#endif
+
+static int
+libxlDomainPMWakeup(virDomainPtr dom,
+ unsigned int flags)
Whitespace is off, but this line could probably be combined with the previous one.
+{
+ 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:
+ virDomainObjEndAPI(&vm);
+ virObjectEventStateQueue(driver->domainEventState, event);
+ return ret;
+}
+
static char *
libxlDomainGetOSType(virDomainPtr dom)
{
@@ -6385,6 +6502,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.8.0 */
+#endif
+ .domainPMWakeup = libxlDomainPMWakeup, /* 4.8.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 */
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
