Quoting Christian Brauner ([email protected]): > This is rather a first-pass suggestion than a full commit to discuss: > > The idea: > --------- > > - If a container has clone-snapshots created by > > lxc-clone -n NAME -N NEWNAME -s > > then even the new version lxc-destroy (see patch on github) cannot remove > the > container because the original container only stores the number of > clone-snapshotted containers in the file "lxc_snapshots". Hence, it has no > way > of looking up the copy-snapshots containers that belong to the container to > delete them and then delete the original container. I suggest modifying > mod_rdep() so it will store not the plain number in the file but rather the > paths and names to the containers that are a clone-snapshots (similar to the > "lxc_rdepends" file for the clones). An example how the file "lxc_snapshots" > of the original container could look like would be: > > /var/lib/lxc:bb > /var/lib/lxc:cc > /opt:dd
Hi, this seems like a good idea to me. Thanks for working on it. A few notes below, but overall +1. > This would be an example of a container that provides the base for > three clone-snapshots bb, cc, and dd. Where bb and cc both are placed > in the usual path for privileged containers and dd is placed in a > custom path. I could then go on to modify lxc-destroy to actually not just > remove the original container including all snapshots but also with all its > clone-snapshots. > > Following is a first idea for rewriting mod_rdep(). If you think its > generally > not something you want just leave a short reply. If you would like to have > this feature but rather implement it in a different way it would be nice if > you could leave some more feedback and suggestions and then I'll rewrite the > patch. Thanks! > > Summary: > -------- > > - Add additional argument to function that takes in the clone-snapshotted > lxc_container. > - Have mod_rdep() write the path and name of the clone-snapshotted container > the > file lxc_snapshots of the original container. > - If a clone-snapshot gets deleted the corresponding line in the file > lxc_snapshot of the original container will be deleted and the file updated. > - Change has_fs_snapshots() to check if the file is empty. If it is it is safe > to delete the original container if the file is not empty then it is not > safe. > > Signed-off-by: Christian Brauner <[email protected]> > --- > src/lxc/lxccontainer.c | 98 > +++++++++++++++++++++++++++++++------------------- > 1 file changed, 61 insertions(+), 37 deletions(-) > > diff --git a/src/lxc/lxccontainer.c b/src/lxc/lxccontainer.c > index 1c103e8..e10ccc7 100644 > --- a/src/lxc/lxccontainer.c > +++ b/src/lxc/lxccontainer.c > @@ -1970,47 +1970,76 @@ out: > > WRAP_API_1(bool, lxcapi_save_config, const char *) > > -static bool mod_rdep(struct lxc_container *c, bool inc) > +static bool mod_rdep(struct lxc_container *c0, struct lxc_container *c, bool > inc) > { > - char path[MAXPATHLEN]; > - int ret, v = 0; > - FILE *f; > + char *buf; please initialize buf to NULL. > + size_t len = 0; > + FILE *f1, *f2; > + char path1[MAXPATHLEN]; > + char path2[MAXPATHLEN]; > + int ret; > bool bret = false; > > - if (container_disk_lock(c)) > + if (container_disk_lock(c0)) > return false; > - ret = snprintf(path, MAXPATHLEN, "%s/%s/lxc_snapshots", c->config_path, > - c->name); > + > + ret = snprintf(path1, MAXPATHLEN, "%s/%s/lxc_snapshots", > c0->config_path, > + c0->name); > if (ret < 0 || ret > MAXPATHLEN) > goto out; > - f = fopen(path, "r"); > - if (f) { > - ret = fscanf(f, "%d", &v); > - fclose(f); > - if (ret != 1) { > - ERROR("Corrupted file %s", path); > + > + if (inc) { > + f1 = fopen(path1, "a"); > + if (!f1) > + goto out; > + > + if (fprintf(f1, "%s:%s\n", c->config_path, c->name) < 0) { > + ERROR("Error writing new snapshots value"); > + fclose(f1); > goto out; > } > - } > - v += inc ? 1 : -1; > - f = fopen(path, "w"); > - if (!f) > - goto out; > - if (fprintf(f, "%d\n", v) < 0) { > - ERROR("Error writing new snapshots value"); > - fclose(f); > - goto out; > - } > - ret = fclose(f); > - if (ret != 0) { > - SYSERROR("Error writing to or closing snapshots file"); > - goto out; > + > + ret = fclose(f1); > + if (ret != 0) { > + SYSERROR("Error writing to or closing snapshots file"); > + goto out; > + } > + } else if (!inc) { > + ret = snprintf(path1, MAXPATHLEN, "%s/%s/lxc_snapshots", > + c0->config_path, c0->name); > + if (ret < 0 || ret > MAXPATHLEN) > + goto out; > + ret = snprintf(path2, MAXPATHLEN, "%s/%s/lxc_snapshots_tmp", > + c0->config_path, c0->name); > + if (ret < 0 || ret > MAXPATHLEN) > + goto out; > + f1 = fopen(path1, "r"); > + if (!f1) > + goto out; > + f2 = fopen(path2, "w"); > + if (!f2) close f1 > + goto out; > + while (getline(&buf, &len, f1) != -1) { > + if (!strstr(strstr(buf, c->config_path), c->name)) { > + if (fprintf(f2, "%s", buf) < 0) { > + ERROR("Error writing new snapshots " > + "value"); > + fclose(f2); close f1 > + remove(path2); > + goto out; > + } > + } > + } > + free(buf); > + fclose(f1); > + fclose(f2); > + rename(path2, path1); I htink it's best to check for failures in these fclose and renames. If either fclose fails, you can salvage the old snapshots file. If rename fails, well you can at least tell the user he's hosed. > } > > bret = true; > > out: > - container_disk_unlock(c); > + container_disk_unlock(c0); > return bret; > } > > @@ -2052,7 +2081,7 @@ static void mod_all_rdeps(struct lxc_container *c, bool > inc) > lxcpath, lxcname); > continue; > } > - if (!mod_rdep(p, inc)) > + if (!mod_rdep(p, c, inc)) > ERROR("Failed to increase numsnapshots for %s:%s", > lxcpath, lxcname); > lxc_container_put(p); > @@ -2067,20 +2096,15 @@ static bool has_fs_snapshots(struct lxc_container *c) > { > char path[MAXPATHLEN]; > int ret, v; > - FILE *f; > + struct stat fbuf; > bool bret = false; > > ret = snprintf(path, MAXPATHLEN, "%s/%s/lxc_snapshots", c->config_path, > c->name); > if (ret < 0 || ret > MAXPATHLEN) > goto out; > - f = fopen(path, "r"); > - if (!f) > - goto out; > - ret = fscanf(f, "%d", &v); > - fclose(f); > - if (ret != 1) > - goto out; > + stat(path, &fbuf); Check for stat failure here. > + v = fbuf.st_size; > bret = v != 0; > > out: > -- > 2.5.0 > > _______________________________________________ > lxc-devel mailing list > [email protected] > http://lists.linuxcontainers.org/listinfo/lxc-devel _______________________________________________ lxc-devel mailing list [email protected] http://lists.linuxcontainers.org/listinfo/lxc-devel
