Add more mount information to errors #837
Conversation
|
The goal is to help with containers/podman#9507 |
|
@edsantiago PTAL |
drivers/overlay/overlay.go
Outdated
| @@ -1169,7 +1169,7 @@ func (d *Driver) get(id string, disableShifting bool, options graphdriver.MountO | |||
| flags, data := mount.ParseOptions(mountData) | |||
| logrus.Debugf("overlay: mount_data=%s", mountData) | |||
| if err := mountFunc("overlay", mountTarget, "overlay", uintptr(flags), data); err != nil { | |||
| return "", fmt.Errorf("error creating overlay mount to %s: %v", mountTarget, err) | |||
| return "", fmt.Errorf("error creating overlay mount to %s, mount_data=%s: %v", mountTarget, mountData, err) | |||
There was a problem hiding this comment.
Will mountData ever include colons or spaces? If so would %q make those instances easier to read? Sorry for not testing on my end; I don't have much time today.
drivers/overlay/overlay.go
Outdated
| @@ -1159,7 +1159,7 @@ func (d *Driver) get(id string, disableShifting bool, options graphdriver.MountO | |||
| } | |||
| mountData = label.FormatMountLabel(opts, options.MountLabel) | |||
| if len(mountData) > pageSize { | |||
| return "", fmt.Errorf("cannot mount layer, mount label too large %d", len(mountData)) | |||
| return "", fmt.Errorf("cannot mount layer, mount label %s too large %d", options.MountLabel, len(mountData)) | |||
There was a problem hiding this comment.
If I were seeing this error as a user my first question would be, "what is the limit?" Is this a likely error condition? If so, it might be polite to include something like "length (%d) must be <= %d", ..., len(mountData), pageSize)
There was a problem hiding this comment.
This is a very unlikely message.
|
@rhatdan needs a rebase and I concur with @edsantiago 's questions. |
Currently it is difficult to diagnose mount failures in overlay storage. This issue is we are not sure of all of the mount options used when attempting to mount the file system. This PR adds missing data. Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
|
LGTM |
There was a problem hiding this comment.
LGTM
the issue with containers/podman#9507 seems to be at unmount time though, should we improve logging there as well?
EDIT: nevermind my suggestion, there is not much to improve there and the podman issue is caused by mount
Currently it is difficult to diagnose mount failures in overlay storage.
This issue is we are not sure of all of the mount options used when
attempting to mount the file system.
This PR adds missing data.
Signed-off-by: Daniel J Walsh dwalsh@redhat.com