[lxc-devel] [PATCH] clean autodev dir on container exit

Michael H. Warfield mhw at WittsEnd.com
Fri Aug 8 20:05:31 UTC 2014


On Fri, 2014-08-08 at 16:41 +0000, Serge Hallyn wrote:
> Quoting Stéphane Graber (stgraber at ubuntu.com):
> > Hello,
> > 
> > We received this patch on github: https://github.com/lxc/lxc/pull/289.patch
> > 
> > While it does appear to make sense to me that we don't want cruft piling
> > up in /dev, especially on systems creating hundreds/thousands of
> > temporary containers, Serge told me that this may have been done by
> > design.

> Right, I wasn't sure whether Michael wanted those to persist.  If not, then
> great let's apply as is.  If so, then how about if we have lxc check at
> start time whether the dir already existed - if so it leaves it be, else it
> removes it at shutdown?

It was intended to be persistent for the duration the host is up.  It
does not persist over host reboots.  The intent was to not be destroying
and recreating those areas in devtmpfs when ever a container reboots and
points back at the same path.  It would then also preserve any udev
changes (which could be a plus or could be a minus).

I don't have a real strong preference.

Regards,
Mike

> > So I'm forwarding this one to the list to get some feedback from Mike
> > and whoever else is involved with that autodev stuff :)
> > 
> > -- 
> > Stéphane Graber
> > Ubuntu developer
> > http://www.ubuntu.com
> 
> > Date: Sat, 2 Aug 2014 14:35:02 +0200
> > From: Jean-Tiare LE BIGOT <jean-tiare.le-bigot at ovh.net>
> > Subject: [PATCH] clean autodev dir on container exit
> > 
> > When "lxc.autodev = 1", LXC creates automatically a "/dev/.lxc/<name>.<hash>"
> > folder to put container's devices in so that they are visible from both
> > the host and the container itself.
> > 
> > On container exit (ne it normal or not), this folder was not cleaned
> > which made "/dev" folder grow continuously.
> > 
> > We fix this by adding a new `int lxc_delete_autodev(struct lxc_handler
> > *handler)` called from `static void lxc_fini(const char *name, struct
> > lxc_handler *handler)`.
> > 
> > Signed-off-by: Jean-Tiare LE BIGOT <jean-tiare.le-bigot at ovh.net>
> > ---
> >  src/lxc/conf.c  | 78 ++++++++++++++++++++++++++++++++++++++++++++++++---------
> >  src/lxc/conf.h  |  1 +
> >  src/lxc/start.c |  1 +
> >  3 files changed, 68 insertions(+), 12 deletions(-)
> > 
> > diff --git a/src/lxc/conf.c b/src/lxc/conf.c
> > index 473d076..3ba118d 100644
> > --- a/src/lxc/conf.c
> > +++ b/src/lxc/conf.c
> > @@ -288,6 +288,9 @@ static struct caps_opt caps_opt[] = {
> >  static struct caps_opt caps_opt[] = {};
> >  #endif
> >  
> > +const char *dev_base_path = "/dev/.lxc";
> > +const char *dev_user_path = "/dev/.lxc/user";
> > +
> >  static int run_buffer(char *buffer)
> >  {
> >  	struct lxc_popen_FILE *f;
> > @@ -1259,13 +1262,11 @@ static char *mk_devtmpfs(const char *name, char *path, const char *lxcpath)
> >  	struct stat s;
> >  	char tmp_path[MAXPATHLEN];
> >  	char fstype[MAX_FSTYPE_LEN];
> > -	char *base_path = "/dev/.lxc";
> > -	char *user_path = "/dev/.lxc/user";
> >  	uint64_t hash;
> >  
> > -	if ( 0 != access(base_path, F_OK) || 0 != stat(base_path, &s) || 0 == S_ISDIR(s.st_mode) ) {
> > +	if ( 0 != access(dev_base_path, F_OK) || 0 != stat(dev_base_path, &s) || 0 == S_ISDIR(s.st_mode) ) {
> >  		/* This is just making /dev/.lxc it better work or we're done */
> > -		ret = mkdir(base_path, S_IRWXU | S_IRGRP | S_IXGRP | S_IROTH | S_IXOTH);
> > +		ret = mkdir(dev_base_path, S_IRWXU | S_IRGRP | S_IXGRP | S_IROTH | S_IXOTH);
> >  		if ( ret ) {
> >  			SYSERROR( "Unable to create /dev/.lxc for autodev" );
> >  			return NULL;
> > @@ -1299,19 +1300,19 @@ static char *mk_devtmpfs(const char *name, char *path, const char *lxcpath)
> >  		}
> >  	}
> >  
> > -	if ( 0 != access(user_path, F_OK) || 0 != stat(user_path, &s) || 0 == S_ISDIR(s.st_mode) ) {
> > +	if ( 0 != access(dev_user_path, F_OK) || 0 != stat(dev_user_path, &s) || 0 == S_ISDIR(s.st_mode) ) {
> >  		/*
> >  		 * This is making /dev/.lxc/user path for non-priv users.
> >  		 * If this doesn't work, we'll have to fall back in the
> >  		 * case of non-priv users.  It's mode 1777 like /tmp.
> >  		 */
> > -		ret = mkdir(user_path, S_IRWXU | S_IRWXG | S_IRWXO | S_ISVTX);
> > +		ret = mkdir(dev_user_path, S_IRWXU | S_IRWXG | S_IRWXO | S_ISVTX);
> >  		if ( ret ) {
> >  			/* Issue an error but don't fail yet! */
> >  			ERROR("Unable to create /dev/.lxc/user");
> >  		}
> >  		/* Umask tends to screw us up here */
> > -		chmod(user_path, S_IRWXU | S_IRWXG | S_IRWXO | S_ISVTX);
> > +		chmod(dev_user_path, S_IRWXU | S_IRWXG | S_IRWXO | S_ISVTX);
> >  	}
> >  
> >  	/*
> > @@ -1326,18 +1327,18 @@ static char *mk_devtmpfs(const char *name, char *path, const char *lxcpath)
> >  
> >  	hash = fnv_64a_buf(tmp_path, ret, FNV1A_64_INIT);
> >  
> > -	ret = snprintf(tmp_path, MAXPATHLEN, "%s/%s.%016" PRIx64, base_path, name, hash);
> > +	ret = snprintf(tmp_path, MAXPATHLEN, "%s/%s.%016" PRIx64, dev_base_path, name, hash);
> >  	if (ret < 0 || ret >= MAXPATHLEN)
> >  		return NULL;
> >  
> >  	if ( 0 != access(tmp_path, F_OK) || 0 != stat(tmp_path, &s) || 0 == S_ISDIR(s.st_mode) ) {
> >  		ret = mkdir(tmp_path, S_IRWXU | S_IRGRP | S_IXGRP | S_IROTH | S_IXOTH);
> >  		if ( ret ) {
> > -			/* Something must have failed with the base_path...
> > -			 * Maybe unpriv user.  Try user_path now... */
> > +			/* Something must have failed with the dev_base_path...
> > +			 * Maybe unpriv user.  Try dev_user_path now... */
> >  			INFO("Setup in /dev/.lxc failed.  Trying /dev/.lxc/user." );
> >  
> > -			ret = snprintf(tmp_path, MAXPATHLEN, "%s/%s.%016" PRIx64, user_path, name, hash);
> > +			ret = snprintf(tmp_path, MAXPATHLEN, "%s/%s.%016" PRIx64, dev_user_path, name, hash);
> >  			if (ret < 0 || ret >= MAXPATHLEN)
> >  				return NULL;
> >  
> > @@ -1355,7 +1356,6 @@ static char *mk_devtmpfs(const char *name, char *path, const char *lxcpath)
> >  	return path;
> >  }
> >  
> > -
> >  /*
> >   * Do we want to add options for max size of /dev and a file to
> >   * specify which devices to create?
> > @@ -1482,6 +1482,60 @@ static int setup_autodev(const char *root)
> >  }
> >  
> >  /*
> > + * Locate allocated devtmpfs mount and purge it.
> > + * path lookup mostly taken from mk_devtmpfs
> > + */
> > +int lxc_delete_autodev(struct lxc_handler *handler)
> > +{
> > +	int ret;
> > +	struct stat s;
> > +	struct lxc_conf *lxc_conf = handler->conf;
> > +	const char *name = handler->name;
> > +	const char *lxcpath = handler->lxcpath;
> > +	char tmp_path[MAXPATHLEN];
> > +	uint64_t hash;
> > +
> > +	if ( lxc_conf->autodev <= 0 )
> > +		return 0;
> > +
> > +    /*
> > +     * Use the same logic as mk_devtmpfs to compute condidate
> > +     * path for cleanup.
> > +     */
> > +
> > +	ret = snprintf(tmp_path, MAXPATHLEN, "%s/%s", lxcpath, name);
> > +	if (ret < 0 || ret >= MAXPATHLEN)
> > +		return -1;
> > +
> > +	hash = fnv_64a_buf(tmp_path, ret, FNV1A_64_INIT);
> > +
> > +	/* Probe /dev/.lxc/<container name>.<hash> */
> > +	ret = snprintf(tmp_path, MAXPATHLEN, "%s/%s.%016" PRIx64, dev_base_path, name, hash);
> > +	if (ret < 0 || ret >= MAXPATHLEN)
> > +		return -1;
> > +
> > +	if ( 0 != access(tmp_path, F_OK) || 0 != stat(tmp_path, &s) || 0 == S_ISDIR(s.st_mode) ) {
> > +		/* Probe /dev/.lxc/user/<container name>.<hash> */
> > +		ret = snprintf(tmp_path, MAXPATHLEN, "%s/%s.%016" PRIx64, dev_user_path, name, hash);
> > +		if (ret < 0 || ret >= MAXPATHLEN)
> > +			return -1;
> > +
> > +		if ( 0 != access(tmp_path, F_OK) || 0 != stat(tmp_path, &s) || 0 == S_ISDIR(s.st_mode) ) {
> > +			WARN("Failed to locate autodev /dev/.lxc and /dev/.lxc/user." );
> > +			return -1;
> > +		}
> > +    }
> > +
> > +	/* Do the cleanup */
> > +	INFO("Cleaning %s", tmp_path );
> > +	if ( 0 != lxc_rmdir_onedev(tmp_path, NULL) ) {
> > +		ERROR("Failed to cleanup autodev" );
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +/*
> >   * I'll forgive you for asking whether all of this is needed :)  The
> >   * answer is yes.
> >   * pivot_root will fail if the new root, the put_old dir, or the parent
> > diff --git a/src/lxc/conf.h b/src/lxc/conf.h
> > index 615f276..99495f8 100644
> > --- a/src/lxc/conf.h
> > +++ b/src/lxc/conf.h
> > @@ -388,6 +388,7 @@ extern int lxc_clear_hooks(struct lxc_conf *c, const char *key);
> >  extern int lxc_clear_idmaps(struct lxc_conf *c);
> >  extern int lxc_clear_groups(struct lxc_conf *c);
> >  extern int lxc_clear_environment(struct lxc_conf *c);
> > +extern int lxc_delete_autodev(struct lxc_handler *handler);
> >  
> >  extern int do_rootfs_setup(struct lxc_conf *conf, const char *name,
> >  			   const char *lxcpath);
> > diff --git a/src/lxc/start.c b/src/lxc/start.c
> > index bb136af..e1ee1ca 100644
> > --- a/src/lxc/start.c
> > +++ b/src/lxc/start.c
> > @@ -477,6 +477,7 @@ static void lxc_fini(const char *name, struct lxc_handler *handler)
> >  
> >  	lxc_console_delete(&handler->conf->console);
> >  	lxc_delete_tty(&handler->conf->tty_info);
> > +	lxc_delete_autodev(handler);
> >  	close(handler->conf->maincmd_fd);
> >  	handler->conf->maincmd_fd = -1;
> >  	free(handler->name);
> > -- 
> > 2.0.3
> 
> 
> 
> 
> > _______________________________________________
> > lxc-devel mailing list
> > lxc-devel at lists.linuxcontainers.org
> > http://lists.linuxcontainers.org/listinfo/lxc-devel
> 

-- 
Michael H. Warfield (AI4NB) | (770) 978-7061 |  mhw at WittsEnd.com
   /\/\|=mhw=|\/\/          | (678) 463-0932 |  http://www.wittsend.com/mhw/
   NIC whois: MHW9          | An optimist believes we live in the best of all
 PGP Key: 0x674627FF        | possible worlds.  A pessimist is sure of it!

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 465 bytes
Desc: This is a digitally signed message part
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20140808/cacfe8b2/attachment.sig>


More information about the lxc-devel mailing list