Fix stats in LXD with ZFS storage pool #2189
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
Author
|
/test pull-cadvisor-e2e |
Collaborator
|
You will need to rebase on #2191, since the update to go 1.12 made our go-lint fail |
dashpole
reviewed
Mar 7, 2019
fs/fs.go
Outdated
| switch partition.fsType { | ||
| var fsType = partition.fsType | ||
| if fsType == ZFS.String() { | ||
| if _, devZfs := os.Stat("/dev/zfs"); os.IsNotExist(devZfs) { |
Collaborator
There was a problem hiding this comment.
I'm not a fan of defining a new fsType. Maybe put this inside of case ZFS.String(), and use goto default to revert to the default behavior when this is the case?
Contributor
Author
There was a problem hiding this comment.
Thank you for the review @dashpole. I moved the /dev/zfs check inside the ZFS case. Are you firmly set on using goto with an extra label (I believe the case label default does not qualify as a goto target)? I went with fallthrough what do you think?
Collaborator
There was a problem hiding this comment.
oh, if you can't goto default, then I don't know if that will work... Let me take another look
ktsakalozos
force-pushed
the
feature/lxd-zfs
branch
from
9e35944 to
03867d4
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When LXD containers are backed up by a ZFS storage pool they do not have the
/dev/zfsdevice. Trying to get the file system stats from that device will fail (va the zfs utility).With this PR, in the above setup (ZFS inside and LXD container), we default to getting the file system stats from VFS.
Here is the error you get when
/dev/zfsis not present: