Skip to content

Improve error message in cloud-init.log#1140

Merged
TheRealFalcon merged 2 commits into
canonical:mainfrom
KsenijaS:ks-fix_cloud_init_log
Dec 9, 2021
Merged

Improve error message in cloud-init.log#1140
TheRealFalcon merged 2 commits into
canonical:mainfrom
KsenijaS:ks-fix_cloud_init_log

Conversation

@KsenijaS
Copy link
Copy Markdown
Contributor

@KsenijaS KsenijaS commented Dec 7, 2021

Improve error message in cloud-init.log for images without ntfs support.

@holmanb
Copy link
Copy Markdown
Member

holmanb commented Dec 8, 2021

Hi @KsenijaS, thanks for the PR!

I agree that the current debug message may leave some things to be desired.

Currently if running on an image without ntfs support, I think an attempt to mount an ntfs file system here would produce a debug message that looks something like: "Failed mount of '/dev/sdaN' as 'ntfs': Stderr: mount: unknown filesystem type 'ntfs'". Is that correct?

I have a comment on the change you requested, as well as some potential ideas.

Since Linux (and BSD) has many different supported filesystems that may have similar error conditions, I think a more generalized solution would be preferred to one isolated error message for a specific fs/error condition combo.

There are a handful of questions that we don't currently answer with this error message that might be helpful to

  • What command was used to attempt the mount?
  • Does the block device exist?
  • What is does <ntfs/reiserfs/btrfs/etc> mean? (the message does not state that this is the mount/filesystem type, and several lesser known filesystem types could be very confusing to a large majority of users without adding the context that this is a filesystem type)

I think that these are things that we could add to keep the logic generalized to all filesystems, while still improving the error message.

Something like this, perhaps?

LOG.debug(
    "Failed to mount device: '%s' with type: '%s' using mount command: '%s' which caused exception: '%s'",
    device, mtype, ' '.join(mountcmd), exc)                                                             

Copy link
Copy Markdown
Contributor

@TheRealFalcon TheRealFalcon left a comment

Choose a reason for hiding this comment

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

Thanks!

@TheRealFalcon TheRealFalcon merged commit b591e9d into canonical:main Dec 9, 2021
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.

3 participants