CARVIEW |
Navigation Menu
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
runtime: deprecate runc --criu / -criu-path option #6496
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
4cf4344
to
0ecad97
Compare
FWIW, I noticed various
|
0ecad97
to
598b386
Compare
@thaJeztah That's odd. I have touched a bunch of protoc/protobuild stuff, but these changes shouldn't be reached to containerd automatically. |
@@ -75,7 +75,7 @@ func init() { | |||
flag.StringVar(&addressFlag, "address", "", "grpc address back to main containerd") | |||
flag.StringVar(&workdirFlag, "workdir", "", "path used to storge large temporary data") | |||
flag.StringVar(&runtimeRootFlag, "runtime-root", process.RuncRoot, "root directory for the runtime") | |||
flag.StringVar(&criuFlag, "criu", "", "path to criu binary") | |||
flag.StringVar(&criuFlag, "criu", "", "path to criu binary (deprecated: do not use)") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The entire shim v1 itself is already deprecated, so maybe this message isn't necessary
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I doubt many people will use it manually from the command-line, but when running the command, it's not printed that it's deprecated, and I think it's useful to at least make it visible that the option itself should not be used?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The v1 shim is deprecated but working. The flag is deprecated and it doesn't work (ignored). So I'd like to keep the message.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@AkihiroSuda Looks good to merge? |
Needs rebase |
You are right. I was assuming that GitHub shows a list of conflicted files when it needs rebase. Where does that go? :( |
I think it performs that check "lazily" (so when you open the page), and it sometimes takes some time to show up. Would be great if they did so "pro-actively" and added a visible warning in overviews (and possibly allowing to filter on it (etc)) I'll try to find some time to rebase tomorrow |
@thaJeztah can you please rebase this? |
runc option --criu is now ignored (with a warning), and the option will be removed entirely in a future release. Users who need a non- standard criu binary should rely on the standard way of looking up binaries in $PATH. Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
598b386
to
d2013d2
Compare
Rebased 👍 |
Build succeeded.
|
Build succeeded.
|
relates to:
runc option --criu is now ignored (with a warning), and the option will be
removed entirely in a future release. Users who need a non- standard criu
binary should rely on the standard way of looking up binaries in $PATH.