[lxc-devel] [PATCH] c/r: force users to provide a fresh directory for criu images

[Tycho Andersen]

On Wed, Apr 22, 2015 at 10:26:17PM +0000, Serge Hallyn wrote:
> Quoting Tycho Andersen (tycho.andersen at canonical.com):
> > On Wed, Apr 22, 2015 at 01:16:08PM -0600, Tycho Andersen wrote:
> > > CRIU can get confused if more than one c/r is done in the same directory, so we
> > > should require lxcapi so that it refuses to dump to a directory with criu
> > > images already in it.
> > 
> > Hmm, actually I'm not sure this is the best way to do this. If users
> > (e.g. LXD) use some sort of mechanism to generate a temporary
> > directory name we could race if the same name is tried twice in a row.
> 
> ?
> 
> You mean if they use something like mkstemp and two threads get the
> same name from it?  (that shouldn't happen)

The problem is that we pass liblxc the dirname, but this patch means
that liblxc creates the directory. mkstemp (and go's equivalent,
ioutil.TempDir) works because it opens the file, so presumably they
test to see that it doesn't exist.

However, we'd have to remove the directory after TempDir creates it,
because liblxc wants to create it. That could get us into a situation
where another TempDir call might generate the same name, in between
when we remove it and when liblxc creates it.

Tycho

> > Instead, maybe it's just best to stat() for the key criu image file
> > (inventory.img), and die if it's there.
> > 
> > Thoughts?
> > 
> > > This won't help if the user copies a checkpoint over another, but at least the
> > > common case will be caught.
> > > 
> > > Signed-off-by: Tycho Andersen <tycho.andersen at canonical.com>
> > > ---
> > >  src/lxc/lxccontainer.c | 7 ++++++-
> > >  1 file changed, 6 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/src/lxc/lxccontainer.c b/src/lxc/lxccontainer.c
> > > index dbcee99..4a290f1 100644
> > > --- a/src/lxc/lxccontainer.c
> > > +++ b/src/lxc/lxccontainer.c
> > > @@ -3687,8 +3687,13 @@ static bool do_lxcapi_checkpoint(struct lxc_container *c, char *directory, bool
> > >  	if (!criu_ok(c))
> > >  		return false;
> > >  
> > > -	if (mkdir(directory, 0700) < 0 && errno != EEXIST)
> > > +	if (mkdir(directory, 0700) < 0) {
> > > +		if (errno == EEXIST)
> > > +			ERROR("please use a new directory for criu state");
> > > +		else
> > > +			SYSERROR("mkdir failed");
> > >  		return false;
> > > +	}
> > >  
> > >  	if (!dump_net_info(c, directory))
> > >  		return false;
> > > -- 
> > > 2.1.4
> > > 
> > _______________________________________________
> > lxc-devel mailing list
> > lxc-devel at lists.linuxcontainers.org
> > http://lists.linuxcontainers.org/listinfo/lxc-devel
> _______________________________________________
> lxc-devel mailing list
> lxc-devel at lists.linuxcontainers.org
> http://lists.linuxcontainers.org/listinfo/lxc-devel