[FstabWatcher] Fix loosing of fstab watcher
ClosedPublic

Authored by bruns on Apr 12 2020, 6:21 PM.

Details

Summary

In case the fstab is deleted (caused by editing it), the watch will be
removed. The code already partially dealed with this, i.e. it tried to
readd the watch, but did not check if the operation succeeded and dropped
any further changes if not.

Keep a watch on the containing folder and readd the fstab on change
events. Unfortunately QFilesystemWatcher does not allow fine granular
change events, though as /etc/ is likely mostly silent this should not
matter much.

The bug becomes apparent when e.g. editing the fstab with VIM, which
has a quite broken implementation of creating temporary/backup files
during save: it moves away the original file and creates the new file
directly under the original name, instead of using a atomic rename.

Test Plan
  1. solid-hardware listen
  2. open fstab with vi, save a few times

Without the change, solid will loose the watch sooner or later.

Depends on D28779

Diff Detail

Repository
R245 Solid
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
bruns created this revision.Apr 12 2020, 6:21 PM
Restricted Application added a project: Frameworks. · View Herald TranscriptApr 12 2020, 6:21 PM
Restricted Application added a subscriber: kde-frameworks-devel. · View Herald Transcript
bruns requested review of this revision.Apr 12 2020, 6:21 PM
apol added a subscriber: apol.Apr 12 2020, 8:47 PM
apol added inline comments.
src/solid/devices/backends/fstab/fstabwatcher.cpp
112

Use m_isFstabWatched? Or maybe m_isFstabWatched isn't necessary altogether and we can check in other places too.

bruns added inline comments.Apr 12 2020, 8:59 PM
src/solid/devices/backends/fstab/fstabwatcher.cpp
112

No. QFileSystemWatcher looses the watch when the file is deleted/renamed, so m_isFstabWatched may be (still) true when contains() returns false.

bruns marked an inline comment as done.

Ping!

apol accepted this revision.Apr 20 2020, 5:32 PM
This revision is now accepted and ready to land.Apr 20 2020, 5:32 PM
ngraham accepted this revision.Apr 20 2020, 6:03 PM
This revision was automatically updated to reflect the committed changes.