[CRIU] AUFS Support in CRIU

[Pavel Emelyanov]

On 08/13/2014 04:29 AM, Saied Kazemi wrote:

Hi, Saied.

> Here is a new patch.  I rebased to the head (commit ded04267f8) and cleaned up the code a
> bit more.  Please use the attached patch instead of the one I sent yesterday.
> 
> Note that I had to delete 3 lines from cgroup.c for properties that I don't have on my
> Ubuntu 14.04.  Also, please note that you have to add --manage-cgroups to criu command
> line for both dump and restore.

Thank you for looking into this. I'm not familiar with AUFS at all, thus I
only have comments about the way AUFS parsing/fixing code is integrated into
the rest of the criu code.

Please, see my comments inline.

> @@ -197,6 +199,19 @@ int fill_fdlink(int lfd, const struct fd_parms *p, struct fd_link *link)
>  		return -1;
>  	}
>  
> +	/*
> +	 * For AUFS support, we need to replace absolute
> +	 * branch pathnames with relative pathnames from root.
> +	 */
> +	if (opts.aufs) {

At this point we have the statfs() information. Can we rely on the
fs_type being aufs magic to check that the file is aufs one?

> +		int n = sizeof link->name - 1;
> +		n = fixup_aufs_path(&link->name[1], n, true);
> +		if (n < 0)
> +			return -1;
> +		if (n > 0)
> +			len = n;
> +	}
> +
>  	link->len = len + 1;
>  	return 0;

> @@ -450,12 +453,24 @@ int parse_smaps(pid_t pid, struct vm_area_list *vma_area_list, bool use_map_file
>  			vma_area->st = prev->st;
>  		} else if (vma_area->vm_file_fd >= 0) {
>  			struct stat *st_buf;
> +			char *f;
>  
>  			st_buf = vma_area->st = xmalloc(sizeof(*st_buf));
>  			if (!st_buf)
>  				goto err;
>  
> -			if (fstat(vma_area->vm_file_fd, st_buf) < 0) {
> +			/*
> +			 * For AUFS support, we cannot fstat() file a descriptor that
> +			 * is a symbolic link to a branch.  Instead, we obtain the
> +			 * pathname of the file from the root and use stat().
> +			 */
> +			if (opts.aufs && (f = fixup_aufs_fd_path(vma_area->vm_file_fd))) {

Would this work if the vm_file_fd sits in another mount namespace? Or
the path reported by fixup_aufs_fd_path() is always in the criu's one?

And another question -- do we need the fixed stat buffer that early?
The vma fd is dumped in dump_filemap() and in that place we call
get_fd_mntid() for missing mount ID. Can we fixup the device there too?

> +				if (stat(f, st_buf) < 0) {
> +					pr_perror("Failed fstat on %d's map %lu (%s)",
> +						pid, start, f);
> +					goto err;
> +				}
> +			} else if (fstat(vma_area->vm_file_fd, st_buf) < 0) {
>  				pr_perror("Failed fstat on %d's map %lu", pid, start);
>  				goto err;
>  			}

> @@ -921,6 +942,16 @@ static int parse_mountinfo_ent(char *str, struct mount_info *new)
>  	if (ret != 3)
>  		return -1;
>  
> +	/* see comments in sysfs_parse.c */
> +	if (opts.aufs && !strcmp(new->mountpoint, "./")) {
> +		if (strcmp(fstype, "aufs")) {

Can we reuse the fstype->parse() callback for this? This one gets called
in parse_mountinfo() in a loop after parse_mountinfo_ent().

> +			pr_err("Expected fstype aufs got %s\n", fstype);
> +			return -1;
> +		}
> +		if (fixup_src_opt(&new->source, &opt) < 0)
> +			return -1;
> +	}
> +
>  	ret = -1;
>  	new->fstype = find_fstype_by_name(fstype);
>  

> +/*
> + * Copy the line in process's mountinfo file that corresponds
> + * to the mountpoint specified by the mntpoint argument.  Return
> + * the number of characters parsed in the line, or -1 on error.
> + */
> +int get_mountinfo_by_mountpoint(pid_t pid, char *mntpoint, char *line, int linelen)

I do not fully understand why this mountpoint->something-else parser
is required. Can we put all the code parsing aufs-specific stuff into
the fstype->parse() callback I mentioned above? When criu starts
parsing mount namespaces (it does this before doing anything else)
this info will be parsed and stored into mount_info->private. Later, 
when we need to e.g. fixup file paths we can get file->mnt_id, get 
mount_info by it, then check the fs being aufs and fix the path 
according to the mount_info->private data we have. Would this work?

> +{
> +	char buf[PATH_MAX];
> +	int n, ret;
> +	FILE *f;

> +
> +	snprintf(buf, sizeof buf, "/proc/%d/mountinfo", pid);
> +	f = fopen(buf, "r");
> +	if (!f) {
> +		pr_perror("Cannot fopen %s", buf);
> +		return -1;
> +	}
> +
> +	do {
> +		n = linelen - 2;
> +		line[n] = '\0';		/* detect long input */
> +		if (fgets(line, linelen, f) == NULL) {
> +			ret = 0;
> +			break;
> +		}
> +		if (line[n] && line[n] != '\n') {
> +			pr_err("Line in mountinfo too long\n");
> +			ret = -1;
> +			goto out;
> +		}
> +
> +		ret = sscanf(line, "%*i %*i %*u:%*u %*s %s %*s - %n", buf, &n);

Wow. Doesn't gcc prints a warning about "too few arguments for format"?

> +		if (ret != 1) {
> +			pr_err("Cannot parse mountpoint (%s)\n", line);
> +			ret = -1;
> +			goto out;
> +		}
> +	} while (strcmp(buf, mntpoint));
> +
> +	if (!ret) {
> +		pr_err("Did not find %s in mountinfo\n", mntpoint);
> +		ret = -1;
> +	} else
> +		ret = n;
> +
> +out:
> +	fclose(f);
> +	return ret;
> +}

Thanks,
Pavel