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);
}
signature.asc
Description: PGP signature
_______________________________________________ lxc-devel mailing list lxc-devel@lists.linuxcontainers.org http://lists.linuxcontainers.org/listinfo/lxc-devel
