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

Re: [libvirt] [PATCH] lxc: suspend/resume support



On Mon, Sep 21, 2009 at 9:05 PM, Daniel P. Berrange <berrange redhat com> wrote:
> On Thu, Sep 17, 2009 at 12:42:08AM +0900, Ryota Ozaki wrote:
>> +static int lxcDomainSuspend(virDomainPtr dom)
>> +{
>> +    lxc_driver_t *driver = dom->conn->privateData;
>> +    virDomainObjPtr vm;
>> +    virDomainEventPtr event = NULL;
>> +    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;
>> +
>> +    event = virDomainEventNewFromObj(vm,
>> +                                     VIR_DOMAIN_EVENT_SUSPENDED,
>> +                                     VIR_DOMAIN_EVENT_SUSPENDED_PAUSED);
>
> The virDomainSaveStatus/virDomainEventNewFromObj  calls need to be moved
> up inside the  "if (vm->state != VIR_DOMAIN_PAUSED) {"  conditional, since
> you don't want to dispatch an event if its already paused.

Oh, you're right.

>
>> +static int lxcDomainResume(virDomainPtr dom)
>> +{
>> +    lxc_driver_t *driver = dom->conn->privateData;
>> +    virDomainObjPtr vm;
>> +    virDomainEventPtr event = NULL;
>> +    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;
>> +
>> +    event = virDomainEventNewFromObj(vm,
>> +                                     VIR_DOMAIN_EVENT_RESUMED,
>> +                                     VIR_DOMAIN_EVENT_RESUMED_UNPAUSED);
>
> Same as above - the virDomainSaveStatus/virDomainEventNewFromObj cals
> need to be moved inside the "if (vm->state == VIR_DOMAIN_PAUSED) "
> conditional

OK, fixed the two parts. Thanks for the review!

And the fixed patch is here!
  ozaki-r

PS: git rebase works well ;-)


>From abe363de43f5053a01593de6e634f654525cd8b4 Mon Sep 17 00:00:00 2001
From: Ryota Ozaki <ozaki ryota gmail com>
Date: Mon, 21 Sep 2009 23:31:22 +0900
Subject: [PATCH] lxc: suspend/resume support

---
 src/conf/domain_conf.c |   27 ++++---
 src/lxc/lxc_driver.c   |  212 +++++++++++++++++++++++++++++++++++++++++++++++-
 src/util/cgroup.c      |   23 +++++-
 src/util/cgroup.h      |    4 +
 4 files changed, 251 insertions(+), 15 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 5ae0775..5e37d96 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -4433,19 +4433,22 @@ char *virDomainObjFormat(virConnectPtr conn,
                       virDomainStateTypeToString(obj->state),
                       obj->pid);

-    switch (obj->monitor_chr->type) {
-    case VIR_DOMAIN_CHR_TYPE_UNIX:
-        monitorpath = obj->monitor_chr->data.nix.path;
-        break;
-    default:
-    case VIR_DOMAIN_CHR_TYPE_PTY:
-        monitorpath = obj->monitor_chr->data.file.path;
-        break;
-    }
+    /* obj->monitor_chr is set only for qemu */
+    if (obj->monitor_chr) {
+        switch (obj->monitor_chr->type) {
+        case VIR_DOMAIN_CHR_TYPE_UNIX:
+            monitorpath = obj->monitor_chr->data.nix.path;
+            break;
+        default:
+        case VIR_DOMAIN_CHR_TYPE_PTY:
+            monitorpath = obj->monitor_chr->data.file.path;
+            break;
+        }

-    virBufferEscapeString(&buf, "  <monitor path='%s'", monitorpath);
-    virBufferVSprintf(&buf, " type='%s'/>¥n",
-                      virDomainChrTypeToString(obj->monitor_chr->type));
+        virBufferEscapeString(&buf, "  <monitor path='%s'", monitorpath);
+        virBufferVSprintf(&buf, " type='%s'/>¥n",
+                          virDomainChrTypeToString(obj->monitor_chr->type));
+    }


     if (obj->nvcpupids) {
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index 0ec1e92..78eb94c 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -1862,6 +1862,214 @@ static char *lxcGetHostname (virConnectPtr conn)
     return result;
 }

+static int lxcFreezeContainer(lxc_driver_t *driver, virDomainObjPtr vm)
+{
+    int timeout = 1000; /* In milliseconds */
+    int check_interval = 1; /* In milliseconds */
+    int exp = 10;
+    int waited_time = 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 (waited_time < timeout) {
+        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;
+        }
+        if (r == -EBUSY)
+            VIR_DEBUG0("Writing freezer.state gets EBUSY");
+
+        /*
+         * 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);
+
+        r = virCgroupGetFreezerState(cgroup, &state);
+
+        if (r < 0) {
+            VIR_DEBUG("Reading freezer.state failed with errno: %d", r);
+            goto error;
+        }
+        VIR_DEBUG("Read freezer.state: %s", state);
+
+        if (STREQ(state, "FROZEN")) {
+            ret = 0;
+            goto cleanup;
+        }
+
+        waited_time += check_interval;
+        /*
+         * Increasing check_interval exponentially starting with
+         * small initial value treats nicely two cases; One is
+         * a container is under no load and waiting for long period
+         * makes no sense. The other is under heavy load. The container
+         * may stay longer time in FREEZING or never transit to FROZEN.
+         * In that case, eager polling will just waste CPU time.
+         */
+        check_interval *= exp;
+        VIR_FREE(state);
+    }
+    VIR_DEBUG0("lxcFreezeContainer timeout");
+error:
+    /*
+     * If timeout or an error on reading the state occurs,
+     * activate the group again and return an error.
+     * This is likely to fall the group back again gracefully.
+     */
+    virCgroupSetFreezerState(cgroup, "THAWED");
+    ret = -1;
+
+cleanup:
+    if (cgroup)
+        virCgroupFree(&cgroup);
+    VIR_FREE(state);
+    return ret;
+}
+
+static int lxcDomainSuspend(virDomainPtr dom)
+{
+    lxc_driver_t *driver = dom->conn->privateData;
+    virDomainObjPtr vm;
+    virDomainEventPtr event = NULL;
+    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;
+
+        event = virDomainEventNewFromObj(vm,
+                                         VIR_DOMAIN_EVENT_SUSPENDED,
+                                         VIR_DOMAIN_EVENT_SUSPENDED_PAUSED);
+    }
+
+    if (virDomainSaveStatus(dom->conn, driver->stateDir, vm) < 0)
+        goto cleanup;
+    ret = 0;
+
+cleanup:
+    if (event)
+        lxcDomainEventQueue(driver, event);
+    if (vm)
+        virDomainObjUnlock(vm);
+    lxcDriverUnlock(driver);
+    return ret;
+}
+
+static int lxcUnfreezeContainer(lxc_driver_t *driver, virDomainObjPtr vm)
+{
+    int ret;
+    virCgroupPtr cgroup = NULL;
+
+    if (!(driver->cgroup &&
+        virCgroupForDomain(driver->cgroup, vm->def->name, &cgroup, 0) == 0))
+        return -1;
+
+    ret = virCgroupSetFreezerState(cgroup, "THAWED");
+
+    virCgroupFree(&cgroup);
+    return ret;
+}
+
+static int lxcDomainResume(virDomainPtr dom)
+{
+    lxc_driver_t *driver = dom->conn->privateData;
+    virDomainObjPtr vm;
+    virDomainEventPtr event = NULL;
+    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;
+
+        event = virDomainEventNewFromObj(vm,
+                                         VIR_DOMAIN_EVENT_RESUMED,
+                                         VIR_DOMAIN_EVENT_RESUMED_UNPAUSED);
+    }
+
+    if (virDomainSaveStatus(dom->conn, driver->stateDir, vm) < 0)
+        goto cleanup;
+    ret = 0;
+
+cleanup:
+    if (event)
+        lxcDomainEventQueue(driver, event);
+    if (vm)
+        virDomainObjUnlock(vm);
+    lxcDriverUnlock(driver);
+    return ret;
+}
+
+
 /* Function Tables */
 static virDriver lxcDriver = {
     VIR_DRV_LXC, /* the number virDrvNo */
@@ -1881,8 +2089,8 @@ static virDriver lxcDriver = {
     lxcDomainLookupByID, /* domainLookupByID */
     lxcDomainLookupByUUID, /* domainLookupByUUID */
     lxcDomainLookupByName, /* domainLookupByName */
-    NULL, /* domainSuspend */
-    NULL, /* domainResume */
+    lxcDomainSuspend, /* domainSuspend */
+    lxcDomainResume, /* domainResume */
     lxcDomainShutdown, /* domainShutdown */
     NULL, /* domainReboot */
     lxcDomainDestroy, /* domainDestroy */
diff --git a/src/util/cgroup.c b/src/util/cgroup.c
index 111601d..2e646fd 100644
--- a/src/util/cgroup.c
+++ b/src/util/cgroup.c
@@ -32,7 +32,8 @@
 #define CGROUP_MAX_VAL 512

 VIR_ENUM_IMPL(virCgroupController, VIR_CGROUP_CONTROLLER_LAST,
-              "cpu", "cpuacct", "cpuset", "memory", "devices");
+              "cpu", "cpuacct", "cpuset", "memory", "devices",
+              "freezer");

 struct virCgroupController {
     int type;
@@ -896,3 +897,23 @@ int virCgroupGetCpuacctUsage(virCgroupPtr group,
unsigned long long *usage)
                                 VIR_CGROUP_CONTROLLER_CPUACCT,
                                 "cpuacct.usage", (uint64_t *)usage);
 }
