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

nydusify: bug fix, check subcommand uses direct mode #494

Merged
merged 1 commit into from
Jun 15, 2022
Merged

nydusify: bug fix, check subcommand uses direct mode #494

merged 1 commit into from
Jun 15, 2022

Conversation

yawqi
Copy link
Contributor

@yawqi yawqi commented Jun 14, 2022

The cache mode is used to speed up. However, when checking, especially the RAFS v6 haven't support the cache mode,
we should directly use direct mode.

What's more, the --fs-version hasn't been add to nydusify check's arg.

Signed-off-by: Qi Wang wangqi@linux.alibaba.com

@yawqi yawqi changed the title nydusify: check subcommand uses direct mode nydusify: bug fix, check subcommand uses direct mode Jun 14, 2022
imeoer
imeoer approved these changes Jun 14, 2022
View reviewed changes
Copy link
Collaborator

@imeoer imeoer left a comment

Choose a reason for hiding this comment

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

LGTM

liubogithub
liubogithub reviewed Jun 14, 2022
View reviewed changes
Copy link
Collaborator

@liubogithub liubogithub left a comment

Choose a reason for hiding this comment

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

What bug is this patch trying fix?

@yawqi
Copy link
Contributor Author

yawqi commented Jun 15, 2022

What bug is this patch trying fix?

The nydusify check subcommand doesn't offer a --fs-version option, so for both v5 and v6, the fs-version is the default 5, therefore, it will always use cache mode for both v5 and v6, which is not implemented for v6.
However the direct mode is implemented for both v5 and v6.

@yawqi yawqi force-pushed the master branch 4 times, most recently from 3cad1a5 to 94d2e6e Compare June 15, 2022 02:59
@jiangliu
Copy link
Collaborator

What bug is this patch trying fix?

The nydusify check subcommand doesn't offer a --fs-version option, so for both v5 and v6, the fs-version is the default 5, therefore, it will always use cache mode for both v5 and v6, which is not implemented for v6. However the direct mode is implemented for both v5 and v6.

Could we enable "--fs-version" for nydusfy check command?

@yawqi
nydusify: check subcommand uses direct mode
3433c4c 
The cache mode is used to speed up. However,
when checking, especially the RAFS v6 haven't
support the cache mode, we should directly use direct mode.

Signed-off-by: Qi Wang <wangqi@linux.alibaba.com>
@yawqi
Copy link
Contributor Author

yawqi commented Jun 15, 2022

What bug is this patch trying fix?

The nydusify check subcommand doesn't offer a --fs-version option, so for both v5 and v6, the fs-version is the default 5, therefore, it will always use cache mode for both v5 and v6, which is not implemented for v6. However the direct mode is implemented for both v5 and v6.

Could we enable "--fs-version" for nydusfy check command?

I am OK with that, but nydusify check actually is able to detect the fs-version I think. The fs-version is only used here actually, do we need to add it?

@jiangliu
Copy link
Collaborator

What bug is this patch trying fix?

The nydusify check subcommand doesn't offer a --fs-version option, so for both v5 and v6, the fs-version is the default 5, therefore, it will always use cache mode for both v5 and v6, which is not implemented for v6. However the direct mode is implemented for both v5 and v6.

Could we enable "--fs-version" for nydusfy check command?

I am OK with that, but nydusify check actually is able to detect the fs-version I think. The fs-version is only used here actually, do we need to add it?

@imeoer any suggestion?

@imeoer
Copy link
Collaborator

imeoer commented Jun 15, 2022 •
edited
Loading

@imeoer any suggestion?

@jiangliu nydusify check is able to detect fs-version of the image (via bootstrap magic number or annotation in image manifest), so we don't need a flag.

@jiangliu jiangliu merged commit 3d3ac61 into dragonflyoss:master Jun 15, 2022
@jiangliu jiangliu mentioned this pull request Jun 15, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@liubogithub liubogithub liubogithub left review comments

@imeoer imeoer imeoer approved these changes

@jiangliu jiangliu jiangliu approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@yawqi @jiangliu @imeoer @liubogithub