-
Notifications
You must be signed in to change notification settings - Fork 2
WIP XXX Migration Statistics #43
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
base: gardenlinux
Are you sure you want to change the base?
WIP XXX Migration Statistics #43
Conversation
e29356d to
4101b11
Compare
| /// The final memory transmission info. | ||
| memory_info: MemoryTransmissionInfo, | ||
| }, | ||
| Failed { |
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.
currently I don't like that this phase is part of this enum. Instead, on the top level we should have
Ongoing(InnerStateA), Cancelled(InnerStateB), Failed(InnerStateC)
vm-migration/src/progress.rs
Outdated
| pub memory_bytes_transmitted: u64, | ||
| pub memory_pages_4k_transmitted: u64, | ||
| pub memory_bytes_remaining_iteration: u64, | ||
| pub memory_bytes_remaining_total: u64, |
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.
qemu also reports the dirty rate. Do we want to report this too?
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.
what's also missing is some sort of status (migration in-progess, migration-finished, ...)
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.
emu also reports the dirty rate. Do we want to report this too?
100%, sure. I just forgot it here
migration in-progess, migration-finished
I have it already, but I want to refactor it to make it more promiment. See my comment #43 (review)
f49e577 to
fdf5858
Compare
| let clear = matches | ||
| .subcommand_matches("migration-progress") | ||
| .unwrap() | ||
| .get_one::<bool>("clear") |
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.
I think we can completely nuke that clear complexity - there is no problem with keeping the old state around. As soon as a new migration starts, the state is overwritten anyway
| } | ||
|
|
||
| #[derive(Clone, Deserialize, Serialize, Debug)] | ||
| pub struct VmMigrationProgressData { |
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.
I think we can completely nuke that clear complexity - there is no problem with keeping the old state around. As soon as a new migration starts, the state is overwritten anyway
Signed-off-by: Philipp Schuster <philipp.schuster@cyberus-technology.de> On-behalf-of: SAP philipp.schuster@sap.com
Signed-off-by: Philipp Schuster <philipp.schuster@cyberus-technology.de> On-behalf-of: SAP philipp.schuster@sap.com
Signed-off-by: Philipp Schuster <philipp.schuster@cyberus-technology.de> On-behalf-of: SAP philipp.schuster@sap.com
f612cf6 to
e9a3321
Compare
No description provided.