[lxc-devel] [PATCH] move monitor-fifo and monitor-sock to /run

Serge Hallyn serge.hallyn at ubuntu.com
Wed Sep 11 17:06:47 UTC 2013


Quoting Dwight Engen (dwight.engen at oracle.com):
> Moving these files should allow $lxcpath to be a read-only fs.

Thanks, nice cleanup too.  One concern though - lxc_monitor_sock_name()
just keeps making a longer and longer path, and it's limited to 108
bytes.  Is there any reason not to use an abstract unix sock for it?
The monitor-fifo doesn't have the length restriction so
$rundir/lxc/$lxcpath/monitor-fifo is ok for it.

> Signed-off-by: Dwight Engen <dwight.engen at oracle.com>
> ---
> Tested lxc-monitor and lxc-wait as root, do not have environment to test
> unprivileged user (XDG_RUNTIME_DIR) case but believe that should work.
> 
>  src/lxc/lxc_monitord.c | 17 +++++++--------
>  src/lxc/lxclock.c      |  5 +----
>  src/lxc/monitor.c      | 56 ++++++++++++++++++++++++++++++++++++++++++++------
>  src/lxc/monitor.h      |  2 ++
>  src/lxc/utils.c        | 10 +++++++++
>  src/lxc/utils.h        |  2 ++
>  6 files changed, 72 insertions(+), 20 deletions(-)
> 
> diff --git a/src/lxc/lxc_monitord.c b/src/lxc/lxc_monitord.c
> index c5e156e..fda6cf4 100644
> --- a/src/lxc/lxc_monitord.c
> +++ b/src/lxc/lxc_monitord.c
> @@ -76,11 +76,9 @@ static int lxc_monitord_fifo_create(struct lxc_monitor *mon)
>  	char fifo_path[PATH_MAX];
>  	int ret;
>  
> -	ret = snprintf(fifo_path, sizeof(fifo_path), "%s/monitor-fifo", mon->lxcpath);
> -	if (ret < 0 || ret >= sizeof(fifo_path)) {
> -		ERROR("lxcpath too long to monitor fifo");
> -		return -1;
> -	}
> +	ret = lxc_monitor_fifo_name(mon->lxcpath, fifo_path, sizeof(fifo_path), 1);
> +	if (ret < 0)
> +		return ret;
>  
>  	ret = mknod(fifo_path, S_IFIFO|S_IRUSR|S_IWUSR, 0);
>  	if (ret < 0) {
> @@ -102,11 +100,10 @@ static int lxc_monitord_fifo_delete(struct lxc_monitor *mon)
>  	char fifo_path[PATH_MAX];
>  	int ret;
>  
> -	ret = snprintf(fifo_path, sizeof(fifo_path), "%s/monitor-fifo", mon->lxcpath);
> -	if (ret < 0 || ret >= sizeof(fifo_path)) {
> -		ERROR("lxcpath too long to monitor fifo");
> -		return -1;
> -	}
> +	ret = lxc_monitor_fifo_name(mon->lxcpath, fifo_path, sizeof(fifo_path), 0);
> +	if (ret < 0)
> +		return ret;
> +
>  	unlink(fifo_path);
>  	return 0;
>  }
> diff --git a/src/lxc/lxclock.c b/src/lxc/lxclock.c
> index 79ebf84..1d6a86c 100644
> --- a/src/lxc/lxclock.c
> +++ b/src/lxc/lxclock.c
> @@ -56,10 +56,7 @@ static char *lxclock_name(const char *p, const char *n)
>  
>  	/* length of "/lock/lxc/" + $lxcpath + "/" + $lxcname + '\0' */
>  	len = strlen("/lock/lxc/") + strlen(n) + strlen(p) + 2;
> -	rundir = getenv("XDG_RUNTIME_DIR");
> -	if (geteuid() == 0 || rundir == NULL)
> -		rundir = "/run";
> -
> +	rundir = get_rundir();
>  	len += strlen(rundir);
>  
>  	if ((dest = malloc(len)) == NULL)
> diff --git a/src/lxc/monitor.c b/src/lxc/monitor.c
> index 412d38f..bdcc581 100644
> --- a/src/lxc/monitor.c
> +++ b/src/lxc/monitor.c
> @@ -40,6 +40,7 @@
>  #include "af_unix.h"
>  
>  #include <lxc/log.h>
> +#include <lxc/lxclock.h>
>  #include <lxc/state.h>
>  #include <lxc/monitor.h>
>  #include <lxc/utils.h>
> @@ -47,17 +48,45 @@
>  lxc_log_define(lxc_monitor, lxc);
>  
>  /* routines used by monitor publishers (containers) */
> +int lxc_monitor_fifo_name(const char *lxcpath, char *fifo_path, size_t fifo_path_sz,
> +			  int do_mkdirp)
> +{
> +	int ret;
> +	const char *rundir;
> +
> +	rundir = get_rundir();
> +	if (do_mkdirp) {
> +		ret = snprintf(fifo_path, fifo_path_sz, "%s/lxc/%s", rundir, lxcpath);
> +		if (ret < 0 || ret >= fifo_path_sz) {
> +			ERROR("rundir/lxcpath (%s/%s) too long for monitor fifo", rundir, lxcpath);
> +			return -1;
> +		}
> +		process_lock();
> +		ret = mkdir_p(fifo_path, 0755);
> +		process_unlock();
> +		if (ret < 0) {
> +			ERROR("unable to create monitor fifo dir %s", fifo_path);
> +			return ret;
> +		}
> +	}
> +	ret = snprintf(fifo_path, fifo_path_sz, "%s/lxc/%s/monitor-fifo", rundir, lxcpath);
> +	if (ret < 0 || ret >= fifo_path_sz) {
> +		ERROR("rundir/lxcpath (%s/%s) too long for monitor fifo", rundir, lxcpath);
> +		return -1;
> +	}
> +	return 0;
> +}
> +
>  static void lxc_monitor_fifo_send(struct lxc_msg *msg, const char *lxcpath)
>  {
>  	int fd,ret;
>  	char fifo_path[PATH_MAX];
>  
>  	BUILD_BUG_ON(sizeof(*msg) > PIPE_BUF); /* write not guaranteed atomic */
> -	ret = snprintf(fifo_path, sizeof(fifo_path), "%s/monitor-fifo", lxcpath);
> -	if (ret < 0 || ret >= sizeof(fifo_path)) {
> -		ERROR("lxcpath too long to open monitor fifo");
> +
> +	ret = lxc_monitor_fifo_name(lxcpath, fifo_path, sizeof(fifo_path), 0);
> +	if (ret < 0)
>  		return;
> -	}
>  
>  	fd = open(fifo_path, O_WRONLY);
>  	if (fd < 0) {
> @@ -98,6 +127,7 @@ int lxc_monitor_sock_name(const char *lxcpath, struct sockaddr_un *addr) {
>  	size_t len;
>  	int ret;
>  	char *sockname = &addr->sun_path[0]; // 1 for abstract
> +	const char *rundir;
>  
>  	/* addr.sun_path is only 108 bytes.
>  	 * should we take a hash of lxcpath? a subset of it? ftok()? we need
> @@ -106,9 +136,23 @@ int lxc_monitor_sock_name(const char *lxcpath, struct sockaddr_un *addr) {
>  	memset(addr, 0, sizeof(*addr));
>  	addr->sun_family = AF_UNIX;
>  	len = sizeof(addr->sun_path) - 1;
> -	ret = snprintf(sockname, len, "%s/monitor-sock", lxcpath);
> +	rundir = get_rundir();
> +	ret = snprintf(sockname, len, "%s/lxc/%s", rundir, lxcpath);
> +	if (ret < 0 || ret >= len) {
> +		ERROR("rundir/lxcpath (%s/%s) too long for monitor unix socket", rundir, lxcpath);
> +		return -1;
> +	}
> +	process_lock();
> +	ret = mkdir_p(sockname, 0755);
> +	process_unlock();
> +	if (ret < 0) {
> +		ERROR("unable to create monitor sock %s", sockname);
> +		return ret;
> +	}
> +
> +	ret = snprintf(sockname, len, "%s/lxc/%s/monitor-sock", rundir, lxcpath);
>  	if (ret < 0 || ret >= len) {
> -		ERROR("lxcpath too long for unix socket");
> +		ERROR("rundir/lxcpath (%s/%s) too long for monitor unix socket", rundir, lxcpath);
>  		return -1;
>  	}
>  	return 0;
> diff --git a/src/lxc/monitor.h b/src/lxc/monitor.h
> index 2a61091..8093919 100644
> --- a/src/lxc/monitor.h
> +++ b/src/lxc/monitor.h
> @@ -41,6 +41,8 @@ struct lxc_msg {
>  
>  extern int lxc_monitor_open(const char *lxcpath);
>  extern int lxc_monitor_sock_name(const char *lxcpath, struct sockaddr_un *addr);
> +extern int lxc_monitor_fifo_name(const char *lxcpath, char *fifo_path,
> +				 size_t fifo_path_sz, int do_mkdirp);
>  extern void lxc_monitor_send_state(const char *name, lxc_state_t state,
>  			    const char *lxcpath);
>  extern int lxc_monitord_spawn(const char *lxcpath);
> diff --git a/src/lxc/utils.c b/src/lxc/utils.c
> index ba0604a..2e66585 100644
> --- a/src/lxc/utils.c
> +++ b/src/lxc/utils.c
> @@ -318,6 +318,16 @@ const char *default_lxc_path(void)
>  	return lxc_global_config_value("lxcpath");
>  }
>  
> +const char *get_rundir()
> +{
> +	const char *rundir;
> +
> +	rundir = getenv("XDG_RUNTIME_DIR");
> +	if (geteuid() == 0 || rundir == NULL)
> +		rundir = "/run";
> +	return rundir;
> +}
> +
>  int wait_for_pid(pid_t pid)
>  {
>  	int status, ret;
> diff --git a/src/lxc/utils.h b/src/lxc/utils.h
> index 1a7b551..9776d18 100644
> --- a/src/lxc/utils.h
> +++ b/src/lxc/utils.h
> @@ -37,6 +37,8 @@ extern int lxc_rmdir_onedev(char *path);
>  extern int lxc_setup_fs(void);
>  extern int get_u16(unsigned short *val, const char *arg, int base);
>  extern int mkdir_p(const char *dir, mode_t mode);
> +extern const char *get_rundir(void);
> +
>  /*
>   * Return a buffer containing the default container path.
>   * Caller must NOT free this buffer, since it may be static.
> -- 
> 1.8.1.4
> 




More information about the lxc-devel mailing list