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