[lxc-devel] [PATCH RFC] split up use of process_lock, and make process_lock return void

Serge Hallyn serge.hallyn at ubuntu.com
Mon Sep 16 23:28:41 UTC 2013


pthread_mutex_lock() will only return an error if it was set to
PTHREAD_MUTEX_ERRORCHECK and we are recursively calling it (and
would otherwise have deadlocked).  If that's the case then log a
message for future debugging and exit.  Trying to "recover" is
nonsense at that point.

process_lock() was held over too long a time in lxcapi_start()
in the daemonize case.  (note the non-daemonized case still needs a
check to enforce that it must NOT be called while threaded).  Add
process_lock() at least across all open/close/socket() calls.

Note that anything after we've done a fork() doesn't need the
locks as it is no longer threaded.

Tested that lp:~serge-hallyn/+junk/lxc-test still works with this
patch.

Signed-off-by: Serge Hallyn <serge.hallyn at ubuntu.com>
---
 src/lxc/af_unix.c      |  10 +++
 src/lxc/bdev.c         |  80 +++++++++++++++++---
 src/lxc/commands.c     |  10 +++
 src/lxc/console.c      |  16 ++--
 src/lxc/lxccontainer.c | 196 ++++++++++++++++++++++++++++++++-----------------
 src/lxc/lxclock.c      |   9 ++-
 src/lxc/lxclock.h      |   2 +-
 src/lxc/monitor.c      |  17 ++++-
 src/lxc/state.c        |   5 ++
 9 files changed, 254 insertions(+), 91 deletions(-)

diff --git a/src/lxc/af_unix.c b/src/lxc/af_unix.c
index d6d18ca..333f05e 100644
--- a/src/lxc/af_unix.c
+++ b/src/lxc/af_unix.c
@@ -43,7 +43,9 @@ int lxc_af_unix_open(const char *path, int type, int flags)
 	if (flags & O_TRUNC)
 		unlink(path);
 
+	process_lock();
 	fd = socket(PF_UNIX, type, 0);
+	process_unlock();
 	if (fd < 0)
 		return -1;
 
@@ -58,7 +60,9 @@ int lxc_af_unix_open(const char *path, int type, int flags)
 	if (path[0]) {
 		len = strlen(path);
 		if (len >= sizeof(addr.sun_path)) {
+			process_lock();
 			close(fd);
+			process_unlock();
 			errno = ENAMETOOLONG;
 			return -1;
 		}
@@ -67,14 +71,18 @@ int lxc_af_unix_open(const char *path, int type, int flags)
 
 	if (bind(fd, (struct sockaddr *)&addr, sizeof(addr))) {
 		int tmp = errno;
+		process_lock();
 		close(fd);
+		process_unlock();
 		errno = tmp;
 		return -1;
 	}
 
 	if (type == SOCK_STREAM && listen(fd, 100)) {
 		int tmp = errno;
+		process_lock();
 		close(fd);
+		process_unlock();
 		errno = tmp;
 		return -1;
 	}
@@ -91,7 +99,9 @@ int lxc_af_unix_close(int fd)
 	    addr.sun_path[0])
 		unlink(addr.sun_path);
 
+	process_lock();
 	close(fd);
+	process_unlock();
 
 	return 0;
 }
diff --git a/src/lxc/bdev.c b/src/lxc/bdev.c
index b45f2cb..7cec5d8 100644
--- a/src/lxc/bdev.c
+++ b/src/lxc/bdev.c
@@ -93,11 +93,15 @@ static int blk_getsize(struct bdev *bdev, unsigned long *size)
 	if (strcmp(bdev->type, "loop") == 0)
 		path = bdev->src + 5;
 
+	process_lock();
 	fd = open(path, O_RDONLY);
+	process_unlock();
 	if (fd < 0)
 		return -1;
 	ret = ioctl(fd, BLKGETSIZE64, size);
+	process_lock();
 	close(fd);
+	process_unlock();
 	return ret;
 }
 
@@ -252,16 +256,23 @@ static int detect_fs(struct bdev *bdev, char *type, int len)
 	if (strcmp(bdev->type, "loop") == 0)
 		srcdev = bdev->src + 5;
 
