[lxc-devel] [PATCH] Add remove_snapshots_entry() (rebased - v2)

[Christian Brauner]

On Mon, Sep 14, 2015 at 10:54:34PM +0000, Serge Hallyn wrote:
> Does it help if we simply define c->delete_with_snapshot_clones(), and have
> src/lxc/destroy.c use that?  Then we can contain mod_all_rdeps to being a
> static function in src/lxc/lxccontainer.c  If not, remind me where else we
> would've needed the mod_all_rdeps?

lxc_destroy.c does not call mod_all_rdeps() directly at all. It reads in the
lxc_snapshots file from the container all at once and finds each container
listed in it and passes it to c->destroy(c). So we should be good regarding
locks on this side. Here is the relevant bit from lxc_destroy.c (omitting the
reading in part of the lxc_snapshots file):

		while ((lxcpath = strtok_r(!counter ? buf : NULL, "\n", &scratch))) {
			if (!(lxcname = strtok_r(NULL, "\n", &scratch)))
				break;
			c1 = lxc_container_new(lxcname, lxcpath);
			if (!c1)
				goto next;
			if (!c1->destroy(c1)) {
				fprintf(stderr, "Destroying snapshot %s of %s failed\n", lxcname, my_args.name);
				lxc_container_put(c1);
				free(buf);
				return -1;
			}
			lxc_container_put(c1);
next:
			counter++;
		}

What I was worried about is the implementation in start.c:

When we have ephemeral containers that delete themselves we need to make sure
that they update the lxc_snapshots file before they die. One way to achieve this
is to have a mod_all_rdeps() version or something similar in start.c.
Specifically modifying this function to update the lxc_snapshots file:

        static void lxc_destroy_container_on_signal(struct lxc_handler *handler,
        					    const char *name)
        {
        	char destroy[MAXPATHLEN];
        	bool bret = true;
        	int ret = 0;
        	struct lxc_container *c;
        	if (handler->conf && handler->conf->rootfs.path && handler->conf->rootfs.mount) {
        		bret = do_destroy_container(handler->conf);
        		if (!bret) {
        			ERROR("Error destroying rootfs for %s", name);
        			return;
        		}
        	}
        	INFO("Destroyed rootfs for %s", name);
        
        	ret = snprintf(destroy, MAXPATHLEN, "%s/%s", handler->lxcpath, name);
        	if (ret < 0 || ret >= MAXPATHLEN) {
        		ERROR("Error printing path for %s", name);
        		ERROR("Error destroying directory for %s", name);
        		return;
        	}
        
        	c = lxc_container_new(name, handler->lxcpath);
        	if (c) {
        		if (container_disk_lock(c)) {
        			INFO("Could not update lxc_snapshots file");
        			lxc_container_put(c);
        		} else {
        			mod_all_rdeps(c, false);
        			container_disk_unlock(c);
        			lxc_container_put(c);
        		}
        	}
        
Right here we should update the lxc_snapshots file. Making mod_all_rdeps() is
the option you suggested. We can either have a special function for start.c or
make one of the functions mod_all_rdeps() or mod_rdep() public. Making
mod_rdep() or mod_all_rdeps() public involves binding in the lxccontainer.h file
into place we might not want them. So the easiest thing seems to be to have a
mod_rdep() function specific to start.c which takes in a lxc_container struct
and protects it with container_disk_lock(). Or do you have another idea?

        	if (am_unpriv())
        		ret = userns_exec_1(handler->conf, lxc_rmdir_onedev_wrapper, destroy);
        	else
        		ret = lxc_rmdir_onedev(destroy, NULL);
        
        	if (ret < 0) {
        		ERROR("Error destroying directory for %s", name);
        		return;
        	}
        	INFO("Destroyed directory for %s", name);
        }
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20150915/8c2e9ee9/attachment.sig>