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
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list