Skip to content

Commit 284d796

Browse files
committed
efi: update the ESP by creating a tmpdir and RENAME_EXCHANGE
See Timothée's comment coreos#454 (comment) logic is like this: - `cp -a fedora fedora.tmp` - We start with a copy to make sure to keep all other files that we do not explicitly track in bootupd - Update the content of `fedora.tmp` with the new binaries - Exchange `fedora.tmp` -> `fedora` - Remove now "old" `fedora.tmp` If we have a file not in a directory in `EFI`, then we can copy it to `foo.tmp` and then act on it and finally rename it. No need to copy the entire `EFI`. Fixes coreos#454
1 parent 8a472ea commit 284d796

File tree

3 files changed

+223
-30
lines changed

3 files changed

+223
-30
lines changed

Cargo.lock

+7
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

+1
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ path = "src/main.rs"
2121
[dependencies]
2222
anyhow = "1.0"
2323
bincode = "1.3.2"
24+
camino = "1.1.7"
2425
chrono = { version = "0.4.38", features = ["serde"] }
2526
clap = { version = "3.2", default-features = false, features = ["cargo", "derive", "std", "suggestions"] }
2627
env_logger = "0.10"

src/filetree.rs

+215-30
Original file line numberDiff line numberDiff line change
@@ -7,17 +7,20 @@
77
#[cfg(any(target_arch = "x86_64", target_arch = "aarch64"))]
88
use anyhow::{bail, Context, Result};
99
#[cfg(any(target_arch = "x86_64", target_arch = "aarch64"))]
10+
use camino::{Utf8Path, Utf8PathBuf};
11+
#[cfg(any(target_arch = "x86_64", target_arch = "aarch64"))]
1012
use openat_ext::OpenatDirExt;
1113
#[cfg(any(target_arch = "x86_64", target_arch = "aarch64"))]
1214
use openssl::hash::{Hasher, MessageDigest};
15+
use rustix::fd::BorrowedFd;
1316
use serde::{Deserialize, Serialize};
1417
#[allow(unused_imports)]
1518
use std::collections::{BTreeMap, HashMap, HashSet};
1619
use std::fmt::Display;
1720
#[cfg(any(target_arch = "x86_64", target_arch = "aarch64"))]
1821
use std::os::unix::io::AsRawFd;
19-
#[cfg(any(target_arch = "x86_64", target_arch = "aarch64"))]
20-
use std::path::Path;
22+
use std::os::unix::process::CommandExt;
23+
use std::process::Command;
2124

