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




src/linux/cgroups2.hpp
Lines 32 (patched)
<https://reviews.apache.org/r/74872/#comment314490>

    which provided options? there are none getting provided here by the caller?



src/linux/cgroups2.hpp
Lines 39-47 (patched)
<https://reviews.apache.org/r/74872/#comment314492>

    We can remove the implicit decision making in these and update cleanup to 
*not unmount* so that the caller can decide whether unmounting is appropriate.
    
    However, it might just be easier here to never actually do the mount in our 
unit tests, I imagine that all the hosts / docker containers / etc we run on 
will have it mounted already in the standard location.



src/linux/cgroups2.cpp
Lines 33-40 (patched)
<https://reviews.apache.org/r/74872/#comment314491>

    let's avoid global implicit state here and for now we'll only support the 
standard location for cgroups2 (/sys/fs/cgroup) using a constant here, e.g. 
ROOT_CGROUP or CGROUP_MOUNT_POINT



src/linux/cgroups2.cpp
Lines 47 (patched)
<https://reviews.apache.org/r/74872/#comment314493>

    nit: brace on the next line on functions like this, I missed it for 
enabled() so feel free to fix that here too :)


- Benjamin Mahler


On Feb. 26, 2024, 10:12 p.m., Devin Leamy wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/74872/
> -----------------------------------------------------------
> 
> (Updated Feb. 26, 2024, 10:12 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Introduces:
> - `cgroups2::mount()`: Mount a cgroup2 hierarchy.
> - `cgroups2::find_mount()`: Find an existing cgroup2 hierarchy.
> - `cgroups2::unmount()`: Unmount the cgroup2 hierarchy, if it was not
>   pre-existing.
> - `cgroups2::cleanup()`: Unmount the cgroup2 hierarchy and cleanup
>   cgroups2. Does nothing if no hierarchy is mounted.
> 
> The mount point is stored in a global variable that is set and unset by
> the `mount()` and `unmount()` methods.
> 
> 
> Diffs
> -----
> 
>   src/linux/cgroups2.hpp 833960d301335541f0bb7ad5c0e05e7222d7bc06 
>   src/linux/cgroups2.cpp 6beb8dd2c43f548dd86be16e828b4abfe6fa3fd3 
> 
> 
> Diff: https://reviews.apache.org/r/74872/diff/2/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Devin Leamy
> 
>

Reply via email to