[lxc-devel] [PATCH] Track snapshot dependencies

[Dwight Engen]

On Wed, 21 Aug 2013 12:18:13 -0500
Serge Hallyn <serge.hallyn at ubuntu.com> wrote:

> Quoting Dwight Engen (dwight.engen at oracle.com):
> > On Wed, 21 Aug 2013 10:47:20 -0500
> > Serge Hallyn <serge.hallyn at ubuntu.com> wrote:
> > 
> > > Quoting Dwight Engen (dwight.engen at oracle.com):
> > > > On Tue, 20 Aug 2013 14:15:26 -0500
> > > > Serge Hallyn <serge.hallyn at ubuntu.com> wrote:
> > > > 
> > > > [...]
> > > > > +static bool mod_rdep(struct lxc_container *c, bool inc)
> > > > > +{
> > > > > +	char path[MAXPATHLEN];
> > > > > +	int ret, v = 0;
> > > > > +	FILE *f;
> > > > > +	bool bret = false;
> > > > > +
> > > > > +	if (container_disk_lock(c))
> > > > > +		return 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) {
> > > > > +		ret = fscanf(f, "%d", &v);
> > > > > +		fclose(f);
> > > > > +		if (ret != 1) {
> > > > > +			ERROR("Corrupted file %s", path);
> > > > > +			goto out;
> > > > > +		}
> > > > > +	}
> > > > > +	v += inc ? 1 : -1;
> > > > > +	f = fopen(path, "w");
> > > > > +	if (!f)
> > > > > +		goto out;
> > > > > +	fprintf(f, "%d\n", v);
> > > > > +	fclose(f);
> > > > 
> > > > Should we check the return value of fclose()? ie. it could fail
> > > > ENOSPC?
> > > 
> > > I had thought about it, but note that the dependency tracking is
> > > best-effort.  I don't want an lxc-clone to fail bc we couldn't
> > > mark the dependency.  Maybe I'm wrong on that, and I should.
> > > What do you think?
> > 
> > Ahh okay. Well I just saw that everything else in the routine was
> > checking for errors and failing so it just looked like the fclose()
> > was missed. I think errors from fclose() are a lot less likely than
> > not being able to open the file in the first place so I guess we can
> > ignore them.
> 
> No I have run into cases where I hadn't checked fclose() and that was
> in fact where it was failing...
> 
> I'll go ahead and update :)
> 
> > This is only done for overlayfs right? And if something does go
> > wrong
> 
> Yup
> 
> > it just means refcounts are off? If we fail to mark the dependency
> > but let the clone go through, then the worst that can happen is the
> > parent gets removed at some later date and the cloned container
> > fails to work then?
> 
> Right, which is always the case right now.
> 
> Of course it can go the other way, where lxc-destroy refuses to
> destroy the container bc we couldn't drop the refcount...  (would be
> weird, but not impossible)  so maybe lxc-destroy -f should ignore the
> refcount... my patch did not do that.

Ahh yeah, thats probably a good idea. They can also get off if people
just rm -rf the container (and I'm guilty of doing that sometimes), so
like you said "best effort".