[lxc-devel] [PATCH] Track snapshot dependencies

[Serge Hallyn]

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.