From 3691e5d93fa7a22142c3a8500c107a6d6bf39e3c Mon Sep 17 00:00:00 2001 From: Jiang Liu Date: Thu, 9 Mar 2023 13:47:03 +0800 Subject: [PATCH] rafs: only invoke v5 related code for v5 builds Only invoke v5 related code for v5 builds, also enforce strictly validation when creating missing directory for tar based builds. Signed-off-by: Jiang Liu --- rafs/src/builder/core/node.rs | 31 +++++++++---------------------- rafs/src/builder/core/v5.rs | 21 +++++++++++++++++++-- rafs/src/builder/tarball.rs | 26 ++++++++++++++++++++------ src/bin/nydus-image/main.rs | 12 ++++++------ 4 files changed, 54 insertions(+), 36 deletions(-) diff --git a/rafs/src/builder/core/node.rs b/rafs/src/builder/core/node.rs index c03286ee061..bed39fe57d2 100644 --- a/rafs/src/builder/core/node.rs +++ b/rafs/src/builder/core/node.rs @@ -235,15 +235,19 @@ impl Node { return Ok(0); } else if self.is_symlink() { if let Some(symlink) = self.info.symlink.as_ref() { - self.inode - .set_digest(RafsDigest::from_buf(symlink.as_bytes(), ctx.digester)); + if self.inode.is_v5() { + self.inode + .set_digest(RafsDigest::from_buf(symlink.as_bytes(), ctx.digester)); + } return Ok(0); } else { return Err(Error::msg("inode's symblink is invalid.")); } } else if self.is_special() { - self.inode - .set_digest(RafsDigest::hasher(ctx.digester).digest_finalize()); + if self.inode.is_v5() { + self.inode + .set_digest(RafsDigest::hasher(ctx.digester).digest_finalize()); + } return Ok(0); } @@ -581,7 +585,7 @@ impl Node { // calculate it later by ourself. if !self.is_dir() { self.inode.set_size(meta.st_size()); - self.set_inode_blocks(); + self.v5_set_inode_blocks(); } self.info = Arc::new(info); @@ -736,23 +740,6 @@ impl Node { &self.info.target } - /// Calculate and set `i_blocks` for inode. - /// - /// In order to support repeatable build, we can't reuse `i_blocks` from source filesystems, - /// so let's calculate it by ourself for stable `i_block`. - /// - /// Normal filesystem includes the space occupied by Xattr into the directory size, - /// let's follow the normal behavior. - pub fn set_inode_blocks(&mut self) { - // Set inode blocks for RAFS v5 inode, v6 will calculate it at runtime. - if let InodeWrapper::V5(_) = self.inode { - self.inode.set_blocks(div_round_up( - self.inode.size() + self.info.xattrs.aligned_size_v5() as u64, - 512, - )); - } - } - /// Set symlink target for the node. pub fn set_symlink(&mut self, symlink: OsString) { let mut info = self.info.deref().clone(); diff --git a/rafs/src/builder/core/v5.rs b/rafs/src/builder/core/v5.rs index ba0c72d7a67..35c0e3497ec 100644 --- a/rafs/src/builder/core/v5.rs +++ b/rafs/src/builder/core/v5.rs @@ -8,7 +8,7 @@ use std::mem::size_of; use anyhow::{bail, Context, Result}; use nydus_utils::digest::{DigestHasher, RafsDigest}; -use nydus_utils::{root_tracer, timing_tracer, try_round_up_4k}; +use nydus_utils::{div_round_up, root_tracer, timing_tracer, try_round_up_4k}; use super::bootstrap::STARGZ_DEFAULT_BLOCK_SIZE; use super::node::Node; @@ -102,7 +102,24 @@ impl Node { // Safe to unwrap() because we have u32 for child count. self.inode.set_size(try_round_up_4k(d_size).unwrap()); } - self.set_inode_blocks(); + self.v5_set_inode_blocks(); + } + + /// Calculate and set `i_blocks` for inode. + /// + /// In order to support repeatable build, we can't reuse `i_blocks` from source filesystems, + /// so let's calculate it by ourself for stable `i_block`. + /// + /// Normal filesystem includes the space occupied by Xattr into the directory size, + /// let's follow the normal behavior. + pub fn v5_set_inode_blocks(&mut self) { + // Set inode blocks for RAFS v5 inode, v6 will calculate it at runtime. + if let InodeWrapper::V5(_) = self.inode { + self.inode.set_blocks(div_round_up( + self.inode.size() + self.info.xattrs.aligned_size_v5() as u64, + 512, + )); + } } } diff --git a/rafs/src/builder/tarball.rs b/rafs/src/builder/tarball.rs index 7a0c47feed7..f426b85cf3e 100644 --- a/rafs/src/builder/tarball.rs +++ b/rafs/src/builder/tarball.rs @@ -399,8 +399,8 @@ impl<'a> TarballTreeBuilder<'a> { } // Update inode.i_blocks for RAFS v5. - if !entry_type.is_dir() { - node.set_inode_blocks(); + if self.ctx.fs_version == RafsVersion::V5 && !entry_type.is_dir() { + node.v5_set_inode_blocks(); } Ok(node) @@ -466,10 +466,24 @@ impl<'a> TarballTreeBuilder<'a> { fn make_lost_dirs>(&mut self, path: P, nodes: &mut Vec) -> Result<()> { 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.len())?; - nodes.push(node); + match self.path_inode_map.get(parent_path) { + Some((i, idx)) => { + if !nodes[*idx].is_dir() { + panic!( + "tarball: existing inode is not a directory {} {} {}", + i, + nodes.len(), + nodes[*idx].is_dir() + ); + } + } + None => { + if !self.path_inode_map.contains_key(parent_path) { + self.make_lost_dirs(parent_path, nodes)?; + let node = self.create_directory(parent_path, nodes.len())?; + nodes.push(node); + } + } } } diff --git a/src/bin/nydus-image/main.rs b/src/bin/nydus-image/main.rs index 95d800bd715..bf4e1e7789f 100644 --- a/src/bin/nydus-image/main.rs +++ b/src/bin/nydus-image/main.rs @@ -665,6 +665,12 @@ impl Command { .unwrap_or_default() .parse()?; let blob_data_size = Self::get_blob_size(matches, conversion_type)?; + let features = Features::try_from( + matches + .get_one::("features") + .map(|s| s.as_str()) + .unwrap_or_default(), + )?; match conversion_type { ConversionType::DirectoryToRafs => { @@ -770,12 +776,6 @@ impl Command { } } - let features = Features::try_from( - matches - .get_one::("features") - .map(|s| s.as_str()) - .unwrap_or_default(), - )?; if features.is_enabled(Feature::BlobToc) && version == RafsVersion::V5 { bail!("`--features blob-toc` can't be used with `--version 5` "); }