Skip to content
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

nydusd: fixed getting the wrong chunk due to using the wrong blob index #573

Merged
merged 1 commit into from
Jul 7, 2022

Conversation

zyfjeff
Copy link
Contributor

@zyfjeff zyfjeff commented Jul 7, 2022

The blob index on the disk where erofs exists starts at 1, but the blob index in BlobInfo starts at 0,
and the blob index on the disk is mistakenly used here instead of blobInfo's index
this will eventually result in the wrong file contents being obtained.

Signed-off-by: zyfjeff zyfjeff@linux.alibaba.com

@zyfjeff zyfjeff changed the title nydus-image: fixed getting the wrong chunk content due to using the w… nydus-image: fixed getting the wrong chunk due to using the wrong blob index Jul 7, 2022
@zyfjeff
Copy link
Contributor Author

zyfjeff commented Jul 7, 2022

The integration test for this PR is covered by #572

@yqleng1987
Copy link
Contributor

@zyfjeff , a new test job has been submitted. Please wait in patience.

@yqleng1987
Copy link
Contributor

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

Test CaseTest Result
merge-target-branch✅SUCCESS
build-docker-image✅SUCCESS
compile-nydus✅SUCCESS
compile-ctr-remote✅SUCCESS
compile-nydus-snapshotter✅SUCCESS
start-nydus-snapshotter-config-containerd✅SUCCESS
run-container-with-nydus-image✅SUCCESS

Congratulations, your test job passed!

@hsiangkao
Copy link
Contributor

hsiangkao commented Jul 7, 2022

could you leave some words on what it will impact? it impact chunk info?

@changweige
Copy link
Contributor

Why does this patch relate to nydus-image? It looks like a change of nydusd runtime

@zyfjeff
Copy link
Contributor Author

zyfjeff commented Jul 7, 2022

Why does this patch relate to nydus-image? It looks like a change of nydusd runtime

You're right, I'll change it.

@zyfjeff zyfjeff changed the title nydus-image: fixed getting the wrong chunk due to using the wrong blob index nydusd: fixed getting the wrong chunk due to using the wrong blob index Jul 7, 2022
@yqleng1987
Copy link
Contributor

@zyfjeff , your pull request has been updated. A new test job will be submitted. Please wait in patience.

@yqleng1987
Copy link
Contributor

@zyfjeff , your test job has passed, and no need to test again.

@zyfjeff
Copy link
Contributor Author

zyfjeff commented Jul 7, 2022

could you leave some words on what it will impact? it impact chunk info?

This will cause the contents of the file to be confused, or the chunk index to go out of bounds

@changweige
Copy link
Contributor

I don't quite understand why data is corrupted. I think it can only cause wrongly generating descs which is a set of backend chunk IO, which means backend IO merging is low-efficient.

@zyfjeff
Copy link
Contributor Author

zyfjeff commented Jul 7, 2022

I don't quite understand why data is corrupted. I think it can only cause wrongly generating descs which is a set of backend chunk IO, which means backend IO merging is low-efficient.

This will result in a BlobIoVec containing different Blobs, but where BlobIoVec is used, it is assumed that they all belong to the same Blob, but it is not, which will result in chunks being taken from the wrong blob

…blob index

The blob index on the disk where erofs exists starts at 1, but the blob index in BlobInfo starts at 0,
and the blob index on the disk is mistakenly used here instead of blobInfo's index

Signed-off-by: zyfjeff <zyfjeff@linux.alibaba.com>
@changweige changweige merged commit 370b9cc into dragonflyoss:master Jul 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants