From 418f279ef36c9ab54223a5428e01a61fbc6ea8ca Mon Sep 17 00:00:00 2001 From: Jiang Liu Date: Wed, 28 Dec 2022 19:37:56 +0800 Subject: [PATCH] stroage: fix a bug in handling last chunk of blob Fix a bug in handling last chunk of blob. Signed-off-by: Jiang Liu --- rafs/src/fs.rs | 2 +- rafs/src/metadata/mod.rs | 8 +-- src/bin/nydus-image/core/context.rs | 20 +++--- src/bin/nydus-image/main.rs | 3 +- src/bin/nydusd/fs_cache.rs | 2 +- storage/src/cache/filecache/mod.rs | 2 +- storage/src/device.rs | 29 ++++++-- storage/src/meta/mod.rs | 102 ++++++++++++++++++++-------- 8 files changed, 115 insertions(+), 53 deletions(-) diff --git a/rafs/src/fs.rs b/rafs/src/fs.rs index a358f912b3f..ef32439c9e2 100644 --- a/rafs/src/fs.rs +++ b/rafs/src/fs.rs @@ -438,7 +438,7 @@ impl Rafs { let batch_size = 1024 * 1024 * 2; for blob in &blob_infos { - let blob_size = blob.compressed_size(); + let blob_size = blob.compressed_data_size(); let count = div_round_up(blob_size, batch_size); let mut pre_offset = 0u64; diff --git a/rafs/src/metadata/mod.rs b/rafs/src/metadata/mod.rs index e43026d75ea..7d9a169d694 100644 --- a/rafs/src/metadata/mod.rs +++ b/rafs/src/metadata/mod.rs @@ -693,7 +693,7 @@ impl RafsSuper { let blobs = rs.superblock.get_blob_infos(); for blob in blobs.iter() { // Fix blob id for new images with old converters. - if blob.has_feature(BlobFeatures::INLINED_META) { + if blob.has_feature(BlobFeatures::INLINED_FS_META) { blob.set_blob_id_from_meta_path(path.as_ref())?; fixed = true; } @@ -702,7 +702,7 @@ impl RafsSuper { // Fix blob id for old images with old converters. let last = blobs.len() - 1; let blob = &blobs[last]; - if !blob.has_feature(BlobFeatures::CAP_INLINED_META) { + if !blob.has_feature(BlobFeatures::CAP_TAR_TOC) { rs.set_blob_id_from_meta_path(path.as_ref())?; } } @@ -738,8 +738,8 @@ impl RafsSuper { pub fn set_blob_id_from_meta_path(&self, meta_path: &Path) -> Result<()> { let blobs = self.superblock.get_blob_infos(); for blob in blobs.iter() { - if blob.has_feature(BlobFeatures::INLINED_META) - || !blob.has_feature(BlobFeatures::CAP_INLINED_META) + if blob.has_feature(BlobFeatures::INLINED_FS_META) + || !blob.has_feature(BlobFeatures::CAP_TAR_TOC) { blob.set_blob_id_from_meta_path(meta_path)?; } diff --git a/src/bin/nydus-image/core/context.rs b/src/bin/nydus-image/core/context.rs index 2b3d7541dc3..85cad7adc1a 100644 --- a/src/bin/nydus-image/core/context.rs +++ b/src/bin/nydus-image/core/context.rs @@ -449,7 +449,7 @@ impl BlobContext { .set_4k_aligned(features.contains(BlobFeatures::ALIGNED)); blob_ctx .blob_meta_header - .set_inlined_meta(features.contains(BlobFeatures::INLINED_META)); + .set_inlined_fs_meta(features.contains(BlobFeatures::INLINED_FS_META)); blob_ctx .blob_meta_header .set_chunk_info_v2(features.contains(BlobFeatures::CHUNK_INFO_V2)); @@ -461,7 +461,7 @@ impl BlobContext { .set_inlined_chunk_digest(features.contains(BlobFeatures::INLINED_CHUNK_DIGEST)); blob_ctx .blob_meta_header - .set_cap_inlined_meta(features.contains(BlobFeatures::CAP_INLINED_META)); + .set_cap_tar_toc(features.contains(BlobFeatures::CAP_TAR_TOC)); blob_ctx } @@ -477,8 +477,8 @@ impl BlobContext { // Fixes up blob info objects from inlined-meta blobs. if chunk_source == ChunkSource::Dict || chunk_source == ChunkSource::Parent { - if features.contains(BlobFeatures::INLINED_META) { - features &= !BlobFeatures::INLINED_META; + if features.contains(BlobFeatures::INLINED_FS_META) { + features &= !BlobFeatures::INLINED_FS_META; if !features.contains(BlobFeatures::ZRAN) { blob_id = blob.blob_id(); @@ -538,9 +538,7 @@ impl BlobContext { } } } - } else if !blob.has_feature(BlobFeatures::CAP_INLINED_META) - && !ctx.can_access_data_blobs - { + } else if !blob.has_feature(BlobFeatures::CAP_TAR_TOC) && !ctx.can_access_data_blobs { blob_id = blob.blob_id(); } } @@ -583,7 +581,7 @@ impl BlobContext { // TODO: check the logic to reset prefetch size pub fn set_blob_prefetch_size(&mut self, ctx: &BuildContext) { - if (self.compressed_blob_size > 0 + if (self.uncompressed_blob_size > 0 || (ctx.conversion_type == ConversionType::EStargzIndexToRef && !self.blob_id.is_empty())) && ctx.prefetch.policy != PrefetchPolicy::Blob @@ -652,7 +650,7 @@ impl BlobContext { /// Get blob id if the blob has some chunks. pub fn blob_id(&mut self) -> Option { - if self.compressed_blob_size > 0 { + if self.uncompressed_blob_size > 0 { Some(self.blob_id.to_string()) } else { None @@ -1070,9 +1068,9 @@ impl BuildContext { features: Features, ) -> Self { let blob_features = if blob_inline_meta { - BlobFeatures::INLINED_META | BlobFeatures::CAP_INLINED_META + BlobFeatures::INLINED_FS_META | BlobFeatures::CAP_TAR_TOC } else { - BlobFeatures::CAP_INLINED_META + BlobFeatures::CAP_TAR_TOC }; BuildContext { blob_id, diff --git a/src/bin/nydus-image/main.rs b/src/bin/nydus-image/main.rs index 1dee96effd8..b427e34b2c8 100644 --- a/src/bin/nydus-image/main.rs +++ b/src/bin/nydus-image/main.rs @@ -995,9 +995,10 @@ impl Command { let mut blob_ids = Vec::new(); for (idx, blob) in blobs.iter().enumerate() { println!( - "\t {}: {}, compressed size 0x{:x}, uncompressed size 0x{:x}, chunks: 0x{:x}, features: {}", + "\t {}: {}, compressed data size 0x{:x}, compressed file size 0x{:x}, uncompressed file size 0x{:x}, chunks: 0x{:x}, features: {}", idx, blob.blob_id(), + blob.compressed_data_size(), blob.compressed_size(), blob.uncompressed_size(), blob.chunk_count(), diff --git a/src/bin/nydusd/fs_cache.rs b/src/bin/nydusd/fs_cache.rs index e0f6fa87d63..8ee8ed76352 100644 --- a/src/bin/nydusd/fs_cache.rs +++ b/src/bin/nydusd/fs_cache.rs @@ -510,7 +510,7 @@ impl FsCacheHandler { Some(1) => nydus_api::default_batch_size() as u64, Some(s) => s as u64, }; - let blob_size = blob_info.compressed_size(); + let blob_size = blob_info.compressed_data_size(); let count = (blob_size + size - 1) / size; let mut blob_req = Vec::with_capacity(count as usize); let mut pre_offset = 0u64; diff --git a/storage/src/cache/filecache/mod.rs b/storage/src/cache/filecache/mod.rs index ccf7407baf8..c444494fff4 100644 --- a/storage/src/cache/filecache/mod.rs +++ b/storage/src/cache/filecache/mod.rs @@ -221,7 +221,7 @@ impl FileCacheEntry { // Set cache file to its expected size. let file_size = file.metadata()?.len(); let cached_file_size = if mgr.cache_raw_data { - blob_info.compressed_size() + blob_info.compressed_data_size() } else { blob_info.uncompressed_size() }; diff --git a/storage/src/device.rs b/storage/src/device.rs index e6479156ab9..0d7674618c5 100644 --- a/storage/src/device.rs +++ b/storage/src/device.rs @@ -52,15 +52,16 @@ bitflags! { /// Uncompressed chunk data is 4K aligned. const ALIGNED = 0x0000_0001; /// RAFS meta data is inlined in the data blob. - const INLINED_META = 0x0000_0002; + const INLINED_FS_META = 0x0000_0002; /// Blob chunk information format v2. const CHUNK_INFO_V2 = 0x0000_0004; /// Blob compression information data include context data for zlib random access. const ZRAN = 0x0000_0008; /// Chunk digest array is inlined in the data blob. const INLINED_CHUNK_DIGEST = 0x0000_0010; - /// Inlined-meta capability, used to support backward compatibility for legacy converters. - const CAP_INLINED_META = 0x4000_0000; + /// Data blob are encoded with Tar header and optionally ToC. + /// It's also a flag indicating that images are generated with `nydus-image` v2.2 or newer. + const CAP_TAR_TOC = 0x4000_0000; /// Rafs V5 image without extended blob table, this is an internal flag. const _V5_NO_EXT_BLOB_TABLE = 0x8000_0000; } @@ -198,8 +199,9 @@ impl BlobInfo { /// Get the id of the blob, with special handling of `inlined-meta` case. pub fn blob_id(&self) -> String { - if (self.has_feature(BlobFeatures::INLINED_META) && !self.has_feature(BlobFeatures::ZRAN)) - || !self.has_feature(BlobFeatures::CAP_INLINED_META) + if (self.has_feature(BlobFeatures::INLINED_FS_META) + && !self.has_feature(BlobFeatures::ZRAN)) + || !self.has_feature(BlobFeatures::CAP_TAR_TOC) { let guard = self.meta_path.lock().unwrap(); if !guard.is_empty() { @@ -214,7 +216,20 @@ impl BlobInfo { &self.blob_id } - /// Get size of the compressed blob. + /// Get size of compressed chunk data, not including `blob.meta`, `blob.chunk`, `toc` etc. + pub fn compressed_data_size(&self) -> u64 { + if self.has_feature(BlobFeatures::ZRAN) { + // It's the size of OCIv1 targz blob. + self.compressed_size + } else if self.has_feature(BlobFeatures::CAP_TAR_TOC) && self.meta_ci_is_valid() { + // There's a tar header between chunk data and compression information. + self.meta_ci_offset - 0x200 + } else { + self.compressed_size + } + } + + /// Get size of the compressed blob, including `blob.meta`, `blob.chunk`, `toc` etc. pub fn compressed_size(&self) -> u64 { self.compressed_size } @@ -453,7 +468,7 @@ impl BlobInfo { /// Get RAFS blob id for ZRan. pub fn get_rafs_blob_id(&self) -> Result { assert!(self.has_feature(BlobFeatures::ZRAN)); - let id = if self.has_feature(BlobFeatures::INLINED_META) { + let id = if self.has_feature(BlobFeatures::INLINED_FS_META) { let guard = self.meta_path.lock().unwrap(); if guard.is_empty() { return Err(einval!("failed to get blob id from meta file name")); diff --git a/storage/src/meta/mod.rs b/storage/src/meta/mod.rs index fe06ab5751b..28f95442bb1 100644 --- a/storage/src/meta/mod.rs +++ b/storage/src/meta/mod.rs @@ -247,11 +247,11 @@ impl BlobMetaHeaderOndisk { } /// Set flag indicating whether RAFS meta is inlined in the data blob. - pub fn set_inlined_meta(&mut self, inlined: bool) { + pub fn set_inlined_fs_meta(&mut self, inlined: bool) { if inlined { - self.s_features |= BlobFeatures::INLINED_META.bits(); + self.s_features |= BlobFeatures::INLINED_FS_META.bits(); } else { - self.s_features &= !BlobFeatures::INLINED_META.bits(); + self.s_features &= !BlobFeatures::INLINED_FS_META.bits(); } } @@ -283,11 +283,11 @@ impl BlobMetaHeaderOndisk { } /// Set flag indicating having inlined-meta capability. - pub fn set_cap_inlined_meta(&mut self, enable: bool) { + pub fn set_cap_tar_toc(&mut self, enable: bool) { if enable { - self.s_features |= BlobFeatures::CAP_INLINED_META.bits(); + self.s_features |= BlobFeatures::CAP_TAR_TOC.bits(); } else { - self.s_features &= !BlobFeatures::CAP_INLINED_META.bits(); + self.s_features &= !BlobFeatures::CAP_TAR_TOC.bits(); } } @@ -404,7 +404,7 @@ impl BlobMetaInfo { let mut state = BlobMetaState { blob_index: blob_info.blob_index(), blob_features: blob_info.features().bits(), - compressed_size: blob_info.compressed_size(), + compressed_size: blob_info.compressed_data_size(), uncompressed_size: round_up_4k(blob_info.uncompressed_size()), chunk_info_array: chunk_infos, chunk_digest_array: Default::default(), @@ -1202,7 +1202,6 @@ impl BlobMetaChunkArray { } } - let mut vec = Vec::with_capacity(128); for entry in &chunk_info_array[index..] { entry.validate(state)?; if !entry.is_zran() { @@ -1212,8 +1211,8 @@ impl BlobMetaChunkArray { } if entry.get_zran_index() != zran_last { let ctx = &state.zran_info_array[entry.get_zran_index() as usize]; - if count + ctx.out_size() as u64 > batch_size - && entry.uncompressed_offset() > end + if count + ctx.out_size() as u64 >= batch_size + && entry.uncompressed_offset() >= end { return Ok(vec); } @@ -1223,7 +1222,17 @@ impl BlobMetaChunkArray { vec.push(BlobMetaChunk::new(index, state)); index += 1; } - return Ok(vec); + + if let Some(c) = vec.last() { + if c.uncompressed_end() >= end { + return Ok(vec); + } + } + return Err(einval!(format!( + "entry not found index {} chunk_info_array.len {}", + index, + chunk_info_array.len(), + ))); } vec.push(BlobMetaChunk::new(index, state)); @@ -1242,7 +1251,7 @@ impl BlobMetaChunkArray { entry.uncompressed_size(), last_end ))); - } else if last_end >= end && entry.aligned_uncompressed_end() > batch_end { + } else if last_end >= end && entry.aligned_uncompressed_end() >= batch_end { // Avoid read amplify if next chunk is too big. return Ok(vec); } @@ -1254,11 +1263,25 @@ impl BlobMetaChunkArray { } } - Err(einval!(format!( - "entry not found index {} chunk_info_array.len {}", - index, - chunk_info_array.len(), - ))) + if last_end >= end { + Ok(vec) + } else { + panic!( + "entry not found index {} chunk_info_array.len {}, last_end 0x{:x}, end 0x{:x}, blob compressed size 0x{:x}", + index, + chunk_info_array.len(), + last_end, + end, + state.uncompressed_size, + ) + /* + Err(einval!(format!( + "entry not found index {} chunk_info_array.len {}", + index, + chunk_info_array.len(), + ))) + */ + } } } @@ -1294,7 +1317,6 @@ impl BlobMetaChunkArray { } } - let mut vec = Vec::with_capacity(128); for entry in &chunk_info_array[index..] { entry.validate(state)?; if !entry.is_zran() { @@ -1314,7 +1336,17 @@ impl BlobMetaChunkArray { vec.push(BlobMetaChunk::new(index, state)); index += 1; } - return Ok(vec); + + if let Some(c) = vec.last() { + if c.uncompressed_end() >= end { + return Ok(vec); + } + } + return Err(einval!(format!( + "entry not found index {} chunk_info_array.len {}", + index, + chunk_info_array.len(), + ))); } vec.push(BlobMetaChunk::new(index, state)); @@ -1338,11 +1370,27 @@ impl BlobMetaChunkArray { } } - Err(einval!(format!( - "entry not found index {} chunk_info_array.len {}", - index, - chunk_info_array.len(), - ))) + if last_end >= end { + Ok(vec) + } else { + panic!( + "entry not found index {} chunk_info_array.len {}, last_end 0x{:x}, end 0x{:x}, blob compressed size 0x{:x}", + index, + chunk_info_array.len(), + last_end, + end, + state.compressed_size, + ) + /* + Err(einval!(format!( + "entry not found index {} chunk_info_array.len {}, last_end 0x{:x}, end 0x{:x}", + index, + chunk_info_array.len(), + last_end, + end, + ))) + */ + } } } @@ -1660,7 +1708,7 @@ pub(crate) mod tests { let path = PathBuf::from(root_dir).join("../tests/texture/zran/233c72f2b6b698c07021c4da367cfe2dff4f049efbaa885ca0ff760ea297865a"); let features = BlobFeatures::ALIGNED - | BlobFeatures::INLINED_META + | BlobFeatures::INLINED_FS_META | BlobFeatures::CHUNK_INFO_V2 | BlobFeatures::ZRAN; let mut blob_info = BlobInfo::new( @@ -1713,7 +1761,7 @@ pub(crate) mod tests { let path = PathBuf::from(root_dir).join("../tests/texture/zran/233c72f2b6b698c07021c4da367cfe2dff4f049efbaa885ca0ff760ea297865a"); let features = BlobFeatures::ALIGNED - | BlobFeatures::INLINED_META + | BlobFeatures::INLINED_FS_META | BlobFeatures::CHUNK_INFO_V2 | BlobFeatures::ZRAN; let mut blob_info = BlobInfo::new( @@ -1770,7 +1818,7 @@ pub(crate) mod tests { let path = PathBuf::from(root_dir).join("../tests/texture/zran/233c72f2b6b698c07021c4da367cfe2dff4f049efbaa885ca0ff760ea297865a"); let features = BlobFeatures::ALIGNED - | BlobFeatures::INLINED_META + | BlobFeatures::INLINED_FS_META | BlobFeatures::CHUNK_INFO_V2 | BlobFeatures::ZRAN; let mut blob_info = BlobInfo::new(