fs2: ignore no device error when reading freezer state

[lifubang]

When reading freezer state, we can safely ignore the error in the following two common situations:

1. The cgroup path does not exist at the time of opening(eg: the kernel is too old) — indicated by os.IsNotExist.
2. The cgroup path is deleted during the seek/read operation — indicated by errors.Is(err, unix.ENODEV). These conditions are expected and do not require special handling.

Unfortunately, the second case was accidentally removed during a code refactoring. We should add it back to ensure correct error handling.

Fix: opencontainers/runc#4798

[kannon92]

cc @kolyshkin

[kolyshkin]

why not put ignore... into the same file, fs2/freezer.go? It is only used from 1 place.

[lifubang]

why not put ignore... into the same file, fs2/freezer.go? It is only used from 1 place.

Just a thought for the future — I saw a similar logic in runc. Maybe we should move it to cgroups, as it could be useful later on.

For example:
https://github.com/opencontainers/runc/blob/main/libcontainer/factory_linux.go#L69-L78

[kolyshkin]

why not put ignore... into the same file, fs2/freezer.go? It is only used from 1 place.

Just a thought for the future — I saw a similar logic in runc. Maybe we should move it to cgroups, as it could be useful later on.

For example: https://github.com/opencontainers/runc/blob/main/libcontainer/factory_linux.go#L69-L78

It is basically a one-liner and I'm not against reimplementing it a few times (rather than exposing as a public API).

When we expose something as a public API, people start depending on it, and then we change the logic (in this case that would be adding another errno, or removing one) people will complain that we break backward compatibility. So, as much as I'm in for code reuse,

• this is not too much code to reuse;
• this better not be added to public API.

[lifubang]

When we expose something as a public API, people start depending on it, and then we change the logic (in this case that would be adding another errno, or removing one) people will complain that we break backward compatibility. So, as much as I'm in for code reuse,

Make sense, removed it now.

[kolyshkin]

LGTM, thanks!

[thaJeztah]

LGTM

[thaJeztah]

Do you think we need to call out that we're deliberately using os.IsNotExist() here, instead of errors.Is (https://pkg.go.dev/os#IsNotExist) to prevent someone from updating the code, or would we be paying attention if that would happen?

[kolyshkin]

Let's address this as a followup.