Enhance localdisk storage backend to manually add blob objects #1337

jiangliu · 2023-06-21T08:25:22Z

Relevant Issue (if applicable)

If there are Issues related to this PullRequest, please list it.

Details

Enhance the localdisk storage backend, so we can manually add blob
objects in the disk, in addition to discovering blob objects by
scanning GPT partition table.

Types of changes

What types of changes does your PullRequest introduce? Put an x in all the boxes that apply:

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)
Documentation Update (if none of the other choices apply)

Checklist

Go over all the following points, and put an x in all the boxes that apply.

I have updated the documentation accordingly.
I have added tests to cover my changes.

anolis-bot · 2023-06-21T08:27:18Z

@jiangliu , a new test job has been submitted. Please wait in patience. The test job url: https://tone.openanolis.cn/ws/nrh4nnio/test_result/80071

codecov · 2023-06-21T08:30:11Z

Codecov Report

Merging #1337 (c1b1c7c) into master (4e3c954) will increase coverage by 0.15%.
The diff coverage is 53.43%.

@@            Coverage Diff             @@
##           master    #1337      +/-   ##
==========================================
+ Coverage   46.01%   46.16%   +0.15%     
==========================================
  Files         122      122              
  Lines       37807    37913     +106     
  Branches    37807    37913     +106     
==========================================
+ Hits        17396    17503     +107     
+ Misses      19501    19490      -11     
- Partials      910      920      +10

Impacted Files	Coverage Δ
api/src/config.rs	`70.56% <ø> (ø)`
storage/src/cache/worker.rs	`62.25% <ø> (ø)`
storage/src/backend/localdisk.rs	`48.10% <53.43%> (+31.35%)`	⬆️

... and 2 files with indirect coverage changes

anolis-bot · 2023-06-21T08:45:19Z

@jiangliu , The CI test is completed, please check result:

Test Case		Test Result
build rust golang image		✅ SUCCESS
compile nydusd		❌ FAIL

Sorry, your test job failed. Please get the details in the link.

anolis-bot · 2023-06-21T08:47:07Z

@jiangliu , the code has been updated, so a new test job has been submitted. Please wait in patience. The test job url: https://tone.openanolis.cn/ws/nrh4nnio/test_result/80080

anolis-bot · 2023-06-21T09:15:23Z

@jiangliu , The CI test is completed, please check result:

Test Case		Test Result
build rust golang image		✅ SUCCESS
compile nydusd		✅ SUCCESS
compile ctr remote		✅ SUCCESS
compile nydus snapshotter		✅ SUCCESS
run container with rafs		✅ SUCCESS
run container with zran		✅ SUCCESS
run container with rafs and compile linux		✅ SUCCESS

Congratulations, your test job passed!

adamqqqplay · 2023-06-23T04:03:52Z

@jiangliu Unit tests on older versions of the localdisk image have passed, which means your code is compatible. For the test code, please refer to https://github.com/adamqqqplay/image-service/tree/localdisk-test

Regarding the review of the code itself, I still need some time. Thanks for your work!

adamqqqplay

Thanks for your work, I made some small comments on the code changes, mainly focusing on the maintainability of the module.

adamqqqplay · 2023-06-27T07:04:40Z

storage/Cargo.toml

@@ -45,7 +45,8 @@ tar = "0.4.38"
 regex = "1.7.0"

 [features]
-backend-localdisk = ["gpt"]
+backend-localdisk = []
+backend-localdisk-gpt = ["gpt", "backend-localdisk"]


I'm a bit confused about this new feature, can you explain what it's for? I only see one feature on other backends.

It's to support localdisk without gpt. Gpt is just one way to discover nydus blobs inside a disk, but not the only way.

OK, got it.

storage/src/backend/localdisk.rs

adamqqqplay · 2023-06-27T07:55:12Z

