Skip to content

Conversation

@hsiangkao
Copy link
Member

What this PR does / why we need it:
Workaround if some SegmentMapping refers to end of tar (because the mapping doesn't record real remote mapping size), which could happen on EROFS meta indexes.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #

Please check the following list:

  • Does the affected code have corresponding tests, e.g. unit test, E2E test?
  • Does this change require a documentation update?
  • Does this introduce breaking changes that would require an announcement or bumping the major version?
  • Do all new files have an appropriate license header?

@hsiangkao
Copy link
Member Author

hsiangkao commented Oct 18, 2024

As pread(2) says:

On success, pread() returns the number of bytes read (a return of zero indicates end of file) and pwrite() returns the number of bytes written.

Note that it is not an error for a successful call to transfer fewer bytes than requested

Add a unit test to test the recent regression.

Signed-off-by: Gao Xiang <[email protected]>
@hsiangkao
Copy link
Member Author

Before:
image
After:
image

LOG_ERRNO_RETURN(0, (int)ret,
"failed to read from `-th file ( ` pread return: ` < size: `)",
m.tag, m_files[m.tag], ret, size);
size_t ret2 = m_files[m.tag]->pread((char *)buf + ret, size - ret, m.moffset * ALIGNMENT + ret);
Copy link
Contributor

Choose a reason for hiding this comment

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

check if ret < 0 here

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed

Currently it could happen on EROFS meta indexes.

Signed-off-by: Gao Xiang <[email protected]>
@BigVan BigVan merged commit b2262fe into containerd:main Nov 12, 2024
2 checks passed
@hsiangkao hsiangkao deleted the turbooci-erofs-fix branch November 12, 2024 07:25
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.

3 participants