cgmanager: don't stay connected

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@ubuntu.com>
Acked-by: Stéphane Graber <stgraber@ubuntu.com>

diff --git a/src/lxc/cgmanager.c b/src/lxc/cgmanager.c
index 73916756c74dc1913870980076c52a0114ac26af..7be94a7b309a40d66878ef657cdb8870a6723825 100644 (file)
--- 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 d931520b26228bf3856342bb2fa27f767d6bcb76..23c8c96e5cdf6fbf4a0c9481015395bb652206ab 100644 (file)
--- 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 e3cf6e0ff60c41151f739dc2bf6a3fda4dd4637a..8cba03066ee6fadd452903633af2f26c4199ed89 100644 (file)
--- 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 fe3d09335b2a0c6cff10c374c13548a438865312..97c82070d1fbdfc9578380ac04d9071494d0786c 100644 (file)
--- 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