It looks relatively good, but one suggestion and two missing pieces: Ryota Ozaki wrote: <snip>
diff --git a/src/lxc_driver.c b/src/lxc_driver.c index 0ec1e92..2399d98 100644 --- a/src/lxc_driver.c +++ b/src/lxc_driver.c @@ -1862,6 +1862,188 @@ static char *lxcGetHostname (virConnectPtr conn) return result; }
+static int lxcFreezeContainer(lxc_driver_t *driver, virDomainObjPtr vm) +{ + int timeout = 3; /* In seconds */ + int check_interval = 500; /* In milliseconds */ + int n_try = (timeout * (1000 / check_interval)); + int i = 0; + int ret = -1; + char *state = NULL; + virCgroupPtr cgroup = NULL; + + if (!(driver->cgroup && + virCgroupForDomain(driver->cgroup, vm->def->name, &cgroup, 0) == 0)) + return -1; + + while (++i <= n_try) { + int r; + /* + * Writing "FROZEN" to the "freezer.state" freezes the group, + * i.e., the container, temporarily transiting "FREEZING" state. + * Once the freezing is completed, the state of the group transits + * to "FROZEN". + * (see linux-2.6/Documentation/cgroups/freezer-subsystem.txt) + */ + r = virCgroupSetFreezerState(cgroup, "FROZEN"); + + /* + * Returning EBUSY explicitly indicates that the group is + * being freezed but incomplete and other errors are true + * errors. + */ + if (r < 0 && r != -EBUSY) { + VIR_DEBUG("Writing freezer.state failed with errno: %d", r); + goto error; + } + + /* + * Unfortunately, returning 0 (success) is likely to happen + * even when the freezing has not been completed. Sometimes + * the state of the group remains "FREEZING" like when + * returning -EBUSY and even worse may never transit to + * "FROZEN" even if writing "FROZEN" again. + * + * So we don't trust the return value anyway and always + * decide that the freezing has been complete only with + * the state actually transit to "FROZEN". + */ + usleep(check_interval * 1000);
Suggestion: it might be better to sleep for shorter periods of time here in a loop. That is, sleep for say 1 ms, check if it's changed, sleep for 1 ms, etc. If it doesn't appear after 500 ms, then fail. That way you'll know about the change faster. <snip>
+static int lxcDomainSuspend(virDomainPtr dom) +{ + lxc_driver_t *driver = dom->conn->privateData; + virDomainObjPtr vm; + int ret = -1; + + lxcDriverLock(driver); + vm = virDomainFindByUUID(&driver->domains, dom->uuid); + + if (!vm) { + char uuidstr[VIR_UUID_STRING_BUFLEN]; + virUUIDFormat(dom->uuid, uuidstr); + lxcError(dom->conn, dom, VIR_ERR_NO_DOMAIN, + _("no domain with matching uuid '%s'"), uuidstr); + goto cleanup; + } + + if (!virDomainIsActive(vm)) { + lxcError(dom->conn, dom, VIR_ERR_OPERATION_INVALID, + "%s", _("domain is not running")); + goto cleanup; + } + + if (vm->state != VIR_DOMAIN_PAUSED) { + if (lxcFreezeContainer(driver, vm) < 0) { + lxcError(dom->conn, dom, VIR_ERR_OPERATION_FAILED, + "%s", _("suspend operation failed")); + goto cleanup; + } + vm->state = VIR_DOMAIN_PAUSED; + } + + if (virDomainSaveStatus(dom->conn, driver->stateDir, vm) < 0) + goto cleanup; + ret = 0;
Here, you'll want to generate an event (virDomainEventNewFromObj) for the change. You can look at lxcDomainCreateAndStart() for an example of what kind of event you would want to generate. <snip>
+static int lxcDomainResume(virDomainPtr dom) +{ + lxc_driver_t *driver = dom->conn->privateData; + virDomainObjPtr vm; + int ret = -1; + + lxcDriverLock(driver); + vm = virDomainFindByUUID(&driver->domains, dom->uuid); + + if (!vm) { + char uuidstr[VIR_UUID_STRING_BUFLEN]; + virUUIDFormat(dom->uuid, uuidstr); + lxcError(dom->conn, dom, VIR_ERR_NO_DOMAIN, + _("no domain with matching uuid '%s'"), uuidstr); + goto cleanup; + } + + if (!virDomainIsActive(vm)) { + lxcError(dom->conn, dom, VIR_ERR_OPERATION_INVALID, + "%s", _("domain is not running")); + goto cleanup; + } + + if (vm->state == VIR_DOMAIN_PAUSED) { + if (lxcUnfreezeContainer(driver, vm) < 0) { + lxcError(dom->conn, dom, VIR_ERR_OPERATION_FAILED, + "%s", _("resume operation failed")); + goto cleanup; + } + vm->state = VIR_DOMAIN_RUNNING; + } + + if (virDomainSaveStatus(dom->conn, driver->stateDir, vm) < 0) + goto cleanup; + ret = 0;
Same thing here. -- Chris Lalancette