Skip to content

Commit 39028de

Browse files
Camelronromoh
authored andcommitted
agent: Handle inplace CopyFile requests
The existing mechanism for forwarding new forward api volumes (/etc/hosts, /etc/resolv.conf, etc) works, but the restored bind mount still references the original, now-empty inode. Here we add a preserve_inode parameter to CopyFileRequest, allowing a file to be replaced in-line. Signed-off-by: Cameron Baird <cameronbaird@microsoft.com> Assisted-by: Sol:5.6
1 parent 99043f4 commit 39028de

6 files changed

Lines changed: 528 additions & 296 deletions

File tree

src/agent/src/rpc.rs

Lines changed: 201 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2402,6 +2402,53 @@ fn do_copy_file(req: &CopyFileRequest, shared_dir: &PathBuf) -> Result<()> {
24022402
std::fs::create_dir_all(shared_dir)?;
24032403
let root = pathrs::Root::open(shared_dir)?;
24042404

2405+
if req.preserve_inode {
2406+
if req.file_mode & libc::S_IFMT != libc::S_IFREG {
2407+
return Err(anyhow!("inode-preserving copy requires a regular file"));
2408+
}
2409+
if req.offset < 0
2410+
|| req.file_size < req.offset
2411+
|| req.data.len() as u64 > (req.file_size - req.offset) as u64
2412+
{
2413+
return Err(anyhow!("invalid inode-preserving copy range"));
2414+
}
2415+
let handle = root
2416+
.resolve_nofollow(path)
2417+
.context("resolve existing file")?;
2418+
let metadata = stat::fstat(&handle).context("stat existing file")?;
2419+
if metadata.st_mode & libc::S_IFMT != libc::S_IFREG {
2420+
return Err(anyhow!(
2421+
"inode-preserving copy target is not a regular file"
2422+
));
2423+
}
2424+
if metadata.st_nlink != 1 {
2425+
return Err(anyhow!(
2426+
"inode-preserving copy target has multiple hard links"
2427+
));
2428+
}
2429+
let file = handle
2430+
.reopen(OpenFlags::O_WRONLY)
2431+
.context("reopen existing file")?;
2432+
if req.offset == 0 {
2433+
file.set_len(0).context("truncate existing file")?;
2434+
}
2435+
file.write_all_at(&req.data, req.offset as u64)
2436+
.context("write existing file")?;
2437+
if req.offset + req.data.len() as i64 == req.file_size {
2438+
file.set_permissions(std::fs::Permissions::from_mode(
2439+
req.file_mode & FILE_PERMISSION_MASK,
2440+
))
2441+
.context("set existing file permissions")?;
2442+
unistd::fchown(
2443+
file,
2444+
Some(Uid::from_raw(req.uid as u32)),
2445+
Some(Gid::from_raw(req.gid as u32)),
2446+
)
2447+
.context("chown existing file")?;
2448+
}
2449+
return Ok(());
2450+
}
2451+
24052452
// Create parent directories if missing
24062453
if let Some(parent) = path.parent() {
24072454
let dir = root
@@ -3925,6 +3972,160 @@ COMMIT
39253972
assert_eq!(ids, vec!["container-1", "container-2"]);
39263973
}
39273974

3975+
#[rstest::rstest]
3976+
#[case("hosts", b"10.0.0.2 new-pod\n")]
3977+
#[case(
3978+
"resolv.conf",
3979+
b"nameserver 10.0.0.10\nsearch default.svc.cluster.local\n"
3980+
)]
3981+
fn test_do_copy_file_refresh_preserves_inode(#[case] name: &str, #[case] data: &[u8]) {
3982+
use std::os::unix::fs::{FileExt, MetadataExt};
3983+
3984+
let temp_dir = tempdir().unwrap();
3985+
let base = temp_dir.path().to_path_buf();
3986+
let path = base.join(name);
3987+
fs::write(&path, b"10.0.0.1 old-pod\n").unwrap();
3988+
let mounted_file = fs::File::open(&path).unwrap();
3989+
fs::OpenOptions::new()
3990+
.write(true)
3991+
.truncate(true)
3992+
.open(&path)
3993+
.unwrap();
3994+
3995+
for contents in [data, &data[..4], b""] {
3996+
let request = CopyFileRequest {
3997+
path: path.to_string_lossy().into_owned(),
3998+
file_mode: 0o640 | libc::S_IFREG,
3999+
file_size: contents.len() as i64,
4000+
data: contents.to_vec(),
4001+
uid: unistd::geteuid().as_raw() as i32,
4002+
gid: unistd::getegid().as_raw() as i32,
4003+
preserve_inode: true,
4004+
..Default::default()
4005+
};
4006+
do_copy_file(&request, &base).unwrap();
4007+
4008+
let mut actual = vec![0; contents.len()];
4009+
mounted_file.read_exact_at(&mut actual, 0).unwrap();
4010+
assert_eq!(actual, contents);
4011+
let metadata = mounted_file.metadata().unwrap();
4012+
assert_eq!(metadata.ino(), fs::metadata(&path).unwrap().ino());
4013+
assert_eq!(metadata.len(), contents.len() as u64);
4014+
assert_eq!(metadata.permissions().mode() & 0o777, 0o640);
4015+
assert_eq!(metadata.uid(), request.uid as u32);
4016+
assert_eq!(metadata.gid(), request.gid as u32);
4017+
}
4018+
}
4019+
4020+
#[test]
4021+
fn test_do_copy_file_preserve_inode_chunked() {
4022+
use std::os::unix::fs::{FileExt, MetadataExt};
4023+
4024+
let temp_dir = tempdir().unwrap();
4025+
let base = temp_dir.path().to_path_buf();
4026+
let path = base.join("hosts");
4027+
fs::write(&path, b"old content with a longer trailing suffix").unwrap();
4028+
let mounted_file = fs::File::open(&path).unwrap();
4029+
let data = b"10.0.0.2 new-pod\n";
4030+
let first_chunk = b"10.0.0.2 ";
4031+
let mut request = CopyFileRequest {
4032+
path: path.to_string_lossy().into_owned(),
4033+
file_mode: 0o644 | libc::S_IFREG,
4034+
file_size: data.len() as i64,
4035+
data: first_chunk.to_vec(),
4036+
uid: unistd::geteuid().as_raw() as i32,
4037+
gid: unistd::getegid().as_raw() as i32,
4038+
preserve_inode: true,
4039+
..Default::default()
4040+
};
4041+
do_copy_file(&request, &base).unwrap();
4042+
assert_eq!(
4043+
mounted_file.metadata().unwrap().len(),
4044+
first_chunk.len() as u64
4045+
);
4046+
request.offset = first_chunk.len() as i64;
4047+
request.data = data[first_chunk.len()..].to_vec();
4048+
do_copy_file(&request, &base).unwrap();
4049+
4050+
let mut actual = vec![0; data.len()];
4051+
mounted_file.read_exact_at(&mut actual, 0).unwrap();
4052+
assert_eq!(actual, data);
4053+
assert_eq!(mounted_file.metadata().unwrap().len(), data.len() as u64);
4054+
assert_eq!(
4055+
mounted_file.metadata().unwrap().ino(),
4056+
fs::metadata(path).unwrap().ino()
4057+
);
4058+
}
4059+
4060+
#[test]
4061+
fn test_do_copy_file_preserve_inode_rejects_unsafe_targets() {
4062+
let temp_dir = tempdir().unwrap();
4063+
let base = temp_dir.path().join("shared");
4064+
fs::create_dir(&base).unwrap();
4065+
let outside = temp_dir.path().join("outside");
4066+
fs::write(&outside, b"unchanged").unwrap();
4067+
std::os::unix::fs::symlink(&outside, base.join("symlink")).unwrap();
4068+
std::os::unix::fs::symlink(temp_dir.path(), base.join("parent-link")).unwrap();
4069+
fs::hard_link(&outside, base.join("hardlink")).unwrap();
4070+
fs::create_dir(base.join("directory")).unwrap();
4071+
unistd::mkfifo(&base.join("fifo"), stat::Mode::S_IRUSR).unwrap();
4072+
4073+
for path in [
4074+
base.join("missing"),
4075+
base.join("symlink"),
4076+
base.join("parent-link/outside"),
4077+
base.join("hardlink"),
4078+
base.join("directory"),
4079+
base.join("fifo"),
4080+
base.join("../outside"),
4081+
outside.clone(),
4082+
] {
4083+
let request = CopyFileRequest {
4084+
path: path.to_string_lossy().into_owned(),
4085+
file_mode: 0o644 | libc::S_IFREG,
4086+
file_size: 3,
4087+
data: b"new".to_vec(),
4088+
preserve_inode: true,
4089+
..Default::default()
4090+
};
4091+
assert!(do_copy_file(&request, &base).is_err(), "{}", path.display());
4092+
assert_eq!(fs::read(&outside).unwrap(), b"unchanged");
4093+
}
4094+
assert!(!base.join("missing").exists());
4095+
assert!(base.join("symlink").is_symlink());
4096+
assert!(base.join("directory").is_dir());
4097+
}
4098+
4099+
#[test]
4100+
fn test_do_copy_file_preserve_inode_rejects_invalid_requests() {
4101+
let temp_dir = tempdir().unwrap();
4102+
let base = temp_dir.path().to_path_buf();
4103+
let path = base.join("hosts");
4104+
fs::write(&path, b"unchanged").unwrap();
4105+
4106+
for (offset, file_size, file_type) in [
4107+
(-1, 3, libc::S_IFREG),
4108+
(0, -1, libc::S_IFREG),
4109+
(4, 3, libc::S_IFREG),
4110+
(1, 3, libc::S_IFREG),
4111+
(i64::MAX, i64::MAX, libc::S_IFREG),
4112+
(0, 3, libc::S_IFDIR),
4113+
(0, 3, libc::S_IFLNK),
4114+
] {
4115+
let request = CopyFileRequest {
4116+
path: path.to_string_lossy().into_owned(),
4117+
file_mode: 0o644 | file_type,
4118+
file_size,
4119+
offset,
4120+
data: b"new".to_vec(),
4121+
preserve_inode: true,
4122+
..Default::default()
4123+
};
4124+
assert!(do_copy_file(&request, &base).is_err());
4125+
assert_eq!(fs::read(&path).unwrap(), b"unchanged");
4126+
}
4127+
}
4128+
39284129
#[tokio::test]
39294130
async fn test_do_copy_file() {
39304131
let temp_dir = tempdir().expect("creating temp dir failed");

src/libs/protocols/protos/agent.proto

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -580,9 +580,9 @@ message CopyFileRequest {
580580
// Path is the destination file in the guest. It must be absolute,
581581
// canonical and below /run.
582582
string path = 1;
583-
// FileSize is the expected file size, for security reasons write operations
584-
// are made in a temporary file, once it has the expected size, it's moved
585-
// to the destination path.
583+
// FileSize is the expected file size. By default, writes are staged in a
584+
// temporary file and moved to the destination once complete. With
585+
// preserve_inode, writes update an existing regular file in place.
586586
int64 file_size = 2;
587587
// FileMode is the file mode.
588588
uint32 file_mode = 3;
@@ -596,6 +596,7 @@ message CopyFileRequest {
596596
int64 offset = 7;
597597
// Data to write in the destination file.
598598
bytes data = 8;
599+
bool preserve_inode = 9;
599600
}
600601

601602
message GetOOMEventRequest {}

src/runtime-rs/crates/agent/src/kata/trans.rs

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -815,6 +815,7 @@ impl From<CopyFileRequest> for agent::CopyFileRequest {
815815
gid: from.gid,
816816
offset: from.offset,
817817
data: from.data,
818+
preserve_inode: from.preserve_inode,
818819
..Default::default()
819820
}
820821
}
@@ -938,6 +939,21 @@ impl From<AddSwapPathRequest> for agent::AddSwapPathRequest {
938939
mod tests {
939940
use super::*;
940941

942+
#[test]
943+
fn copy_file_preserves_inode_option() {
944+
for preserve_inode in [false, true] {
945+
let request = CopyFileRequest {
946+
preserve_inode,
947+
..Default::default()
948+
};
949+
950+
assert_eq!(
951+
agent::CopyFileRequest::from(request).preserve_inode,
952+
preserve_inode
953+
);
954+
}
955+
}
956+
941957
#[test]
942958
fn ip_address_round_trip_preserves_address_and_mask() {
943959
let address = IPAddress {

src/runtime-rs/crates/agent/src/types.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -558,6 +558,7 @@ pub struct CopyFileRequest {
558558
pub gid: i32,
559559
pub offset: i64,
560560
pub data: ::std::vec::Vec<u8>,
561+
pub preserve_inode: bool,
561562
}
562563

563564
#[derive(PartialEq, Clone, Default, Debug)]

src/runtime-rs/crates/resource/src/volume/share_fs_volume.rs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -461,7 +461,7 @@ impl ShareFsVolume {
461461
let guest_path = generate_copy_file_guest_path(cid, m.destination())
462462
.context("generate path failed")?;
463463
// Copy a single file
464-
Self::copy_file_to_guest(&src, &guest_path, &agent)
464+
Self::copy_file_to_guest(&src, &guest_path, &agent, false)
465465
.await
466466
.context("copy file to guest")?;
467467

@@ -592,6 +592,7 @@ impl ShareFsVolume {
592592
src: &Path,
593593
guest_path: &str,
594594
agent: &Arc<dyn Agent>,
595+
preserve_inode: bool,
595596
) -> Result<()> {
596597
// Read file metadata
597598
let file_metadata = std::fs::metadata(src)
@@ -613,6 +614,7 @@ impl ShareFsVolume {
613614
gid: file_metadata.gid() as i32,
614615
file_mode: file_metadata.mode(),
615616
data: buffer,
617+
preserve_inode,
616618
..Default::default()
617619
};
618620

@@ -693,7 +695,7 @@ pub(crate) async fn refresh_guest_path(
693695
if metadata.is_dir() {
694696
ShareFsVolume::copy_directory_to_guest(&source, guest_path, agent).await
695697
} else {
696-
ShareFsVolume::copy_file_to_guest(&source, guest_path, agent).await
698+
ShareFsVolume::copy_file_to_guest(&source, guest_path, agent, true).await
697699
}
698700
}
699701

0 commit comments

Comments
 (0)