<div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote">On Wed, Aug 20, 2014 at 3:39 AM, Pavel Emelyanov <span dir="ltr"><<a href="mailto:xemul@parallels.com" target="_blank">xemul@parallels.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div class="">On 08/20/2014 10:21 AM, Saied Kazemi wrote:<br>
<br>
> > Thanks again for your feedback. I hope the answers are clear. If it's easier or more productive,<br>
> > I am available on video chat to discuss in greater detail. My schedule is flexible, so time<br>
> > difference shouldn't be a problem.<br>
><br>
> OK, I'll keep that in mind :) Your timezone is US Pacific one, right?<br>
><br>
><br>
> Yes, I am in Mountain View.<br>
><br>
> Looking forward to your comments on this patch which I think has addressed everything above.<br>
<br>
</div>Yup, I think this patch is perfect. Only one question left:<br></blockquote><div><br></div><div>Great!</div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<br>
> @@ -240,6 +244,31 @@ static int vma_get_mapfile(struct vma_area *vma, DIR *mfd,<br>
> vma->vm_socket_id = buf.st_ino;<br>
> } else if (errno != ENOENT)<br>
> return -1;<br>
> + } else if (opts.aufs) {<br>
> + /*<br>
> + * AUFS support to compensate for the kernel bug<br>
> + * exposing branch pathnames in map_files.<br>
> + *<br>
> + * If the link points inside a branch, replace it<br>
> + * with a pathname from the root for later use in<br>
> + * dump_filemap().<br>
> + */<br>
> + char p[PATH_MAX];<br>
> + int n;<br>
> +<br>
> + p[0] = '.';<br>
> + n = read_fd_link(vma->vm_file_fd, &p[1], sizeof p - 1);<br>
<div class="">> + if (n < 0)<br>
> + return -1;<br>
</div>> + n = fixup_aufs_path(&p[1], sizeof p - 1, true);<br>
<div class="">> + if (n < 0)<br>
> + return -1;<br>
> + if (n > 0) {<br>
</div>> + vma->aufs_rpath = xmalloc(n + 2);<br>
> + if (!vma->aufs_rpath)<br>
> + return -1;<br>
> + strcpy(vma->aufs_rpath, p);<br>
> + }<br>
> }<br>
><br>
> return 0;<br>
> @@ -450,12 +479,24 @@ int parse_smaps(pid_t pid, struct vm_area_list *vma_area_list, bool use_map_file<br>
<div class="">> vma_area->st = prev->st;<br>
> } else if (vma_area->vm_file_fd >= 0) {<br>
> struct stat *st_buf;<br>
> + char *f;<br>
><br>
> st_buf = vma_area->st = xmalloc(sizeof(*st_buf));<br>
> if (!st_buf)<br>
> goto err;<br>
><br>
> - if (fstat(vma_area->vm_file_fd, st_buf) < 0) {<br>
> + /*<br>
</div>> + * For AUFS support, we cannot fstat() a file descriptor that<br>
<div class="">> + * is a symbolic link to a branch. Instead, we obtain the<br>
> + * pathname of the file from the root and use stat().<br>
> + */<br>
> + if (opts.aufs && (f = fixup_aufs_fd_path(vma_area->vm_file_fd))) {<br>
</div><div class="">> + if (stat(f, st_buf) < 0) {<br>
</div>> + pr_perror("Failed stat on %d's map %lu (%s)",<br>
<div class="">> + pid, start, f);<br>
> + goto err;<br>
> + }<br>
> + } else if (fstat(vma_area->vm_file_fd, st_buf) < 0) {<br>
> pr_perror("Failed fstat on %d's map %lu", pid, start);<br>
> goto err;<br>
> }<br>
<br>
</div>Why do we need the 2nd call to fixup branch name? Can't we just stat<br>
the vma->aufs_rpath here?<br></blockquote><div><br></div><div>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:</div>
<div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div>- if (opts.aufs && (f = fixup_aufs_fd_path(vma_area->vm_file_fd))) {</div>
<div>- if (stat(f, st_buf) < 0) {</div><div>+ if (opts.aufs && vma_area->aufs_rpath && opts.aufs_root) {</div><div>+ char path[PATH_MAX];</div>
<div>+ int n;</div><div>+</div><div>+ n = snprintf(path, PATH_MAX, "%s/%s", opts.aufs_root, vma_area->aufs_rpath);</div><div>+ if (n >= PATH_MAX) {</div>
<div>+ pr_err("Path %s/%s too long\n", opts.aufs_root, vma_area->aufs_rpath);</div><div>+ goto err;</div><div>+ }</div>
<div>+ if (stat(path, st_buf) < 0) {</div></blockquote><div><br></div><div>If you prefer this approach, I will send you a new patch with this change and removal of fixup_aufs_fd_path().</div>
<div><br></div>
<div>Cheers!</div><div><br></div><div>--Saied</div><div><br></div></div></div></div>