[lxc-devel] [PATCH] clean autodev dir on container exit
Michael H. Warfield
mhw at WittsEnd.com
Fri Aug 8 20:52:12 UTC 2014
On Fri, 2014-08-08 at 16:12 -0400, Stéphane Graber wrote:
> On Fri, Aug 08, 2014 at 04:05:31PM -0400, Michael H. Warfield wrote:
> > 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.
> Ok, so per the above, it'd be fine calling the autodev destroy function
> in a container shutdown case which isn't a reboot right?
> That way we keep the autodev around for when the container reboots but
> we clear up all the autodev entries from /dev when it gets stopped for
> good.
Yeah, I think that works for me.
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!
> >
>
>
>
> > _______________________________________________
> > lxc-devel mailing list
> > lxc-devel at lists.linuxcontainers.org
> > http://lists.linuxcontainers.org/listinfo/lxc-devel
>
>
> _______________________________________________
> 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/9243a443/attachment-0001.sig>
More information about the lxc-devel
mailing list