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.

Sign up for GitHub

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

Jump to bottom

fix: amplify io is too large to hold in fuse buffer #1311

Merged
merged 2 commits into from
Jul 12, 2023
Merged

fix: amplify io is too large to hold in fuse buffer #1311

merged 2 commits into from
Jul 12, 2023

Conversation

ccx1024cc
Copy link
Contributor

@ccx1024cc ccx1024cc commented Jun 5, 2023 •
edited
Loading

Fuse request buffer is fixed by FUSE_KERN_BUF_SIZE * pagesize() + FUSE_HEADER_ SIZE. When amplify io is larger than it, FuseDevWriter suffers from smaller buffer. As a result, invalid data error is returned.

Reproduction

run nydusd with 3MB amplify_io
error from random io:
    reply error header OutHeader { len: 16, error: -5, unique: 108 }, error Custom { kind: InvalidData, error: "data out of range, available 1052656 requested 1250066" }

Details

size of fuse buffer = 1052656 + 16 (size of inner header) = 256(FUSE_KERN_BUF_SIZE) * 4096(page size) + 4096(fuse header)
let amplify_io = min(user_specified, fuseWriter.available_bytes())

Resolution

Evalucation of amplify_io will be replaced with ZeroCopyWriter.available_bytes().

Relevant Issue

#1270
ci: https://github.com/dragonflyoss/image-service/actions/runs/4931748472/jobs/8814099799
#1312

@ccx1024cc ccx1024cc requested a review from a team as a code owner June 5, 2023 12:58
@ccx1024cc ccx1024cc requested review from imeoer, jiangliu and luodw and removed request for a team June 5, 2023 12:58
@anolis-bot
Copy link
Collaborator

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

@anolis-bot
Copy link
Collaborator

@ccx1024cc , 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/77064

@ccx1024cc ccx1024cc requested a review from bergwolf June 5, 2023 13:04
@anolis-bot
Copy link
Collaborator

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

Test CaseTest 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!

@anolis-bot
Copy link
Collaborator

@ccx1024cc , 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/77074

@anolis-bot
Copy link
Collaborator

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

Test CaseTest 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❌ FAIL

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

@anolis-bot
Copy link
Collaborator

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

Test CaseTest 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!

@anolis-bot
Copy link
Collaborator

@ccx1024cc , 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/77083

@anolis-bot
Copy link
Collaborator

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

Test CaseTest 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!

@anolis-bot
Copy link
Collaborator

@ccx1024cc , 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/77111

@anolis-bot
Copy link
Collaborator

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

Test CaseTest 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!

bergwolf
bergwolf approved these changes Jul 11, 2023
View reviewed changes
Copy link
Member

@bergwolf bergwolf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@bergwolf bergwolf mentioned this pull request Jul 11, 2023
Merged
6 tasks
fix: amplify io is too large to hold in fuse buffer
7e5b7e7 
Fuse request buffer is fixed by `FUSE_KERN_BUF_SIZE * pagesize() + FUSE_HEADER_ SIZE`. When amplify io is larger than it, FuseDevWriter suffers from smaller buffer. As a result, invalid data error is returned.

Reproduction:
    run nydusd with 3MB amplify_io
    error from random io:
        reply error header OutHeader { len: 16, error: -5, unique: 108 }, error Custom { kind: InvalidData, error: "data out of range, available 1052656 requested 1250066" }

Details:
    size of fuse buffer = 1052656 + 16 (size of inner header) = 256(page number) * 4096(page size) + 4096(fuse header)
    let amplify_io = min(user_specified, fuseWriter.available_bytes())

Resolution:
    This pr is not best implements, but independent of modification to [fuse-backend-rs]("https://github.com/cloud-hypervisor/fuse-backend-rs").
    In future, evalucation of amplify_io will be replaced with [ZeroCopyWriter.available_bytes()]("cloud-hypervisor/fuse-backend-rs#135").

Signed-off-by: 泰友 <[email protected]>
@anolis-bot
Copy link
Collaborator

@ccx1024cc , 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/84091

@anolis-bot
Copy link
Collaborator

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

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

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

@anolis-bot
Copy link
Collaborator

@ccx1024cc , 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/84094

@anolis-bot
Copy link
Collaborator

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

Test CaseTest 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!

