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-3.0.0.beta1~130^2~6
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=bc631984fcc1c8b69cafbcce448f98961bca1e55;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 d7e41e5a7..d95530ec7 100644
--- a/src/lxc/commands.c
+++ b/src/lxc/commands.c
@@ -893,9 +893,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,
@@ -904,17 +903,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] == 1) {
-		TRACE("Container is in %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;
@@ -924,11 +916,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;
 }
 
@@ -947,10 +947,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 394f0a1d7..32831e2b6 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 lxc_state_client *newclient;
 	struct lxc_list *tmplist;
 
@@ -211,11 +212,17 @@ int lxc_add_state_client(int state_client_fd, struct lxc_handler *handler,
 	}
 
 	process_lock();
-	lxc_list_add_elem(tmplist, newclient);
-	lxc_list_add_tail(&handler->conf->state_clients, tmplist);
-	process_unlock();
+	state = handler->state;
+	if (states[state] != 1) {
+		lxc_list_add_elem(tmplist, newclient);
+		lxc_list_add_tail(&handler->conf->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 ce13a3c50..72e3f0146 100644
--- a/src/lxc/start.c
+++ b/src/lxc/start.c
@@ -358,10 +358,10 @@ static int lxc_serve_state_clients(const char *name,
 	struct lxc_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->conf->state_clients)) {
 		TRACE("No state clients registered");
 		process_unlock();