Hi, this is actually the old version. I implemented it differently using mmap() + + memmem() + memmove() + munmap() + ftruncate() to avoid using a temporary file. It's present as a PR on Github. Would be nice if you could say something about it too:
https://github.com/lxc/lxc/pull/641 Christian On Thu, Aug 27, 2015 at 04:17:28PM +0000, Serge Hallyn wrote: > Quoting Christian Brauner (christianvanbrau...@gmail.com): > > 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 <christianvanbrau...@gmail.com> > > --- > > 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 > > lxc-devel@lists.linuxcontainers.org > > http://lists.linuxcontainers.org/listinfo/lxc-devel
signature.asc
Description: PGP signature
_______________________________________________ lxc-devel mailing list lxc-devel@lists.linuxcontainers.org http://lists.linuxcontainers.org/listinfo/lxc-devel