From: Christian Brauner <christian.brauner@ubuntu.com>
Date: Fri, 24 Nov 2017 11:51:04 +0000 (+0100)
Subject: commands: tweak locking
X-Git-Tag: lxc-2.0.10~515
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=7ccbce50ac6b6e6c96057b86235c3ebb435c4bca;p=thirdparty%2Flxc.git

commands: tweak locking

Take the lock on the list after we've done all necessary work and check state.
If we are in requested state, do cleanup and return without adding the state
client to the state client list.

Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
---

diff --git a/src/lxc/commands.c b/src/lxc/commands.c
index a6948b2f2..5b82fadd2 100644
--- a/src/lxc/commands.c
+++ b/src/lxc/commands.c
@@ -829,9 +829,8 @@ int lxc_cmd_add_state_client(const char *name, const char *lxcpath,
 			     lxc_state_t states[MAX_STATE],
 			     int *state_client_fd)
 {
-	int stopped;
+	int state, stopped;
 	ssize_t ret;
-	int state = -1;
 	struct lxc_cmd_rr cmd = {
 	    .req = {
 		.cmd     = LXC_CMD_ADD_STATE_CLIENT,
@@ -840,17 +839,10 @@ int lxc_cmd_add_state_client(const char *name, const char *lxcpath,
 	    },
 	};
 
-	/* Check if already in requested state. */
-	state = lxc_getstate(name, lxcpath);
-	if (state < 0) {
-		TRACE("%s - Failed to retrieve state of container", strerror(errno));
-		return -1;
-	} else if (states[state]) {
-		TRACE("Container is %s state", lxc_state2str(state));
-		return state;
-	}
-
 	ret = lxc_cmd(name, &cmd, &stopped, lxcpath, NULL);
+	if (states[STOPPED] != 0 && stopped != 0)
+		return STOPPED;
+
 	if (ret < 0) {
 		ERROR("%s - Failed to execute command", strerror(errno));
 		return -1;
@@ -860,11 +852,19 @@ int lxc_cmd_add_state_client(const char *name, const char *lxcpath,
 	 * function.
 	 */
 	if (cmd.rsp.ret < 0) {
-		ERROR("Failed to receive socket fd");
+		ERROR("%s - Failed to receive socket fd", strerror(-cmd.rsp.ret));
 		return -1;
 	}
 
+	state = PTR_TO_INT(cmd.rsp.data);
+	if (state < MAX_STATE) {
+		TRACE("Container is already in requested state %s",
+		      lxc_state2str(state));
+		return state;
+	}
+
 	*state_client_fd = cmd.rsp.ret;
+	TRACE("Added state client %d to state client list", cmd.rsp.ret);
 	return MAX_STATE;
 }
 
@@ -883,10 +883,7 @@ static int lxc_cmd_add_state_client_callback(int fd, struct lxc_cmd_req *req,
 		return -1;
 
 	rsp.ret = lxc_add_state_client(fd, handler, (lxc_state_t *)req->data);
-	if (rsp.ret < 0)
-		ERROR("Failed to add state client %d to state client list", fd);
-	else
-		TRACE("Added state client %d to state client list", fd);
+	rsp.data = INT_TO_PTR(rsp.ret);
 
 	return lxc_cmd_rsp_send(fd, &rsp);
 }
diff --git a/src/lxc/commands_utils.c b/src/lxc/commands_utils.c
index e03022f28..986d9904b 100644
--- a/src/lxc/commands_utils.c
+++ b/src/lxc/commands_utils.c
@@ -193,6 +193,7 @@ int lxc_cmd_connect(const char *name, const char *lxcpath,
 int lxc_add_state_client(int state_client_fd, struct lxc_handler *handler,
 			 lxc_state_t states[MAX_STATE])
 {
+	int state;
 	struct state_client *newclient;
 	struct lxc_list *tmplist;
 
@@ -210,12 +211,17 @@ int lxc_add_state_client(int state_client_fd, struct lxc_handler *handler,
 		return -ENOMEM;
 	}
 
-	process_lock();
-	lxc_list_add_elem(tmplist, newclient);
-	lxc_list_add_tail(&handler->state_clients, tmplist);
-	process_unlock();
+	state = handler->state;
+	if (states[state] != 1) {
+		lxc_list_add_elem(tmplist, newclient);
+		lxc_list_add_tail(&handler->state_clients, tmplist);
+		process_unlock();
+	} else {
+		process_unlock();
+		free(newclient);
+		return state;
+	}
 
 	TRACE("added state client %d to state client list", state_client_fd);
-
-	return 0;
+	return MAX_STATE;
 }
diff --git a/src/lxc/start.c b/src/lxc/start.c
index 4e8773562..1892df652 100644
--- a/src/lxc/start.c
+++ b/src/lxc/start.c
@@ -351,10 +351,10 @@ static int lxc_serve_state_clients(const char *name,
 	struct state_client *client;
 	struct lxc_msg msg = {.type = lxc_msg_state, .value = state};
 
+	process_lock();
 	handler->state = state;
 	TRACE("Set container state to %s", lxc_state2str(state));
 
-	process_lock();
 	if (lxc_list_empty(&handler->state_clients)) {
 		TRACE("No state clients registered");
 		process_unlock();