2225
/// The prefix we apply to our temporary files.
2326
#[cfg(any(target_arch = "x86_64", target_arch = "aarch64"))]
@@ -272,22 +275,50 @@ pub(crate) struct ApplyUpdateOptions {
272275
// Let's just fork off a helper process for now.
273276
#[cfg(any(target_arch = "x86_64", target_arch = "aarch64"))]
274277
pub(crate) fn syncfs(d: &openat::Dir) -> Result<()> {
275-
use rustix::fd::BorrowedFd;
276278
use rustix::fs::{Mode, OFlags};
277279
let d = unsafe { BorrowedFd::borrow_raw(d.as_raw_fd()) };
278280
let oflags = OFlags::RDONLY | OFlags::CLOEXEC | OFlags::DIRECTORY;
279281
let d = rustix::fs::openat(d, ".", oflags, Mode::empty())?;
280282
rustix::fs::syncfs(d).map_err(Into::into)
281283
}
282284

283-
#[cfg(any(target_arch = "x86_64", target_arch = "aarch64"))]
284-
fn tmpname_for_path<P: AsRef<Path>>(path: P) -> std::path::PathBuf {
285+
fn tmpname_for_path<P: AsRef<Utf8Path>>(path: P) -> Utf8PathBuf {
285286
let path = path.as_ref();
286-
let mut buf = path.file_name().expect("filename").to_os_string();
287-
buf.push(TMP_PREFIX);
287+
let mut buf = path.file_name().expect("filename").to_string();
288+
buf.insert_str(0, TMP_PREFIX);
288289
path.with_file_name(buf)
289290
}
290291

292+
/// Copy from src to dst at root dir
293+
#[cfg(any(target_arch = "x86_64", target_arch = "aarch64"))]
294+
fn copy_dir(root: &openat::Dir, src: &str, dst: &str) -> Result<()> {
295+
let rootfd = unsafe { BorrowedFd::borrow_raw(root.as_raw_fd()) };
296+
let r = unsafe {
297+
Command::new("cp")
298+
.args(["-a"])
299+
.arg(src)
300+
.arg(dst)
301+
.pre_exec(move || rustix::process::fchdir(rootfd).map_err(Into::into))
302+
.status()?
303+
};
304+
if !r.success() {
305+
anyhow::bail!("Failed to copy {src} to {dst}");
306+
}
307+
log::debug!("Copy {src} to {dst}");
308+
Ok(())
309+
}
310+
311+
/// Get first sub dir and tmp sub dir for the path
312+
/// "fedora/foo/bar" -> ("fedora", ".btmp.fedora")
313+
/// "foo" -> ("foo", ".btmp.foo")
314+
#[cfg(any(target_arch = "x86_64", target_arch = "aarch64"))]
315+
fn get_subdir(path: &Utf8Path) -> Result<(String, String)> {
316+
let buf = path.iter().next().unwrap().to_owned();
317+
let mut buf_tmp = buf.clone();
318+
buf_tmp.insert_str(0, TMP_PREFIX);
319+
Ok((buf, buf_tmp))
320+
}
321+
291322
/// Given two directories, apply a diff generated from srcdir to destdir
292323
#[cfg(any(target_arch = "x86_64", target_arch = "aarch64"))]
293324
pub(crate) fn apply_diff(
@@ -302,41 +333,101 @@ pub(crate) fn apply_diff(
302333
let opts = opts.unwrap_or(&default_opts);
303334
cleanup_tmp(destdir).context("cleaning up temporary files")?;
304335

305-
// Write new and changed files
306-
for pathstr in diff.additions.iter().chain(diff.changes.iter()) {
307-
let path = Path::new(pathstr);
308-
if let Some(parent) = path.parent() {
309-
destdir.ensure_dir_all(parent, DEFAULT_FILE_MODE)?;
336+
let mut updates = HashMap::new();
337+
// Handle removals in temp dir
338+
if !opts.skip_removals {
339+
for pathstr in diff.removals.iter() {
340+
let path = Utf8Path::new(pathstr);
341+
let (subdir, subdir_tmp) = get_subdir(path)?;
342+
let path_tmp;
343+
if subdir != *pathstr {
344+
// need to copy to temp subdir and remember
345+
if !destdir.exists(&subdir_tmp)? {
346+
copy_dir(destdir, &subdir, &subdir_tmp)?;
347+
updates.insert(subdir.clone(), subdir_tmp.clone());
348+
}
349+
path_tmp = Utf8Path::new(&subdir_tmp).join(path.strip_prefix(&subdir)?);
350+
} else {
351+
// remove the file directly that is not in dir
352+
path_tmp = path.to_path_buf();
353+
}
354+
destdir
355+
.remove_file(path_tmp.as_std_path())
356+
.with_context(|| format!("removing {:?}", path_tmp))?;
310357
}
311-
let destp = tmpname_for_path(path);
312-
srcdir
313-
.copy_file_at(path, destdir, destp.as_path())
314-
.with_context(|| format!("writing {}", &pathstr))?;
315358
}
316-
// Ensure all of the new files are written persistently to disk
317-
if !opts.skip_sync {
318-
syncfs(destdir)?;
359+
// Write changed files to temp dir and exchange finally
360+
for pathstr in diff.changes.iter() {
361+
let path = Utf8Path::new(pathstr);
362+
let (subdir, subdir_tmp) = get_subdir(path)?;
363+
let path_tmp;
364+
if subdir != *pathstr {
365+
if !destdir.exists(&subdir_tmp)? {
366+
// need to copy to temp subdir and remember
367+
copy_dir(destdir, &subdir, &subdir_tmp)?;
368+
updates.insert(subdir.clone(), subdir_tmp.clone());
369+
}
370+
path_tmp = Utf8Path::new(&subdir_tmp).join(path.strip_prefix(&subdir)?);
371+
destdir
372+
.remove_file_optional(path_tmp.as_std_path())
373+
.with_context(|| format!("removing {path_tmp}"))?;
374+
} else {
375+
// file that is not in dir, copy file to foo.tmp
376+
updates.insert(subdir.clone(), subdir_tmp.clone());
377+
path_tmp = Utf8PathBuf::from(&subdir_tmp);
378+
}
379+
srcdir
380+
.copy_file_at(path.as_std_path(), destdir, path_tmp.as_std_path())
381+
.with_context(|| format!("copying {:?} to {:?}", path, path_tmp))?;
319382
}
320-
// Now move them all into place (TODO track interruption)
321-
for path in diff.additions.iter().chain(diff.changes.iter()) {
322-
let pathtmp = tmpname_for_path(path);
323-
destdir
324-
.local_rename(&pathtmp, path)
325-
.with_context(|| format!("renaming {path}"))?;
383+
// Write new files to temp dir if exists, else write to tmp
384+
for pathstr in diff.additions.iter() {
385+
let path = Utf8Path::new(pathstr);
386+
let (subdir, subdir_tmp) = get_subdir(path)?;
387+
let path_tmp;
388+
if destdir.exists(&subdir_tmp)? {
389+
path_tmp = Utf8Path::new(&subdir_tmp).join(path.strip_prefix(&subdir)?);
390+
} else {
391+
path_tmp = tmpname_for_path(path);
392+
updates.insert(pathstr.to_owned(), path_tmp.to_string());
393+
}
394+
// ensure new additions dir exists
395+
if let Some(parent) = path_tmp.parent().filter(|&v| !v.as_os_str().is_empty()) {
396+
destdir.ensure_dir_all(parent.as_std_path(), DEFAULT_FILE_MODE)?;
397+
}
398+
srcdir
399+
.copy_file_at(path.as_std_path(), destdir, path_tmp.as_std_path())
400+
.with_context(|| format!("copying {:?} to {:?}", path, path_tmp))?;
326401
}
327-
if !opts.skip_removals {
328-
for path in diff.removals.iter() {
402+
403+
// do local exchange
404+
for (src, tmp) in updates.iter() {
405+
log::trace!("doing local exchange/rename for {} and {}", tmp, src);
406+
if destdir.exists(src)? {
329407
destdir
330-
.remove_file_optional(path)
331-
.with_context(|| format!("removing {path}"))?;
408+
.local_exchange(tmp, src)
409+
.with_context(|| format!("exchange for {} and {}", tmp, src))?;
410+
} else {
411+
destdir
412+
.local_rename(tmp, src)
413+
.with_context(|| format!("rename for {} and {}", tmp, src))?;
332414
}
333415
}
416+
// Ensure all of the updates & changes are written persistently to disk
417+
if !opts.skip_sync {
418+
syncfs(destdir)?;
419+
}
420+
421+
// finally remove the temp dir
422+
for (_, tmp) in updates.iter() {
423+
log::trace!("cleanup: {}", tmp);
424+
destdir.remove_all(tmp).context("clean up temp")?;
425+
}
334426
// A second full filesystem sync to narrow any races rather than
335427
// waiting for writeback to kick in.
336428
if !opts.skip_sync {
337429
syncfs(destdir)?;
338430
}
339-
340431
Ok(())
341432
}
342433

@@ -345,6 +436,7 @@ mod tests {
345436
use super::*;
346437
use std::fs;
347438
use std::io::Write;
439+
use std::path::Path;
348440

349441
fn run_diff(a: &openat::Dir, b: &openat::Dir) -> Result<FileTreeDiff> {
350442
let ta = FileTree::new_from_dir(a)?;
@@ -508,4 +600,97 @@ mod tests {
508600
assert!(!a.join(relp).join("shim.x64").exists());
509601
Ok(())
510602
}
603+
#[test]
604+
fn test_get_subdir() -> Result<()> {
605+
// test path
606+
let path = Utf8Path::new("foo/subdir/bar");
607+
let (tp, tp_tmp) = get_subdir(path)?;
608+
assert_eq!(tp.eq("foo"), tp_tmp.eq(".btmp.foo"));
609+
// test file
610+
let path = Utf8Path::new("foo");
611+
let (tp, tp_tmp) = get_subdir(path)?;
612+
assert_eq!(tp.eq("foo"), tp_tmp.eq(".btmp.foo"));
613+
Ok(())
614+
}
615+
#[test]
616+
fn test_apply_with_file() -> Result<()> {
617+
let tmpd = tempfile::tempdir()?;
618+
let p = tmpd.path();
619+
let pa = p.join("a");
620+
let pb = p.join("b");
621+
std::fs::create_dir(&pa)?;
622+
std::fs::create_dir(&pb)?;
623+
let a = openat::Dir::open(&pa)?;
624+
let b = openat::Dir::open(&pb)?;
625+
a.create_dir("foo", 0o755)?;
626+
a.create_dir("bar", 0o755)?;
627+
let foo = Path::new("foo/bar");
628+
let bar = Path::new("bar/foo");
629+
let testfile = "testfile";
630+
{
631+
let mut buf = a.write_file(foo, 0o644)?;
632+
buf.write_all("foocontents".as_bytes())?;
633+
let mut buf = a.write_file(bar, 0o644)?;
634+
buf.write_all("barcontents".as_bytes())?;
635+
let mut buf = a.write_file(testfile, 0o644)?;
636+
buf.write_all("testfilecontents".as_bytes())?;
637+
}
638+
639+
let diff = run_diff(&a, &b)?;
640+
assert_eq!(diff.count(), 3);
641+
b.create_dir("foo", 0o755)?;
642+
{
643+
let mut buf = b.write_file(foo, 0o644)?;
644+
buf.write_all("foocontents".as_bytes())?;
645+
}
646+
let b_btime_foo = fs::metadata(pb.join(foo))?.created()?;
647+
648+
{
649+
let diff = run_diff(&b, &a)?;
650+
assert_eq!(diff.count(), 2);
651+
apply_diff(&a, &b, &diff, None).context("test additional files")?;
652+
assert_eq!(
653+
String::from_utf8(std::fs::read(pb.join(testfile))?)?,
654+
"testfilecontents"
655+
);
656+
assert_eq!(
657+
String::from_utf8(std::fs::read(pb.join(bar))?)?,
658+
"barcontents"
659+
);
660+
// creation time is not changed for unchanged file
661+
let b_btime_foo_new = fs::metadata(pb.join(foo))?.created()?;
662+
assert_eq!(b_btime_foo_new, b_btime_foo);
663+
}
664+
{
665+
fs::write(pa.join(testfile), "newtestfile")?;
666+
fs::write(pa.join(bar), "newbar")?;
667+
let diff = run_diff(&b, &a)?;
668+
assert_eq!(diff.count(), 2);
669+
apply_diff(&a, &b, &diff, None).context("test changed files")?;
670+
assert_eq!(
671+
String::from_utf8(std::fs::read(pb.join(testfile))?)?,
672+
"newtestfile"
673+
);
674+
assert_eq!(String::from_utf8(std::fs::read(pb.join(bar))?)?, "newbar");
675+
// creation time is not changed for unchanged file
676+
let b_btime_foo_new = fs::metadata(pb.join(foo))?.created()?;
677+
assert_eq!(b_btime_foo_new, b_btime_foo);
678+
}
679+
{
680+
a.remove_file(testfile)?;
681+
a.remove_file(bar)?;
682+
let diff = run_diff(&b, &a)?;
683+
assert_eq!(diff.count(), 2);
684+
apply_diff(&a, &b, &diff, None).context("test removed files")?;
685+
assert_eq!(b.exists(testfile)?, false);
686+
assert_eq!(b.exists(bar)?, false);
687+
let diff = run_diff(&b, &a)?;
688+
assert_eq!(diff.count(), 0);
689+
// creation time is not changed for unchanged file
690+
let b_btime_foo_new = fs::metadata(pb.join(foo))?.created()?;
691+
assert_eq!(b_btime_foo_new, b_btime_foo);
692+
}
693+
694+
Ok(())
695+
}
511696
}

0 commit comments

Comments
 (0)