-
Notifications
You must be signed in to change notification settings - Fork 214
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
storage: introduce new backend localdisk in nydusd #972
storage: introduce new backend localdisk in nydusd #972
Conversation
@adamqqqplay , the title of this PR starts with "WIP" or "[WIP]", so skip testing for now. |
} | ||
|
||
// Disk names in GPT tables cannot store full 64-byte blob ids, so we should truncate them to 32 bytes. | ||
fn truncate_blob_id(blob_id: &str) -> LocalDiskResult<&str> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about storing binary rather than strings? Then 32 bytes can hold the whole blob id
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about storing binary rather than strings? Then 32 bytes can hold the whole blob id
Very good suggestion, I will try to modify this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried to use the storage method of Hex to String, but there are still problems in the implementation, and I will improve it later.
In the future, I will use two different methods to obtain the blob_id in get_blob() to maintain compatibility with different versions of the localdisk image.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about storing binary rather than strings? Then 32 bytes can hold the whole blob id
Problem fixed in 7f8a808 @changweige
How to create partitions filled with blobs and run with this PR? |
https://github.com/adamqqqplay/nydus-localdisk |
0abd285
to
b0684c1
Compare
@adamqqqplay , the title of this PR starts with "WIP" or "[WIP]", so skip testing for now. |
b0684c1
to
d2dabdc
Compare
@adamqqqplay , the title of this PR starts with "WIP" or "[WIP]", so skip testing for now. |
@adamqqqplay , the title 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/47122 |
@adamqqqplay , The CI test is completed, please check result:
Congratulations, your test job passed! |
@adamqqqplay , the title 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/47247 |
@adamqqqplay , The CI test is completed, please check result:
Sorry, your test job failed. Please get the details in the link. |
@adamqqqplay , 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/48450 |
@adamqqqplay , The CI test is completed, please check result:
Congratulations, your test job passed! |
@@ -151,6 +151,25 @@ We are working on enabling cloud-hypervisor support for nydus. | |||
} | |||
``` | |||
|
|||
##### Localdisk Backend | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we explain what scenario will use localdisk and how we make a localdisk device here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I'll add more description here.
} | ||
|
||
// Disk names in GPT tables cannot store full 64-byte blob ids, so we should truncate them to 32 bytes. | ||
fn truncate_blob_id(blob_id: &str) -> LocalDiskResult<&str> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the blob id is truncated in GPT table, how do we ensure no conflicts between different blobs with the same sha256 prefix?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When the output of SHA-256 is truncated to 128 bits, its collision resistance is still 2^64, which is still enough to prevent blob collisions. And in the SHA standard, the result is also allowed to be truncated.
Please refer to:
https://security.stackexchange.com/questions/34796/truncating-the-output-of-sha256-to-128-bits
https://security.stackexchange.com/questions/72673/how-bad-is-it-to-truncate-a-hash/72685#72685
Despite this, I'll try to store the full blob id in the future, while maintaining compatibility with images generated in the past.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the blob id is truncated in GPT table, how do we ensure no conflicts between different blobs with the same sha256 prefix?
Problem fixed in latest commit. Now the full 64-bit layer digest (blob id) stored in the Partition Name and Partition GUID respectively.
At the same time, forward and backward compatibility is maintained.
d1c0649
to
7f8a808
Compare
@adamqqqplay , 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/48844 |
@adamqqqplay , The CI test is completed, please check result:
Congratulations, your test job passed! |
docs/nydusd.md
Outdated
@@ -151,6 +151,31 @@ We are working on enabling cloud-hypervisor support for nydus. | |||
} | |||
``` | |||
|
|||
##### Localdisk Backend |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe mark this Localdisk Backend (Experimental)
before we have supported it in nydusify.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe mark this
Localdisk Backend (Experimental)
before we have supported it in nydusify.
OK, that's good.
7f8a808
to
5b54510
Compare
@adamqqqplay , 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/49092 |
@adamqqqplay , The CI test is completed, please check result:
Congratulations, your test job passed! |
This patch adds a new storage backend localdisk which supports reading blobs on disk. In this scenario, each layer of the blob is stored in partitions, and multiple partitions are addressed in the local raw disk via the GUID partition table (GPT), which means that this disk stores the entire image. Signed-off-by: Qinqi Qu <[email protected]>
5b54510
to
44ced99
Compare
@adamqqqplay , 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/49212 |
@adamqqqplay , The CI test is completed, please check result:
Congratulations, your test job passed! |
The localdisk backend adds support for storing images in disks. In this scenario, each layer of the blob is stored in partitions, and multiple partitions are addressed in the local raw disk via the GUID partition table (GPT), which means that this disk stores the entire image.
Currently, generating a localdisk image through
nydusify
is not supported for the time being. You need to use thenydus-localdisk
tool to complete this step.Test document located at: https://github.com/adamqqqplay/nydus-localdisk/blob/master/README.md
This PR updates: #971
Signed-off-by: Qinqi Qu [email protected]