Closed behouba closed 11 months ago
Patch coverage: 76.00
% and project coverage change: -0.49
:warning:
Comparison is base (
c3bcdc6
) 80.00% compared to head (9efa578
) 79.51%.
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.
26 testsβ Β±0βββ26 :heavy_check_mark: Β±0βββ0s :stopwatch: Β±0s ββ1 suites Β±0βββββ0 :zzz: Β±0β ββ1 filesββ Β±0βββββ0 :x: Β±0β
Results for commit 5f24f390.βΒ± Comparison against base commit ba8d41b1.
:recycle: This comment has been updated with latest results.
Now that #56 is merged maybe you could extend the process view with the full command-line. Instead of:
counter
βββ [1] bash
βββ [7] bash
βββ [7] counter.py
βββ [8] bash
βββ [8] tee
This uses the information from the core image filed comm
. In you example I see the output that can also be seen when running ps aux
. You copy the complete command-line from the pages images. Maybe that would be nice first use case of your go-criu interface.
You can just include go-criu with the SHA and date, not using a released version, but the current latest version from git.
With the introduction of the inspect
sub-command, I believe that some of the features for reading memory pages content should also be part of inspect
parameters. I would like to discuss how we should incorporate these features.
comm
) and add another flag (e.g: --ps-args
only usable with --ps-tree
) to display the full command line from memory pages (Only in this case we will need to extract all pages-*.img
, pagemap-*img
and mm-*.img
images)? Examples:
$ checkpointctl inspect /path/to/checkpoint.tar.gz --ps-tree
counter
βββ [1] bash
βββ [7] bash
βββ [7] counter.py
βββ [8] bash
βββ [8] tee
$ checkpointctl inspect /path/to/checkpoint.tar.gz --ps-tree --ps-args
counter
βββ [1] bash
βββ [7] bash -c 'python counter.py'
βββ [7] python counter.py --input data.txt --output result.txt
βββ [8] bash -c 'tee output.log'
βββ [8] tee output.log
--ps-env-vars
) only usable with --ps-tree
.Example:
$ checkpointctl inspect /path/to/checkpoint.tar.gz --ps-tree --ps-env-vars
counter
βββ [1] bash
βββ [7] bash
β βββ [7] counter.py
β β βββ [7] PYTHONPATH=/usr/local/lib/python3.9/site-packages
β β βββ [7] DEBUG_MODE=true
β βββ [8] bash
β βββ [8] tee
βββ [8] tee
βββ [8] OUTPUT_DIR=/var/log
What do you think @rst0git , @adrianreber ?
I am wondering if we can keep a default output and add another flag to display the full command line from memory pages?
It makes sense. For example, we can introduce something like the following two options:
--ps-tree-cmd
: Show command-line arguments--ps-tree-env
: Show environment variablesfor inspecting the content of processes memory pages. I am not sure if it will make sense to have it alongside the tree or json view.
The content of memory pages could be several gigabytes and it might not be appropriate to show all of it in a terminal. However, we can introduce a sub-command, for example checkpointctl memparser
, that can save the output in a file. We can also extend this sub-command with additional options for more effective memory analysis.
@adrianreber What do you think?
What do you think @rst0git , @adrianreber ?
Looks good.
@behouba A container checkpoint may include multiple processes, each with potentially large amount memory. To analyze the memory of a checkpoint it might be useful to display an overview of the memory size of each process, as well as to show the memory pages for a specific process.
Would it make sense to implement this as a table showing all PIDs, process names, and memory size for each process? For example, when the --pid
option has been specified it could show the memory pages (vaddr, hexadecimal, ascii) for the specified PID, and use --output
option that could allow to write the output to a specified file instead of STDOUT.
We could implement this as new sub-command for checkpointctl with description "Analyze container checkpoint memory", or extend the existing (show
and inspect
) sub-commands.
@behouba @adrianreber What do you think?
Would it make sense to implement this as a table showing all PIDs, process names, and memory size for each process? For example, when the
--pid
option has been specified it could show the memory pages (vaddr, hexadecimal, ascii) for the specified PID, and use--output
option that could allow to write the output to a specified file instead of STDOUT.
Implementing this as a table with PIDs, process names, and memory sizes for each process seems to me like a good approach. Also, the --pid an --output options would add flexibility and usability to view to content of the memory pages.
We could implement this as new sub-command for checkpointctl with description "Analyze container checkpoint memory", or extend the existing (show and inspect) sub-commands.
In my option, adding a new sub-command for this use case is more appropriate. However, I am unsure about the most suitable name for this sub-command. What about memscan
or memparser
as you suggested earlier @rst0git ?
What about
memscan
ormemparser
as you suggested earlier @rst0git ?
checkpointctl memparse
might be more appropriate.
"parse" is defined as "to examine computer data and change it into a form that can be easily read or understood".
The following wiki page has some relevant examples of the Volatility interface for memory analysis: https://github.com/volatilityfoundation/volatility/wiki/Linux-Command-Reference#process-memory
"parse" is defined as "to examine computer data and change it into a form that can be easily read or understood".
I totally agree, memparse
is more appropriate according to the definition. Thank you @rst0git, I will work on that.
Closing in favour of https://github.com/checkpoint-restore/checkpointctl/pull/95
This proof of concept introduces the following new flags:
Example output
Information about the new flags:
Example of ps-args:
Example of ps-env-vars:
Example of ps-memory-pages:
I am still uncertain about the design of the new flags. π€
Please note that the CI is expected to fail because the version of the
crit
package used in this code is copied from https://github.com/checkpoint-restore/go-criu/pull/133.