[cgmanager-devel] [PATCH RFC] Undo the connection timeouts
Serge Hallyn
serge.hallyn at ubuntu.com
Thu May 1 04:52:22 UTC 2014
They are wreak havoc on container proxies.
However do keep the increased nfiles rlimit (which came in with that
same commit).
Please shout if you can think of a good way to distinguish a lazy
slow who left his connection open from an active container. (Then
again, perhaps cgroxies should simply allow their connections to remain
closed until they need it again? that would be an alternative to this
patch.)
Signed-off-by: Serge Hallyn <serge.hallyn at ubuntu.com>
---
Makefile.am | 10 +--
cgmanager-proxy.c | 2 -
cgmanager.c | 2 -
frontend.c | 123 -------------------------------------
frontend.h | 1 -
tests/test-timeout.c | 169 ---------------------------------------------------
6 files changed, 1 insertion(+), 306 deletions(-)
delete mode 100644 tests/test-timeout.c
diff --git a/Makefile.am b/Makefile.am
index a60c1bf..fec3bd8 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -113,12 +113,4 @@ TESTS_CGM_CONCURRENT: tests/cgm-concurrent.o
$(CCLD) -o tests/cgm-concurrent tests/cgm-concurrent.o \
$(NIH_LIBS) $(NIH_DBUS_LIBS) $(DBUS_LIBS) -lpthread -lcgmanager
-tests/test-timeout.o: tests/test-timeout.c
- $(CC) -I. $(NIH_CFLAGS) $(NIH_DBUS_CFLAGS) $(DBUS_CFLAGS) -c \
- -fPIC -DPIC -o tests/test-timeout.o tests/test-timeout.c
-
-TESTS_CGM_TIMEOUT: tests/test-timeout.o
- $(CCLD) -o tests/test-timeout tests/test-timeout.o \
- $(NIH_LIBS) $(NIH_DBUS_LIBS) $(DBUS_LIBS) -lcgmanager
-
-tests: TESTS_CGM_CONCURRENT TESTS_CGM_TIMEOUT
+tests: TESTS_CGM_CONCURRENT
diff --git a/cgmanager-proxy.c b/cgmanager-proxy.c
index efdeac5..508cbd5 100644
--- a/cgmanager-proxy.c
+++ b/cgmanager-proxy.c
@@ -1075,8 +1075,6 @@ main (int argc, char *argv[])
exit(1);
}
- connection_timeout_init();
-
if (setup_proxy() < 0) {
nih_fatal ("Failed to set up as proxy");
exit(1);
diff --git a/cgmanager.c b/cgmanager.c
index cee7cf1..1160949 100644
--- a/cgmanager.c
+++ b/cgmanager.c
@@ -907,8 +907,6 @@ main (int argc, char *argv[])
setns_user_supported = true;
}
- connection_timeout_init();
-
newrlimit.rlim_cur = 10000;
newrlimit.rlim_max = 10000;
if (setrlimit(RLIMIT_NOFILE, &newrlimit) < 0)
diff --git a/frontend.c b/frontend.c
index 2740d1c..fbe4b12 100644
--- a/frontend.c
+++ b/frontend.c
@@ -22,9 +22,6 @@
#define __frontend_c
#include <frontend.h>
-#include <nih/hash.h>
-#include <nih/list.h>
-#include <nih/timer.h>
int daemonise = FALSE;
int sigstop = FALSE;
@@ -33,91 +30,6 @@ unsigned long mypidns;
bool setns_user_supported = false;
unsigned long myuserns;
-/* functions to time out connections */
-
-typedef struct timer_hash_entry {
- NihList entry;
- DBusConnection *conn;
- NihTimer *timer;
-} timer_hash_entry;
-
-static NihHash *timer_hash;
-
-static const void *timer_key_fn(NihList *entry)
-{
- timer_hash_entry *e = (timer_hash_entry *)entry;
- return e->conn;
-}
-
-static uint32_t timer_hash_fn(const void *conn)
-{
- unsigned long val = (unsigned long)conn;
-
- if (sizeof(conn) > 4)
- val ^= (val >> 32);
- return (uint32_t)val;
-}
-
-static int timer_cmp_fn(const void *v1, const void *v2)
-{
- unsigned long val1 = (unsigned long)v1;
- unsigned long val2 = (unsigned long)v2;
-
- if (val1 > val2)
- return 1;
- else if (val1 < val2)
- return -1;
- else
- return 0;
-}
-
-static void timeout_remove(DBusConnection *conn)
-{
- timer_hash_entry *t;
-
- t = (timer_hash_entry *) nih_hash_lookup(timer_hash, conn);
- if (!t)
- return;
-
- nih_free (t->timer);
- nih_free (t);
-}
-
-static void timeout_handler(void *data, NihTimer *timer)
-{
- timer_hash_entry *t = (timer_hash_entry *)data;
- DBusConnection *conn = t->conn;
-
- dbus_connection_close(conn);
- dbus_connection_unref(conn);
- /* nih will take care of the NihTimer itself */
- nih_free(t);
-}
-
-static void timeout_add(DBusConnection *);
-
-static void timeout_reset(DBusConnection *conn)
-{
- timeout_remove(conn);
- timeout_add(conn);
-}
-
-static void timeout_add(DBusConnection *conn)
-{
- timer_hash_entry *t;
- NihTimer *timer;
-
- t = NIH_MUST (nih_new (NULL, timer_hash_entry));
- nih_list_init (&t->entry);
- nih_alloc_set_destructor (t, nih_list_destroy);
- timer = NIH_MUST (nih_timer_add_timeout (NULL, 20, timeout_handler, t));
- t->conn = conn;
- t->timer = timer;
-
- nih_hash_add (timer_hash, &t->entry);
-}
-
-/* reject unsafe cgroups */
bool sane_cgroup(const char *cgroup)
{
if (!cgroup)
@@ -304,7 +216,6 @@ int cgmanager_get_pid_cgroup_scm (void *data, NihDBusMessage *message,
{
struct scm_sock_data *d;
- timeout_reset(message->connection);
d = alloc_scm_sock_data(message, sockfd, REQ_TYPE_GET_PID);
if (!d)
return -1;
@@ -347,7 +258,6 @@ int cgmanager_get_pid_cgroup (void *data, NihDBusMessage *message,
return -1;
}
- timeout_reset(message->connection);
if (!dbus_connection_get_socket(message->connection, &fd)) {
nih_dbus_error_raise_printf (DBUS_ERROR_INVALID_ARGS,
"Could not get client socket.");
@@ -417,7 +327,6 @@ int cgmanager_get_pid_cgroup_abs_scm (void *data, NihDBusMessage *message,
{
struct scm_sock_data *d;
- timeout_reset(message->connection);
d = alloc_scm_sock_data(message, sockfd, REQ_TYPE_GET_PID_ABS);
if (!d)
return -1;
@@ -461,7 +370,6 @@ int cgmanager_get_pid_cgroup_abs (void *data, NihDBusMessage *message,
return -1;
}
- timeout_reset(message->connection);
if (!dbus_connection_get_socket(message->connection, &fd)) {
nih_dbus_error_raise_printf (DBUS_ERROR_INVALID_ARGS,
"Could not get client socket.");
@@ -541,7 +449,6 @@ int cgmanager_move_pid_scm (void *data, NihDBusMessage *message,
{
struct scm_sock_data *d;
- timeout_reset(message->connection);
d = alloc_scm_sock_data(message, sockfd, REQ_TYPE_MOVE_PID);
if (!d)
return -1;
@@ -584,7 +491,6 @@ int cgmanager_move_pid (void *data, NihDBusMessage *message,
return -1;
}
- timeout_reset(message->connection);
if (!dbus_connection_get_socket(message->connection, &fd)) {
nih_dbus_error_raise_printf (DBUS_ERROR_INVALID_ARGS,
"Could not get client socket.");
@@ -636,7 +542,6 @@ int cgmanager_move_pid_abs_scm (void *data, NihDBusMessage *message,
{
struct scm_sock_data *d;
- timeout_reset(message->connection);
d = alloc_scm_sock_data(message, sockfd, REQ_TYPE_MOVE_PID_ABS);
if (!d)
return -1;
@@ -674,7 +579,6 @@ int cgmanager_move_pid_abs (void *data, NihDBusMessage *message,
return -1;
}
- timeout_reset(message->connection);
if (!dbus_connection_get_socket(message->connection, &fd)) {
nih_dbus_error_raise_printf (DBUS_ERROR_INVALID_ARGS,
"Could not get client socket.");
@@ -758,7 +662,6 @@ int cgmanager_create_scm (void *data, NihDBusMessage *message,
{
struct scm_sock_data *d;
- timeout_reset(message->connection);
d = alloc_scm_sock_data(message, sockfd, REQ_TYPE_CREATE);
if (!d)
return -1;
@@ -803,7 +706,6 @@ int cgmanager_create (void *data, NihDBusMessage *message,
return -1;
}
- timeout_reset(message->connection);
if (!dbus_connection_get_socket(message->connection, &fd)) {
nih_dbus_error_raise_printf (DBUS_ERROR_INVALID_ARGS,
"Could not get client socket.");
@@ -845,7 +747,6 @@ int cgmanager_chown_scm (void *data, NihDBusMessage *message,
{
struct scm_sock_data *d;
- timeout_reset(message->connection);
d = alloc_scm_sock_data(message, sockfd, REQ_TYPE_CHOWN);
if (!d)
return -1;
@@ -893,7 +794,6 @@ int cgmanager_chown (void *data, NihDBusMessage *message,
return -1;
}
- timeout_reset(message->connection);
if (!dbus_connection_get_socket(message->connection, &fd)) {
nih_dbus_error_raise_printf (DBUS_ERROR_INVALID_ARGS,
"Could not get client socket.");
@@ -952,7 +852,6 @@ int cgmanager_chmod_scm (void *data, NihDBusMessage *message,
{
struct scm_sock_data *d;
- timeout_reset(message->connection);
d = alloc_scm_sock_data(message, sockfd, REQ_TYPE_CHMOD);
if (!d)
return -1;
@@ -997,7 +896,6 @@ int cgmanager_chmod (void *data, NihDBusMessage *message,
return -1;
}
- timeout_reset(message->connection);
if (!dbus_connection_get_socket(message->connection, &fd)) {
nih_dbus_error_raise_printf (DBUS_ERROR_INVALID_ARGS,
"Could not get client socket.");
@@ -1042,7 +940,6 @@ int cgmanager_get_value_scm (void *data, NihDBusMessage *message,
{
struct scm_sock_data *d;
- timeout_reset(message->connection);
d = alloc_scm_sock_data(message, sockfd, REQ_TYPE_GET_VALUE);
if (!d)
return -1;
@@ -1094,7 +991,6 @@ int cgmanager_get_value (void *data, NihDBusMessage *message,
return -1;
}
- timeout_reset(message->connection);
if (!dbus_connection_get_socket(message->connection, &fd)) {
nih_dbus_error_raise_printf (DBUS_ERROR_INVALID_ARGS,
"Could not get client socket.");
@@ -1135,7 +1031,6 @@ int cgmanager_set_value_scm (void *data, NihDBusMessage *message,
{
struct scm_sock_data *d;
- timeout_reset(message->connection);
d = alloc_scm_sock_data(message, sockfd, REQ_TYPE_SET_VALUE);
if (!d)
return -1;
@@ -1184,7 +1079,6 @@ int cgmanager_set_value (void *data, NihDBusMessage *message,
return -1;
}
- timeout_reset(message->connection);
if (!dbus_connection_get_socket(message->connection, &fd)) {
nih_dbus_error_raise_printf (DBUS_ERROR_INVALID_ARGS,
"Could not get client socket.");
@@ -1228,7 +1122,6 @@ int cgmanager_remove_scm (void *data, NihDBusMessage *message,
{
struct scm_sock_data *d;
- timeout_reset(message->connection);
d = alloc_scm_sock_data(message, sockfd, REQ_TYPE_REMOVE);
if (!d)
return -1;
@@ -1273,7 +1166,6 @@ int cgmanager_remove (void *data, NihDBusMessage *message, const char *controlle
"message was null");
return -1;
}
- timeout_reset(message->connection);
if (!dbus_connection_get_socket(message->connection, &fd)) {
nih_dbus_error_raise_printf (DBUS_ERROR_INVALID_ARGS,
@@ -1331,7 +1223,6 @@ int cgmanager_get_tasks_scm (void *data, NihDBusMessage *message,
{
struct scm_sock_data *d;
- timeout_reset(message->connection);
d = alloc_scm_sock_data(message, sockfd, REQ_TYPE_GET_TASKS);
if (!d)
return -1;
@@ -1375,7 +1266,6 @@ int cgmanager_get_tasks (void *data, NihDBusMessage *message, const char *contro
return -1;
}
- timeout_reset(message->connection);
if (!dbus_connection_get_socket(message->connection, &fd)) {
nih_dbus_error_raise_printf (DBUS_ERROR_INVALID_ARGS,
"Could not get client socket.");
@@ -1460,7 +1350,6 @@ int cgmanager_list_children_scm (void *data, NihDBusMessage *message,
{
struct scm_sock_data *d;
- timeout_reset(message->connection);
d = alloc_scm_sock_data(message, sockfd, REQ_TYPE_LIST_CHILDREN);
if (!d)
return -1;
@@ -1504,7 +1393,6 @@ int cgmanager_list_children (void *data, NihDBusMessage *message,
"message was null");
return -1;
}
- timeout_reset(message->connection);
if (!dbus_connection_get_socket(message->connection, &fd)) {
nih_dbus_error_raise_printf (DBUS_ERROR_INVALID_ARGS,
@@ -1548,7 +1436,6 @@ int cgmanager_remove_on_empty_scm (void *data, NihDBusMessage *message,
{
struct scm_sock_data *d;
- timeout_reset(message->connection);
d = alloc_scm_sock_data(message, sockfd, REQ_TYPE_REMOVE_ON_EMPTY);
if (!d)
return -1;
@@ -1591,7 +1478,6 @@ int cgmanager_remove_on_empty (void *data, NihDBusMessage *message,
"message was null");
return -1;
}
- timeout_reset(message->connection);
if (!dbus_connection_get_socket(message->connection, &fd)) {
nih_dbus_error_raise_printf (DBUS_ERROR_INVALID_ARGS,
@@ -1622,7 +1508,6 @@ int cgmanager_remove_on_empty (void *data, NihDBusMessage *message,
int
cgmanager_get_api_version(void *data, NihDBusMessage *message, int *version)
{
- timeout_reset(message->connection);
nih_assert(message);
nih_assert(version);
*version = API_VERSION;
@@ -1650,7 +1535,6 @@ int client_connect (DBusServer *server, DBusConnection *conn)
"/org/linuxcontainers/cgmanager",
cgmanager_interfaces, NULL));
- timeout_add(conn);
return TRUE;
}
@@ -1659,12 +1543,5 @@ void client_disconnect (DBusConnection *conn)
if (conn == NULL)
return;
- timeout_remove(conn);
nih_info (_("Disconnected from private client"));
}
-
-void connection_timeout_init(void)
-{
- timer_hash = NIH_MUST( nih_hash_new(NULL, 1024,
- timer_key_fn, timer_hash_fn, timer_cmp_fn) );
-}
diff --git a/frontend.h b/frontend.h
index 20ed266..3e41cb3 100644
--- a/frontend.h
+++ b/frontend.h
@@ -155,7 +155,6 @@ int cgmanager_ping (void *data, NihDBusMessage *message, int junk);
int client_connect (DBusServer *server, DBusConnection *conn);
void client_disconnect (DBusConnection *conn);
-void connection_timeout_init(void);
bool sane_cgroup(const char *cgroup);
diff --git a/tests/test-timeout.c b/tests/test-timeout.c
deleted file mode 100644
index 2c9be61..0000000
--- a/tests/test-timeout.c
+++ /dev/null
@@ -1,169 +0,0 @@
-/* concurrent.c
- *
- * Copyright © 2013 S.Çağlar Onur <caglar at 10ur.org>
- * Copyright © 2014 Serge Hallyn <serge.hallyn at ubuntu.com>
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2, as
- * published by the Free Software Foundation.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License along
- * with this program; if not, write to the Free Software Foundation, Inc.,
- * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
- *
- * compile with
-gcc -o cgm-concurrent cgm-concurrent.c -I/usr/include/dbus-1.0 -I/usr/lib/x86_64-linux-gnu/dbus-1.0/include -I/usr/include/cgmanager -lcgmanager -lnih -lnih-dbus -ldbus-1 -I/usr/include/dbus-1.0 -I/usr/lib/x86_64-linux-gnux/dbus-1.0/include -ldbus-1 -lpthread -g XXX
- */
-
-#include <stdio.h>
-#include <stdlib.h>
-#include <errno.h>
-#include <unistd.h>
-#include <string.h>
-#include <dirent.h>
-#include <fcntl.h>
-#include <ctype.h>
-#include <pthread.h>
-#include <grp.h>
-#include <sys/types.h>
-#include <sys/stat.h>
-#include <sys/param.h>
-#include <sys/inotify.h>
-#include <sys/mount.h>
-#include <netinet/in.h>
-#include <net/if.h>
-#include <sys/syscall.h>
-
-#include <limits.h>
-#include <malloc.h>
-#include <getopt.h>
-#include <stdlib.h>
-
-#include <nih-dbus/dbus_connection.h>
-#include <cgmanager/cgmanager-client.h>
-#include <nih/alloc.h>
-#include <nih/error.h>
-#include <nih/string.h>
-#include <stdbool.h>
-#include <cgmanager-client.h>
-
-NihDBusProxy *cgroup_manager = NULL;
-DBusConnection *connection = NULL;
-
-static const struct option options[] = {
- { "threads", required_argument, NULL, 'j' },
- { "iterations", required_argument, NULL, 'i' },
- { "connect-only", no_argument, NULL, 'c' },
- { 0, 0, 0, 0 },
-};
-
-void cgm_dbus_disconnected(DBusConnection *connection)
-{
- cgroup_manager = NULL;
- connection = NULL;
-}
-
-static void cgm_dbus_disconnect(void)
-{
- if (cgroup_manager)
- nih_free(cgroup_manager);
- cgroup_manager = NULL;
- if (connection) {
- dbus_connection_flush(connection);
- dbus_connection_close(connection);
- dbus_connection_unref(connection);
- }
- connection = NULL;
-}
-
-#define CGMANAGER_DBUS_SOCK "unix:path=/sys/fs/cgroup/cgmanager/sock"
-static bool cgm_dbus_connect(void)
-{
- DBusError dbus_error;
-
- dbus_error_init(&dbus_error);
-
- connection = nih_dbus_connect(CGMANAGER_DBUS_SOCK, cgm_dbus_disconnected);
- if (!connection) {
- dbus_error_free(&dbus_error);
- return false;
- }
- dbus_connection_set_exit_on_disconnect(connection, FALSE);
- dbus_error_free(&dbus_error);
- cgroup_manager = nih_dbus_proxy_new(NULL, connection,
- NULL /* p2p */,
- "/org/linuxcontainers/cgmanager", NULL, NULL);
- if (!cgroup_manager) {
- NihError *nerr;
- nerr = nih_error_get();
- nih_free(nerr);
- cgm_dbus_disconnect();
- return false;
- }
-
- // force fd passing negotiation
- if (cgmanager_ping_sync(NULL, cgroup_manager, 0) != 0) {
- NihError *nerr;
- nerr = nih_error_get();
- nih_free(nerr);
- cgm_dbus_disconnect();
- return false;
- }
- return true;
-}
-
-int main(int argc, char *argv[]) {
-
- int32_t v;
- NihError *nerr;
-
- if (!cgm_dbus_connect()) {
- fprintf(stderr, "Error connecting to dbus\n");
- exit(1);
- }
-
- sleep(30);
- if (cgmanager_ping_sync(NULL, cgroup_manager, v) == 0) {
- fprintf(stderr, "connection did not time out");
- exit(1);
- }
- nerr = nih_error_get();
- nih_free(nerr);
-
- /* Now open a connection, wait 15 seconds, do a request,
- * wait another 15 seconds - connection should still be open;
- * wait anothe 15 seconds, it should be closed.
- */
- if (!cgm_dbus_connect()) {
- fprintf(stderr, "Error connecting to dbus\n");
- exit(1);
- }
- sleep(15);
- if (cgmanager_get_api_version_sync(NULL, cgroup_manager, &v) != 0) {
- nerr = nih_error_get();
- nih_free(nerr);
- fprintf(stderr, "Error creating freezer cgroup\n");
- exit(1);
- }
- sleep(15);
- if (cgmanager_ping_sync(NULL, cgroup_manager, v) != 0) {
- nerr = nih_error_get();
- nih_free(nerr);
- fprintf(stderr, "timeout was not reset");
- exit(1);
- }
- sleep(30);
- if (cgmanager_ping_sync(NULL, cgroup_manager, v) == 0) {
- fprintf(stderr, "connection did not time out after a reset");
- exit(1);
- }
- nerr = nih_error_get();
- nih_free(nerr);
- fprintf(stderr, "all tests passed\n");
- exit(0);
-}
--
1.9.1
More information about the cgmanager-devel
mailing list