Document Pause and Resume #156
Conversation
| let cgroups_path = | ||
| utils::get_cgroup_path(&spec.linux.unwrap().cgroups_path, &self.container_id); | ||
| // get cgroup path defined in spec | ||
| let path_in_spec = match spec.linux { |
There was a problem hiding this comment.
Since when create container we guarantee spec.linux must not be None also by unwrap it directly (https://github.com/containers/youki/blob/main/src/container/builder_impl.rs#L62)
I think it's unnecessary and we can keep it simple here.
(Actually we unwrap it directly in all commands)
There was a problem hiding this comment.
Ohk, I'll revert these chagnes back.
I had a question though, if we can guarantee that it won't be None, then why we have kept it as Option type? In case it is for the case when loading spec from JSON it might be None, should we try to make some workaround so that it will never be None?
The point I am trying to make is that if we are calling unwrap at all places, then Youki will not work when it is None, and we may as well as move that error to the place where we load the spec, and convert Option<T> to the T .
I might be missing something of the big picture 😅 , but can you help me understanding this? @utam0k can you take a look as well?
Thanks
There was a problem hiding this comment.
Yeah, I agree with you.
It seems the default() for Linux has been implemented, I think we can remove Option for Linux.
Or at least we should add a validate function when create container to check input meets our expectation(like Linux is not None and so on), otherwise it would panic and unreadable for users.
WDYT? @utam0k
Documents pause and resume commands, and add references for freezer cgroups in doc-draft.
@duduainankai I have changed an unwrap statement to a temp variable with match statement. It compiles correctly, can you verify that it does not do anything incorrect? I changed it to prevent potential panic due to unwrap. Also please verify the comments as well.