Skip to content

LCOW differ return ErrNotImplemented for wrong mount type#7112

Merged
estesp merged 1 commit intocontainerd:mainfrom
dcantah:lcow-notimpl
Jun 30, 2022
Merged

LCOW differ return ErrNotImplemented for wrong mount type#7112
estesp merged 1 commit intocontainerd:mainfrom
dcantah:lcow-notimpl

Conversation

@dcantah
Copy link
Copy Markdown
Member

@dcantah dcantah commented Jun 29, 2022

On Windows the two differs we register by default are the "windows" and "windows-lcow" differs. The diff service checks if Apply returns ErrNotImplemented and will move on to the next differ in the line if so. The Windows differ makes use of this to fallback to LCOW if it's determined the mount type passed is incorrect, but the LCOW differ does not return ErrNotImplemented for the same scenario. This puts a strict ordering requirement on the default differ entries in the config, namely that ["windows", "windows-lcow"] will work, as windows will correctly fall back to the lcow differ, but ["windows-lcow", "windows"] won't as the diff services Apply will just return the error directly. Not too much of an issue, but if you're hand editing a config and place lcow first it can be ran into.

On Windows the two differs we register by default are the "windows" and
"windows-lcow" differs. The diff service checks if Apply returns
ErrNotImplemented and will move on to the next differ in the line.
The Windows differ makes use of this to fallback to LCOW if it's
determined the mount type passed is incorrect, but the LCOW differ
does not return ErrNotImplemented for the same scenario. This puts
a strict ordering requirement on the default differ entries in the config,
namely that ["windows", "windows-lcow"] will work, as windows will correctly
fall back to the lcow differ, but ["windows-lcow", "windows"] won't as
the diff services Apply will just return the error directly.

Signed-off-by: Daniel Canter <[email protected]>
Copy link
Copy Markdown
Member

@kevpar kevpar left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Copy Markdown
Member

@estesp estesp left a comment

Choose a reason for hiding this comment

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

LGTM

@estesp estesp merged commit 7eae7f2 into containerd:main Jun 30, 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.

4 participants