Draggon / hassio-hdd-tools

27 stars 13 forks source link

Update permissions to latest Supervisor #13

Closed McGiverGim closed 3 years ago

McGiverGim commented 3 years ago

Seems to fix https://github.com/Draggon/hassio-hdd-tools/issues/12 and fix https://github.com/Draggon/hassio-hdd-tools/issues/9

What I don't know is what will happen with people with "old" versions of Home Assistant, I can't find when this configuration was changed and since when is mandatory some of them. We can add a "homeassistant" tag too with the minimum version, but I don't know what will be the correct version to limit.

Another doubt: I've configured the addon with startup: "application" because the data can't be sent to HA until this has started, but maybe for some reason it was configured to start before (maybe because the speed test that is optional?). The options are:

Default application. initialize will start add-on on setup of Home Assistant. system is for things like databases and not dependent on other things. services will start before Home Assistant, while application is started afterwards. Finally once is for applications that don't run as a daemon.

Other testing is welcome to be sure I've not broken anything.

Another benefit of this changes is that the "full access" is not needed anymore in HA. At least in my tests it seems to work now without it.

McGiverGim commented 3 years ago

Looking at this again, maybe without full access we can't access devices if they are at a different mount point than /dev/sda.

SPEC1AL1ST commented 3 years ago

Looking at this again, maybe without full access we can't access devices if they are at a different mount point than /dev/sda.

Maybe you can fork and fix this git? If you know how.

McGiverGim commented 3 years ago

@SPEC1AL1ST I'm not too sure what are the final "needed" changes. With this changes it works in my system. I have my repo containing the changes of this PR in a temporary way (until merged in this official repo).

If you want, please test my repo: https://github.com/McGiverGim/hassio-hdd-tools

If it works, perfect, if not we can try to find a fix for your system.

boesing commented 3 years ago

Can confirm that this works for me on 2021.3.0

massive commented 3 years ago

Unfortunately this patch seems only to fix the permissions if you happen to monitor /dev/sda. I'm monitoring /dev/sdb, which still gives Operation not permitted even with this patch.

McGiverGim commented 3 years ago

@massive true, for this reason I created this other: https://github.com/Draggon/hassio-hdd-tools/pull/19 that let's disable the protection for any other mapping.

massive commented 3 years ago

@McGiverGim great. I already created a quick and dirty PR to fix this issue, but seems that you had a better fix in mind. I'll close mine.

McGiverGim commented 3 years ago

@massive I don't dislike your solution, it was discussed here: https://github.com/Draggon/hassio-hdd-tools/issues/10

Both solutions have pros and cons. If the repo was mine I will implement a fusion of both, but is not so it depends on others opinion too.