+
+int virCgroupSetFreezerState(virCgroupPtr group, const char *state)
+{
+    return virCgroupSetValueStr(group,
+                                VIR_CGROUP_CONTROLLER_CPU,
+                                "freezer.state", state);
+}
+
+int virCgroupGetFreezerState(virCgroupPtr group, char **state)
+{
+    int ret;
+    ret = virCgroupGetValueStr(group,
+                                VIR_CGROUP_CONTROLLER_CPU,
+                                "freezer.state", state);
+    if (ret == 0) {
+        char *p = strchr(*state, '¥n');
+        if (p) *p = '¥0';
+    }
+    return ret;
+}
diff --git a/src/util/cgroup.h b/src/util/cgroup.h
index 6d43b14..aba56c6 100644
--- a/src/util/cgroup.h
+++ b/src/util/cgroup.h
@@ -21,6 +21,7 @@ enum {
     VIR_CGROUP_CONTROLLER_CPUSET,
     VIR_CGROUP_CONTROLLER_MEMORY,
     VIR_CGROUP_CONTROLLER_DEVICES,
+    VIR_CGROUP_CONTROLLER_FREEZER,

     VIR_CGROUP_CONTROLLER_LAST
 };
@@ -68,6 +69,9 @@ int virCgroupGetCpuShares(virCgroupPtr group,
unsigned long long *shares);

 int virCgroupGetCpuacctUsage(virCgroupPtr group, unsigned long long *usage);

+int virCgroupSetFreezerState(virCgroupPtr group, const char *state);
+int virCgroupGetFreezerState(virCgroupPtr group, char **state);
+
 int virCgroupRemove(virCgroupPtr group);

 void virCgroupFree(virCgroupPtr *group);
-- 
1.6.0.2

Attachment: 0001-lxc-suspend-resume-support.patch
Description: Binary data


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