Skip to content

Backport cancellation from upstream - #209

Open
phip1611 wants to merge 10 commits into
cyberus-technology:gardenlinuxfrom
phip1611:backport-gardenlinux-cancellation
Open

Backport cancellation from upstream#209
phip1611 wants to merge 10 commits into
cyberus-technology:gardenlinuxfrom
phip1611:backport-gardenlinux-cancellation

Conversation

@phip1611

@phip1611 phip1611 commented Sep 28, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

@phip1611 phip1611 self-assigned this Sep 28, 2026
@arctic-alpaca

arctic-alpaca commented Sep 29, 2026

Copy link
Copy Markdown

I'll wait with my review until the libvirt pipeline is green.

@phip1611

phip1611 commented Sep 29, 2026

Copy link
Copy Markdown
Member Author

Should be fixed now! Explanation is in the new commit's message.

@phip1611
phip1611 force-pushed the backport-gardenlinux-cancellation branch from cc13988 to c946fb2 Compare September 29, 2026 13:05
arctic-alpaca
arctic-alpaca reviewed Sep 29, 2026
View reviewed changes

@arctic-alpaca arctic-alpaca left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The commits refer to your fork instead of this repo or upstream. Some commits have a Assisted-by: Codex:GPT-6 trailer.

@phip1611

phip1611 commented Sep 29, 2026 •
edited
Loading

Copy link
Copy Markdown
Member Author

The commits refer to your fork instead of this repo or upstream.#

Ah, let me double check. I took the commits now from GitHub. I think I've looked at the wrong base when I did this in my IDE. Great catch!

Some commits have a Assisted-by: Codex:GPT-6 trailer.

What, I have no idea why. I did the cherry-pick by hand. A local review assisted by Codex must have added them 🫨 Thanks, great catch!

@phip1611
phip1611 force-pushed the backport-gardenlinux-cancellation branch from c946fb2 to 559d42f Compare September 29, 2026 17:19
@phip1611
phip1611 marked this pull request as draft September 29, 2026 20:28
@phip1611

phip1611 commented Sep 29, 2026

Copy link
Copy Markdown
Member Author

Drafted to double check if I cherry picked the right commits. Looks like I have cherry picked commits from some intermediate state

@arctic-alpaca

arctic-alpaca commented Sep 30, 2026

Copy link
Copy Markdown

The commits refer to your fork instead of this repo or upstream.

Just to clarify, the (cherry picked from commit 7e8d947) in the commit message links to https://github.com/phip1611/cloud-hypervisor instead of https://github.com/cloud-hypervisor/cloud-hypervisor.

phip1611 added 10 commits October 1, 2026 18:39
@phip1611
Revert "ch-remote: add cancel-migration"
68cc06f 
This reverts commit caf70eb.

On-behalf-of: SAP philipp.schuster@sap.com
Signed-off-by: Philipp Schuster <philipp.schuster@cyberus-technology.de>
@phip1611
Revert "vmm: migration cancellation: integrate into TCP threads"
f19f6cd 
This reverts commit c48d640

On-behalf-of: SAP philipp.schuster@sap.com
Signed-off-by: Philipp Schuster <philipp.schuster@cyberus-technology.de>
@phip1611
Revert "vmm: migration: early cancellation (add more checks)"
e717483 
This reverts commit e298b86

On-behalf-of: SAP philipp.schuster@sap.com
Signed-off-by: Philipp Schuster <philipp.schuster@cyberus-technology.de>
@phip1611
Revert "vmm: migration: actually support cancellation"
b999ec3 
This reverts commit f145ee6.

On-behalf-of: SAP philipp.schuster@sap.com
Signed-off-by: Philipp Schuster <philipp.schuster@cyberus-technology.de>
@phip1611
Revert "vmm: http api: add VmCancelMigration action"
0711e0f 
This reverts commit 5d08e56.

On-behalf-of: SAP philipp.schuster@sap.com
Signed-off-by: Philipp Schuster <philipp.schuster@cyberus-technology.de>
@phip1611
Revert "vmm: api: add VmCancelMigration action"
f51ce6b 
This reverts commit 93e9901.

On-behalf-of: SAP philipp.schuster@sap.com
Signed-off-by: Philipp Schuster <philipp.schuster@cyberus-technology.de>
@phip1611
vmm: migration: support cancellation
657e936 
Introduce the minimal functionality required to support canceling a live
migration. This establishes the basic mechanism and adds a
synchronization point to check whether the VM should be cancelled before
the migration is completed.

While the next commit will introduce the cancellation in the precopy
workers, this commit only uses a single check. Adding more checks is
disproportionate as everything except the memory migration takes fairly
little time.

(cherry picked from commit 85fa59c)

On-behalf-of: SAP philipp.schuster@sap.com
Signed-off-by: Philipp Schuster <philipp.schuster@cyberus-technology.de>
@phip1611
vmm: migration: cancel early when sending memory (precopy)
575c679 
This commit reduces the delay in stopping memory transmission during the
precopy phase when a migration is cancelled. The cancellation handling
is implemented in SendAdditionalConnections, which coordinates all
memory transmission threads.

(cherry picked from commit 66e96bd)

On-behalf-of: SAP philipp.schuster@sap.com
Signed-off-by: Philipp Schuster <philipp.schuster@cyberus-technology.de>
@phip1611
ch-remote: add cancel-migration
9737f0a 
(cherry picked from commit 7e8d947)

On-behalf-of: SAP philipp.schuster@sap.com
Signed-off-by: Philipp Schuster <philipp.schuster@cyberus-technology.de>
@phip1611
vmm: migration: abort receive on unexpected EOF while receiving memory
ec4f30d 
This cherry-pick is needed as the new approach doesn't send the
Request::abandon() anymore.

If the sender closes the connection while the receiver is reading
memory, e.g. after a canceled migration, read_volatile_from() returns
Ok(0). The receive loop treated this as a zero-progress read and spun
forever, leaving receive-migration (and thus the destination VMM's
API) hanging indefinitely.

Found by the migration cancellation integration tests.

On-behalf-of: SAP philipp.schuster@sap.com
Signed-off-by: Philipp Schuster <philipp.schuster@cyberus-technology.de>
@phip1611
phip1611 force-pushed the backport-gardenlinux-cancellation branch from 559d42f to ec4f30d Compare October 1, 2026 16:43
@phip1611
phip1611 marked this pull request as ready for review October 1, 2026 16:43
@phip1611

phip1611 commented Oct 1, 2026

Copy link
Copy Markdown
Member Author

I think it is fixed now.

@phip1611
phip1611 requested a review from arctic-alpaca October 1, 2026 17:24
@arctic-alpaca

arctic-alpaca commented Oct 2, 2026

Copy link
Copy Markdown

Interesting, depending on the view, GitHub links to a different repo for the commit in the (cherry picked from commit ...) message. In the commit view, it links to https://github.com/phip1611/cloud-hypervisor, when looking at the individual commit, it links to https://github.com/cyberus-technology/cloud-hypervisor. Slightly confusing GitHub...

arctic-alpaca
arctic-alpaca reviewed Oct 2, 2026
View reviewed changes

@arctic-alpaca arctic-alpaca left a comment •
edited
Loading

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I think this looks good, only the last commit is missing the hash of the commit the cherry pick is coming from.

Also when the runners are up again, could you start another libvirt run?

CI failure should be handled by #210

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@arctic-alpaca arctic-alpaca arctic-alpaca left review comments
@scholzp scholzp Awaiting requested review from scholzp

At least 2 approving reviews are required to merge this pull request.

Assignees

@phip1611 phip1611

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@phip1611 @arctic-alpaca