[lxc-devel] [PATCH] cgmanager: have cgm_set and cgm_get use absolute path when possible
Stéphane Graber
stgraber at ubuntu.com
Fri Jun 27 17:08:35 UTC 2014
On Thu, Jun 26, 2014 at 04:44:46PM -0500, Serge Hallyn wrote:
> This allows users to get/set cgroup settings when logged into a different
> session than that from which they started the container.
>
> There is no cgmanager command to do an _abs variant of cgmanager_get_value
> and cgmanager_set_value. So we fork off a new task, which enters the
> parent cgroup of the started container, then can get/set the value from
> there. The reason not to go straight into the container's cgroup is that
> if we are freezing the container, or the container is already frozen, we'll
> freeze as well :) The reason to fork off a new task is that if we are
> in a cgroup which is set to remove-on-empty, we may not be able to return
> to our original cgroup after making the change.
>
> This should fix https://github.com/lxc/lxc/issues/246
>
> Signed-off-by: Serge Hallyn <serge.hallyn at ubuntu.com>
Acked-by: Stéphane Graber <stgraber at ubuntu.com>
> ---
> src/lxc/cgmanager.c | 253 +++++++++++++++++++++++++++++++++++++++-------------
> 1 file changed, 189 insertions(+), 64 deletions(-)
>
> diff --git a/src/lxc/cgmanager.c b/src/lxc/cgmanager.c
> index 3a5525a..d18ec2c 100644
> --- a/src/lxc/cgmanager.c
> +++ b/src/lxc/cgmanager.c
> @@ -701,52 +701,124 @@ static inline void free_abs_cgroup(char *cgroup)
> free(cgroup);
> }
>
> -/* cgm_get is called to get container cgroup settings, not during startup */
> -static int cgm_get(const char *filename, char *value, size_t len, const char *name, const char *lxcpath)
> +static void do_cgm_get(const char *name, const char *lxcpath, const char *filename, int outp, bool sendvalue)
> {
> - char *result, *controller, *key, *cgroup = NULL;
> - size_t newlen;
> + char *controller, *key, *cgroup = NULL, *cglast;
> + int len = -1;
> + int ret;
> + nih_local char *result = NULL;
>
> controller = alloca(strlen(filename)+1);
> strcpy(controller, filename);
> key = strchr(controller, '.');
> - if (!key)
> - return -1;
> + if (!key) {
> + ret = write(outp, &len, sizeof(len));
> + if (ret != sizeof(len))
> + WARN("Failed to warn cgm_get of error; parent may hang");
> + exit(1);
> + }
> *key = '\0';
>
> - /* use the command interface to look for the cgroup */
> - cgroup = lxc_cmd_get_cgroup_path(name, lxcpath, controller);
> - if (!cgroup)
> - return -1;
> -
> if (!cgm_dbus_connect()) {
> ERROR("Error connecting to cgroup manager");
> - return -1;
> + ret = write(outp, &len, sizeof(len));
> + if (ret != sizeof(len))
> + WARN("Failed to warn cgm_get of error; parent may hang");
> + exit(1);
> }
> -
> - if (cgmanager_get_value_sync(NULL, cgroup_manager, controller, cgroup, filename, &result) != 0) {
> - /*
> - * must consume the nih error
> - * However don't print out an error as the key may simply not exist
> - * on the host
> - */
> + cgroup = try_get_abs_cgroup(name, lxcpath, subsystems[0]);
> + if (!cgroup) {
> + cgm_dbus_disconnect();
> + ret = write(outp, &len, sizeof(len));
> + if (ret != sizeof(len))
> + WARN("Failed to warn cgm_get of error; parent may hang");
> + exit(1);
> + }
> + cglast = strrchr(cgroup, '/');
> + if (!cglast) {
> + cgm_dbus_disconnect();
> + free_abs_cgroup(cgroup);
> + ret = write(outp, &len, sizeof(len));
> + if (ret != sizeof(len))
> + WARN("Failed to warn cgm_get of error; parent may hang");
> + exit(1);
> + }
> + *cglast = '\0';
> + if (!lxc_cgmanager_enter(getpid(), controller, cgroup, abs_cgroup_supported())) {
> + ERROR("Failed to enter container cgroup %s:%s", controller, cgroup);
> + ret = write(outp, &len, sizeof(len));
> + if (ret != sizeof(len))
> + WARN("Failed to warn cgm_get of error; parent may hang");
> + cgm_dbus_disconnect();
> + free_abs_cgroup(cgroup);
> + exit(1);
> + }
> + if (cgmanager_get_value_sync(NULL, cgroup_manager, controller, cglast+1, filename, &result) != 0) {
> NihError *nerr;
> nerr = nih_error_get();
> nih_free(nerr);
> - free(cgroup);
> + free_abs_cgroup(cgroup);
> cgm_dbus_disconnect();
> - return -1;
> + ret = write(outp, &len, sizeof(len));
> + if (ret != sizeof(len))
> + WARN("Failed to warn cgm_get of error; parent may hang");
> + exit(1);
> }
> + free_abs_cgroup(cgroup);
> cgm_dbus_disconnect();
> - free(cgroup);
> - newlen = strlen(result);
> - if (!len || !value) {
> - // user queries the size
> - nih_free(result);
> - return newlen+1;
> + len = strlen(result);
> + ret = write(outp, &len, sizeof(len));
> + if (ret != sizeof(len)) {
> + WARN("Failed to send length to parent");
> + exit(1);
> }
> + if (!len || !sendvalue) {
> + exit(0);
> + }
> + ret = write(outp, result, len);
> + if (ret < 0)
> + exit(1);
> + exit(0);
> +}
>
> - strncpy(value, result, len);
> +/* cgm_get is called to get container cgroup settings, not during startup */
> +static int cgm_get(const char *filename, char *value, size_t len, const char *name, const char *lxcpath)
> +{
> + pid_t pid;
> + int p[2], ret, newlen, readlen;
> +
> + if (pipe(p) < 0)
> + return -1;
> + if ((pid = fork()) < 0) {
> + close(p[0]);
> + close(p[1]);
> + return -1;
> + }
> + if (!pid)
> + do_cgm_get(name, lxcpath, filename, p[1], len && value);
> + close(p[1]);
> + ret = read(p[0], &newlen, sizeof(newlen));
> + if (ret != sizeof(newlen)) {
> + close(p[0]);
> + return -1;
> + }
> + if (!len || !value) {
> + close(p[0]);
> + return newlen;
> + }
> + if (newlen < 0) { // child is reporting an error
> + close(p[0]);
> + return -1;
> + }
> + if (newlen == 0) { // empty read
> + close(p[0]);
> + return 0;
> + }
> + readlen = newlen > len ? len : newlen;
> + ret = read(p[0], value, readlen);
> + close(p[0]);
> + if (ret != readlen)
> + return -1;
> if (newlen >= len) {
> value[len-1] = '\0';
> newlen = len-1;
> @@ -755,57 +827,106 @@ static int cgm_get(const char *filename, char *value, size_t len, const char *na
> value[newlen++] = '\n';
> value[newlen] = '\0';
> }
> - nih_free(result);
> return newlen;
> }
>
> -/* internal helper - call with cgmanager dbus connection open */
> -static int cgm_do_set(const char *controller, const char *file,
> - const char *cgroup, const char *value)
> +static void do_cgm_set(const char *name, const char *lxcpath, const char *filename, const char *value, int outp)
> {
> + char *controller, *key, *cgroup = NULL;
> + int retval = 0; // value we are sending to the parent over outp
> int ret;
> - ret = cgmanager_set_value_sync(NULL, cgroup_manager, controller,
> - cgroup, file, value);
> - if (ret != 0) {
> + char *cglast;
> +
> + controller = alloca(strlen(filename)+1);
> + strcpy(controller, filename);
> + key = strchr(controller, '.');
> + if (!key) {
> + ret = write(outp, &retval, sizeof(retval));
> + if (ret != sizeof(retval))
> + WARN("Failed to warn cgm_set of error; parent may hang");
> + exit(1);
> + }
> + *key = '\0';
> +
> + if (!cgm_dbus_connect()) {
> + ERROR("Error connecting to cgroup manager");
> + ret = write(outp, &retval, sizeof(retval));
> + if (ret != sizeof(retval))
> + WARN("Failed to warn cgm_set of error; parent may hang");
> + exit(1);
> + }
> + cgroup = try_get_abs_cgroup(name, lxcpath, subsystems[0]);
> + if (!cgroup) {
> + cgm_dbus_disconnect();
> + ret = write(outp, &retval, sizeof(retval));
> + if (ret != sizeof(retval))
> + WARN("Failed to warn cgm_set of error; parent may hang");
> + exit(1);
> + }
> + cglast = strrchr(cgroup, '/');
> + if (!cglast) {
> + cgm_dbus_disconnect();
> + free_abs_cgroup(cgroup);
> + ret = write(outp, &retval, sizeof(retval));
> + if (ret != sizeof(retval))
> + WARN("Failed to warn cgm_set of error; parent may hang");
> + exit(1);
> + }
> + *cglast = '\0';
> + if (!lxc_cgmanager_enter(getpid(), controller, cgroup, abs_cgroup_supported())) {
> + ERROR("Failed to enter container cgroup %s:%s", controller, cgroup);
> + ret = write(outp, &retval, sizeof(retval));
> + if (ret != sizeof(retval))
> + WARN("Failed to warn cgm_set of error; parent may hang");
> + cgm_dbus_disconnect();
> + free_abs_cgroup(cgroup);
> + exit(1);
> + }
> + if (cgmanager_set_value_sync(NULL, cgroup_manager, controller, cglast+1, filename, value) != 0) {
> NihError *nerr;
> nerr = nih_error_get();
> + ERROR("Error setting cgroup value %s for %s:%s", filename, controller, cgroup);
> ERROR("call to cgmanager_set_value_sync failed: %s", nerr->message);
> nih_free(nerr);
> - ERROR("Error setting cgroup %s limit %s", file, cgroup);
> + free_abs_cgroup(cgroup);
> + cgm_dbus_disconnect();
> + ret = write(outp, &retval, sizeof(retval));
> + if (ret != sizeof(retval))
> + WARN("Failed to warn cgm_set of error; parent may hang");
> + exit(1);
> }
> - return ret;
> + free_abs_cgroup(cgroup);
> + cgm_dbus_disconnect();
> + /* tell parent that we are done */
> + retval = 1;
> + ret = write(outp, &retval, sizeof(retval));
> + if (ret != sizeof(retval)) {
> + exit(1);
> + }
> + exit(0);
> }
>
> /* cgm_set is called to change cgroup settings, not during startup */
> static int cgm_set(const char *filename, const char *value, const char *name, const char *lxcpath)
> {
> - char *controller, *key, *cgroup = NULL;
> - int ret;
> + pid_t pid;
> + int p[2], ret, v;
>
> - controller = alloca(strlen(filename)+1);
> - strcpy(controller, filename);
> - key = strchr(controller, '.');
> - if (!key)
> + if (pipe(p) < 0)
> return -1;
> - *key = '\0';
> -
> - /* use the command interface to look for the cgroup */
> - cgroup = lxc_cmd_get_cgroup_path(name, lxcpath, controller);
> - if (!cgroup) {
> - ERROR("Failed to get cgroup for controller %s for %s:%s",
> - controller, lxcpath, name);
> + if ((pid = fork()) < 0) {
> + close(p[1]);
> + close(p[0]);
> return -1;
> }
> -
> - if (!cgm_dbus_connect()) {
> - ERROR("Error connecting to cgroup manager");
> - free(cgroup);
> - return false;
> - }
> - ret = cgm_do_set(controller, filename, cgroup, value);
> - cgm_dbus_disconnect();
> - free(cgroup);
> - return ret;
> + if (!pid)
> + do_cgm_set(name, lxcpath, filename, value, p[1]);
> + close(p[1]);
> + ret = read(p[0], &v, sizeof(v));
> + close(p[0]);
> + if (ret != sizeof(v) || !v)
> + return -1;
> + return 0;
> }
>
> static void free_subsystems(void)
> @@ -976,10 +1097,14 @@ static bool cgm_setup_limits(void *hdata, struct lxc_list *cgroup_settings, bool
> p = strchr(controller, '.');
> if (p)
> *p = '\0';
> - if (cgm_do_set(controller, cg->subsystem, d->cgroup_path
> - , cg->value) < 0) {
> - ERROR("Error setting %s to %s for %s",
> - cg->subsystem, cg->value, d->name);
> + if (cgmanager_set_value_sync(NULL, cgroup_manager, controller,
> + d->cgroup_path, cg->subsystem, cg->value) != 0) {
> + NihError *nerr;
> + nerr = nih_error_get();
> + ERROR("call to cgmanager_set_value_sync failed: %s", nerr->message);
> + nih_free(nerr);
> + ERROR("Error setting cgroup %s:%s limit type %s", controller,
> + d->cgroup_path, cg->subsystem);
> goto out;
> }
>
> --
> 1.9.1
>
> _______________________________________________
> lxc-devel mailing list
> lxc-devel at lists.linuxcontainers.org
> http://lists.linuxcontainers.org/listinfo/lxc-devel
--
Stéphane Graber
Ubuntu developer
http://www.ubuntu.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20140627/b80a729e/attachment.sig>
More information about the lxc-devel
mailing list