-	if (pipe(p) < 0)
+	process_lock();
+	ret = pipe(p);
+	process_unlock();
+	if (ret < 0)
 		return -1;
 	if ((pid = fork()) < 0)
 		return -1;
 	if (pid > 0) {
 		int status;
+		process_lock();
 		close(p[1]);
+		process_unlock();
 		memset(type, 0, len);
 		ret = read(p[0], type, len-1);
+		process_lock();
 		close(p[0]);
+		process_unlock();
 		if (ret < 0) {
 			SYSERROR("error reading from pipe");
 			wait(&status);
@@ -488,7 +499,10 @@ static int zfs_list_entry(const char *path, char *output, size_t inlen)
 	FILE *f;
 	int found=0;
 
-	if ((f = popen("zfs list 2> /dev/null", "r")) == NULL) {
+	process_lock();
+	f = popen("zfs list 2> /dev/null", "r");
+	process_unlock();
+	if (f == NULL) {
 		SYSERROR("popen failed");
 		return 0;
 	}
@@ -498,7 +512,9 @@ static int zfs_list_entry(const char *path, char *output, size_t inlen)
 			break;
 		}
 	}
+	process_lock();
 	(void) pclose(f);
+	process_unlock();
 
 	return found;
 }
@@ -750,11 +766,15 @@ static int lvm_detect(const char *path)
 		ERROR("lvm uuid pathname too long");
 		return 0;
 	}
+	process_lock();
 	fout = fopen(devp, "r");
+	process_unlock();
 	if (!fout)
 		return 0;
 	ret = fread(buf, 1, 4, fout);
+	process_lock();
 	fclose(fout);
+	process_unlock();
 	if (ret != 4 || strncmp(buf, "LVM-", 4) != 0)
 		return 0;
 	return 1;
@@ -1052,13 +1072,17 @@ static bool is_btrfs_fs(const char *path)
 	struct btrfs_ioctl_space_args sargs;
 
 	// make sure this is a btrfs filesystem
+	process_lock();
 	fd = open(path, O_RDONLY);
+	process_unlock();
 	if (fd < 0)
 		return false;
 	sargs.space_slots = 0;
 	sargs.total_spaces = 0;
 	ret = ioctl(fd, BTRFS_IOC_SPACE_INFO, &sargs);
+	process_lock();
 	close(fd);
+	process_unlock();
 	if (ret < 0)
 		return false;
 
@@ -1155,7 +1179,10 @@ static int btrfs_subvolume_create(const char *path)
 	}
 	*p = '\0';
 
-	if ((fd = open(newfull, O_RDONLY)) < 0) {
+	process_lock();
+	fd = open(newfull, O_RDONLY);
+	process_unlock();
+	if (fd < 0) {
 		ERROR("Error opening %s", newfull);
 		free(newfull);
 		return -1;
@@ -1168,7 +1195,9 @@ static int btrfs_subvolume_create(const char *path)
 	INFO("btrfs: snapshot create ioctl returned %d", ret);
 
 	free(newfull);
+	process_lock();
 	close(fd);
+	process_unlock();
 	return ret;
 }
 
@@ -1190,12 +1219,14 @@ static int btrfs_snapshot(const char *orig, const char *new)
 	}
 	newname = basename(newfull);
 	newdir = dirname(newfull);
+	process_lock();
 	fd = open(orig, O_RDONLY);
+	fddst = open(newdir, O_RDONLY);
+	process_unlock();
 	if (fd < 0) {
 		SYSERROR("Error opening original rootfs %s", orig);
 		goto out;
 	}
-	fddst = open(newdir, O_RDONLY);
 	if (fddst < 0) {
 		SYSERROR("Error opening new container dir %s", newdir);
 		goto out;
@@ -1209,10 +1240,12 @@ static int btrfs_snapshot(const char *orig, const char *new)
 	INFO("btrfs: snapshot create ioctl returned %d", ret);
 
 out:
+	process_lock();
 	if (fddst != -1)
 		close(fddst);
 	if (fd != -1)
 		close(fd);
+	process_unlock();
 	if (newfull)
 		free(newfull);
 	return ret;
@@ -1282,7 +1315,10 @@ static int btrfs_destroy(struct bdev *orig)
 	}
 	*p = '\0';
 
