[live-migration] Renegotiate GCS version on restore and drop Plan9 from HCS Document - #2845
[live-migration] Renegotiate GCS version on restore and drop Plan9 from HCS Document#2845rawahars wants to merge 3 commits into
Conversation
After a migration blackout the guest re-dials and resets its GCS protocol version, so version-gated RPCs (e.g. exec) failed on source resume. Renegotiate via connect() in ResumeOnConn before adopting the new connection. - GuestConnection.ResumeOnConn now takes a ctx and re-runs the version handshake after swapping the bridge transport. - Fix PrepareConnection idempotency to key only on the armed listener (gcListener) so the log/GCS listener can be re-armed on resume even when a GuestConnection already exists. - Clarify resume comments and error messages. Signed-off-by: Harsh Rawat <harshrawat@microsoft.com>
| } | ||
|
|
||
| switch { | ||
| case rebuildBridge: |
There was a problem hiding this comment.
Unrelated to this change, but it is harder to read this switch case. Can you re-write it as simple if-else block?
There was a problem hiding this comment.
Addressed in the latest commit
Signed-off-by: Harsh Rawat <harshrawat@microsoft.com>
Signed-off-by: Harsh Rawat <harshrawat@microsoft.com>
rawahars
changed the title
| func (gc *GuestConnection) ResumeOnConn(ctx context.Context, conn io.ReadWriteCloser) error { | ||
| if gc.brdg == nil { | ||
| // Not adopting conn; close it so the accepted socket does not leak. | ||
| _ = conn.Close() | ||
| return ErrBridgeClosed | ||
| } | ||
| return gc.brdg.ResumeOnConn(conn) |
There was a problem hiding this comment.
connect's doc comment still says "It must not be called more than once"
Also just a question is there nothing that issues container RPCs while a source rollback resume is in progress/inflight ? I see container RPCs go sright to c.gc.brdg.RPC and never take the guest mutex, this function holding that lock doesn't cover the swap and negotiate landing
| } else { | ||
| // Destination: reuse the connection already armed at start. | ||
| if err := c.guest.CreateConnection(ctx, false); err != nil { | ||
| return fmt.Errorf("resume guest connection: %w", err) |
There was a problem hiding this comment.
Nit: both branches still wrap with "resume guest connection", so source and destination are indistinguishable in logs. You retitled the prepare message, worth splitting these too.
shreyanshjain7174
left a comment
shreyanshjain7174
left a comment
There was a problem hiding this comment.
Nothing in CI exercises LM, so a unit test is the only thing that catches a regression here. guestconnection_test.go's simpleGcs already answers RPCNegotiateProtocol: set migrating, call ResumeOnConn with a second fake pipe, assert the second negotiate arrived. Worth adding?
|
Everything else looks good! |
3 participants
After a migration blackout the guest re-dials and resets its GCS protocol version, so version-gated RPCs (e.g. exec) failed on source resume. Renegotiate via connect() in ResumeOnConn before adopting the new connection.
An empty Plan9 element makes HCS attach a file-sharing device that becomes part of the VM's saved device state and fails the save/restore path, breaking migration even when nothing is shared. Omit Plan9 for live-migratable sandboxes.