Skip to content

Conversation

@last-genius
Copy link
Contributor

localhost_migration was introduced as the last parameter for migrate in xenops_interface.ml, but was second to last in xenops_server.ml.

This meant that non-localhost migrations with verify_dest=true were classed as localhost and did not balloon down before migration.

Fixes: 0911604 ("xenopsd: Don't balloon down memory on same-host migration")

localhost_migration was introduced as the last parameter for migrate in
xenops_interface.ml, but was second to last in xenops_server.ml.

This meant that non-localhost migrations with verify_dest=true were classed as
localhost and did not balloon down before migration.

Fixes: 0911604 ("xenopsd: Don't balloon down memory on same-host migration")

Signed-off-by: Andrii Sultanov <[email protected]>
Copy link
Member

@robhoes robhoes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops.

@andyhhp
Copy link
Collaborator

andyhhp commented Jan 8, 2026

Not related to this change specifically, but ballooning a VM down for migrate is a terrible thing to be doing generally.

It's far more expensive than just sending a page which isn't in active use, and it prevents using host superpages on the destination.

Copy link
Member

@psafont psafont left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can these arguments become named?

@last-genius
Copy link
Contributor Author

Can these arguments become named?

We could create separate types for these arguments in a newtype fashion (so that they're not both bool, but LocalhostMigrationBool and VerifyDest bool, but I think this is more trouble than it's worth), but I don't think xapi-idl supports named arguments

@last-genius last-genius added this pull request to the merge queue Jan 8, 2026
Merged via the queue into xapi-project:master with commit 4e2ef11 Jan 8, 2026
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants