vm-migration: improve debuggability on receiver for failed migrations

We cannot reliably send Request::abandon() on every kind of failure on
the sender side, as we might be in the middle of a memory transmission.
The receiver would not reliably know what to do with that. So instead,
when the receiver cannot read from the socket, we log that the migration
sender failed, which is the only likely cause of that failure.

On-behalf-of: SAP philipp.schuster@sap.com
Signed-off-by: Philipp Schuster <philipp.schuster@cyberus-technology.de>

offload_daemon/src/main.rs

@@ -283,7 +283,7 @@ fn run_snapshot(socket_path: &Path, output_dir: &Path) -> Result<()> {
                 break;
             }
             Command::Abandon => {
-                // ACK before bailing so CH's ok_or_abandon read returns
+                // ACK before bailing so CH's ok_or_fatal_error() read returns
                 // cleanly instead of hitting EOF.
                 Response::ok().write_to(&mut stream).ok();
                 return Err(Error::Abandoned);

vm-migration/src/protocol.rs

@@ -363,10 +363,19 @@ impl Response {
         })
     }
 
-    /// Return the response if its status is `Ok`; return the caller-provided error for any other status.
+    /// Return the response if its status is `Ok`.
-    pub fn ok_or_error(self, sender_error: MigratableError) -> Result<Response, MigratableError> {
+    ///
+    /// Otherwise, returns an error and logs that the receiving VMM responded
+    /// with an error, which aborts the migration.
+    pub fn ok_or_fatal_error(
+        self,
+        sender_error: MigratableError,
+    ) -> Result<Response, MigratableError> {
         if self.status != Status::Ok {
             error!("Receiver reported error: aborting migration");
+            // `sender_error` identifies the sender-side operation that was in
+            // progress when the receiver reported failure; the receiver's
+            // actual error is unknown to the sender VMM.
             return Err(sender_error);
         }
         Ok(self)

vmm/src/lib.rs

@@ -30,7 +30,7 @@ use event_monitor::event;
 use hypervisor::arch::x86;
 use landlock::LandlockError;
 use libc::{EFD_NONBLOCK, SIGINT, SIGTERM, TCSANOW, tcsetattr, termios};
-use log::{debug, error, info, trace, warn};
+use log::{debug, error, info, warn};
 use memory_manager::MemoryManagerSnapshotData;
 use pci::PciBdf;
 use seccompiler::{BpfProgram, SeccompAction, apply_filter};
@@ -3040,9 +3040,20 @@ impl RequestHandler for Vmm {
         let mut state = ReceiveMigrationState::Established;
 
         while !state.finished() {
-            let req = Request::read_from(&mut socket)?;
+            let req = Request::read_from(&mut socket).inspect_err(|error| {
-            trace!("Command {:?} received", req.command());
+                if matches!(
+                    error,
+                    MigratableError::MigrateSocket(io_error)
+                        if io_error.kind() == io::ErrorKind::UnexpectedEof
+                ) {
+                    error!("Failed to read migration request: sender likely failed, aborting");
+                }
+            })?;
+            debug!("Command '{:?}' received", req.command());
 
+            // If sender-side migration causes any error propagated here, the
+            // next loop iteration logs a helpful error when reading the next
+            // request (which will fail as the sender closed the socket).
             let (response, new_state) = match self.vm_receive_migration_step(
                 &mut socket,
                 &listener,

vmm/src/migration/transport.rs

@@ -557,10 +557,9 @@ impl ReceiveAdditionalConnections {
                     // header. Each memory chunk is fully received and acked
                     // before the worker loops back to Request::read_from(), so
                     // EOF at this point means the sender finished sending
-                    // memory rather than dropping a chunk mid-transfer.
+                    // memory rather than dropping a chunk mid-transfer (happy
-                    debug!(
+                    // path) or the sender failed (error path).
-                        "Connection closed by peer as expected (sender finished sending memory)"
+                    debug!("Connection closed by peer");
-                    );
                     return Ok(());
                 }
                 Err(e) => return Err(e),
@@ -1115,7 +1114,9 @@ pub(crate) fn expect_ok_response(
     socket: &mut SocketStream,
     error: MigratableError,
 ) -> Result<(), MigratableError> {
-    Response::read_from(socket)?.ok_or_error(error).map(|_| ())
+    Response::read_from(socket)?
+        .ok_or_fatal_error(error)
+        .map(|_| ())
 }
 
 /// Send a request and validate that the peer responds with OK.

vmm/src/vm.rs

@@ -3026,9 +3026,9 @@ impl Vm {
                 .context("Error sending memory fd")
                 .map_err(MigratableError::MigrateSend)?;
 
-            Response::read_from(socket)?.ok_or_error(MigratableError::MigrateSend(anyhow!(
+            Response::read_from(socket)?.ok_or_fatal_error(MigratableError::MigrateSend(
-                "Error during memory fd migration"
+                anyhow!("Error during memory fd migration"),
-            )))?;
+            ))?;
         }
 
         Ok(())