[lxc-devel] [PATCH 1/1] snapshots: move snapshot directory

[Stéphane Graber]

On Wed, Apr 16, 2014 at 02:04:45PM -0500, Serge Hallyn wrote:
> Originally we kept snapshots under /var/lib/lxcsnaps.  If a
> separate btrfs is mounted at /var/lib/lxc, then we can't
> make btrfs snapshots under /var/lib/lxcsnaps.
> 
> This patch moves the default directory to /var/lib/lxc/lxcsnaps.
> If /var/lib/lxcsnaps already exists, then use that.  Don't allow
> any container to be used with the name 'lxcsnaps'.

Just to follow up on thoughts I shared on IRC.

I wonder why we shouldn't move those to
/var/lib/lxc/<container>/snapshots/ instead. Regardless of where they
go, the testcase should also be updated to match.

> 
> Signed-off-by: Serge Hallyn <serge.hallyn at ubuntu.com>
> ---
>  src/lxc/lxccontainer.c | 49 +++++++++++++++++++++++++++++++++++++++----------
>  src/lxc/lxccontainer.h |  4 ++--
>  2 files changed, 41 insertions(+), 12 deletions(-)
> 
> diff --git a/src/lxc/lxccontainer.c b/src/lxc/lxccontainer.c
> index c90b564..1059961 100644
> --- a/src/lxc/lxccontainer.c
> +++ b/src/lxc/lxccontainer.c
> @@ -2823,16 +2823,42 @@ static int get_next_index(const char *lxcpath, char *cname)
>  	}
>  }
>  
> +static bool get_snappath_dir(struct lxc_container *c, char *snappath)
> +{
> +	int ret;
> +	/*
> +	 * If the old style snapshot path exists, use it
> +	 * /var/lib/lxc -> /var/lib/lxcsnaps
> +	 */
> +	ret = snprintf(snappath, MAXPATHLEN, "%ssnaps", c->config_path);
> +	if (ret < 0 || ret >= MAXPATHLEN)
> +		return false;
> +	if (dir_exists(snappath)) {
> +		ret = snprintf(snappath, MAXPATHLEN, "%ssnaps/%s", c->config_path, c->name);
> +		if (ret < 0 || ret >= MAXPATHLEN)
> +			return false;
> +		return true;
> +	}
> +
> +	/*
> +	 * Use the new style path
> +	 * /var/lib/lxc -> /var/lib/lxc + /lxcsnaps/ + c->name + \0
> +	 */
> +	ret = snprintf(snappath, MAXPATHLEN, "%s/lxcsnaps/%s", c->config_path, c->name);
> +	if (ret < 0 || ret >= MAXPATHLEN)
> +		return false;
> +	return true;
> +}
> +
>  static int lxcapi_snapshot(struct lxc_container *c, const char *commentfile)
>  {
>  	int i, flags, ret;
>  	struct lxc_container *c2;
>  	char snappath[MAXPATHLEN], newname[20];
>  
> -	// /var/lib/lxc -> /var/lib/lxcsnaps \0
> -	ret = snprintf(snappath, MAXPATHLEN, "%ssnaps/%s", c->config_path, c->name);
> -	if (ret < 0 || ret >= MAXPATHLEN)
> +	if (!get_snappath_dir(c, snappath)) {
>  		return -1;
> +	}
>  	i = get_next_index(snappath, c->name);
>  
>  	if (mkdir_p(snappath, 0755) < 0) {
> @@ -2966,7 +2992,7 @@ static char *get_timestamp(char* snappath, char *name)
>  static int lxcapi_snapshot_list(struct lxc_container *c, struct lxc_snapshot **ret_snaps)
>  {
>  	char snappath[MAXPATHLEN], path2[MAXPATHLEN];
> -	int dirlen, count = 0, ret;
> +	int count = 0, ret;
>  	struct dirent dirent, *direntp;
>  	struct lxc_snapshot *snaps =NULL, *nsnaps;
>  	DIR *dir;
> @@ -2974,9 +3000,7 @@ static int lxcapi_snapshot_list(struct lxc_container *c, struct lxc_snapshot **r
>  	if (!c || !lxcapi_is_defined(c))
>  		return -1;
>  
> -	// snappath is ${lxcpath}snaps/${lxcname}/
> -	dirlen = snprintf(snappath, MAXPATHLEN, "%ssnaps/%s", c->config_path, c->name);
> -	if (dirlen < 0 || dirlen >= MAXPATHLEN) {
> +	if (!get_snappath_dir(c, snappath)) {
>  		ERROR("path name too long");
>  		return -1;
>  	}
> @@ -3044,7 +3068,7 @@ out_free:
>  static bool lxcapi_snapshot_restore(struct lxc_container *c, const char *snapname, const char *newname)
>  {
>  	char clonelxcpath[MAXPATHLEN];
> -	int flags = 0,ret;
> +	int flags = 0;
>  	struct lxc_container *snap, *rest;
>  	struct bdev *bdev;
>  	bool b = false;
> @@ -3067,8 +3091,7 @@ static bool lxcapi_snapshot_restore(struct lxc_container *c, const char *snapnam
>  			return false;
>  		}
>  	}
> -	ret = snprintf(clonelxcpath, MAXPATHLEN, "%ssnaps/%s", c->config_path, c->name);
> -	if (ret < 0 || ret >= MAXPATHLEN) {
> +	if (!get_snappath_dir(c, clonelxcpath)) {
>  		bdev_put(bdev);
>  		return false;
>  	}
> @@ -3284,6 +3307,12 @@ struct lxc_container *lxc_container_new(const char *name, const char *configpath
>  {
>  	struct lxc_container *c;
>  
> +	if (!name)
> +		return NULL;
> +
> +	if (strcmp(name, "lxcsnaps") == 0)
> +		return NULL;
> +
>  	c = malloc(sizeof(*c));
>  	if (!c) {
>  		fprintf(stderr, "failed to malloc lxc_container\n");
> diff --git a/src/lxc/lxccontainer.h b/src/lxc/lxccontainer.h
> index ba15ab7..946a662 100644
> --- a/src/lxc/lxccontainer.h
> +++ b/src/lxc/lxccontainer.h
> @@ -649,7 +649,7 @@ struct lxc_container {
>  	 * \brief Create a container snapshot.
>  	 *
>  	 * Assuming default paths, snapshots will be created as
> -	 * \c /var/lib/lxcsnaps/\<c\>/snap\<n\>
> +	 * \c /var/lib/lxc/lxcsnaps/\<c\>/snap\<n\>
>  	 * where \c \<c\> represents the container name and \c \<n\>
>  	 * represents the zero-based snapshot number.
>  	 *
> @@ -691,7 +691,7 @@ struct lxc_container {
>  	 *  fail if the  snapshot is overlay-based, since the snapshots
>  	 *  will pin the original container.
>  	 * \note As an example, if the container exists as \c /var/lib/lxc/c1, snapname might be \c 'snap0'
> -	 *  (representing \c /var/lib/lxcsnaps/c1/snap0). If \p newname is \p c2,
> +	 *  (representing \c /var/lib/lxc/lxcsnaps/c1/snap0). If \p newname is \p c2,
>  	 *  then \c snap0 will be copied to \c /var/lib/lxc/c2.
>  	 */
>  	bool (*snapshot_restore)(struct lxc_container *c, const char *snapname, const char *newname);
> -- 
> 1.8.3.2
> 
> _______________________________________________
> 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/20140430/70beedc1/attachment-0001.sig>