Improving user feedback when timeshift fails

[prl]

Continuing the discussion about user feedback when creating hard links for timeshift fails (from Initial observations from a new user):

The problem with reformatting the HDD is that may do a lot more than just change the filesystem. If the HDD has multiple partitions, they will all be lost and replaced with one partition that spans the entire device.

I'd go with something along the lines of:

Timeshift directory is set to a location that does not support required features.

followed by a suggestion message that is specific to the error condition and possible remedy.

In the case of the timeshift location residing on a filesystem that is located on a device attached to a SATA/USB port, suggest using the HDD format option in Setup to wipe, repartition and format the entire device. If possible, identify the physical connection and brand/model of the drive that is the intended candidate.

Consider using Setup/Storage/Format HDD on device
WD40EZRX attached to Internal SATA port.

If the timeshift location appears to be on internal flash, warn about this being an unsupported scenario that could potentially corrupt the firmware. If the timeshift location appears to be on a network drive that does not support hard links, identify it as such. In both cases, suggest changing timeshift directory to a different device.

Either:

Timeshift location is on internal flash - this could lead to firmware corruption.

or:

Timeshift location is on a network drive.

in both cases followed by

Consider changing timeshift location to a different device.


Or to simplify matters:
1. If timeshift is on a SATA/USB storage device, suggest reformatting the drive and provide hints.
2. In all other cases, suggest changing timeshift location.

[prl]

There are rather a lot of error returns from link(2).

EPERM: (filesystem doesn't support hard links) Possible - this is the error that the code assumes to be the cause of the exception.

EIO: Possible, but probably should show different message and perhaps disable timeshift.
ENOSPC: (no space for directory entry) Possible, but probably should show different message and perhaps disable timeshift.
EFAULT: Possible but unlikely, but probably should re-throw the exception and cause a crash.

ELOOP: Possible, but would require some interference by the user.
EMLINK: Possible, but would require some interference by the user (more than 65000 links would be needed to be made to the file).
ENAMETOOLONG: Should not be possible without user interference in the settings file.

ENOMEM: Possible, but the whole system may be about to go down.

EXDEV: Probably not possible (the time gap available would make it hard for even deliberate user interference to cause this).

EACCES: Not possible (enigma2 runs as root)
EDQUOT: Not possible (enigma2 runs as root, no quotas enabled).
EEXIST: Not possible, link targets are removed first.
ENOENT: Not possible, the existence of the file and timeshift directory is verified in the code.
ENOTDIR: Not possible, the existence of the timeshift directory is implicitly verified in the code.
EPERM: (oldpath is marked immutable or append-only) Not possible: those flags are not applied to timeshift files.
EPERM: (oldpath is a directory) Not possible, the source (oldpath) of the link has been verified to be a file.
EROFS: Not possible, the link is being made to a file that has just been created on the device, and its existence has been checked.

[peteru]

Rather than waiting for an error to occur when making hard links, it may make more sense to perform the capability feature check first and report on the outcome of that. If the capability check passes (i.e. it looks like the file system ought to support timeshift), but there are errors, just report the errors. Maybe suggest to the user to check the timeshift location setting.

[peteru]

I had a look at the implementation of the hard link capability/feature check. I think it needs a bit of a refactor or at least a closer examination.

checkReadWriteDir() is mostly redundant, since it first calls checkTSFS() and that will only succeed if the directory is writable.

It's probably a good idea for checkTSFS() to cache the result of the most recent path it was called with to avoid unneeded I/O. The function could probably use a longer, more descriptive name.

[peteru]

BTW: A number of the errors you flagged as "not possible" are actually possible, but are not very likely. Think race conditions and kernel level security. This is another argument for not trying to handle the errors with numerous specific cases.

[prl]

I had a look at the implementation of the hard link capability/feature check. I think it needs a bit of a refactor or at least a closer examination.

Agreed.

It's probably a good idea for checkTSFS() to cache the result of the most recent path it was called with to avoid unneeded I/O.

That's not entirely safe, either, but then neither would it be if uncached or if its os.path.realpath() was cached and compared, and the latter would negate much of the I/O savings.