storage/src/backend/localdisk.rs

            } else {
-                v.name.clone() + guid.to_simple().to_string().as_str() // The 64-byte blob_id is stored in two parts
+                // The 64-byte blob_id is stored in two parts


I think this case should also belong to gpt mode.

The reason for the judgment of part_type_guid above comes from a compatibility issue.

When part_type_guid == gpt::partition_types::BASIC, the blob_id only exists in the name field of the partition.

When part_type_guid == gpt::partition_types::LINUX_FS, the blob_id exists in the name field and in the guid field.

For the corresponding changes, please refer to: adamqqqplay/nydus-localdisk@4812dcb . BTW, the design of the image building did have some hacks before. If you have any suggestions, I could refactor them.

I missed your point here. This piece of code is enabled for gpt only.
For the backward compatibility, it would be better to remove if we can.

Okay, I will refactor this part later. Now, I think the code might look like this to be compatible.

let name = if v.part_type_guid == gpt::partition_types::BASIC { is_gpt_mode = true; // Compatible with old versions of localdisk image v.name.clone() } else if v.part_type_guid == gpt::partition_types::LINUX_FS { is_gpt_mode = true; // The 64-byte blob_id is stored in two parts v.name.clone() + guid.to_simple().to_string().as_str() } else { // **Your new code block. The part_type_guid not found** // The 64-byte blob_id is stored in two parts v.name.clone() + guid.to_simple().to_string().as_str() }

storage/src/backend/localdisk.rs

adamqqqplay · 2023-06-27T08:04:26Z

api/src/config.rs

@@ -443,6 +443,9 @@ pub struct LocalDiskConfig {
    /// Mounted block device path or original localdisk image file path.
    #[serde(default)]
    pub device_path: String,
+    /// Disable discover blob objects by scanning GPT table.
+    #[serde(default)]
+    pub disable_gpt: bool,


The variable disable_gpt has been used here to control the localdisk mode. Is it necessary to use the feature backend-localdisk-gpt for conditional compilation at the same time?

Let's keep it for simplicity?

anolis-bot · 2023-06-30T08:20:20Z

@jiangliu , the code has been updated, so a new test job has been submitted. Please wait in patience. The test job url: https://tone.openanolis.cn/ws/nrh4nnio/test_result/81810

anolis-bot · 2023-06-30T08:50:06Z

@jiangliu , The CI test is completed, please check result:

Test Case		Test Result
build rust golang image		✅ SUCCESS
compile nydusd		✅ SUCCESS
compile ctr remote		✅ SUCCESS
compile nydus snapshotter		✅ SUCCESS
run container with rafs		✅ SUCCESS
run container with zran		✅ SUCCESS
run container with rafs and compile linux		✅ SUCCESS

Congratulations, your test job passed!

adamqqqplay · 2023-07-06T02:22:04Z

I have no other comments, @imeoer please take a look. Thanks!

storage/src/backend/localdisk.rs

Introduce feature `backend-localdisk-gpt` for localdisk storage backend, so it can be optionally disabled. Signed-off-by: Jiang Liu <gerry@linux.alibaba.com>

Use File instead of RawFd in struct LocalDiskBlob to avoid possible race conditions. Signed-off-by: Jiang Liu <gerry@linux.alibaba.com>

Enhance the localdisk storage backend, so we can manually add blob objects in the disk, in addition to discovering blob objects by scanning GPT partition table. Signed-off-by: Jiang Liu <gerry@linux.alibaba.com>

anolis-bot · 2023-07-18T01:55:27Z

@jiangliu , the code has been updated, so a new test job has been submitted. Please wait in patience. The test job url: https://tone.openanolis.cn/ws/nrh4nnio/test_result/85262

anolis-bot · 2023-07-18T02:06:42Z

@jiangliu , The CI test is completed, please check result:

Test Case		Test Result
build rust golang image		✅ SUCCESS
compile nydusd		✅ SUCCESS
compile ctr remote		✅ SUCCESS
compile nydus snapshotter		✅ SUCCESS
run container with rafs		✅ SUCCESS
run container with zran		✅ SUCCESS
run container with rafs and compile linux		✅ SUCCESS

Congratulations, your test job passed!

jiangliu requested a review from a team as a code owner June 21, 2023 08:25

jiangliu requested review from liubogithub, imeoer and hsiangkao and removed request for a team June 21, 2023 08:25

anolis-bot added the anolis_testing label Jun 21, 2023

adamqqqplay self-requested a review June 21, 2023 08:27

anolis-bot added anolis_test_fail and removed anolis_testing labels Jun 21, 2023

jiangliu force-pushed the localdisk branch from a720da9 to 5dc39e9 Compare June 21, 2023 08:46

anolis-bot added anolis_testing and removed anolis_test_fail labels Jun 21, 2023

anolis-bot added anolis_test_pass and removed anolis_testing labels Jun 21, 2023

adamqqqplay reviewed Jun 27, 2023

View reviewed changes

jiangliu force-pushed the localdisk branch from 5dc39e9 to 4e42902 Compare June 30, 2023 08:20

anolis-bot added anolis_testing and removed anolis_test_pass labels Jun 30, 2023

anolis-bot added anolis_test_pass and removed anolis_testing labels Jun 30, 2023

jiangliu force-pushed the localdisk branch from 4e42902 to 7a0e4c4 Compare July 14, 2023 09:15

imeoer reviewed Jul 15, 2023

View reviewed changes

storage/src/backend/localdisk.rs Outdated Show resolved Hide resolved

jiangliu added 3 commits July 18, 2023 09:49

storage: introduce feature backend-localdisk-gpt

6cf4a35

Introduce feature `backend-localdisk-gpt` for localdisk storage backend, so it can be optionally disabled. Signed-off-by: Jiang Liu <gerry@linux.alibaba.com>

storage: use File instead of RawFd to avoid possible race conditions

92113d0

Use File instead of RawFd in struct LocalDiskBlob to avoid possible race conditions. Signed-off-by: Jiang Liu <gerry@linux.alibaba.com>

storage: support manually add blob object to localdisk backend driver

c1b1c7c

Enhance the localdisk storage backend, so we can manually add blob objects in the disk, in addition to discovering blob objects by scanning GPT partition table. Signed-off-by: Jiang Liu <gerry@linux.alibaba.com>

jiangliu force-pushed the localdisk branch from 7a0e4c4 to c1b1c7c Compare July 18, 2023 01:53

anolis-bot added anolis_testing and removed anolis_test_pass labels Jul 18, 2023

anolis-bot added anolis_test_pass and removed anolis_testing labels Jul 18, 2023

imeoer approved these changes Jul 18, 2023

View reviewed changes

imeoer merged commit be52ebd into dragonflyoss:master Jul 18, 2023
24 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enhance localdisk storage backend to manually add blob objects #1337

Enhance localdisk storage backend to manually add blob objects #1337

jiangliu commented Jun 21, 2023

anolis-bot commented Jun 21, 2023

codecov bot commented Jun 21, 2023 •

edited

anolis-bot commented Jun 21, 2023

anolis-bot commented Jun 21, 2023

anolis-bot commented Jun 21, 2023

adamqqqplay commented Jun 23, 2023

adamqqqplay left a comment

adamqqqplay Jun 27, 2023

jiangliu Jun 30, 2023

adamqqqplay Jul 6, 2023

adamqqqplay Jun 27, 2023

jiangliu Jun 30, 2023

adamqqqplay Jul 6, 2023

adamqqqplay Jun 27, 2023

jiangliu Jun 30, 2023

adamqqqplay Jul 6, 2023

anolis-bot commented Jun 30, 2023

anolis-bot commented Jun 30, 2023

adamqqqplay commented Jul 6, 2023

anolis-bot commented Jul 18, 2023

anolis-bot commented Jul 18, 2023

Enhance localdisk storage backend to manually add blob objects #1337

Enhance localdisk storage backend to manually add blob objects #1337

Conversation

jiangliu commented Jun 21, 2023

Relevant Issue (if applicable)

Details

Types of changes

Checklist

anolis-bot commented Jun 21, 2023

codecov bot commented Jun 21, 2023 • edited

Codecov Report

anolis-bot commented Jun 21, 2023

anolis-bot commented Jun 21, 2023

anolis-bot commented Jun 21, 2023

adamqqqplay commented Jun 23, 2023

adamqqqplay left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

anolis-bot commented Jun 30, 2023

anolis-bot commented Jun 30, 2023

adamqqqplay commented Jul 6, 2023

anolis-bot commented Jul 18, 2023

anolis-bot commented Jul 18, 2023

codecov bot commented Jun 21, 2023 •

edited