-
Notifications
You must be signed in to change notification settings - Fork 97
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
Filesystem: refactor estargz support #77
Conversation
Prepare for removing stargz filesystem implementation. Since the stargz filesystem implementation is very similar to the nydus filesystem implementation, much of the code can be shared, for example the shared daemon related codes. We are going to merge them together, this PR moves codes, and breaks the current stargz implementation. Signed-off-by: Yan Song <yansong.ys@antfin.com>
Move pkg/filesystem/stargz to pkg/filesystem/fs/stargz. And remove stargz related codes from pkg/filesystem/fs, it's prepare for stargz support in pkg/filesystem/fs package directly. Signed-off-by: Yan Song <yansong.ys@antfin.com>
Add stargz handle logic in filesystem implementation, this commit recovery stargz support. Signed-off-by: Yan Song <yansong.ys@antfin.com>
Codecov Report
@@ Coverage Diff @@
## main #77 +/- ##
==========================================
+ Coverage 28.78% 29.44% +0.66%
==========================================
Files 20 17 -3
Lines 1567 1467 -100
==========================================
- Hits 451 432 -19
+ Misses 1049 971 -78
+ Partials 67 64 -3
Continue to review full report at Codecov.
|
pkg/filesystem/fs/fs.go
Outdated
// So we only need to read the MaxSuperBlockSize size to include both v5 and v6 superblocks | ||
const MaxSuperBlockSize = 8 * 1024 | ||
const RafsV6Magic = 0xE0F5E1E2 | ||
const ChunkInfoOffset = 1024 + 128 + 24 |
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.
RafsV6ChunkInfoOffset?
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.
Fixed, actually, these codes just moved from other place. :)
pkg/filesystem/fs/fs.go
Outdated
log.G(ctx).Infof("total nydus prepare data layer duration %d", duration.Milliseconds()) | ||
}() | ||
|
||
if fs.imageMode == OnDemand { |
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.
Move it to before the duration calculation? No need to calculate duration for preload mode.
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.
Also it's better to check fs.imageMode == Preload
directly.
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.
Fixed
pkg/filesystem/fs/fs.go
Outdated
} | ||
defer readerCloser.Close() | ||
|
||
blobFile, err := os.OpenFile(blobPath, os.O_CREATE|os.O_RDWR, 0755) |
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.
0666
?
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.
Fixed
return errors.Wrap(err, "failed to create blob file") | ||
} | ||
defer blobFile.Close() | ||
_, err = io.Copy(blobFile, readerCloser) |
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.
remove blob file upon error?
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.
Fixed
Signed-off-by: Yan Song <yansong.ys@antfin.com>
Since the stargz filesystem implementation is very similar
to the nydus filesystem implementation, much of the code
can be shared, for example the shared daemon related codes.
We are going to merge them together, this PR moves codes,
and adjust the code path for stargz support.