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

[Chris Lalancette]

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