@ccx1024cc ccx1024cc merged commit c8a39c8 into dragonflyoss:master Jul 12, 2023
@ccx1024cc ccx1024cc deleted the morgan/fix_amplify_io branch July 12, 2023 01:00
ccx1024cc added a commit to ccx1024cc/image-service that referenced this pull request Jul 12, 2023
@ccx1024cc
fix: amplify io is too large to hold in fuse buffer (dragonflyoss#1311)
ee79cfa 
* fix: amplify io is too large to hold in fuse buffer

Fuse request buffer is fixed by `FUSE_KERN_BUF_SIZE * pagesize() + FUSE_HEADER_ SIZE`. When amplify io is larger than it, FuseDevWriter suffers from smaller buffer. As a result, invalid data error is returned.

Reproduction:
    run nydusd with 3MB amplify_io
    error from random io:
        reply error header OutHeader { len: 16, error: -5, unique: 108 }, error Custom { kind: InvalidData, error: "data out of range, available 1052656 requested 1250066" }

Details:
    size of fuse buffer = 1052656 + 16 (size of inner header) = 256(page number) * 4096(page size) + 4096(fuse header)
    let amplify_io = min(user_specified, fuseWriter.available_bytes())

Resolution:
    This pr is not best implements, but independent of modification to [fuse-backend-rs]("https://github.com/cloud-hypervisor/fuse-backend-rs").
    In future, evalucation of amplify_io will be replaced with [ZeroCopyWriter.available_bytes()]("cloud-hypervisor/fuse-backend-rs#135").

Signed-off-by: 泰友 <[email protected]>

* feat: e2e for amplify io larger than fuse buffer

Signed-off-by: 泰友 <[email protected]>

---------

Signed-off-by: 泰友 <[email protected]>
Co-authored-by: 泰友 <[email protected]>
@ccx1024cc ccx1024cc mentioned this pull request Jul 12, 2023
Merged
ccx1024cc added a commit to ccx1024cc/image-service that referenced this pull request Jul 12, 2023
@ccx1024cc
fix: amplify io is too large to hold in fuse buffer (dragonflyoss#1311)
baf6304 
* fix: amplify io is too large to hold in fuse buffer

Fuse request buffer is fixed by `FUSE_KERN_BUF_SIZE * pagesize() + FUSE_HEADER_ SIZE`. When amplify io is larger than it, FuseDevWriter suffers from smaller buffer. As a result, invalid data error is returned.

Reproduction:
    run nydusd with 3MB amplify_io
    error from random io:
        reply error header OutHeader { len: 16, error: -5, unique: 108 }, error Custom { kind: InvalidData, error: "data out of range, available 1052656 requested 1250066" }

Details:
    size of fuse buffer = 1052656 + 16 (size of inner header) = 256(page number) * 4096(page size) + 4096(fuse header)
    let amplify_io = min(user_specified, fuseWriter.available_bytes())

Resolution:
    This pr is not best implements, but independent of modification to [fuse-backend-rs]("https://github.com/cloud-hypervisor/fuse-backend-rs").
    In future, evalucation of amplify_io will be replaced with [ZeroCopyWriter.available_bytes()]("cloud-hypervisor/fuse-backend-rs#135").

Signed-off-by: 泰友 <[email protected]>

* feat: e2e for amplify io larger than fuse buffer

Signed-off-by: 泰友 <[email protected]>

---------

Signed-off-by: 泰友 <[email protected]>
Co-authored-by: 泰友 <[email protected]>
bergwolf pushed a commit that referenced this pull request Jul 13, 2023
@ccx1024cc @bergwolf
fix: amplify io is too large to hold in fuse buffer (#1311)
b6ee7bb 
* fix: amplify io is too large to hold in fuse buffer

Fuse request buffer is fixed by `FUSE_KERN_BUF_SIZE * pagesize() + FUSE_HEADER_ SIZE`. When amplify io is larger than it, FuseDevWriter suffers from smaller buffer. As a result, invalid data error is returned.

Reproduction:
    run nydusd with 3MB amplify_io
    error from random io:
        reply error header OutHeader { len: 16, error: -5, unique: 108 }, error Custom { kind: InvalidData, error: "data out of range, available 1052656 requested 1250066" }

Details:
    size of fuse buffer = 1052656 + 16 (size of inner header) = 256(page number) * 4096(page size) + 4096(fuse header)
    let amplify_io = min(user_specified, fuseWriter.available_bytes())

Resolution:
    This pr is not best implements, but independent of modification to [fuse-backend-rs]("https://github.com/cloud-hypervisor/fuse-backend-rs").
    In future, evalucation of amplify_io will be replaced with [ZeroCopyWriter.available_bytes()]("cloud-hypervisor/fuse-backend-rs#135").

Signed-off-by: 泰友 <[email protected]>

* feat: e2e for amplify io larger than fuse buffer

Signed-off-by: 泰友 <[email protected]>

---------

Signed-off-by: 泰友 <[email protected]>
Co-authored-by: 泰友 <[email protected]>
bergwolf pushed a commit to bergwolf/nydus that referenced this pull request Jul 13, 2023
@ccx1024cc @bergwolf
fix: amplify io is too large to hold in fuse buffer (dragonflyoss#1311)
dfc08f1 
backport the logic of master commit c8a39c8

* fix: amplify io is too large to hold in fuse buffer

Fuse request buffer is fixed by `FUSE_KERN_BUF_SIZE * pagesize() + FUSE_HEADER_ SIZE`. When amplify io is larger than it, FuseDevWriter suffers from smaller buffer. As a result, invalid data error is returned.

Reproduction:
    run nydusd with 3MB amplify_io
    error from random io:
        reply error header OutHeader { len: 16, error: -5, unique: 108 }, error Custom { kind: InvalidData, error: "data out of range, available 1052656 requested 1250066" }

Details:
    size of fuse buffer = 1052656 + 16 (size of inner header) = 256(page number) * 4096(page size) + 4096(fuse header)
    let amplify_io = min(user_specified, fuseWriter.available_bytes())

Resolution:
    This pr is not best implements, but independent of modification to [fuse-backend-rs]("https://github.com/cloud-hypervisor/fuse-backend-rs").
    In future, evalucation of amplify_io will be replaced with [ZeroCopyWriter.available_bytes()]("cloud-hypervisor/fuse-backend-rs#135").

Signed-off-by: 泰友 <[email protected]>

* feat: e2e for amplify io larger than fuse buffer

Signed-off-by: 泰友 <[email protected]>

---------

Signed-off-by: 泰友 <[email protected]>
Co-authored-by: 泰友 <[email protected]>
Signed-off-by: Peng Tao <[email protected]>
@bergwolf bergwolf mentioned this pull request Jul 13, 2023
Merged
imeoer pushed a commit that referenced this pull request Jul 17, 2023
@ccx1024cc @imeoer
fix: amplify io is too large to hold in fuse buffer (#1311)
9727b85 
backport the logic of master commit c8a39c8

* fix: amplify io is too large to hold in fuse buffer

Fuse request buffer is fixed by `FUSE_KERN_BUF_SIZE * pagesize() + FUSE_HEADER_ SIZE`. When amplify io is larger than it, FuseDevWriter suffers from smaller buffer. As a result, invalid data error is returned.

Reproduction:
    run nydusd with 3MB amplify_io
    error from random io:
        reply error header OutHeader { len: 16, error: -5, unique: 108 }, error Custom { kind: InvalidData, error: "data out of range, available 1052656 requested 1250066" }

Details:
    size of fuse buffer = 1052656 + 16 (size of inner header) = 256(page number) * 4096(page size) + 4096(fuse header)
    let amplify_io = min(user_specified, fuseWriter.available_bytes())

Resolution:
    This pr is not best implements, but independent of modification to [fuse-backend-rs]("https://github.com/cloud-hypervisor/fuse-backend-rs").
    In future, evalucation of amplify_io will be replaced with [ZeroCopyWriter.available_bytes()]("cloud-hypervisor/fuse-backend-rs#135").

Signed-off-by: 泰友 <[email protected]>

* feat: e2e for amplify io larger than fuse buffer

Signed-off-by: 泰友 <[email protected]>

---------

Signed-off-by: 泰友 <[email protected]>
Co-authored-by: 泰友 <[email protected]>
Signed-off-by: Peng Tao <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bergwolf bergwolf bergwolf approved these changes

@jiangliu jiangliu Awaiting requested review from jiangliu jiangliu is a code owner automatically assigned from dragonflyoss/nydus-reviewers

@luodw luodw Awaiting requested review from luodw luodw is a code owner automatically assigned from dragonflyoss/nydus-reviewers

@imeoer imeoer Awaiting requested review from imeoer

Assignees
No one assigned
Labels
anolis_test_pass backport/v2.2
Projects
None yet
Milestone
No milestone
6 participants
@ccx1024cc @anolis-bot @jiangliu @bergwolf @adamqqqplay @imeoer