Install to /cache/rayhunter-data for tplink, add --data-dir parameter#886
Install to /cache/rayhunter-data for tplink, add --data-dir parameter#886
Conversation
3f85ed0 to
7f692af
Compare
da21bf1 to
f34e103
Compare
This fixes several space-related issues at once. We have observed the following phenomenon on TP-Link, Orbic and Moxee: - Filling /data bricks the device (broken wifi, broken rndis, broken display) - Filling /cache does not (it only bricks rayhunter if it's installed there, and it might break firmware updates) Therefore it would make sense to store the entire rayhunter installation in /cache. This is a great idea for TP-Link and Moxee, because /cache is significantly larger than /data. However, on Orbic, /data is significantly larger than /cache! This PR refactors orbic-network and tplink to use a shared codepath for setting up the data directory. A symlink is created at /data/rayhunter, and what it points to is device-specific: - Orbic will have its data at `/data/rayhunter-data` - There is a new alias `installer moxee` that overrides this to `/cache/rayhunter-data` - TP-Link will have its data at /cache/rayhunter-data when there's no SD card, and /media/whatever when there is one. In all cases, existing data is migrated to the new location. The user can switch back and forth between two values of --data-dir and the data will be moved over every time. This PR has one huge wart, and that is that the USB installer for Orbic remains untouched. The annoying reason for this is that the DeviceConnection trait is insufficient to reflect all the different kinds of shells you can have over USB: adb with fakeroot, and serial with real root. I think it's not possible to create the right directories with 'rootshell -c'. I'm thinking of spawning a telnet server over serial, so that we can just do telnet again, but this is for another time.
f34e103 to
b7e4293
Compare
|
Looks good to me, no concerns |
|
Is additional testing necessary? I've got 2 unbricked Moxees I can test on |
|
I've tested that the installer works on tplink orbit and moxee, and I think I did a test to manually fill the cache partition too. |
| let result = output | ||
| .lines() | ||
| .find_map(|line| line.trim().strip_prefix("RL:")) | ||
| .unwrap_or("") |
There was a problem hiding this comment.
is this not an error case?
| /// - If `/data/rayhunter` is a symlink to a different location, moves from the old target | ||
| /// - If `/data/rayhunter` doesn't exist, just creates the symlink | ||
| /// - If `/data/rayhunter` is a symlink to `data_dir`, does nothing | ||
| pub async fn setup_data_directory<C: DeviceConnection>(conn: &mut C, data_dir: &str) -> Result<()> { |
There was a problem hiding this comment.
it seems this only results in a new directory at data_dir if there was old data that needed migrating. should it not do a mkdir -p {data_dir} otherwise?
There was a problem hiding this comment.
Isn't that's what's on line 139?
| /// Overwrite config.toml even if it already exists on the device. | ||
| #[arg(long)] | ||
| reset_config: bool, | ||
|
|
There was a problem hiding this comment.
is this right? why are we migrating data_dir by default on Orbics?
There was a problem hiding this comment.
it was easier to make /data/rayhunter a symlink to some other directory in all cases than to special-case the orbic where /data/rayhunter is currently a dir. that's why the layout for orbic would end up with a symlink from /data/rayhunter to /data/rayhunter-data even though it's not really necessary
| /// Override the data directory path. Defaults to /cache/rayhunter-data (or SD card path when | ||
| /// SD card is used). Must not be /data/rayhunter. |
There was a problem hiding this comment.
i think including a small snippet explaining why would help illustrate why this parameter exists:
| /// Override the data directory path. Defaults to /cache/rayhunter-data (or SD card path when | |
| /// SD card is used). Must not be /data/rayhunter. | |
| /// Override the data directory path. Defaults to /cache/rayhunter-data (or SD card path when | |
| /// SD card is used). Must not be /data/rayhunter, which lives on a storage partition that's | |
| /// too small for normal Rayhunter operation. |
| } | ||
|
|
||
| async fn setup_rayhunter(admin_ip: &str, reset_config: bool) -> Result<()> { | ||
| async fn setup_rayhunter(admin_ip: &str, reset_config: bool, data_dir: &str) -> Result<()> { |
There was a problem hiding this comment.
i feel like this should be an optional arg, and we don't call setup_data_directory if it's None
|
|
||
| /// Check if a directory exists using a DeviceConnection | ||
| pub async fn dir_exists<C: DeviceConnection>(conn: &mut C, path: &str) -> bool { | ||
| conn.run_command(&format!("test -d {path} && echo exists || echo missing")) |
There was a problem hiding this comment.
The helpers (setup_data_directory, readlink, dir_exists, is_symlink) interpolate paths directly into shell commands format!("mv {old_source} {data_dir}").
A --data-dir path containing spaces would break here.
This fixes several space-related issues at once. Ref #865
We have observed the following phenomenon on TP-Link, Orbic and Moxee:
Filling
/databricks the device (broken wifi, broken rndis, brokendisplay)
Filling
/cachedoes not (it only bricks rayhunter if it's installedthere, and it might break firmware updates)
Therefore it would make sense to store the entire rayhunter installation
in
/cache.This is a great idea for TP-Link and Moxee, because
/cacheissignificantly larger than /data. However, on Orbic,
/dataissignificantly larger than
/cache!This PR refactors orbic-network and tplink to use a shared codepath for
setting up the data directory. A symlink is created at /data/rayhunter,
and what it points to is device-specific:
Orbic will have its data at
/data/rayhunter-dataMoxee will have its data at
/cache/rayhunter-data(you have to useinstaller moxeethough)TP-Link will have its data at
/cache/rayhunter-datawhen there's no SDcard, and
/media/whateverwhen there is one.In all cases, existing data is migrated to the new location. The user
can switch back and forth between two values of
--data-dirand the datawill be moved over every time.
This PR has one huge wart, and that is that the USB installer for Orbic
remains untouched. The annoying reason for this is that the
DeviceConnection trait is insufficient to reflect all the different
kinds of shells you can have over USB: adb with fakeroot, and serial
with real root. I think it's not possible to create the right
directories with
rootshell -c.I'm thinking of spawning a telnet server over serial, so that we can
just do telnet again, but this is for another time.