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

Fix VFS vs quota regression #35827

Merged
merged 2 commits into from Dec 22, 2017

Conversation

Projects
None yet
6 participants
@kolyshkin
Contributor

kolyshkin commented Dec 18, 2017

- What I did
Fixed #35815

- How I did it

  1. ENOSYS from mknod() is now treated as "quota not supported".
  2. Any errors returned from quota.NewControl() are now ignored (and logged).

- How to verify it
Start daemon with vfs driver backed by osxfs

- Description for the changelog
Fix vfs graph driver failure to initialize because of failure to setup fs quota

projectquota: treat ENOSYS as quota unsupported
If mknod() returns ENOSYS, it most probably means quota is not supported
here, so return the appropriate error.

This is a conservative* fix to regression in vfs graph driver introduced
by commit 7a1618c ("add quota support to VFS graphdriver").
On some filesystems, vfs fails to init with the following error:

> Error starting daemon: error initializing graphdriver: Failed to mknod
> /go/src/github.com/docker/docker/bundles/test-integration/d6bcf6de610e9/root/vfs/backingFsBlockDev:
> function not implemented

Reported-by: Brian Goff <cpuguy83@gmail.com>
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
@thaJeztah

This comment has been minimized.

Show comment
Hide comment
@thaJeztah

thaJeztah Dec 18, 2017

Member

ping @cpuguy83 PTAL

/cc @sargun

Member

thaJeztah commented Dec 18, 2017

ping @cpuguy83 PTAL

/cc @sargun

@kolyshkin

This comment has been minimized.

Show comment
Hide comment
@kolyshkin

kolyshkin Dec 18, 2017

Contributor

Now this is a rather conservative fix, only making ENOSYS from mknod() a special case. A less conservative fix would be to treat any error from quota.NewControl() as "quota not supported", essentially ignoring any errors during quota init.

Contributor

kolyshkin commented Dec 18, 2017

Now this is a rather conservative fix, only making ENOSYS from mknod() a special case. A less conservative fix would be to treat any error from quota.NewControl() as "quota not supported", essentially ignoring any errors during quota init.

@cpuguy83

LGTM

@sargun

This comment has been minimized.

Show comment
Hide comment
@sargun

sargun Dec 19, 2017

Contributor

@kolyshkin I would change to making it so that any error returned from initializing quota in the VFS driver code marks it as quota not supported. It'd also fix the user namespaces case in a general way.

Contributor

sargun commented Dec 19, 2017

@kolyshkin I would change to making it so that any error returned from initializing quota in the VFS driver code marks it as quota not supported. It'd also fix the user namespaces case in a general way.

@kolyshkin

This comment has been minimized.

Show comment
Hide comment
@kolyshkin

kolyshkin Dec 19, 2017

Contributor

@sargun makes sense. I will keep this patch (as it makes sense for other graphdrivers, too) and add another one.

Contributor

kolyshkin commented Dec 19, 2017

@sargun makes sense. I will keep this patch (as it makes sense for other graphdrivers, too) and add another one.

vfs gd: ignore quota setup errors
This is a fix to regression in vfs graph driver introduced by
commit 7a1618c ("add quota support to VFS graphdriver").

On some filesystems, vfs fails to init with the following error:

> Error starting daemon: error initializing graphdriver: Failed to mknod
> /go/src/github.com/docker/docker/bundles/test-integration/d6bcf6de610e9/root/vfs/backingFsBlockDev:
> function not implemented

As quota is not essential for vfs, let's ignore (but log as a warning) any error
from quota init.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
@sargun

This comment has been minimized.

Show comment
Hide comment
@sargun

sargun Dec 20, 2017

Contributor

LGTM.

Contributor

sargun commented Dec 20, 2017

LGTM.

@thaJeztah

LGTM

@yongtang

This comment has been minimized.

Show comment
Hide comment
@yongtang

yongtang Dec 22, 2017

Member

All tests passed now, including z 🎉

Member

yongtang commented Dec 22, 2017

All tests passed now, including z 🎉

@yongtang yongtang merged commit 3e1df95 into moby:master Dec 22, 2017

6 checks passed

dco-signed All commits are signed
experimental Jenkins build Docker-PRs-experimental 38526 has succeeded
Details
janky Jenkins build Docker-PRs 47235 has succeeded
Details
powerpc Jenkins build Docker-PRs-powerpc 7649 has succeeded
Details
windowsRS1 Jenkins build Docker-PRs-WoW-RS1 18747 has succeeded
Details
z Jenkins build Docker-PRs-s390x 7494 has succeeded
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment