Query size of device using devpath rather than devname - also enables more logging on error#144
Open
ogayot wants to merge 4 commits intocanonical:mainfrom
Open
Query size of device using devpath rather than devname - also enables more logging on error#144ogayot wants to merge 4 commits intocanonical:mainfrom
ogayot wants to merge 4 commits intocanonical:mainfrom
Conversation
The existing read_sys_block_size_bytes functions forces us to guess where to find a given device in the /sys/class/block hierarchy. While most of the time, /sys/class/block/<device>/size seems to be the right place to look, there have been reports of the file missing. We now provide another way to read the size using the DEVPATH udev variable instead. Hopefully, this should be a more accurate way to find the information we are looking for. Signed-off-by: Olivier Gayot <olivier.gayot@canonical.com>
Signed-off-by: Olivier Gayot <olivier.gayot@canonical.com>
If read_sys_devpath_size_bytes fails with ENOENT, we want to know whether it's the directory that does not exist of the file named "size". If the caller requests it, on error we now list the files contained in the /sys/<devpath> directory. If the directory does not exist, we will have a nested FileNotFoundError exception. Signed-off-by: Olivier Gayot <olivier.gayot@canonical.com>
This also enabled logging so we can figure out whether the directory or the file does not exist. Signed-off-by: Olivier Gayot <olivier.gayot@canonical.com>
Member
Author
|
Looks like it requires #145 (or I can simply revisit the type hints to use Union) |
Collaborator
|
Isn't it at least possible this is the race condition where udev processing a block device causes the device nodes for the devices partitions to disappear briefly? In that case I'd expect the devpath-based path to be missing too. Do we understand / remember why storage.py:blockdev_probe doesn't just look at the size attribute from udev? (it might give strange results for extended partitions perhaps?) |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
In LP:#2063433 and private reports associated to it, we observe that sometimes, accessing
/sys/class/block/<devname>/sizefails withFileNotFoundError.It isn't easy to determine whether the
sizefile does not exist or the parent directory itself.We now rely on a new way to retrieve the size, using the devpath instead of the devname. Hopefully, the devpath should always exist in sysfs if udev says so?
As for errors, we will now try to list the files from the parent directory when a
FileNotFoundExceptionis found as the result of the new function.