-	if ((fd = open(newfull, O_RDONLY)) < 0) {
+	process_lock();
+	fd = open(newfull, O_RDONLY);
+	process_unlock();
+	if (fd < 0) {
 		ERROR("Error opening %s", newfull);
 		free(newfull);
 		return -1;
@@ -1295,7 +1331,9 @@ static int btrfs_destroy(struct bdev *orig)
 	INFO("btrfs: snapshot create ioctl returned %d", ret);
 
 	free(newfull);
+	process_lock();
 	close(fd);
+	process_unlock();
 	return ret;
 }
 
@@ -1335,7 +1373,10 @@ static int find_free_loopdev(int *retfd, char *namep)
 	DIR *dir;
 	int fd = -1;
 
-	if (!(dir = opendir("/dev"))) {
+	process_lock();
+	dir = opendir("/dev");
+	process_unlock();
+	if (!dir) {
 		SYSERROR("Error opening /dev");
 		return -1;
 	}
@@ -1345,10 +1386,15 @@ static int find_free_loopdev(int *retfd, char *namep)
 			break;
 		if (strncmp(direntp->d_name, "loop", 4) != 0)
 			continue;
-		if ((fd = openat(dirfd(dir), direntp->d_name, O_RDWR)) < 0)
+		process_lock();
+		fd = openat(dirfd(dir), direntp->d_name, O_RDWR);
+		process_unlock();
+		if (fd < 0)
 			continue;
 		if (ioctl(fd, LOOP_GET_STATUS64, &lo) == 0 || errno != ENXIO) {
+			process_lock();
 			close(fd);
+			process_unlock();
 			fd = -1;
 			continue;
 		}
@@ -1356,7 +1402,9 @@ static int find_free_loopdev(int *retfd, char *namep)
 		snprintf(namep, 100, "/dev/%s", direntp->d_name);
 		break;
 	}
+	process_lock();
 	closedir(dir);
+	process_unlock();
 	if (fd == -1) {
 		ERROR("No loop device found");
 		return -1;
@@ -1379,7 +1427,10 @@ static int loop_mount(struct bdev *bdev)
 	if (find_free_loopdev(&lfd, loname) < 0)
 		return -22;
 
-	if ((ffd = open(bdev->src + 5, O_RDWR)) < 0) {
+	process_lock();
+	ffd = open(bdev->src + 5, O_RDWR);
+	process_unlock();
+	if (ffd < 0) {
 		SYSERROR("Error opening backing file %s\n", bdev->src);
 		goto out;
 	}
@@ -1402,12 +1453,14 @@ static int loop_mount(struct bdev *bdev)
 		bdev->lofd = lfd;
 
 out:
+	process_lock();
 	if (ffd > -1)
 		close(ffd);
 	if (ret < 0) {
 		close(lfd);
 		bdev->lofd = -1;
 	}
+	process_unlock();
 	return ret;
 }
 
@@ -1421,7 +1474,9 @@ static int loop_umount(struct bdev *bdev)
 		return -22;
 	ret = umount(bdev->dest);
 	if (bdev->lofd >= 0) {
+		process_lock();
 		close(bdev->lofd);
+		process_unlock();
 		bdev->lofd = -1;
 	}
 	return ret;
@@ -1429,9 +1484,11 @@ static int loop_umount(struct bdev *bdev)
 
 static int do_loop_create(const char *path, unsigned long size, const char *fstype)
 {
-	int fd;
+	int fd, ret;
 	// create the new loopback file.
+	process_lock();
 	fd = creat(path, S_IRUSR|S_IWUSR);
+	process_unlock();
 	if (fd < 0)
 		return -1;
 	if (lseek(fd, size, SEEK_SET) < 0) {
@@ -1444,7 +1501,10 @@ static int do_loop_create(const char *path, unsigned long size, const char *fsty
 		close(fd);
 		return -1;
 	}
-	if (close(fd) < 0) {
+	process_lock();
+	ret = close(fd);
+	process_unlock();
+	if (ret < 0) {
 		SYSERROR("Error closing new loop file");
 		return -1;
 	}
diff --git a/src/lxc/commands.c b/src/lxc/commands.c
index c706a7a..59ea9cb 100644
--- a/src/lxc/commands.c
+++ b/src/lxc/commands.c
@@ -720,7 +720,9 @@ static void lxc_cmd_fd_cleanup(int fd, struct lxc_handler *handler,
 {
 	lxc_console_free(handler->conf, fd);
 	lxc_mainloop_del_handler(descr, fd);
+	process_lock();
 	close(fd);
+	process_unlock();
 }
 
 static int lxc_cmd_handler(int fd, void *data, struct lxc_epoll_descr *descr)
@@ -791,7 +793,9 @@ static int lxc_cmd_accept(int fd, void *data, struct lxc_epoll_descr *descr)
 {
 	int opt = 1, ret = -1, connection;
 
+	process_lock();
 	connection = accept(fd, NULL, 0);
+	process_unlock();
 	if (connection < 0) {
 		SYSERROR("failed to accept connection");
 		return -1;
@@ -818,7 +822,9 @@ out:
 	return ret;
 
 out_close:
+	process_lock();
 	close(connection);
+	process_unlock();
 	goto out;
 }
 
@@ -847,7 +853,9 @@ int lxc_cmd_init(const char *name, struct lxc_handler *handler,
 
 	if (fcntl(fd, F_SETFD, FD_CLOEXEC)) {
 		SYSERROR("failed to set sigfd to close-on-exec");
+		process_lock();
 		close(fd);
+		process_unlock();
 		return -1;
 	}
 
@@ -864,7 +872,9 @@ int lxc_cmd_mainloop_add(const char *name,
 	ret = lxc_mainloop_add_handler(descr, fd, lxc_cmd_accept, handler);
 	if (ret) {
 		ERROR("failed to add handler for command socket");
+		process_lock();
 		close(fd);
+		process_unlock();
 	}
 
 	return ret;
diff --git a/src/lxc/console.c b/src/lxc/console.c
index e35a811..f503f18 100644
--- a/src/lxc/console.c
+++ b/src/lxc/console.c
@@ -100,16 +100,16 @@ static void lxc_console_winch(struct lxc_tty_state *ts)
 
 void lxc_console_sigwinch(int sig)
 {
-	if (process_lock() == 0) {
-		struct lxc_list *it;
-		struct lxc_tty_state *ts;
+	struct lxc_list *it;
+	struct lxc_tty_state *ts;
 
-		lxc_list_for_each(it, &lxc_ttys) {
-			ts = it->elem;
-			lxc_console_winch(ts);
-		}
-		process_unlock();
+	process_lock();
+
+	lxc_list_for_each(it, &lxc_ttys) {
+		ts = it->elem;
+		lxc_console_winch(ts);
 	}
+	process_unlock();
 }
 
 static int lxc_console_cb_sigwinch_fd(int fd, void *cbdata,
diff --git a/src/lxc/lxccontainer.c b/src/lxc/lxccontainer.c
index 3c51c4a..9458b4b 100644
--- a/src/lxc/lxccontainer.c
+++ b/src/lxc/lxccontainer.c
@@ -100,12 +100,12 @@ int ongoing_create(struct lxc_container *c)
 
 	if (!file_exists(path))
 		return 0;
-	if (process_lock())
-		return -1;
-	if ((fd = open(path, O_RDWR)) < 0) {
+	process_lock();
+	fd = open(path, O_RDWR);
+	process_unlock();
+	if (fd < 0) {
 		// give benefit of the doubt
 		SYSERROR("Error opening partial file");
-		process_unlock();
 		return 0;
 	}
 	lk.l_type = F_WRLCK;
@@ -115,11 +115,13 @@ int ongoing_create(struct lxc_container *c)
 	lk.l_pid = -1;
 	if (fcntl(fd, F_GETLK, &lk) == 0 && lk.l_pid != -1) {
 		// create is still ongoing
+		process_lock();
 		close(fd);
 		process_unlock();
 		return 1;
 	}
 	// create completed but partial is still there.
+	process_lock();
 	close(fd);
 	process_unlock();
 	return 2;
@@ -138,8 +140,7 @@ int create_partial(struct lxc_container *c)
 		ERROR("Error writing partial pathname");
 		return -1;
 	}
-	if (process_lock())
-		return -1;
+	process_lock();
 	if ((fd=open(path, O_RDWR | O_CREAT | O_EXCL, 0755)) < 0) {
 		SYSERROR("Erorr creating partial file");
 		process_unlock();
@@ -167,17 +168,16 @@ void remove_partial(struct lxc_container *c, int fd)
 	char *path = alloca(len);
 	int ret;
 
+	process_lock();
 	close(fd);
+	process_unlock();
 	ret = snprintf(path, len, "%s/%s/partial", c->config_path, c->name);
 	if (ret < 0 || ret >= len) {
 		ERROR("Error writing partial pathname");
 		return;
 	}
-	if (process_lock())
-		return;
 	if (unlink(path) < 0)
 		SYSERROR("Error unlink partial file %s", path);
-	process_unlock();
 }
 
 /* LOCKING
@@ -546,20 +546,14 @@ static bool lxcapi_start(struct lxc_container *c, int useinit, char * const argv
 			return false;
 		lxc_monitord_spawn(c->config_path);
 
-		if (process_lock())
-			return false;
 		pid_t pid = fork();
 		if (pid < 0) {
 			lxc_container_put(c);
-			process_unlock();
 			return false;
 		}
-		if (pid != 0) {
-			ret = wait_on_daemonized_start(c);
-			process_unlock();
-			return ret;
-		}
-		process_unlock();
+		if (pid != 0)
+			return wait_on_daemonized_start(c);
+
 		/* second fork to be reparented by init */
 		pid = fork();
 		if (pid < 0) {
@@ -882,56 +876,44 @@ bool prepend_lxc_header(char *path, const char *t, char *const argv[])
 	long flen;
 	char *contents;
 	FILE *f;
+	int ret = -1;
 #if HAVE_LIBGNUTLS
-	int i, ret;
+	int i;
 	unsigned char md_value[SHA_DIGEST_LENGTH];
 	char *tpath;
 	bool have_tpath = false;
 #endif
 
-	if ((f = fopen(path, "r")) == NULL) {
-		SYSERROR("Opening old config");
-		return false;
-	}
-	if (fseek(f, 0, SEEK_END) < 0) {
-		SYSERROR("Seeking to end of old config file");
-		fclose(f);
-		return false;
-	}
-	if ((flen = ftell(f)) < 0) {
-		SYSERROR("telling size of old config");
-		fclose(f);
-		return false;
-	}
-	if (fseek(f, 0, SEEK_SET) < 0) {
-		SYSERROR("rewinding old config");
-		fclose(f);
-		return false;
-	}
-	if ((contents = malloc(flen + 1)) == NULL) {
-		SYSERROR("out of memory");
-		fclose(f);
-		return false;
-	}
-	if (fread(contents, 1, flen, f) != flen) {
-		SYSERROR("Reading old config");
-		free(contents);
-		fclose(f);
+	process_lock();
+	f = fopen(path, "r");
+	process_unlock();
+	if (f == NULL)
 		return false;
-	}
+
+	if (fseek(f, 0, SEEK_END) < 0)
+		goto out_error;
+	if ((flen = ftell(f)) < 0)
+		goto out_error;
+	if (fseek(f, 0, SEEK_SET) < 0)
+		goto out_error;
+	if ((contents = malloc(flen + 1)) == NULL)
+		goto out_error;
+	if (fread(contents, 1, flen, f) != flen)
+		goto out_free_contents;
+
 	contents[flen] = '\0';
-	if (fclose(f) < 0) {
-		SYSERROR("closing old config");
-		free(contents);
-		return false;
-	}
+	process_lock();
+	ret = fclose(f);
+	process_unlock();
+	f = NULL;
+	if (ret < 0)
+		goto out_free_contents;
 
 #if HAVE_LIBGNUTLS
 	tpath = get_template_path(t);
 	if (tpath == (char *) -1) {
 		ERROR("bad template: %s\n", t);
-		free(contents);
-		return false;
+		goto out_free_contents;
 	}
 
 	if (tpath) {
@@ -939,14 +921,16 @@ bool prepend_lxc_header(char *path, const char *t, char *const argv[])
 		ret = sha1sum_file(tpath, md_value);
 		if (ret < 0) {
 			ERROR("Error getting sha1sum of %s", tpath);
-			free(contents);
-			return false;
+			goto out_free_contents;
 		}
 		free(tpath);
 	}
 #endif
 
-	if ((f = fopen(path, "w")) == NULL) {
+	process_lock();
+	f = fopen(path, "w");
+	process_unlock();
+	if (f == NULL) {
 		SYSERROR("reopening config for writing");
 		free(contents);
 		return false;
@@ -971,12 +955,25 @@ bool prepend_lxc_header(char *path, const char *t, char *const argv[])
 	if (fwrite(contents, 1, flen, f) != flen) {
 		SYSERROR("Writing original contents");
 		free(contents);
+		process_lock();
 		fclose(f);
+		process_unlock();
 		return false;
 	}
+	ret = 0;
+out_free_contents:
 	free(contents);
-	if (fclose(f) < 0) {
-		SYSERROR("Closing config file after write");
+out_error:
+	if (f) {
+		int newret;
+		process_lock();
+		newret = fclose(f);
+		process_unlock();
+		if (ret == 0)
+			ret = newret;
+	}
+	if (ret < 0) {
+		SYSERROR("Error prepending header");
 		return false;
 	}
 	return true;
@@ -1196,7 +1193,9 @@ char** lxcapi_get_ips(struct lxc_container *c, char* interface, char* family, in
 		goto out;
 
 	/* Save reference to old netns */
+	process_lock();
 	old_netns = open("/proc/self/ns/net", O_RDONLY);
+	process_unlock();
 	if (old_netns < 0) {
 		SYSERROR("failed to open /proc/self/ns/net");
 		goto out;
@@ -1207,7 +1206,9 @@ char** lxcapi_get_ips(struct lxc_container *c, char* interface, char* family, in
 	if (ret < 0 || ret >= MAXPATHLEN)
 		goto out;
 
+	process_lock();
 	new_netns = open(new_netns_path, O_RDONLY);
+	process_unlock();
 	if (new_netns < 0) {
 		SYSERROR("failed to open %s", new_netns_path);
 		goto out;
@@ -1270,10 +1271,12 @@ out:
 	/* Switch back to original netns */
 	if (old_netns >= 0 && setns(old_netns, CLONE_NEWNET))
 		SYSERROR("failed to setns");
+	process_lock();
 	if (new_netns >= 0)
 		close(new_netns);
 	if (old_netns >= 0)
 		close(old_netns);
+	process_unlock();
 
 	/* Append NULL to the array */
 	if (count) {
@@ -1396,25 +1399,36 @@ static bool mod_rdep(struct lxc_container *c, bool inc)
 			c->name);
 	if (ret < 0 || ret > MAXPATHLEN)
 		goto out;
+	process_lock();
 	f = fopen(path, "r");
+	process_unlock();
 	if (f) {
 		ret = fscanf(f, "%d", &v);
+		process_lock();
 		fclose(f);
+		process_unlock();
 		if (ret != 1) {
 			ERROR("Corrupted file %s", path);
 			goto out;
 		}
 	}
 	v += inc ? 1 : -1;
+	process_lock();
 	f = fopen(path, "w");
+	process_unlock();
 	if (!f)
 		goto out;
 	if (fprintf(f, "%d\n", v) < 0) {
 		ERROR("Error writing new snapshots value");
+		process_lock();
 		fclose(f);
+		process_unlock();
 		goto out;
 	}
-	if (fclose(f) != 0) {
+	process_lock();
+	ret = fclose(f);
+	process_unlock();
+	if (ret != 0) {
 		SYSERROR("Error writing to or closing snapshots file");
 		goto out;
 	}
@@ -1449,7 +1463,10 @@ static void mod_all_rdeps(struct lxc_container *c, bool inc)
 		ERROR("Path name too long");
 		return;
 	}
-	if ((f = fopen(path, "r")) == NULL)
+	process_lock();
+	f = fopen(path, "r");
+	process_unlock();
+	if (f == NULL)
 		return;
 	while (getline(&lxcpath, &pathlen, f) != -1) {
 		if (getline(&lxcname, &namelen, f) == -1) {
@@ -1471,7 +1488,9 @@ static void mod_all_rdeps(struct lxc_container *c, bool inc)
 out:
 	if (lxcpath) free(lxcpath);
 	if (lxcname) free(lxcname);
+	process_lock();
 	fclose(f);
+	process_unlock();
 }
 
 static bool has_snapshots(struct lxc_container *c)
@@ -1485,11 +1504,15 @@ static bool has_snapshots(struct lxc_container *c)
 			c->name);
 	if (ret < 0 || ret > MAXPATHLEN)
 		goto out;
+	process_lock();
 	f = fopen(path, "r");
+	process_unlock();
 	if (!f)
 		goto out;
 	ret = fscanf(f, "%d", &v);
+	process_lock();
 	fclose(f);
+	process_unlock();
 	if (ret != 1)
 		goto out;
 	bret = v != 0;
@@ -1741,15 +1764,21 @@ static int copy_file(char *old, char *new)
 		return -1;
 	}
 
+	process_lock();
 	in = open(old, O_RDONLY);
+	process_unlock();
 	if (in < 0) {
 		SYSERROR("Error opening original file %s", old);
 		return -1;
 	}
+	process_lock();
 	out = open(new, O_CREAT | O_EXCL | O_WRONLY, 0644);
+	process_unlock();
 	if (out < 0) {
 		SYSERROR("Error opening new file %s", new);
+		process_lock();
 		close(in);
+		process_unlock();
 		return -1;
 	}
 
@@ -1767,8 +1796,10 @@ static int copy_file(char *old, char *new)
 			goto err;
 		}
 	}
+	process_lock();
 	close(in);
 	close(out);
+	process_unlock();
 
 	// we set mode, but not owner/group
 	ret = chmod(new, sbuf.st_mode);
@@ -1780,8 +1811,10 @@ static int copy_file(char *old, char *new)
 	return 0;
 
 err:
+	process_lock();
 	close(in);
 	close(out);
+	process_unlock();
 	return -1;
 }
 
@@ -1821,13 +1854,18 @@ static int copyhooks(struct lxc_container *oldc, struct lxc_container *c)
 
 static void new_hwaddr(char *hwaddr)
 {
-	FILE *f = fopen("/dev/urandom", "r");
+	FILE *f;
+	process_lock();
+	f = fopen("/dev/urandom", "r");
+	process_unlock();
 	if (f) {
 		unsigned int seed;
 		int ret = fread(&seed, sizeof(seed), 1, f);
 		if (ret != 1)
 			seed = time(NULL);
+		process_lock();
 		fclose(f);
+		process_unlock();
 		srand(seed);
 	} else
 		srand(time(NULL));
@@ -1917,15 +1955,19 @@ static bool add_rdepends(struct lxc_container *c, struct lxc_container *c0)
 		c->name);
 	if (ret < 0 || ret >= MAXPATHLEN)
 		return false;
+	process_lock();
 	f = fopen(path, "a");
+	process_unlock();
 	if (!f)
 		return false;
 	bret = true;
 	// if anything goes wrong, just return an error
 	if (fprintf(f, "%s\n%s\n", c0->config_path, c0->name) < 0)
 		bret = false;
+	process_lock();
 	if (fclose(f) != 0)
 		bret = false;
+	process_unlock();
 	return bret;
 }
 
@@ -2103,13 +2145,17 @@ struct lxc_container *lxcapi_clone(struct lxc_container *c, const char *newname,
 	}
 
 	// copy the configuration, tweak it as needed,
+	process_lock();
 	fout = fopen(newpath, "w");
+	process_unlock();
 	if (!fout) {
 		SYSERROR("open %s", newpath);
 		goto out;
 	}
 	write_config(fout, c->lxc_conf);
+	process_lock();
 	fclose(fout);
+	process_unlock();
 
 	sprintf(newpath, "%s/%s/rootfs", l, n);
 	if (mkdir(newpath, 0755) < 0) {
@@ -2256,6 +2302,7 @@ static int lxcapi_snapshot(struct lxc_container *c, char *commentfile)
 	time_t timer;
 	char buffer[25];
 	struct tm* tm_info;
+	FILE *f;
 
 	time(&timer);
 	tm_info = localtime(&timer);
@@ -2264,7 +2311,9 @@ static int lxcapi_snapshot(struct lxc_container *c, char *commentfile)
 
 	char *dfnam = alloca(strlen(snappath) + strlen(newname) + 5);
 	sprintf(dfnam, "%s/%s/ts", snappath, newname);
-	FILE *f = fopen(dfnam, "w");
+	process_lock();
+	f = fopen(dfnam, "w");
+	process_unlock();
 	if (!f) {
 		ERROR("Failed to open %s\n", dfnam);
 		return -1;
@@ -2274,7 +2323,10 @@ static int lxcapi_snapshot(struct lxc_container *c, char *commentfile)
 		fclose(f);
 		return -1;
 	}
-	if (fclose(f) != 0) {
+	process_lock();
+	ret = fclose(f);
+	process_unlock();
+	if (ret != 0) {
 		SYSERROR("Writing timestamp");
 		return -1;
 	}
@@ -2327,7 +2379,10 @@ static char *get_timestamp(char* snappath, char *name)
 	ret = snprintf(path, MAXPATHLEN, "%s/%s/ts", snappath, name);
 	if (ret < 0 || ret >= MAXPATHLEN)
 		return NULL;
-	if ((fin = fopen(path, "r")) == NULL)
+	process_lock();
+	fin = fopen(path, "r");
+	process_unlock();
+	if (!fin)
 		return NULL;
 	(void) fseek(fin, 0, SEEK_END);
 	len = ftell(fin);
@@ -2343,7 +2398,9 @@ static char *get_timestamp(char* snappath, char *name)
 			}
 		}
 	}
+	process_lock();
 	fclose(fin);
+	process_unlock();
 	return s;
 }
 
@@ -2363,7 +2420,10 @@ static int lxcapi_snapshot_list(struct lxc_container *c, struct lxc_snapshot **r
 		ERROR("path name too long");
 		return -1;
 	}
-	if (!(dir = opendir(snappath))) {
+	process_lock();
+	dir = opendir(snappath);
+	process_unlock();
+	if (!dir) {
 		INFO("failed to open %s - assuming no snapshots", snappath);
 		return 0;
 	}
@@ -2405,8 +2465,10 @@ static int lxcapi_snapshot_list(struct lxc_container *c, struct lxc_snapshot **r
 		count++;
 	}
 
+	process_lock();
 	if (closedir(dir))
 		WARN("failed to close directory");
+	process_unlock();
 
 	*ret_snaps = snaps;
 	return count;
diff --git a/src/lxc/lxclock.c b/src/lxc/lxclock.c
index 1d6a86c..1733fd1 100644
--- a/src/lxc/lxclock.c
+++ b/src/lxc/lxclock.c
@@ -271,13 +271,14 @@ void lxc_putlock(struct lxc_lock *l)
 	free(l);
 }
 
-int process_lock(void)
+void process_lock(void)
 {
 	int ret;
-	ret = pthread_mutex_lock(&thread_mutex);
-	if (ret != 0)
+
+	if ((ret = pthread_mutex_lock(&thread_mutex)) != 0) {
 		ERROR("pthread_mutex_lock returned:%d %s", ret, strerror(ret));
-	return ret;
+		exit(1);
+	}
 }
 
 void process_unlock(void)
diff --git a/src/lxc/lxclock.h b/src/lxc/lxclock.h
index fae7e4d..dcdf79d 100644
--- a/src/lxc/lxclock.h
+++ b/src/lxc/lxclock.h
@@ -85,7 +85,7 @@ extern int lxcunlock(struct lxc_lock *lock);
 
 extern void lxc_putlock(struct lxc_lock *l);
 
-extern int process_lock(void);
+extern void process_lock(void);
 extern void process_unlock(void);
 struct lxc_container;
 extern int container_mem_lock(struct lxc_container *c);
diff --git a/src/lxc/monitor.c b/src/lxc/monitor.c
index 64e9987..9b8aa16 100644
--- a/src/lxc/monitor.c
+++ b/src/lxc/monitor.c
@@ -90,7 +90,9 @@ static void lxc_monitor_fifo_send(struct lxc_msg *msg, const char *lxcpath)
 	if (ret < 0)
 		return;
 
+	process_lock();
 	fd = open(fifo_path, O_WRONLY);
+	process_unlock();
 	if (fd < 0) {
 		/* it is normal for this open to fail when there is no monitor
 		 * running, so we don't log it
@@ -100,12 +102,16 @@ static void lxc_monitor_fifo_send(struct lxc_msg *msg, const char *lxcpath)
 
 	ret = write(fd, msg, sizeof(*msg));
 	if (ret != sizeof(*msg)) {
+		process_lock();
 		close(fd);
+		process_unlock();
 		SYSERROR("failed to write monitor fifo %s", fifo_path);
 		return;
 	}
 
+	process_lock();
 	close(fd);
+	process_unlock();
 }
 
 void lxc_monitor_send_state(const char *name, lxc_state_t state, const char *lxcpath)
@@ -122,7 +128,12 @@ void lxc_monitor_send_state(const char *name, lxc_state_t state, const char *lxc
 /* routines used by monitor subscribers (lxc-monitor) */
 int lxc_monitor_close(int fd)
 {
-	return close(fd);
+	int ret;
+
+	process_lock();
+	ret = close(fd);
+	process_unlock();
+	return ret;
 }
 
 /* Note we don't use SHA-1 here as we don't want to depend on HAVE_GNUTLS.
@@ -187,7 +198,9 @@ int lxc_monitor_open(const char *lxcpath)
 	if (lxc_monitor_sock_name(lxcpath, &addr) < 0)
 		return -1;
 
+	process_lock();
 	fd = socket(PF_UNIX, SOCK_STREAM, 0);
+	process_unlock();
 	if (fd < 0) {
 		ERROR("socket : %s", strerror(errno));
 		return -1;
@@ -207,7 +220,9 @@ int lxc_monitor_open(const char *lxcpath)
 	}
 	return fd;
 err1:
+	process_lock();
 	close(fd);
+	process_unlock();
 	return ret;
 }
 
diff --git a/src/lxc/state.c b/src/lxc/state.c
index 398833a..92be560 100644
--- a/src/lxc/state.c
+++ b/src/lxc/state.c
@@ -39,6 +39,7 @@
 #include <lxc/monitor.h>
 #include "commands.h"
 #include "config.h"
+#include "lxclock.h"
 
 lxc_log_define(lxc_state, lxc);
 
@@ -83,14 +84,18 @@ static lxc_state_t freezer_state(const char *name, const char *lxcpath)
 	if (ret < 0 || ret >= MAXPATHLEN)
 		goto out;
 
+	process_lock();
 	file = fopen(freezer, "r");
+	process_unlock();
 	if (!file) {
 		ret = -1;
 		goto out;
 	}
 
 	ret = fscanf(file, "%s", status);
+	process_lock();
 	fclose(file);
+	process_unlock();
 
 	if (ret == EOF) {
 		SYSERROR("failed to read %s", freezer);
-- 
1.8.3.2





More information about the lxc-devel mailing list