[lxc-devel] [PATCH RFC] cgmanager: don't stay connected

[Serge Hallyn]

There are only a few times when we need to be connected to the
cgroup manager:

* when starting a container, from cgm_init until we've set cgroup limits
* when changing a cgroup setting (while running)
* when cleaning up (when shutting down)
* around the cgroup entering at attach

So only connect/disconnect the cgmanager socket on-demand as
needed.  This should have a few benefits.

1. Reduce the # open fds when many containers are running
2. if cgmanager is stopped and restarted, the container
   doesn't have to deal with the disconnection.

This is currently RFC.  There are a few issues outstanding:

1. the cgm_set and cgm_get may need to be made thread-safe.
2. a non-daemonized start which fails while cgm is connected,
will not disconnected.

Signed-off-by: Serge Hallyn <serge.hallyn at ubuntu.com>
---
 src/lxc/cgmanager.c | 19 +++++++++++++++++--
 src/lxc/cgroup.c    |  6 ++++++
 src/lxc/cgroup.h    |  1 +
 src/lxc/start.c     |  2 ++
 4 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/src/lxc/cgmanager.c b/src/lxc/cgmanager.c
index 7391675..7be94a7 100644
--- a/src/lxc/cgmanager.c
+++ b/src/lxc/cgmanager.c
@@ -92,13 +92,13 @@ static NihDBusProxy *cgroup_manager = NULL;
 static struct cgroup_ops cgmanager_ops;
 static int nr_subsystems;
 static char **subsystems;
+static DBusConnection *connection;
 
 #define CGMANAGER_DBUS_SOCK "unix:path=/sys/fs/cgroup/cgmanager/sock"
 static void cgm_dbus_disconnected(DBusConnection *connection);
 static bool cgm_dbus_connect(void)
 {
 	DBusError dbus_error;
-	DBusConnection *connection;
 	dbus_error_init(&dbus_error);
 
 	lock_mutex(&thread_mutex);
@@ -118,7 +118,6 @@ static bool cgm_dbus_connect(void)
 	cgroup_manager = nih_dbus_proxy_new(NULL, connection,
 				NULL /* p2p */,
 				"/org/linuxcontainers/cgmanager", NULL, NULL);
-	dbus_connection_unref(connection);
 	if (!cgroup_manager) {
 		NihError *nerr;
 		nerr = nih_error_get();
@@ -145,13 +144,19 @@ static void cgm_dbus_disconnect(void)
 	if (cgroup_manager)
 		nih_free(cgroup_manager);
 	cgroup_manager = NULL;
+	dbus_connection_unref(connection);
+	connection = NULL;
 	unlock_mutex(&thread_mutex);
 }
 
 static void cgm_dbus_disconnected(DBusConnection *connection)
 {
+	lock_mutex(&thread_mutex);
 	WARN("Cgroup manager connection was terminated");
 	cgroup_manager = NULL;
+	dbus_connection_unref(connection);
+	connection = NULL;
+	unlock_mutex(&thread_mutex);
 	if (cgm_dbus_connect()) {
 		INFO("New cgroup manager connection was opened");
 	} else {
@@ -398,6 +403,7 @@ static void *cgm_init(const char *name)
 {
 	struct cgm_data *d;
 
+	cgm_dbus_connect();
 	d = malloc(sizeof(*d));
 	if (!d)
 		return NULL;
@@ -430,6 +436,7 @@ static void cgm_destroy(void *hdata)
 
 	if (!d)
 		return;
+	cgm_dbus_connect();
 	for (i = 0; i < nr_subsystems; i++)
 		cgm_remove_cgroup(subsystems[i], d->cgroup_path);
 
@@ -437,6 +444,7 @@ static void cgm_destroy(void *hdata)
 	if (d->cgroup_path)
 		free(d->cgroup_path);
 	free(d);
+	cgm_dbus_disconnect();
 }
 
 /*
@@ -602,6 +610,7 @@ static int cgm_get(const char *filename, char *value, size_t len, const char *na
 	cgroup = lxc_cmd_get_cgroup_path(name, lxcpath, controller);
 	if (!cgroup)
 		return -1;
+	cgm_dbus_connect();
 	lock_mutex(&thread_mutex);
 	if (cgmanager_get_value_sync(NULL, cgroup_manager, controller, cgroup, filename, &result) != 0) {
 		/*
@@ -614,9 +623,11 @@ static int cgm_get(const char *filename, char *value, size_t len, const char *na
 		nih_free(nerr);
 		free(cgroup);
 		unlock_mutex(&thread_mutex);
+		cgm_dbus_disconnect();
 		return -1;
 	}
 	unlock_mutex(&thread_mutex);
+	cgm_dbus_disconnect();
 	free(cgroup);
 	newlen = strlen(result);
 	if (!value) {
@@ -675,7 +686,9 @@ static int cgm_set(const char *filename, const char *value, const char *name, co
 			controller, lxcpath, name);
 		return -1;
 	}
+	cgm_dbus_connect();
 	ret = cgm_do_set(controller, filename, cgroup, value);
+	cgm_dbus_disconnect();
 	free(cgroup);
 	return ret;
 }
@@ -875,8 +888,10 @@ static bool cgm_attach(const char *name, const char *lxcpath, pid_t pid)
 		goto out;
 	}
 
+	cgm_dbus_connect();
 	if (!(pass = do_cgm_enter(pid, cgroup)))
 		ERROR("Failed to enter group %s", cgroup);
+	cgm_dbus_disconnect();
 
 out:
 	free(cgroup);
diff --git a/src/lxc/cgroup.c b/src/lxc/cgroup.c
index d931520..23c8c96 100644
--- a/src/lxc/cgroup.c
+++ b/src/lxc/cgroup.c
@@ -175,3 +175,9 @@ void restart_cgroups(void)
 	ops = NULL;
 	cgroup_ops_init();
 }
+
+void cgroup_disconnect(void)
+{
+	if (ops && ops->disconnect)
+		ops->disconnect();
+}
diff --git a/src/lxc/cgroup.h b/src/lxc/cgroup.h
index e3cf6e0..8cba030 100644
--- a/src/lxc/cgroup.h
+++ b/src/lxc/cgroup.h
@@ -66,5 +66,6 @@ extern int cgroup_nrtasks(struct lxc_handler *handler);
 extern const char *cgroup_get_cgroup(struct lxc_handler *handler, const char *subsystem);
 extern bool cgroup_unfreeze(struct lxc_handler *handler);
 extern void restart_cgroups(void);
+extern void cgroup_disconnect(void);
 
 #endif
diff --git a/src/lxc/start.c b/src/lxc/start.c
index fe3d093..97c8207 100644
--- a/src/lxc/start.c
+++ b/src/lxc/start.c
@@ -952,6 +952,8 @@ static int lxc_spawn(struct lxc_handler *handler)
 		goto out_delete_net;
 	}
 
+	cgroup_disconnect();
+
 	/* Tell the child to complete its initialization and wait for
 	 * it to exec or return an error.  (the child will never
 	 * return LXC_SYNC_POST_CGROUP+1.  It will either close the
-- 
1.9.0