-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37684/#review96068
-----------------------------------------------------------

Ship it!


Thanks


src/tests/containerizer/rootfs.hpp (lines 111 - 126)
<https://reviews.apache.org/r/37684/#comment151272>

    Can you combine the this loop with the loop below (like the following). 
Also, could you use 'realpath' here to resolve the symlink.
    
    ```
    foreach (const std::string& directory, directories) {
      // Some linux distros are moving all binaries and libraries
      // to /usr, in which case /bin, /lib, and /lib64 will be
      // symlinks to their equivalent directories in /usr. So we
      // call the realpath here first. 
      Result<std::string> realpath = os::realpath(directory);
      if (!realpath.isSome()) {
        return Error("Failed to get realpath for '" + directory + "': " +
                     (realpath.isError() ? realpath.error() : "No such 
directory");
      }
      
      Try<Nothing> result = rootfs->add(realpath.get());
      ...
    }
    ```


- Jie Yu


On Aug. 21, 2015, 4:51 p.m., Greg Mann wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37684/
> -----------------------------------------------------------
> 
> (Updated Aug. 21, 2015, 4:51 p.m.)
> 
> 
> Review request for mesos, Marco Massenzio and Vinod Kone.
> 
> 
> Bugs: MESOS-3296 and MESOS-3297
>     https://issues.apache.org/jira/browse/MESOS-3296
>     https://issues.apache.org/jira/browse/MESOS-3297
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added symlink test for /bin, /lib, and /lib64.
> 
> 
> Diffs
> -----
> 
>   src/tests/containerizer/rootfs.hpp 961003fd6768ee0f7effd2804d1c453f8d45e058 
> 
> Diff: https://reviews.apache.org/r/37684/diff/
> 
> 
> Testing
> -------
> 
> On CentOS 7.1 and other distros with symlinked /bin, /lib, & /lib64:
> 
> "sudo make check"
> 
> 
> Some tests will fail due to other root test issues being tracked at 
> MESOS-3292, MESOS-3293, MESOS-3294, MESOS-3295
> 
> This patch solves bugs in MesosContainerizerLaunchTest.ROOT_ChangeRootfs and 
> the LinuxFilesystemIsolatorTest.* tests, so those should all pass.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Reply via email to