[lxc-devel] [PATCH 1/1] clone: don't ever mark the clone's rootfs as being the old, on disk

[Stéphane Graber]

On Thu, Feb 27, 2014 at 09:49:27PM -0600, Serge Hallyn wrote:
> Otherwise an interrupted clone can lead to the original rootfs
> being delete.
> 
> There is a period during lxcapi_clone during which we have written down
> a temporary configuration file on disk, for the new container, using the
> old rootfs. Interruption of clone doesn't allow us to do the cleanup we
> do in error paths, so a subsequent lxc-destroy removes the old rootfs.
> 
> Fix this by doing the copy_storage as early as possible, and not
> writing down the rootfs when we write down the temporary configuration
> file.
> 
> (note - I tested this by putting a series of
> 'if (strcmp(newname, "u%d") == 0) exit(1)' inline to trigger
> interruption between most blocks.  If someone has a good idea
> for a generic way to regression-test this henceforth that'd be
> great)
> 
> See https://bugs.launchpad.net/lxc/+bug/1285850
> 
> Signed-off-by: Serge Hallyn <serge.hallyn at ubuntu.com>

Acked-by: Stéphane Graber <stgraber at ubuntu.com>

> ---
>  src/lxc/lxccontainer.c | 17 ++++++++++++-----
>  1 file changed, 12 insertions(+), 5 deletions(-)
> 
> diff --git a/src/lxc/lxccontainer.c b/src/lxc/lxccontainer.c
> index d9aa973..c60f927 100644
> --- a/src/lxc/lxccontainer.c
> +++ b/src/lxc/lxccontainer.c
> @@ -2610,6 +2610,7 @@ static struct lxc_container *lxcapi_clone(struct lxc_container *c, const char *n
>  	char newpath[MAXPATHLEN];
>  	int ret, storage_copied = 0;
>  	const char *n, *l;
> +	char *origroot = NULL;
>  	struct clone_update_data data;
>  	FILE *fout;
>  	pid_t pid;
> @@ -2645,6 +2646,10 @@ static struct lxc_container *lxcapi_clone(struct lxc_container *c, const char *n
>  	}
>  
>  	// copy the configuration, tweak it as needed,
> +	if (c->lxc_conf->rootfs.path) {
> +		origroot = c->lxc_conf->rootfs.path;
> +		c->lxc_conf->rootfs.path = NULL;
> +	}
>  	fout = fopen(newpath, "w");
>  	if (!fout) {
>  		SYSERROR("open %s", newpath);
> @@ -2652,6 +2657,7 @@ static struct lxc_container *lxcapi_clone(struct lxc_container *c, const char *n
>  	}
>  	write_config(fout, c->lxc_conf);
>  	fclose(fout);
> +	c->lxc_conf->rootfs.path = origroot;
>  
>  	sprintf(newpath, "%s/%s/rootfs", l, n);
>  	if (mkdir(newpath, 0755) < 0) {
> @@ -2671,6 +2677,12 @@ static struct lxc_container *lxcapi_clone(struct lxc_container *c, const char *n
>  		ERROR("clone: failed to create new container (%s %s)", n, l);
>  		goto out;
>  	}
> +	c2->lxc_conf->rootfs.path = origroot;
> +
> +	// copy/snapshot rootfs's
> +	ret = copy_storage(c, c2, bdevtype, flags, bdevdata, newsize);
> +	if (ret < 0)
> +		goto out;
>  
>  	// update utsname
>  	if (!set_config_item_locked(c2, "lxc.utsname", newname)) {
> @@ -2694,11 +2706,6 @@ static struct lxc_container *lxcapi_clone(struct lxc_container *c, const char *n
>  	if (!(flags & LXC_CLONE_KEEPMACADDR))
>  		network_new_hwaddrs(c2);
>  
> -	// copy/snapshot rootfs's
> -	ret = copy_storage(c, c2, bdevtype, flags, bdevdata, newsize);
> -	if (ret < 0)
> -		goto out;
> -
>  	// We've now successfully created c2's storage, so clear it out if we
>  	// fail after this
>  	storage_copied = 1;
> -- 
> 1.9.0
> 
> _______________________________________________
> lxc-devel mailing list
> lxc-devel at lists.linuxcontainers.org
> http://lists.linuxcontainers.org/listinfo/lxc-devel

-- 
Stéphane Graber
Ubuntu developer
http://www.ubuntu.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20140303/31cfced0/attachment.pgp>