Handle invalid FS type on mount. #639
Conversation
|
Fixes #638 |
| @@ -82,7 +82,7 @@ | |||
| # Filesystem type | |||
| # This option is handled in the volume-plugin at the docker host, and tracked in volume metadata. | |||
| FILESYSTEM_TYPE = 'fstype' | |||
| DEFAULT_FILESYSTEM_TYPE = '' | |||
| DEFAULT_FILESYSTEM_TYPE = 'ext4' | |||
There was a problem hiding this comment.
Aren't old volumes formatted with ext2?
govint
force-pushed
the
fix-fstype
branch
2 times, most recently
from
f021a86
to
91af52e
Compare
There was a problem hiding this comment.
I am fine with proposed change. Report an appropriate error. No backward compatibility.
I believe this change will also help if Host is attempting to mount volume created and formatted with filesystem which is not available on current Host.
Minor comment below. Please update and then LGTM.
Forgot to mention that you should update PR description to reflect new behavior.
| log.WithFields( | ||
| log.Fields{"name": r.Name, "error": "Invalid filesystem type."}, | ||
| ).Error("Failed to mount ") | ||
| return volume.Response{Err: "Failed mount - invalid filesystem type."} |
There was a problem hiding this comment.
minor edit: "Failed to mount - Invalid filesystem type"
| @@ -84,14 +84,18 @@ func getMountPoint(volName string) string { | |||
|
|
|||
| // Get info about a single volume | |||
| func (d *vmdkDriver) Get(r volume.Request) volume.Response { | |||
| log.Debug("calling GET ") | |||
There was a problem hiding this comment.
I think this debug should be one layer above, when misc. methods are called.
There was a problem hiding this comment.
Sorry this was a debug added to root cause, removed this.
There was a problem hiding this comment.
Removed the code to return a default FS type. The change to add FS type for a volume effectively uses the KV on the server to store meta-data that the client wants persisted. Am changing the client to also fall back to ext2 in case the server sends an invalid response for fstype (empty)
| status, err := d.ops.Get(r.Name) | ||
| log.Debug("returned GET ") |
|
|
||
| if !exists { | ||
| log.WithFields( | ||
| log.Fields{"name": r.Name, "error": "Invalid filesystem type."}, |
There was a problem hiding this comment.
can you use a var for message instead of copy-n-paste here and on line 374 ? Also I believe the text ("failed mount - invalid filesystem type" ) should be the same in both cases, and ideally include some hint on what to do next. Though I have no idea what to do next ...
govint
changed the title
|
With old volumes the client plugin just appears hung (no response causing Yes, ext2 is the default. On Fri, Oct 21, 2016 at 6:39 AM, Prashant Dhamdhere <
|
* master: (25 commits) Update new ESX IP added forgotten .so file Install sqlite3 py libs on ESX and load for Python2 Added py code and binaries for sqlite3 python libs Update drone security Removed accidental .pyc files Handle byte to string conversions for status command. Auth configuration and operation admission check (Auth.liping) (vmware-archive#603) Revert "Cli auth.liping" Cli auth.liping (vmware-archive#640) Handle missing or invalid fs type on mount. (vmware-archive#639) Updated Admin CLI commands to support tenants. (vmware-archive#620) Workaround older versions of e2fsprogs (vmware-archive#631) Add auth proposal Made handing of missing metafile less harsh. (vmware-archive#627) Fixed ACLs in payload bin dir (vmware-archive#624) Fixed error handling for set command. (vmware-archive#610) Use new error variables when rolling back volume creation to avoid nil reassignment. (vmware-archive#617) Change wording Fix broken link ...
Handle invalid FS type returned by a GET call to the plugin server and fail back to using ext2 as FS type in such a case. This handles both any corruption on the server side plus makes a best effort to mount a volume with a default FS type.