Abort upload after an out-of-order write attempt.

Signed-off-by: Alessandro Passaro <[email protected]>

mountpoint-s3/src/fs.rs

@@ -76,15 +76,10 @@ impl<Client: ObjectClient> UploadState<Client> {
             Ok(len) => Ok(len as u32),
             Err(e) => {
                 error!("write failed: {e}");
-                match e {
-                    UploadWriteError::PutRequestFailed(_) | UploadWriteError::ObjectTooBig { .. } => {
-                        // Abort the request.
-                        let ret: libc::c_int = e.into();
-                        *self = Self::Failed(ret);
-                        Err(ret)
-                    }
-                    UploadWriteError::OutOfOrderWrite { .. } => Err(e.into()),
-                }
+                // Abort the request.
+                let ret: libc::c_int = e.into();
+                *self = Self::Failed(ret);
+                Err(ret)
             }
         }
     }

mountpoint-s3/tests/fs.rs

@@ -350,8 +350,10 @@ async fn test_sequential_write(write_size: usize) {
     fs.release(file_ino, fh, 0, None, true).await.unwrap();
 }
 
+#[test_case(-27; "earlier offset")]
+#[test_case(28; "later offset")]
 #[tokio::test]
-async fn test_unordered_write_fails() {
+async fn test_unordered_write_fails(offset: i64) {
     const BUCKET_NAME: &str = "test_unordered_write_fails";
 
     let (_client, fs) = make_test_filesystem(BUCKET_NAME, &Default::default(), Default::default());
@@ -374,15 +376,15 @@ async fn test_unordered_write_fails() {
     assert_eq!(written, 27);
 
     let err = fs
-        .write(file_ino, fh, 0, &[0xaa; 27], 0, 0, None)
+        .write(file_ino, fh, written as i64 + offset, &[0xaa; 27], 0, 0, None)
         .await
-        .expect_err("writes to earlier offsets should fail");
+        .expect_err("writes to out-of-order offsets should fail");
     assert_eq!(err, libc::EINVAL);
 
     let err = fs
-        .write(file_ino, fh, 55, &[0xaa; 27], 0, 0, None)
+        .write(file_ino, fh, written as i64, &[0xaa; 27], 0, 0, None)
         .await
-        .expect_err("writes to later offsets should fail");
+        .expect_err("any write after an error should fail");
     assert_eq!(err, libc::EINVAL);
 }
 

mountpoint-s3/tests/fuse_tests/write_test.rs

@@ -329,7 +329,7 @@ fn write_too_big_test_mock(write_size: usize) {
     write_too_big_test(crate::fuse_tests::mock_session::new, write_size);
 }
 
-fn out_of_order_write_test<F>(creator_fn: F)
+fn out_of_order_write_test<F>(creator_fn: F, offset: i64)
 where
     F: FnOnce(&str, TestSessionConfig) -> (TempDir, BackgroundSession, TestClientBox),
 {
@@ -350,25 +350,18 @@ where
     let mut body = vec![0u8; OBJECT_SIZE];
     rng.fill(&mut body[..]);
 
-    let part1 = &body[..(OBJECT_SIZE / 2)];
-    let part2 = &body[(OBJECT_SIZE / 2)..];
-
-    f.write_all(part1).unwrap();
+    f.write_all(&body).unwrap();
 
-    // Attempt to seek to start and write over.
-    f.seek(std::io::SeekFrom::Start(0)).unwrap();
-    let err = f.write_all(part2).expect_err("out of order write should fail");
+    // Attempt to write out-of-order.
+    f.seek(std::io::SeekFrom::Current(offset)).unwrap();
+    let err = f.write_all(&body).expect_err("out of order write should fail");
     assert_eq!(err.raw_os_error(), Some(libc::EINVAL));
 
-    // Attempt to seek beyond end and write.
-    f.seek(std::io::SeekFrom::Start((part1.len() + 1) as u64)).unwrap();
-    let err = f.write_all(part2).expect_err("out of order write should fail");
+    // Seek where we left off and attempt to write.
+    f.seek(std::io::SeekFrom::Start((OBJECT_SIZE) as u64)).unwrap();
+    let err = f.write_all(&body).expect_err("writes after an error should fail");
     assert_eq!(err.raw_os_error(), Some(libc::EINVAL));
 
-    // Seek where we left off and write sequentially.
-    f.seek(std::io::SeekFrom::Start((part1.len()) as u64)).unwrap();
-    f.write_all(part2).unwrap();
-
     drop(f);
 
     // The kernel doesn't guarantee to flush the data as soon as the file is closed. Currently,
@@ -378,20 +371,19 @@ where
     // during writes
     std::thread::sleep(Duration::from_secs(5));
 
-    let m = metadata(&path).unwrap();
-    assert_eq!(m.len(), body.len() as u64);
-
-    let buf = read(&path).unwrap();
-    assert_eq!(&buf[..], &body[..]);
+    let err = metadata(&path).expect_err("upload shouldn't have succeeded");
+    assert_eq!(err.raw_os_error(), Some(libc::ENOENT));
 }
 
 #[cfg(feature = "s3_tests")]
-#[test]
-fn out_of_order_write_test_s3() {
-    out_of_order_write_test(crate::fuse_tests::s3_session::new);
+#[test_case(-1; "earlier offset")]
+#[test_case(1; "later offset")]
+fn out_of_order_write_test_s3(offset: i64) {
+    out_of_order_write_test(crate::fuse_tests::s3_session::new, offset);
 }
 
-#[test]
-fn out_of_order_write_test_mock() {
-    out_of_order_write_test(crate::fuse_tests::mock_session::new);
+#[test_case(-1; "earlier offset")]
+#[test_case(1; "later offset")]
+fn out_of_order_write_test_mock(offset: i64) {
+    out_of_order_write_test(crate::fuse_tests::mock_session::new, offset);
 }