> On March 9, 2016, 6:03 p.m., Jiang Yan Xu wrote:
> > src/slave/containerizer/mesos/isolators/disk/xfs.hpp, line 87
> > <https://reviews.apache.org/r/44342/diff/3/?file=1279807#file1279807line87>
> >
> >     Ditto about uint32_t vs prid_t.

We don't use ``prid_t`` in the header so that the XFS and quota headers don't 
pollute the namespace of other parts of Mesos.


> On March 9, 2016, 6:03 p.m., Jiang Yan Xu wrote:
> > configure.ac, line 923
> > <https://reviews.apache.org/r/44342/diff/3/?file=1279804#file1279804line923>
> >
> >     Where is the corresponding AC_ARG_WITH?
> >     
> >     i.e. we should only do this if the user wants to use XFS.

There's no ``AC_ARG_WITH``. If the dependencies area available we will build 
the isolator and the operator can configure it at runtime. I don't think 
there's any need to disable this at build time since it is not enabled by 
default anyway.


> On March 9, 2016, 6:03 p.m., Jiang Yan Xu wrote:
> > src/slave/containerizer/mesos/isolators/disk/xfs.cpp, line 17
> > <https://reviews.apache.org/r/44342/diff/3/?file=1279808#file1279808line17>
> >
> >     Add a link that explains this?

A link to what?


> On March 9, 2016, 6:03 p.m., Jiang Yan Xu wrote:
> > src/slave/containerizer/mesos/isolators/disk/xfs.cpp, line 128
> > <https://reviews.apache.org/r/44342/diff/3/?file=1279808#file1279808line128>
> >
> >     Can this and other xfs* methods be put under a xfs namespace instead?

They are internal to the file, so I don't think we need another namespace.


> On March 9, 2016, 6:03 p.m., Jiang Yan Xu wrote:
> > src/slave/containerizer/mesos/isolators/disk/xfs.cpp, line 154
> > <https://reviews.apache.org/r/44342/diff/3/?file=1279808#file1279808line154>
> >
> >     This doesn't appear to be XFS specific. Can we pull this into stout? At 
> > lease name it such that it's clear.

I don't know that is belongs in stout since it pulls in a dependency on 
``libblkid``. I can make the name generic though.


> On March 9, 2016, 6:03 p.m., Jiang Yan Xu wrote:
> > src/slave/containerizer/mesos/isolators/disk/xfs.cpp, lines 190-194
> > <https://reviews.apache.org/r/44342/diff/3/?file=1279808#file1279808line190>
> >
> >     Why list initialize it here?

It's not a list initialization just a regular struct initialization.


> On March 9, 2016, 6:03 p.m., Jiang Yan Xu wrote:
> > src/slave/containerizer/mesos/isolators/disk/xfs.cpp, lines 191-194
> > <https://reviews.apache.org/r/44342/diff/3/?file=1279808#file1279808line191>
> >
> >     Why a union here?

The union is to avoid a ``reinterpret_cast`` later.


- James


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


On March 7, 2016, 6:38 p.m., James Peach wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44342/
> -----------------------------------------------------------
> 
> (Updated March 7, 2016, 6:38 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-4828
>     https://issues.apache.org/jira/browse/MESOS-4828
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Track sandbox directory usage by dynamically assigning XFS project
> quotas. We track a range of XFS project IDs, assigning a project ID
> and a project quota to each sandbox as it is created. When the task
> reaches the quota, writes will fail with EDQUOT, and the task will have
> an opportunity to handle that.
> 
> Quotas are not applied to volume resources since the isolator interface
> has no insight into the volume lifecycle. Thus it is not currently
> possible to accurately assign and reclaim project IDs.
> 
> If LOW is the lower bound of the project ID range and HIGH is the upper
> bound, you can show the currently allocated project quotas using the
> xfs_quota command:
> 
>   $ xfs_quota -x -c "report -a -n -L LOW -U HIGH"
> 
> To show the project ID assigned to the file PATH, use the xfs_io command:
> 
>   $ xfs_io -r -c stat PATH
> 
> 
> Diffs
> -----
> 
>   configure.ac a20382e8d425eb297492a6e6c2c75ea59be097c2 
>   docs/configuration.md 305ba2c801c2060db6dcb4ef83c1043aaa7d520c 
>   docs/mesos-containerizer.md 15fb5bdbe74e059614b8948108f32cd04b623305 
>   src/Makefile.am a41e95ddeb838fdebf4ced953c4a29181916e261 
>   src/slave/containerizer/mesos/containerizer.cpp 
> af3ff5750649497d8852b4761c78d4cae5455a02 
>   src/slave/containerizer/mesos/isolators/disk/xfs.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/disk/xfs.cpp PRE-CREATION 
>   src/slave/flags.hpp feb095da4521f678c96f4cc53bdfda262d350388 
>   src/slave/flags.cpp 6e3fd69c06eefd40bc0e5c222ea72f34144c5534 
> 
> Diff: https://reviews.apache.org/r/44342/diff/
> 
> 
> Testing
> -------
> 
> Manual testing on Fedora 23 w/ XFS. Make check on Fedora and OS X.
> 
> 
> Thanks,
> 
> James Peach
> 
>

Reply via email to