Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Minor improvements to nydus-image #1135

Merged
merged 2 commits into from
Mar 6, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/bin/nydus-image/builder/directory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -117,10 +117,10 @@ impl Builder for DirectoryBuilder {
bootstrap_mgr: &mut BootstrapManager,
blob_mgr: &mut BlobManager,
) -> Result<BuildOutput> {
let mut bootstrap_ctx = bootstrap_mgr.create_ctx(ctx.blob_inline_meta)?;
let mut bootstrap_ctx = bootstrap_mgr.create_ctx()?;
let layer_idx = u16::from(bootstrap_ctx.layered);
let mut blob_writer = if let Some(blob_stor) = ctx.blob_storage.clone() {
ArtifactWriter::new(blob_stor, ctx.blob_inline_meta)?
ArtifactWriter::new(blob_stor)?
} else {
return Err(anyhow!(
"target blob path should always be valid for directory builder"
Expand Down
4 changes: 2 additions & 2 deletions src/bin/nydus-image/builder/stargz.rs
Original file line number Diff line number Diff line change
Expand Up @@ -811,10 +811,10 @@ impl Builder for StargzBuilder {
bootstrap_mgr: &mut BootstrapManager,
blob_mgr: &mut BlobManager,
) -> Result<BuildOutput> {
let mut bootstrap_ctx = bootstrap_mgr.create_ctx(ctx.blob_inline_meta)?;
let mut bootstrap_ctx = bootstrap_mgr.create_ctx()?;
let layer_idx = u16::from(bootstrap_ctx.layered);
let mut blob_writer = if let Some(blob_stor) = ctx.blob_storage.clone() {
Some(ArtifactWriter::new(blob_stor, ctx.blob_inline_meta)?)
Some(ArtifactWriter::new(blob_stor)?)
} else {
return Err(anyhow!("missing configuration for target path"));
};
Expand Down
27 changes: 16 additions & 11 deletions src/bin/nydus-image/builder/tarball.rs
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ impl<'a> TarballTreeBuilder<'a> {

// Generate the root node in advance, it may be overwritten by entries from the tar stream.
let mut nodes = Vec::with_capacity(10240);
let root = self.create_directory(Path::new("/"), &nodes)?;
let root = self.create_directory(Path::new("/"), 0)?;
nodes.push(root.clone());

// Generate RAFS node for each tar entry, and optionally adding missing parents.
Expand Down Expand Up @@ -192,10 +192,13 @@ impl<'a> TarballTreeBuilder<'a> {
) -> Result<Node> {
let header = entry.header();
let entry_type = header.entry_type();
assert!(!entry_type.is_gnu_longname());
assert!(!entry_type.is_gnu_longlink());
assert!(!entry_type.is_pax_local_extensions());
if entry_type.is_pax_global_extensions() {
if entry_type.is_gnu_longname() {
return Err(anyhow!("unsupported gnu_longname from tar header"));
} else if entry_type.is_gnu_longlink() {
return Err(anyhow!("unsupported gnu_longlink from tar header"));
} else if entry_type.is_pax_local_extensions() {
return Err(anyhow!("unsupported pax_local_extensions from tar header"));
} else if entry_type.is_pax_global_extensions() {
return Err(anyhow!("unsupported pax_global_extensions from tar header"));
} else if entry_type.is_contiguous() {
return Err(anyhow!("unsupported contiguous entry type from tar header"));
Expand Down Expand Up @@ -455,15 +458,15 @@ impl<'a> TarballTreeBuilder<'a> {
if let Some(parent_path) = path.as_ref().parent() {
if !self.path_inode_map.contains_key(parent_path) {
self.make_lost_dirs(parent_path, nodes)?;
let node = self.create_directory(parent_path, nodes)?;
let node = self.create_directory(parent_path, nodes.len())?;
nodes.push(node);
}
}

Ok(())
}

fn create_directory(&mut self, path: &Path, nodes: &[Node]) -> Result<Node> {
fn create_directory(&mut self, path: &Path, nodes_index: usize) -> Result<Node> {
let ino = (self.path_inode_map.len() + 1) as Inode;
let name = Self::get_file_name(path)?;
let mut inode = InodeWrapper::new(self.ctx.fs_version);
Expand Down Expand Up @@ -502,7 +505,7 @@ impl<'a> TarballTreeBuilder<'a> {
};

self.path_inode_map
.insert(path.to_path_buf(), (ino, nodes.len()));
.insert(path.to_path_buf(), (ino, nodes_index));

Ok(node)
}
Expand Down Expand Up @@ -530,11 +533,13 @@ impl<'a> TarballTreeBuilder<'a> {
}
}

pub(crate) struct TarballBuilder {
/// Builder to create RAFS filesystems from tarballs.
pub struct TarballBuilder {
ty: ConversionType,
}

impl TarballBuilder {
/// Create a new instance of [TarballBuilder] to build a RAFS filesystem from a tarball.
pub fn new(conversion_type: ConversionType) -> Self {
Self {
ty: conversion_type,
Expand All @@ -549,7 +554,7 @@ impl Builder for TarballBuilder {
bootstrap_mgr: &mut BootstrapManager,
blob_mgr: &mut BlobManager,
) -> Result<BuildOutput> {
let mut bootstrap_ctx = bootstrap_mgr.create_ctx(ctx.blob_inline_meta)?;
let mut bootstrap_ctx = bootstrap_mgr.create_ctx()?;
let layer_idx = u16::from(bootstrap_ctx.layered);
let mut blob_writer = match self.ty {
ConversionType::EStargzToRafs
Expand All @@ -558,7 +563,7 @@ impl Builder for TarballBuilder {
| ConversionType::EStargzToRef
| ConversionType::TargzToRef => {
if let Some(blob_stor) = ctx.blob_storage.clone() {
ArtifactWriter::new(blob_stor, ctx.blob_inline_meta)?
ArtifactWriter::new(blob_stor)?
} else {
return Err(anyhow!("missing configuration for target path"));
}
Expand Down
4 changes: 2 additions & 2 deletions src/bin/nydus-image/core/blob_compact.rs
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ impl ChunkSet {
aligned_chunk: bool,
backend: &Arc<dyn BlobBackend + Send + Sync>,
) -> Result<Vec<(ChunkWrapper, ChunkWrapper)>> {
let mut blob_writer = ArtifactWriter::new(blob_storage, build_ctx.blob_inline_meta)?;
let mut blob_writer = ArtifactWriter::new(blob_storage)?;

// sort chunks first, don't break order in original blobs
let mut chunks = self.chunks.values().collect::<Vec<&ChunkWrapper>>();
Expand Down Expand Up @@ -578,7 +578,7 @@ impl BlobCompactor {
);
let mut bootstrap_mgr =
BootstrapManager::new(Some(ArtifactStorage::SingleFile(d_bootstrap)), None);
let mut bootstrap_ctx = bootstrap_mgr.create_ctx(false)?;
let mut bootstrap_ctx = bootstrap_mgr.create_ctx()?;
let mut ori_blob_mgr = BlobManager::new(rs.meta.get_digester());
ori_blob_mgr.extend_from_blob_table(&build_ctx, rs.superblock.get_blob_infos())?;
if let Some(dict) = chunk_dict {
Expand Down
29 changes: 14 additions & 15 deletions src/bin/nydus-image/core/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,13 @@ use std::any::Any;
use std::borrow::Cow;
use std::collections::{HashMap, VecDeque};
use std::convert::TryFrom;
use std::fmt;
use std::fs::{remove_file, rename, File, OpenOptions};
use std::io::{BufWriter, Cursor, Read, Seek, Write};
use std::os::unix::fs::FileTypeExt;
use std::path::{Display, Path, PathBuf};
use std::str::FromStr;
use std::sync::{Arc, Mutex};
use std::{fmt, fs};

use anyhow::{Context, Error, Result};
use sha2::{Digest, Sha256};
Expand Down Expand Up @@ -245,15 +246,18 @@ impl Write for ArtifactWriter {
}

impl ArtifactWriter {
pub fn new(storage: ArtifactStorage, fifo: bool) -> Result<Self> {
pub fn new(storage: ArtifactStorage) -> Result<Self> {
match storage {
ArtifactStorage::SingleFile(ref p) => {
let mut opener = &mut OpenOptions::new();
opener = opener.write(true).create(true);
// Make it as the writer side of FIFO file, no truncate flag because it has
// been created by the reader side.
if !fifo {
opener = opener.truncate(true);
if let Ok(md) = fs::metadata(p) {
let ty = md.file_type();
// Make it as the writer side of FIFO file, no truncate flag because it has
// been created by the reader side.
if !ty.is_fifo() {
opener = opener.truncate(true);
}
}
let b = BufWriter::with_capacity(
BUF_WRITER_CAPACITY,
Expand Down Expand Up @@ -938,10 +942,9 @@ pub struct BootstrapContext {
}

impl BootstrapContext {
pub fn new(storage: Option<ArtifactStorage>, layered: bool, fifo: bool) -> Result<Self> {
pub fn new(storage: Option<ArtifactStorage>, layered: bool) -> Result<Self> {
let writer = if let Some(storage) = storage {
Box::new(ArtifactFileWriter(ArtifactWriter::new(storage, fifo)?))
as Box<dyn RafsIoWrite>
Box::new(ArtifactFileWriter(ArtifactWriter::new(storage)?)) as Box<dyn RafsIoWrite>
} else {
Box::<ArtifactMemoryWriter>::default() as Box<dyn RafsIoWrite>
};
Expand Down Expand Up @@ -1013,12 +1016,8 @@ impl BootstrapManager {
}
}

pub fn create_ctx(&self, fifo: bool) -> Result<BootstrapContext> {
BootstrapContext::new(
self.bootstrap_storage.clone(),
self.f_parent_path.is_some(),
fifo,
)
pub fn create_ctx(&self) -> Result<BootstrapContext> {
BootstrapContext::new(self.bootstrap_storage.clone(), self.f_parent_path.is_some())
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/bin/nydus-image/core/node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1469,7 +1469,7 @@ mod tests {

let bootstrap_path = TempFile::new().unwrap();
let storage = ArtifactStorage::SingleFile(bootstrap_path.as_path().to_path_buf());
let mut bootstrap_ctx = BootstrapContext::new(Some(storage), false, false).unwrap();
let mut bootstrap_ctx = BootstrapContext::new(Some(storage), false).unwrap();
bootstrap_ctx.offset = 0;

// reg file.
Expand Down
12 changes: 7 additions & 5 deletions src/bin/nydus-image/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -869,10 +869,9 @@ impl Command {
)?;

// Some operations like listing xattr pairs of certain namespace need the process
// to be privileged. Therefore, trace what euid and egid are
// to be privileged. Therefore, trace what euid and egid are.
event_tracer!("euid", "{}", geteuid());
event_tracer!("egid", "{}", getegid());

info!("successfully built RAFS filesystem: \n{}", build_output);
OutputSerializer::dump(matches, build_output, build_info)
}
Expand Down Expand Up @@ -1284,9 +1283,12 @@ impl Command {
}
}
Some(v) => {
let param = v.trim_start_matches("0x").trim_start_matches("0X");
let chunk_size =
u32::from_str_radix(param, 16).context(format!("invalid chunk size {}", v))?;
let chunk_size = if v.starts_with("0x") || v.starts_with("0X") {
u32::from_str_radix(&v[2..], 16).context(format!("invalid chunk size {}", v))?
} else {
v.parse::<u32>()
.context(format!("invalid chunk size {}", v))?
};
if chunk_size as u64 > RAFS_MAX_CHUNK_SIZE
|| chunk_size < 0x1000
|| !chunk_size.is_power_of_two()
Expand Down
2 changes: 1 addition & 1 deletion src/bin/nydus-image/merge.rs
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,7 @@ impl Merger {
ctx.chunk_size = chunk_size;
}

let mut bootstrap_ctx = BootstrapContext::new(Some(target.clone()), false, false)?;
let mut bootstrap_ctx = BootstrapContext::new(Some(target.clone()), false)?;
let mut bootstrap = Bootstrap::new()?;
bootstrap.build(ctx, &mut bootstrap_ctx, &mut tree)?;
let blob_table = blob_mgr.to_blob_table(ctx)?;
Expand Down