Storage managment API: disks infos#1953
Conversation
cb97422 to
6dbe2e1
Compare
6dbe2e1 to
a2b7e81
Compare
a2b7e81 to
a1a394f
Compare
|
Ok I think this is a good first step. Here is an exemple of the result this produces: In a future PR, I'd like to add the device type (HDD, SSD, …) and RPM for HDDs. I don't really know how to do that. TrueNAS does it by using its own library. In the meantime, this is ready for review. |
81d9fa7 to
e83e7b3
Compare
|
A few notes here: I rewrote this PR to use the udisks2 API which provides much more details about disks and is easier to use that udev + other sources of infos. But as a pure dbus API, the code is less self-explanatory. It's mainly about parsing dictionary. Maybe I can make use of something like Tell me if it's OK to add the udisk daemon to Ynh. I'm not aware of everything it entails. |
|
Naively udisk sounds fine to me 👍 |
|
Yes i think udisk is a good idea, also because it could help to automount external usb or disk... |
You can know if it's a HDD or SSD by checking directly the info inside /sys... 0 -> SSD /sys/block/sda/queue/removable to know if it's an usb removable disk i guess We could also imagine check for ssd how many written is made, in order to compute an end of life percentage. |
udisks2 provides the information so I don't need a solution anymore. Thanks. |
7d9c4de to
06e2219
Compare
| for k, v in manager.get_dbus_method( | ||
| "GetManagedObjects", "org.freedesktop.DBus.ObjectManager" | ||
| )().items(): |
There was a problem hiding this comment.
Does someone knows a more efficient way to interact with dbus than just list all the objects managed by Udisks2 here?
|
I don't believe it is useful to test this. We'd basically be testing the |
06e2219 to
bcfd1ae
Compare
bcfd1ae to
ea7051a
Compare
ea7051a to
0961615
Compare
zamentur
left a comment
There was a problem hiding this comment.
I suggest to split into 2 commands
yunohos storage disk list
yunohos storage disk info <disk>
| , python3-ldap, python3-zeroconf (>= 0.47), python3-lexicon, | ||
| , python3-cryptography, python3-jwt, python3-passlib, python3-magic | ||
| , python-is-python3, python3-pydantic, python3-email-validator | ||
| , udisks2, |
There was a problem hiding this comment.
We might add udisks2 suggested package to be sure those package will be installed on arm.
| , udisks2, | |
| , udisks2, udisks2-bcache udisks2-btrfs udisks2-lvm2 udisks2-zram |
| subcategory_help: Manage et get infos about hard-drives | ||
| actions: | ||
| # storage_disks_list | ||
| infos: |
There was a problem hiding this comment.
I suggest to be more consistent with the existing other command lines by splitting in 2 commands:
$ yunohost storage disk list [--with-info [--human-readable]]
$ yunohost storage disk info <disk_name> [--human-readable]
name: XXXX
model: YYYY
serial: ZZZZ
size: 120GB
type: SSD
rpm: None
There was a problem hiding this comment.
About --human-readable we are doing this in several part of the app, but regarding the cli is often use by a human, i ask me if we should not put by default the --human-readable and configure an other option to get the size in Bytes...
| "size": drive["Size"], | ||
| "type": "HDD" if rotation_rate != 0 else "SSD", | ||
| "rpm": rotation_rate if rotation_rate != 0 else None, | ||
| } |
There was a problem hiding this comment.
What if there are no disks at all ? We probably should display a message or something. Currently, it's just like this:
root@ynh:/ynh-dev# yunohost storage disk infos
root@ynh:/ynh-dev#
There was a problem hiding this comment.
When using the --human-readable option you mean?
There was a problem hiding this comment.
With --human-readable :
size: 60 GB
Without:
size: 64424509440
There was a problem hiding this comment.
No I meant about the case where there's no disks at all. With --human-readable, a message, without, empty list. Is that OK?
| "model": drive["Model"], | ||
| "serial": drive["Serial"], | ||
| "size": drive["Size"], | ||
| "type": "HDD" if rotation_rate != 0 else "SSD", |
There was a problem hiding this comment.
I have tested on my computer with an USB key, and i got -1
| "type": "HDD" if rotation_rate != 0 else "SSD", | |
| "type": "HDD" if rotation_rate <= 0 else "SSD", |
I made this suggestion, but we probably should distinguish USB key, SSD, HDD, SSHD (Nvme ?)
| "name": name, | ||
| "model": drive["Model"], | ||
| "serial": drive["Serial"], | ||
| "size": drive["Size"], |
There was a problem hiding this comment.
We probably need the accurate value in B and a more human value.
|
I tried to test into an incus container, but drives are not discovered by dbus (lsblk does). It's due to the fact that the main storage is on /dev/loopXX on incus. I tried to add an entire usb key, but it was a fail. |
d3c3f61 to
0877f01
Compare
|
@zamentur So I implemented you suggestions and tested with a USB flash drive, USB hard drive and SD card. Here are the resultsWith Without Weirdly, the USB flash drive is detected as being rotational with unkown RPM. I don't know how this should be handled. |
0877f01 to
45e30e6
Compare
45e30e6 to
7f659f7
Compare
| if human_readable and not result: | ||
| return "No external media found" | ||
|
|
||
| return result |
There was a problem hiding this comment.
| return result | |
| return {"disks": result} |
It's done like that in user_list backup_list and permission_list, with disks: no need to add a message to explain that there is no disks at all i guess.
There was a problem hiding this comment.
Ok, then I guess result should be a list and not a dict. What do you say?
9a14fcf to
00b390a
Compare
d2ea214 to
34b801f
Compare
| ACTIONS = [ | ||
| action | ||
| for action in OPTION_SUBTREE[category]["actions"].keys() | ||
| for action in OPTION_SUBTREE[category].get("actions", {}).keys() |
There was a problem hiding this comment.
Il n'y a pas de section action dans storage pour le moment.
83b05ba to
f22365d
Compare
f22365d to
d2a40cd
Compare
|
We probably should merge into a temporary branch to run all CI tests... |
The problem
Rel: YunoHost/issues#1823.
Add API for getting infos on hard drives.