[CRIU] AUFS Support in CRIU

[Pavel Emelyanov]

On 08/20/2014 08:54 PM, Saied Kazemi wrote:

> 
>     > @@ -240,6 +244,31 @@ static int vma_get_mapfile(struct vma_area *vma, DIR *mfd,
>     >                       vma->vm_socket_id = buf.st_ino;
>     >               } else if (errno != ENOENT)
>     >                       return -1;
>     > +     } else if (opts.aufs) {
>     > +             /*
>     > +              * AUFS support to compensate for the kernel bug
>     > +              * exposing branch pathnames in map_files.
>     > +              *
>     > +              * If the link points inside a branch, replace it
>     > +              * with a pathname from the root for later use in
>     > +              * dump_filemap().
>     > +              */
>     > +             char p[PATH_MAX];
>     > +             int n;
>     > +
>     > +             p[0] = '.';
>     > +             n = read_fd_link(vma->vm_file_fd, &p[1], sizeof p - 1);
>     > +             if (n < 0)
>     > +                     return -1;
>     > +             n = fixup_aufs_path(&p[1], sizeof p - 1, true);
>     > +             if (n < 0)
>     > +                     return -1;
>     > +             if (n > 0) {
>     > +                     vma->aufs_rpath = xmalloc(n + 2);
>     > +                     if (!vma->aufs_rpath)
>     > +                             return -1;
>     > +                     strcpy(vma->aufs_rpath, p);
>     > +             }
>     >       }
>     >
>     >       return 0;
>     > @@ -450,12 +479,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() a file 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))) {
>     > +                             if (stat(f, st_buf) < 0) {
>     > +                                     pr_perror("Failed stat 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;
>     >                       }
> 
>     Why do we need the 2nd call to fixup branch name? Can't we just stat
>     the vma->aufs_rpath here?
> 
> 
> That's because vma->aufs_rpath is relative from the root of the mount namespace (e.g., ./bin/busybox) but we are not in that directory (we're in /).  To avoid calling fixup we can construct the full path by prepending aufs_root as follows:
> 
>     -                       if (opts.aufs && (f = fixup_aufs_fd_path(vma_area->vm_file_fd))) {
>     -                               if (stat(f, st_buf) < 0) {
>     +                       if (opts.aufs && vma_area->aufs_rpath && opts.aufs_root) {
>     +                               char path[PATH_MAX];
>     +                               int n;
>     +
>     +                               n = snprintf(path, PATH_MAX, "%s/%s", opts.aufs_root, vma_area->aufs_rpath);
>     +                               if (n >= PATH_MAX) {
>     +                                       pr_err("Path %s/%s too long\n", opts.aufs_root, vma_area->aufs_rpath);
>     +                                       goto err;
>     +                               }
>     +                               if (stat(path, st_buf) < 0) {
> 
> 
> If you prefer this approach, I will send you a new patch with this change and removal of fixup_aufs_fd_path().

Well yes :) I was confused by the 2nd call to fixup_aufs_path from inside
fixup_aufs_fd_path. It would call read_fd_link and walk the array of branches 
again, which is not necessary, since we did it already in parse_smaps(). So
it's better, from my perspective, not to do extra work.

Just an optimization suggestion -- since we see the full path in parse_smaps(),
we can save one on vma->ppage_bitmap (it's also restore-only field) and just
re-use one here.

Thanks,
Pavel