[lxc-devel] [PATCH 2/1] lxc_monitor_open: prepend lxcpath

Stéphane Graber stgraber at ubuntu.com
Tue Feb 19 19:45:39 UTC 2013


On 02/14/2013 11:30 AM, Serge Hallyn wrote:
> This is needed for lxc_wait and lxc_monitor to handle lxcpath.  However,
> the full path name is limited to 108 bytes.  Should we use a md5sum of
> the lxcpath instead of the path itself?


I thought of instead using the cgroup name (including group and parent,
so lxc/group/name) but that wouldn't work as the name isn't guaranteed
to match that of the container (in the case where we have multiple
container of the same name in the same group).

So yeah, md5 of the config path seems like a good idea but I'd prefer we
only use it when we exceed the 108 bytes limit as I kind of like our
current path (very easy to read ;)).

108 chars is very long, even my longest container path is only at 64
chars: /home/stgraber/data/vm/lxc/lib/tpl-precise-amd64-nh55jb/command

Acked-by: Stéphane Graber <stgraber at ubuntu.com>

> 
> In any case, with this patch, lxc-wait and lxc-monitor work right with
> respect to multiple lxcpaths.
> 
> The lxcpath is added to the lxc_handler to make it available most of the
> places we need it.
> 
> I also remove function prototypes in monitor.h for two functions which
> are not defined or used anywhere.
> 
> TODO: make cgroups tolerate multiple same-named containers.
> 
> Signed-off-by: Serge Hallyn <serge.hallyn at ubuntu.com>
> ---
>  src/lxc/freezer.c      | 14 +++++++-------
>  src/lxc/lxc.h          |  6 +++---
>  src/lxc/lxc_freeze.c   |  2 +-
>  src/lxc/lxc_monitor.c  |  2 +-
>  src/lxc/lxc_unfreeze.c |  2 +-
>  src/lxc/lxccontainer.c |  4 ++--
>  src/lxc/monitor.c      | 36 ++++++++++++++++++++++++++++--------
>  src/lxc/monitor.h      |  5 ++---
>  src/lxc/start.c        |  3 ++-
>  src/lxc/start.h        |  1 +
>  src/lxc/state.c        |  2 +-
>  src/lxc/stop.c         |  2 +-
>  12 files changed, 50 insertions(+), 29 deletions(-)
> 
> diff --git a/src/lxc/freezer.c b/src/lxc/freezer.c
> index 3e4f55d..e8b1311 100644
> --- a/src/lxc/freezer.c
> +++ b/src/lxc/freezer.c
> @@ -40,7 +40,7 @@
>  
>  lxc_log_define(lxc_freezer, lxc);
>  
> -static int freeze_unfreeze(const char *name, int freeze)
> +static int freeze_unfreeze(const char *name, int freeze, const char *lxcpath)
>  {
>  	char *nsgroup;
>  	char freezer[MAXPATHLEN], *f;
> @@ -98,7 +98,7 @@ static int freeze_unfreeze(const char *name, int freeze)
>  		ret = strncmp(f, tmpf, strlen(f));
>  		if (!ret)
>  		{
> -			lxc_monitor_send_state(name, freeze ? FROZEN : THAWED);
> +			lxc_monitor_send_state(name, freeze ? FROZEN : THAWED, lxcpath);
>  			break;		/* Success */
>  		}
>  
> @@ -122,14 +122,14 @@ out:
>  	return ret;
>  }
>  
> -int lxc_freeze(const char *name)
> +int lxc_freeze(const char *name, const char *lxcpath)
>  {
> -	lxc_monitor_send_state(name, FREEZING);
> -	return freeze_unfreeze(name, 1);
> +	lxc_monitor_send_state(name, FREEZING, lxcpath);
> +	return freeze_unfreeze(name, 1, lxcpath);
>  }
>  
> -int lxc_unfreeze(const char *name)
> +int lxc_unfreeze(const char *name, const char *lxcpath)
>  {
> -	return freeze_unfreeze(name, 0);
> +	return freeze_unfreeze(name, 0, lxcpath);
>  }
>  
> diff --git a/src/lxc/lxc.h b/src/lxc/lxc.h
> index 4e770a6..0fa9480 100644
> --- a/src/lxc/lxc.h
> +++ b/src/lxc/lxc.h
> @@ -74,7 +74,7 @@ extern int lxc_execute(const char *name, char *const argv[], int quiet,
>   * The function will return an fd corresponding to the events
>   * Returns a file descriptor on success, < 0 otherwise
>   */
> -extern int lxc_monitor_open(void);
> +extern int lxc_monitor_open(const char *lxcpath);
>  
>  /*
>   * Read the state of the container if this one has changed
> @@ -108,14 +108,14 @@ extern int lxc_console(const char *name, int ttynum, int *fd, const char *lxcpat
>   * @name : the container name
>   * Returns 0 on success, < 0 otherwise
>   */
> -extern int lxc_freeze(const char *name);
> +extern int lxc_freeze(const char *name, const char *lxcpath);
>  
>  /*
>   * Unfreeze all previously frozen tasks.
>   * @name : the name of the container
>   * Return 0 on sucess, < 0 otherwise
>   */
> -extern int lxc_unfreeze(const char *name);
> +extern int lxc_unfreeze(const char *name, const char *lxcpath);
>  
>  /*
>   * Retrieve the container state
> diff --git a/src/lxc/lxc_freeze.c b/src/lxc/lxc_freeze.c
> index 4da5654..77d5388 100644
> --- a/src/lxc/lxc_freeze.c
> +++ b/src/lxc/lxc_freeze.c
> @@ -58,6 +58,6 @@ int main(int argc, char *argv[])
>  			 my_args.progname, my_args.quiet))
>  		return -1;
>  
> -	return lxc_freeze(my_args.name);
> +	return lxc_freeze(my_args.name, my_args.lxcpath);
>  }
>  
> diff --git a/src/lxc/lxc_monitor.c b/src/lxc/lxc_monitor.c
> index 2b8b0a0..b308019 100644
> --- a/src/lxc/lxc_monitor.c
> +++ b/src/lxc/lxc_monitor.c
> @@ -87,7 +87,7 @@ int main(int argc, char *argv[])
>  		return -1;
>  	}
>  
> -	fd = lxc_monitor_open();
> +	fd = lxc_monitor_open(my_args.lxcpath);
>  	if (fd < 0)
>  		return -1;
>  
> diff --git a/src/lxc/lxc_unfreeze.c b/src/lxc/lxc_unfreeze.c
> index 02e9a47..4a9ecd1 100644
> --- a/src/lxc/lxc_unfreeze.c
> +++ b/src/lxc/lxc_unfreeze.c
> @@ -57,6 +57,6 @@ int main(int argc, char *argv[])
>  			 my_args.progname, my_args.quiet))
>  		return -1;
>  
> -	return lxc_unfreeze(my_args.name);
> +	return lxc_unfreeze(my_args.name, my_args.lxcpath);
>  }
>  
> diff --git a/src/lxc/lxccontainer.c b/src/lxc/lxccontainer.c
> index 1e257c0..f24c39f 100644
> --- a/src/lxc/lxccontainer.c
> +++ b/src/lxc/lxccontainer.c
> @@ -195,7 +195,7 @@ static bool lxcapi_freeze(struct lxc_container *c)
>  
>  	if (lxclock(c->slock, 0))
>  		return false;
> -	ret = lxc_freeze(c->name);
> +	ret = lxc_freeze(c->name, c->config_path);
>  	lxcunlock(c->slock);
>  	if (ret)
>  		return false;
> @@ -210,7 +210,7 @@ static bool lxcapi_unfreeze(struct lxc_container *c)
>  
>  	if (lxclock(c->slock, 0))
>  		return false;
> -	ret = lxc_unfreeze(c->name);
> +	ret = lxc_unfreeze(c->name, c->config_path);
>  	lxcunlock(c->slock);
>  	if (ret)
>  		return false;
> diff --git a/src/lxc/monitor.c b/src/lxc/monitor.c
> index 2eb0397..1caecd7 100644
> --- a/src/lxc/monitor.c
> +++ b/src/lxc/monitor.c
> @@ -47,13 +47,23 @@ lxc_log_define(lxc_monitor, lxc);
>  #define UNIX_PATH_MAX 108
>  #endif
>  
> -static void lxc_monitor_send(struct lxc_msg *msg)
> +static void lxc_monitor_send(struct lxc_msg *msg, const char *lxcpath)
>  {
>  	int fd;
>  	struct sockaddr_un addr = { .sun_family = AF_UNIX };
>  	char *offset = &addr.sun_path[1];
> -
> -	strcpy(offset, "lxc-monitor");
> +	size_t ret, len;
> +
> +	/*
> +	 * addr.sun_path is only 108 bytes.
> +	 * should we take a hash of lxcpath?  a subset of it?
> +	 */
> +	len = sizeof(addr.sun_path) - 1;
> +	ret = snprintf(offset, len, "%s/lxc-monitor", lxcpath);
> +	if (ret < 0 || ret >= len) {
> +		ERROR("lxcpath too long to open monitor");
> +		return;
> +	}
>  
>  	fd = socket(PF_UNIX, SOCK_DGRAM, 0);
>  	if (fd < 0)
> @@ -65,23 +75,33 @@ static void lxc_monitor_send(struct lxc_msg *msg)
>  	close(fd);
>  }
>  
> -void lxc_monitor_send_state(const char *name, lxc_state_t state)
> +void lxc_monitor_send_state(const char *name, lxc_state_t state, const char *lxcpath)
>  {
>  	struct lxc_msg msg = { .type = lxc_msg_state,
>  			       .value = state };
>  	strncpy(msg.name, name, sizeof(msg.name));
>  	msg.name[sizeof(msg.name) - 1] = 0;
>  
> -	lxc_monitor_send(&msg);
> +	lxc_monitor_send(&msg, lxcpath);
>  }
>  
> -int lxc_monitor_open(void)
> +int lxc_monitor_open(const char *lxcpath)
>  {
>  	struct sockaddr_un addr = { .sun_family = AF_UNIX };
>  	char *offset = &addr.sun_path[1];
>  	int fd;
> -
> -	strcpy(offset, "lxc-monitor");
> +	size_t ret, len;
> +
> +	/*
> +	 * addr.sun_path is only 108 bytes.
> +	 * should we take a hash of lxcpath?  a subset of it?
> +	 */
> +	len = sizeof(addr.sun_path) - 1;
> +	ret = snprintf(offset, len, "%s/lxc-monitor", lxcpath);
> +	if (ret < 0 || ret >= len) {
> +		ERROR("lxcpath too long to open monitor");
> +		return -1;
> +	}
>  
>  	fd = socket(PF_UNIX, SOCK_DGRAM, 0);
>  	if (fd < 0) {
> diff --git a/src/lxc/monitor.h b/src/lxc/monitor.h
> index ab08f10..46e35a9 100644
> --- a/src/lxc/monitor.h
> +++ b/src/lxc/monitor.h
> @@ -36,8 +36,7 @@ struct lxc_msg {
>  	int value;
>  };
>  
> -void lxc_monitor_send_state(const char *name, lxc_state_t state);
> -void lxc_monitor_send_priority(const char *name, int priority);
> -void lxc_monitor_cleanup(const char *name);
> +void lxc_monitor_send_state(const char *name, lxc_state_t state,
> +			    const char *lxcpath);
>  
>  #endif
> diff --git a/src/lxc/start.c b/src/lxc/start.c
> index 139be08..de431be 100644
> --- a/src/lxc/start.c
> +++ b/src/lxc/start.c
> @@ -311,7 +311,7 @@ int lxc_clone_flags_callback(int fd, struct lxc_request *request,
>  int lxc_set_state(const char *name, struct lxc_handler *handler, lxc_state_t state)
>  {
>  	handler->state = state;
> -	lxc_monitor_send_state(name, state);
> +	lxc_monitor_send_state(name, state, handler->lxcpath);
>  	return 0;
>  }
>  
> @@ -379,6 +379,7 @@ struct lxc_handler *lxc_init(const char *name, struct lxc_conf *conf, const char
>  	memset(handler, 0, sizeof(*handler));
>  
>  	handler->conf = conf;
> +	handler->lxcpath = lxcpath;
>  
>  	apparmor_handler_init(handler);
>  	handler->name = strdup(name);
> diff --git a/src/lxc/start.h b/src/lxc/start.h
> index 8c8cbaf..a0afa87 100644
> --- a/src/lxc/start.h
> +++ b/src/lxc/start.h
> @@ -50,6 +50,7 @@ struct lxc_handler {
>  	int aa_enabled;
>  #endif
>  	int pinfd;
> +	const char *lxcpath;
>  };
>  
>  extern struct lxc_handler *lxc_init(const char *name, struct lxc_conf *, const char *);
> diff --git a/src/lxc/state.c b/src/lxc/state.c
> index 7d6c0d5..9fcb06d 100644
> --- a/src/lxc/state.c
> +++ b/src/lxc/state.c
> @@ -200,7 +200,7 @@ extern int lxc_wait(const char *lxcname, const char *states, int timeout, const
>  	if (fillwaitedstates(states, s))
>  		return -1;
>  
> -	fd = lxc_monitor_open();
> +	fd = lxc_monitor_open(lxcpath);
>  	if (fd < 0)
>  		return -1;
>  
> diff --git a/src/lxc/stop.c b/src/lxc/stop.c
> index fa1e375..c796283 100644
> --- a/src/lxc/stop.c
> +++ b/src/lxc/stop.c
> @@ -85,7 +85,7 @@ extern int lxc_stop_callback(int fd, struct lxc_request *request,
>  
>  	answer.ret = kill(handler->pid, SIGKILL);
>  	if (!answer.ret) {
> -		ret = lxc_unfreeze(handler->name);
> +		ret = lxc_unfreeze(handler->name, handler->lxcpath);
>  		if (!ret)
>  			return 0;
>  
> 


-- 
Stéphane Graber
Ubuntu developer
http://www.ubuntu.com

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 901 bytes
Desc: OpenPGP digital signature
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20130219/acf4fe86/attachment.pgp>


More information about the lxc-devel mailing list