Merge pull request #1135 from jiangliu/nydus-image-simplify

Minor improvements to nydus-image

src/bin/nydus-image/builder/directory.rs

@@ -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"

src/bin/nydus-image/builder/stargz.rs

@@ -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"));
         };

src/bin/nydus-image/builder/tarball.rs

@@ -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.
@@ -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"));
@@ -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);
@@ -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)
     }
@@ -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,
@@ -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
@@ -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"));
                 }

src/bin/nydus-image/core/blob_compact.rs

@@ -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>>();
@@ -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 {

src/bin/nydus-image/core/context.rs

@@ -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};
@@ -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,
@@ -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>
         };
@@ -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())
     }
 }
 

src/bin/nydus-image/core/node.rs

@@ -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.

src/bin/nydus-image/main.rs

@@ -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)
     }
@@ -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()

src/bin/nydus-image/merge.rs

@@ -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)?;