From 13e9c100c5c42fa36321fdb354de596c503ab510 Mon Sep 17 00:00:00 2001 From: Kevin Wang Date: Sun, 20 Sep 2026 00:06:44 -0700 Subject: [PATCH 1/2] fix(supervisor): reopen the log only when it is actually rotated The logrotate watcher matched `is_remove() || is_modify()`, and on Linux `recommended_watcher` is inotify, where `is_modify()` also matches `IN_MODIFY` -- which is what the supervisor's own `write_all` generates. So a chatty child paid a synchronous `fsync` plus a reopen per read buffer, on its own output, with nothing rotating anything. Measured with `strace -e trace=fsync,openat` against the real binary, 8 MiB of child output through one supervised process: before: 1145 fsync, 1146 opens of a log nothing had rotated after: 0 fsync, 1 open Match the events that mean the path stopped naming the file we hold: a remove, or a rename. inotify reports `IN_MOVED_FROM` as `Modify(Name(_))` rather than as a remove, so `logrotate(8)`'s default mode is still followed. A truncate-in-place rotation -- what `dstack-vmm`'s own `logrotate` module does, and `logrotate(8)`'s `copytruncate` -- needs no reopen: the file is opened `O_APPEND`, so the next write lands at its new end. That is the requirement the vmm module documents, and the "reopens them when they change" half of its comment was never what made it hold. --- dstack/Cargo.lock | 1 + dstack/supervisor/Cargo.toml | 3 + dstack/supervisor/src/process.rs | 226 ++++++++++++++++++++++++++++++- dstack/vmm/src/app.rs | 6 +- 4 files changed, 231 insertions(+), 5 deletions(-) diff --git a/dstack/Cargo.lock b/dstack/Cargo.lock index 96b7a81ad..564eca86c 100644 --- a/dstack/Cargo.lock +++ b/dstack/Cargo.lock @@ -7545,6 +7545,7 @@ dependencies = [ "rocket", "serde", "serde_json", + "tempfile", "tokio", "tracing", "tracing-subscriber", diff --git a/dstack/supervisor/Cargo.toml b/dstack/supervisor/Cargo.toml index 3d7b3439d..d7fc97840 100644 --- a/dstack/supervisor/Cargo.toml +++ b/dstack/supervisor/Cargo.toml @@ -34,3 +34,6 @@ tokio = { workspace = true, features = [ ] } tracing.workspace = true tracing-subscriber.workspace = true + +[dev-dependencies] +tempfile.workspace = true diff --git a/dstack/supervisor/src/process.rs b/dstack/supervisor/src/process.rs index 94e0c61e5..cd3f21194 100644 --- a/dstack/supervisor/src/process.rs +++ b/dstack/supervisor/src/process.rs @@ -376,6 +376,38 @@ async fn redirect(mut input: impl AsyncRead + Unpin, to: String) { } } +/// How many times a redirect target has been opened, counted for the tests +/// below. A reopen leaves nothing behind in the file it reopens, so counting is +/// the only way to see the thing that was wrong here: not what was written, but +/// how often the writing stopped to `fsync` and start over. +#[cfg(test)] +static LOG_OPENS: std::sync::atomic::AtomicUsize = std::sync::atomic::AtomicUsize::new(0); + +/// Whether a watcher event means the log path stopped naming the file this +/// task holds open, so the next write has to go to a new one. +/// +/// Deliberately not `is_modify()`. On Linux `recommended_watcher` is inotify, +/// where `is_modify()` also matches `IN_MODIFY` -- which is exactly what the +/// `write_all` below generates, on its own output. Streaming 8 MiB from a +/// supervised child through the previous condition cost 1145 `fsync` calls and +/// 1146 reopens of a log nothing had rotated, one per read buffer, each one a +/// synchronous flush on the path a chatty child writes fastest. +/// +/// A rename is `IN_MOVED_FROM`, which inotify reports as `Modify(Name(_))` and +/// not as a remove, so `logrotate(8)` in its default mode still gets a reopen. +/// A truncate-in-place rotation -- what `dstack-vmm`'s own `logrotate` module +/// does, and what `logrotate(8)`'s `copytruncate` does -- needs no reopen at +/// all: the file is opened with `O_APPEND`, so every write goes to the current +/// end of it whether or not it was just emptied. That is the requirement the +/// vmm module's documentation states, and it is what makes this safe to drop. +fn is_rotation(kind: ¬ify::EventKind) -> bool { + use notify::event::{EventKind, ModifyKind}; + matches!( + kind, + EventKind::Remove(_) | EventKind::Modify(ModifyKind::Name(_)) + ) +} + async fn try_redirect(input: &mut (impl AsyncRead + Unpin), to: String) -> Result<()> { let dst_path = Path::new(&to); let dst_path_buf = dst_path.to_path_buf(); @@ -386,9 +418,7 @@ async fn try_redirect(input: &mut (impl AsyncRead + Unpin), to: String) -> Resul notify::recommended_watcher(move |res: Result| { if let Ok(event) = res { // Check if the event affects our specific file - if (event.kind.is_remove() || event.kind.is_modify()) - && event.paths.iter().any(|p| p == &dst_path_buf) - { + if is_rotation(&event.kind) && event.paths.iter().any(|p| p == &dst_path_buf) { let _ = reopen_tx.blocking_send(()); } } @@ -406,6 +436,8 @@ async fn try_redirect(input: &mut (impl AsyncRead + Unpin), to: String) -> Resul .create(true) .append(true) .open(dst_path)?; + #[cfg(test)] + LOG_OPENS.fetch_add(1, std::sync::atomic::Ordering::SeqCst); loop { tokio::select! { @@ -438,3 +470,191 @@ async fn try_redirect(input: &mut (impl AsyncRead + Unpin), to: String) -> Resul } } } + +#[cfg(test)] +mod log_rotation_tests { + use super::*; + use std::sync::atomic::Ordering; + use std::time::Duration; + + /// [`LOG_OPENS`] is process-wide, so the tests that read it take turns. + static SERIAL: Mutex<()> = Mutex::new(()); + + struct Redirect { + _serial: std::sync::MutexGuard<'static, ()>, + opens_at_start: usize, + } + + impl Redirect { + fn start() -> Self { + let serial = SERIAL.lock().unwrap_or_else(|err| err.into_inner()); + Self { + opens_at_start: LOG_OPENS.load(Ordering::SeqCst), + _serial: serial, + } + } + + fn opens(&self) -> usize { + LOG_OPENS.load(Ordering::SeqCst) - self.opens_at_start + } + } + + /// Wait for `condition`, or give the assertion after it something to say. + async fn settle(condition: impl Fn() -> bool) { + for _ in 0..400 { + if condition() { + return; + } + tokio::time::sleep(Duration::from_millis(10)).await; + } + } + + fn size(path: &Path) -> u64 { + fs::metadata(path).map(|meta| meta.len()).unwrap_or(0) + } + + /// A child's own output must not look like a rotation. + /// + /// inotify reports every `write_all` below as `IN_MODIFY` on the log path, + /// which the watcher used to accept, so each read buffer cost an `fsync` + /// and a reopen. Measured on the real binary before this changed: 8 MiB of + /// child output produced 1145 `fsync` calls and 1146 opens of a log nothing + /// had rotated. + #[tokio::test] + async fn streaming_output_does_not_reopen_the_log() { + let counter = Redirect::start(); + let dir = tempfile::tempdir().unwrap(); + let log = dir.path().join("stdout.log"); + + let (mut writer, mut reader) = tokio::io::duplex(64 * 1024); + let path = log.to_string_lossy().into_owned(); + let redirect = tokio::spawn(async move { try_redirect(&mut reader, path).await }); + + let chunk = vec![b'x'; 8192]; + for _ in 0..128 { + tokio::io::AsyncWriteExt::write_all(&mut writer, &chunk) + .await + .unwrap(); + } + drop(writer); + redirect.await.unwrap().unwrap(); + + assert_eq!(size(&log), 128 * 8192); + assert_eq!( + counter.opens(), + 1, + "1 MiB of output reopened the log {} times", + counter.opens() + ); + } + + /// A rotation that renames the log away still has to be followed. + /// + /// inotify reports the rename as `IN_MOVED_FROM`, which `notify` maps to + /// `Modify(Name(_))` rather than to a remove -- so dropping `is_modify()` + /// wholesale would have left `logrotate(8)`'s default mode writing into an + /// unlinked inode forever. + #[tokio::test] + async fn a_renamed_log_is_reopened() { + let counter = Redirect::start(); + let dir = tempfile::tempdir().unwrap(); + let log = dir.path().join("stdout.log"); + let rotated = dir.path().join("stdout.log.1"); + + let (mut writer, mut reader) = tokio::io::duplex(64 * 1024); + let path = log.to_string_lossy().into_owned(); + let redirect = tokio::spawn(async move { try_redirect(&mut reader, path).await }); + + tokio::io::AsyncWriteExt::write_all(&mut writer, b"before\n") + .await + .unwrap(); + settle(|| size(&log) == 7).await; + + fs::rename(&log, &rotated).unwrap(); + settle(|| log.exists()).await; + assert!(log.exists(), "the log was never reopened after the rename"); + + tokio::io::AsyncWriteExt::write_all(&mut writer, b"after\n") + .await + .unwrap(); + drop(writer); + redirect.await.unwrap().unwrap(); + + assert_eq!(fs::read_to_string(&rotated).unwrap(), "before\n"); + assert_eq!(fs::read_to_string(&log).unwrap(), "after\n"); + // One rename reaches the watcher twice: `notify` reports `IN_MOVED_FROM` + // as `Modify(Name(From))` and then synthesizes a `Modify(Name(Both))` + // carrying the old path as well. Both name this log, so a rotation + // costs a reopen more than it strictly needs. Bounded by how often a + // log is rotated, which is the point. + assert!( + counter.opens() >= 2, + "the rename was never followed: {} opens", + counter.opens() + ); + } + + /// The rotation dstack actually performs needs no reopen. + /// + /// `dstack-vmm`'s `logrotate` module copies the log and then truncates it + /// in place, precisely so a live writer's fd stays valid; `logrotate(8)`'s + /// `copytruncate` does the same. The file is opened with `O_APPEND`, so the + /// next write lands at the new end of it. Nothing to follow, and the + /// truncation is an `IN_MODIFY` this must not react to -- reacting is what + /// made a chatty child cost an `fsync` per buffer. + #[tokio::test] + async fn a_truncated_log_keeps_appending_without_a_reopen() { + let counter = Redirect::start(); + let dir = tempfile::tempdir().unwrap(); + let log = dir.path().join("stdout.log"); + + let (mut writer, mut reader) = tokio::io::duplex(64 * 1024); + let path = log.to_string_lossy().into_owned(); + let redirect = tokio::spawn(async move { try_redirect(&mut reader, path).await }); + + tokio::io::AsyncWriteExt::write_all(&mut writer, b"before\n") + .await + .unwrap(); + settle(|| size(&log) == 7).await; + + fs::write(&log, b"").unwrap(); + tokio::io::AsyncWriteExt::write_all(&mut writer, b"after\n") + .await + .unwrap(); + drop(writer); + redirect.await.unwrap().unwrap(); + + // No sparse hole: `O_APPEND` put the write at the current end of file, + // not at the offset the writer had reached before the truncation. + assert_eq!(fs::read_to_string(&log).unwrap(), "after\n"); + assert_eq!(counter.opens(), 1, "a truncation is not a rotation"); + } + + #[test] + fn only_a_removal_or_a_rename_counts_as_a_rotation() { + use notify::event::{ + AccessKind, CreateKind, DataChange, EventKind, MetadataKind, ModifyKind, RemoveKind, + RenameMode, + }; + + for kind in [ + EventKind::Remove(RemoveKind::File), + EventKind::Remove(RemoveKind::Any), + EventKind::Modify(ModifyKind::Name(RenameMode::From)), + EventKind::Modify(ModifyKind::Name(RenameMode::Any)), + ] { + assert!(is_rotation(&kind), "{kind:?} is a rotation"); + } + for kind in [ + // What every `write_all` in `try_redirect` produces. + EventKind::Modify(ModifyKind::Data(DataChange::Any)), + EventKind::Modify(ModifyKind::Data(DataChange::Content)), + EventKind::Modify(ModifyKind::Metadata(MetadataKind::Any)), + EventKind::Modify(ModifyKind::Any), + EventKind::Create(CreateKind::File), + EventKind::Access(AccessKind::Any), + ] { + assert!(!is_rotation(&kind), "{kind:?} is not a rotation"); + } + } +} diff --git a/dstack/vmm/src/app.rs b/dstack/vmm/src/app.rs index df3e99281..bbacf4c42 100644 --- a/dstack/vmm/src/app.rs +++ b/dstack/vmm/src/app.rs @@ -1889,8 +1889,10 @@ fn append_boot_separator(path: &std::path::Path) { /// Logs a CVM writes into its work directory, subject to retention. /// /// stdout and stderr are written by the supervisor, which always opens them -/// with `append(true)` and reopens them when they change, so they satisfy -/// [`crate::logrotate`]'s contract no matter which VMM launched the VM. +/// with `append(true)`, so they satisfy [`crate::logrotate`]'s contract no +/// matter which VMM launched the VM. That is the whole requirement: the +/// truncation below lands at the writer's next append either way, and the +/// supervisor reopens only when the path stops naming the file it holds. /// serial.log is written by QEMU, whose fd only appends when *we* passed /// `logappend=on`, so it is included only when `serial` says so. fn rotatable_logs(work_dir: &VmWorkDir, serial: bool) -> Vec { From 167f7d0fa43ba83ccb13628f433075ad7571f711 Mon Sep 17 00:00:00 2001 From: Kevin Wang Date: Thu, 24 Sep 2026 01:58:02 -0700 Subject: [PATCH 2/2] refactor(supervisor): trim log rotation comments and tests --- dstack/Cargo.lock | 1 - dstack/supervisor/Cargo.toml | 3 - dstack/supervisor/src/process.rs | 219 ++----------------------------- dstack/vmm/src/app.rs | 4 +- 4 files changed, 13 insertions(+), 214 deletions(-) diff --git a/dstack/Cargo.lock b/dstack/Cargo.lock index 564eca86c..96b7a81ad 100644 --- a/dstack/Cargo.lock +++ b/dstack/Cargo.lock @@ -7545,7 +7545,6 @@ dependencies = [ "rocket", "serde", "serde_json", - "tempfile", "tokio", "tracing", "tracing-subscriber", diff --git a/dstack/supervisor/Cargo.toml b/dstack/supervisor/Cargo.toml index d7fc97840..3d7b3439d 100644 --- a/dstack/supervisor/Cargo.toml +++ b/dstack/supervisor/Cargo.toml @@ -34,6 +34,3 @@ tokio = { workspace = true, features = [ ] } tracing.workspace = true tracing-subscriber.workspace = true - -[dev-dependencies] -tempfile.workspace = true diff --git a/dstack/supervisor/src/process.rs b/dstack/supervisor/src/process.rs index cd3f21194..f80272fbc 100644 --- a/dstack/supervisor/src/process.rs +++ b/dstack/supervisor/src/process.rs @@ -376,30 +376,8 @@ async fn redirect(mut input: impl AsyncRead + Unpin, to: String) { } } -/// How many times a redirect target has been opened, counted for the tests -/// below. A reopen leaves nothing behind in the file it reopens, so counting is -/// the only way to see the thing that was wrong here: not what was written, but -/// how often the writing stopped to `fsync` and start over. -#[cfg(test)] -static LOG_OPENS: std::sync::atomic::AtomicUsize = std::sync::atomic::AtomicUsize::new(0); - -/// Whether a watcher event means the log path stopped naming the file this -/// task holds open, so the next write has to go to a new one. -/// -/// Deliberately not `is_modify()`. On Linux `recommended_watcher` is inotify, -/// where `is_modify()` also matches `IN_MODIFY` -- which is exactly what the -/// `write_all` below generates, on its own output. Streaming 8 MiB from a -/// supervised child through the previous condition cost 1145 `fsync` calls and -/// 1146 reopens of a log nothing had rotated, one per read buffer, each one a -/// synchronous flush on the path a chatty child writes fastest. -/// -/// A rename is `IN_MOVED_FROM`, which inotify reports as `Modify(Name(_))` and -/// not as a remove, so `logrotate(8)` in its default mode still gets a reopen. -/// A truncate-in-place rotation -- what `dstack-vmm`'s own `logrotate` module -/// does, and what `logrotate(8)`'s `copytruncate` does -- needs no reopen at -/// all: the file is opened with `O_APPEND`, so every write goes to the current -/// end of it whether or not it was just emptied. That is the requirement the -/// vmm module's documentation states, and it is what makes this safe to drop. +/// inotify reports our own `write_all` as `Modify(Data)`, so only a removal or a +/// rename (logrotate's default mode) means the path no longer names our file. fn is_rotation(kind: ¬ify::EventKind) -> bool { use notify::event::{EventKind, ModifyKind}; matches!( @@ -436,8 +414,6 @@ async fn try_redirect(input: &mut (impl AsyncRead + Unpin), to: String) -> Resul .create(true) .append(true) .open(dst_path)?; - #[cfg(test)] - LOG_OPENS.fetch_add(1, std::sync::atomic::Ordering::SeqCst); loop { tokio::select! { @@ -473,188 +449,17 @@ async fn try_redirect(input: &mut (impl AsyncRead + Unpin), to: String) -> Resul #[cfg(test)] mod log_rotation_tests { - use super::*; - use std::sync::atomic::Ordering; - use std::time::Duration; - - /// [`LOG_OPENS`] is process-wide, so the tests that read it take turns. - static SERIAL: Mutex<()> = Mutex::new(()); - - struct Redirect { - _serial: std::sync::MutexGuard<'static, ()>, - opens_at_start: usize, - } - - impl Redirect { - fn start() -> Self { - let serial = SERIAL.lock().unwrap_or_else(|err| err.into_inner()); - Self { - opens_at_start: LOG_OPENS.load(Ordering::SeqCst), - _serial: serial, - } - } - - fn opens(&self) -> usize { - LOG_OPENS.load(Ordering::SeqCst) - self.opens_at_start - } - } - - /// Wait for `condition`, or give the assertion after it something to say. - async fn settle(condition: impl Fn() -> bool) { - for _ in 0..400 { - if condition() { - return; - } - tokio::time::sleep(Duration::from_millis(10)).await; - } - } - - fn size(path: &Path) -> u64 { - fs::metadata(path).map(|meta| meta.len()).unwrap_or(0) - } - - /// A child's own output must not look like a rotation. - /// - /// inotify reports every `write_all` below as `IN_MODIFY` on the log path, - /// which the watcher used to accept, so each read buffer cost an `fsync` - /// and a reopen. Measured on the real binary before this changed: 8 MiB of - /// child output produced 1145 `fsync` calls and 1146 opens of a log nothing - /// had rotated. - #[tokio::test] - async fn streaming_output_does_not_reopen_the_log() { - let counter = Redirect::start(); - let dir = tempfile::tempdir().unwrap(); - let log = dir.path().join("stdout.log"); - - let (mut writer, mut reader) = tokio::io::duplex(64 * 1024); - let path = log.to_string_lossy().into_owned(); - let redirect = tokio::spawn(async move { try_redirect(&mut reader, path).await }); - - let chunk = vec![b'x'; 8192]; - for _ in 0..128 { - tokio::io::AsyncWriteExt::write_all(&mut writer, &chunk) - .await - .unwrap(); - } - drop(writer); - redirect.await.unwrap().unwrap(); - - assert_eq!(size(&log), 128 * 8192); - assert_eq!( - counter.opens(), - 1, - "1 MiB of output reopened the log {} times", - counter.opens() - ); - } - - /// A rotation that renames the log away still has to be followed. - /// - /// inotify reports the rename as `IN_MOVED_FROM`, which `notify` maps to - /// `Modify(Name(_))` rather than to a remove -- so dropping `is_modify()` - /// wholesale would have left `logrotate(8)`'s default mode writing into an - /// unlinked inode forever. - #[tokio::test] - async fn a_renamed_log_is_reopened() { - let counter = Redirect::start(); - let dir = tempfile::tempdir().unwrap(); - let log = dir.path().join("stdout.log"); - let rotated = dir.path().join("stdout.log.1"); - - let (mut writer, mut reader) = tokio::io::duplex(64 * 1024); - let path = log.to_string_lossy().into_owned(); - let redirect = tokio::spawn(async move { try_redirect(&mut reader, path).await }); - - tokio::io::AsyncWriteExt::write_all(&mut writer, b"before\n") - .await - .unwrap(); - settle(|| size(&log) == 7).await; - - fs::rename(&log, &rotated).unwrap(); - settle(|| log.exists()).await; - assert!(log.exists(), "the log was never reopened after the rename"); - - tokio::io::AsyncWriteExt::write_all(&mut writer, b"after\n") - .await - .unwrap(); - drop(writer); - redirect.await.unwrap().unwrap(); - - assert_eq!(fs::read_to_string(&rotated).unwrap(), "before\n"); - assert_eq!(fs::read_to_string(&log).unwrap(), "after\n"); - // One rename reaches the watcher twice: `notify` reports `IN_MOVED_FROM` - // as `Modify(Name(From))` and then synthesizes a `Modify(Name(Both))` - // carrying the old path as well. Both name this log, so a rotation - // costs a reopen more than it strictly needs. Bounded by how often a - // log is rotated, which is the point. - assert!( - counter.opens() >= 2, - "the rename was never followed: {} opens", - counter.opens() - ); - } - - /// The rotation dstack actually performs needs no reopen. - /// - /// `dstack-vmm`'s `logrotate` module copies the log and then truncates it - /// in place, precisely so a live writer's fd stays valid; `logrotate(8)`'s - /// `copytruncate` does the same. The file is opened with `O_APPEND`, so the - /// next write lands at the new end of it. Nothing to follow, and the - /// truncation is an `IN_MODIFY` this must not react to -- reacting is what - /// made a chatty child cost an `fsync` per buffer. - #[tokio::test] - async fn a_truncated_log_keeps_appending_without_a_reopen() { - let counter = Redirect::start(); - let dir = tempfile::tempdir().unwrap(); - let log = dir.path().join("stdout.log"); - - let (mut writer, mut reader) = tokio::io::duplex(64 * 1024); - let path = log.to_string_lossy().into_owned(); - let redirect = tokio::spawn(async move { try_redirect(&mut reader, path).await }); - - tokio::io::AsyncWriteExt::write_all(&mut writer, b"before\n") - .await - .unwrap(); - settle(|| size(&log) == 7).await; - - fs::write(&log, b"").unwrap(); - tokio::io::AsyncWriteExt::write_all(&mut writer, b"after\n") - .await - .unwrap(); - drop(writer); - redirect.await.unwrap().unwrap(); - - // No sparse hole: `O_APPEND` put the write at the current end of file, - // not at the offset the writer had reached before the truncation. - assert_eq!(fs::read_to_string(&log).unwrap(), "after\n"); - assert_eq!(counter.opens(), 1, "a truncation is not a rotation"); - } + use super::is_rotation; + use notify::event::{DataChange, EventKind, ModifyKind, RemoveKind, RenameMode}; #[test] - fn only_a_removal_or_a_rename_counts_as_a_rotation() { - use notify::event::{ - AccessKind, CreateKind, DataChange, EventKind, MetadataKind, ModifyKind, RemoveKind, - RenameMode, - }; - - for kind in [ - EventKind::Remove(RemoveKind::File), - EventKind::Remove(RemoveKind::Any), - EventKind::Modify(ModifyKind::Name(RenameMode::From)), - EventKind::Modify(ModifyKind::Name(RenameMode::Any)), - ] { - assert!(is_rotation(&kind), "{kind:?} is a rotation"); - } - for kind in [ - // What every `write_all` in `try_redirect` produces. - EventKind::Modify(ModifyKind::Data(DataChange::Any)), - EventKind::Modify(ModifyKind::Data(DataChange::Content)), - EventKind::Modify(ModifyKind::Metadata(MetadataKind::Any)), - EventKind::Modify(ModifyKind::Any), - EventKind::Create(CreateKind::File), - EventKind::Access(AccessKind::Any), - ] { - assert!(!is_rotation(&kind), "{kind:?} is not a rotation"); - } + fn only_a_removal_or_a_rename_is_a_rotation() { + assert!(is_rotation(&EventKind::Remove(RemoveKind::File))); + assert!(is_rotation(&EventKind::Modify(ModifyKind::Name( + RenameMode::From + )))); + assert!(!is_rotation(&EventKind::Modify(ModifyKind::Data( + DataChange::Any + )))); } } diff --git a/dstack/vmm/src/app.rs b/dstack/vmm/src/app.rs index bbacf4c42..a1c8cab41 100644 --- a/dstack/vmm/src/app.rs +++ b/dstack/vmm/src/app.rs @@ -1890,9 +1890,7 @@ fn append_boot_separator(path: &std::path::Path) { /// /// stdout and stderr are written by the supervisor, which always opens them /// with `append(true)`, so they satisfy [`crate::logrotate`]'s contract no -/// matter which VMM launched the VM. That is the whole requirement: the -/// truncation below lands at the writer's next append either way, and the -/// supervisor reopens only when the path stops naming the file it holds. +/// matter which VMM launched the VM. /// serial.log is written by QEMU, whose fd only appends when *we* passed /// `logappend=on`, so it is included only when `serial` says so. fn rotatable_logs(work_dir: &VmWorkDir, serial: bool) -> Vec {