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




src/slave/containerizer/device_manager/device_manager.cpp
Lines 234 (patched)
<https://reviews.apache.org/r/75143/#comment315074>

    maybe just s/access_map/states/



src/slave/containerizer/device_manager/device_manager.cpp
Lines 237-239 (patched)
<https://reviews.apache.org/r/75143/#comment315073>

    nit: 4 space indent on paren wrapping



src/slave/containerizer/device_manager/device_manager.cpp
Lines 238 (patched)
<https://reviews.apache.org/r/75143/#comment315076>

    how about: s/cgroup_device_state/access/ ?



src/slave/containerizer/device_manager/device_manager.cpp
Lines 240 (patched)
<https://reviews.apache.org/r/75143/#comment315075>

    maybe just s/message_instance/state/



src/slave/containerizer/device_manager/device_manager.cpp
Lines 250-251 (patched)
<https://reviews.apache.org/r/75143/#comment315077>

    use std::move here, or manipulate directly:
    
    ```
    CgroupDeviceAccessMessage state = 
&(*(access_map.mutable_device_access_per_cgroup()))[cgroup];
    
    foreach (const Entry& entry, cgroup_device_state.allow_list) {
      state->add_allow_list(stringify(entry));
    }
    foreach (const Entry& entry, cgroup_device_state.deny_list) {
      state->add_deny_list(stringify(entry));
    }
    ```


- Benjamin Mahler


On Aug. 6, 2024, 4:29 p.m., Jason Zhou wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/75143/
> -----------------------------------------------------------
> 
> (Updated Aug. 6, 2024, 4:29 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Currently we do not checkpoint the device access state of each cgroup
> when the configure or reconfigure is called. Meaning that we have
> no way of recovering a cgroup's device access state.
> 
> We will checkpoint state of the device manager whenever its state
> is being changed to ensure that we can recover the most recent state
> when necessary.
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/device_manager/device_manager.cpp 
> e613323dc47a7980984426d37b6fc5cfc52dffe0 
> 
> 
> Diff: https://reviews.apache.org/r/75143/diff/2/
> 
> 
> Testing
> -------
> 
> In later patches of the stack, we were able to ensure that the state was 
> persisted and was able to be recovered.
> 
> 
> Thanks,
> 
> Jason Zhou
> 
>

Reply via email to