This commit is still work-in-progress, but I would really like to get some feedback to the approach. Does it make sense to add a new backend? Or should this functionality be merged with one of the other backends (I was considering the fstab backend)?

Thanks for working on this.

I'd probably call this fusemounts backend as it handles only FUSE mounts and nothing else.

As for the teardown operation, some FUSE backends (cryfs >0.10) have their own unmount commands instead of fusermount -u.

In D20938#459076, @ivan wrote:

Thanks for working on this.

I'd probably call this fusemounts backend as it handles only FUSE mounts and nothing else.

As for the teardown operation, some FUSE backends (cryfs >0.10) have their own unmount commands instead of fusermount -u.

Good idea for the name :D

How would I know how to unmount? As far as I can tell there is no "metadata" describing the mounted filesystem, so I guess the only way would be to have a list of known filesystems that need a special command, was that what you were thinking?

I'm torn between two approaches:

• doing what you have done, maybe with a customization point - fusermount -u by default, something else for specific mount types;
• disabling the teardown operation

The rationale for the second one:

• The users which create fuse mounts from the shell know how to unmount them;
• Users which used a tool to mount something (like Plasma Vault) should use the same tool to unmount (these tools potentially do more than simple unmounting - like calling a destructor in C++ instead of just free :) );
• Some applications will have their own mounts (for example, akonadi will use encrypted storage for storing indexes of encrypted mails) - you don't want to have those controlled by the user.*

(*) we will also need to hide these from Places, but that is not important at this point.

BTW I for one would really like FUSE mounts to show up in the Places panel. See for example https://bugs.kde.org/show_bug.cgi?id=404828

@ngraham

I guess that is the main point of this patch. I don't think that anyone is against that. :)

After reading the code a bit, I wonder

• can't some of the parsing code be made testable (and tests added)
  • This likely also requires to get rid of the global static.
• rather than poll /proc/mounts every second, I was wondering if one of the filesystem notification things (QFileSystemWatcher, KDirWatch) could tell when to re-parse it

WRT implementation, I'd also add a link to KMountPoint (https://api.kde.org/frameworks/kio/html/classKMountPoint.html) which provides an interface to fstab/mtab so that Solid/FuseMounts plugin does not need to do any parsing by itself.

It works with 'fuse.*'

Solid already has a working implementation for reading from /proc/mounts, the fstab backend.

Contrary to this code, the fstab backend does not poll every second using a timer, but correctly uses /proc/mounts changes notification via blocking read.

The only thing needed is a small extension to also hand out information for other filesystems than nfs and cifs.

In D20938#459149, @ivan wrote:

I'm torn between two approaches:

• doing what you have done, maybe with a customization point - fusermount -u by default, something else for specific mount types;
• disabling the teardown operation
  
  The rationale for the second one:
• The users which create fuse mounts from the shell know how to unmount them;
• Users which used a tool to mount something (like Plasma Vault) should use the same tool to unmount (these tools potentially do more than simple unmounting - like calling a destructor in C++ instead of just free :) );
• Some applications will have their own mounts (for example, akonadi will use encrypted storage for storing indexes of encrypted mails) - you don't want to have those controlled by the user.*
  
  (*) we will also need to hide these from Places, but that is not important at this point.

I am actually leaning mostly towards your second suggestion, disabling the teardown action. I think you described it very good that the user already knows how to use fuse and in the cases where he doesn't (e.g. when using RDP or something) then he probably shouldn't unmount the filesystem anyway.

@ngraham - do you have an opinion on this?

In D20938#459218, @bruns wrote:

Solid already has a working implementation for reading from /proc/mounts, the fstab backend.

Contrary to this code, the fstab backend does not poll every second using a timer, but correctly uses /proc/mounts changes notification via blocking read.

The only thing needed is a small extension to also hand out information for other filesystems than nfs and cifs.

This was exactly the kind of review comments I was looking for :D Let me take a new stab at this where I extend the current fstab with this functionality so we can see how that solution compares to this one.

In D20938#459627, @hallas wrote:

In D20938#459218, @bruns wrote:

Solid already has a working implementation for reading from /proc/mounts, the fstab backend.

Contrary to this code, the fstab backend does not poll every second using a timer, but correctly uses /proc/mounts changes notification via blocking read.

The only thing needed is a small extension to also hand out information for other filesystems than nfs and cifs.

This was exactly the kind of review comments I was looking for :D Let me take a new stab at this where I extend the current fstab with this functionality so we can see how that solution compares to this one.

I already have/had some code for this laying on my hard disk, I will post this as a WIP later today ...

In D20938#459628, @bruns wrote:

In D20938#459627, @hallas wrote:

In D20938#459218, @bruns wrote:

Solid already has a working implementation for reading from /proc/mounts, the fstab backend.

Contrary to this code, the fstab backend does not poll every second using a timer, but correctly uses /proc/mounts changes notification via blocking read.

The only thing needed is a small extension to also hand out information for other filesystems than nfs and cifs.

This was exactly the kind of review comments I was looking for :D Let me take a new stab at this where I extend the current fstab with this functionality so we can see how that solution compares to this one.

I already have/had some code for this laying on my hard disk, I will post this as a WIP later today ...

Great! Then I will hold my horses a bit!

I have tried to modify the fstab backend to also show fuse mounts and a very simple prototype is this:

diff --git a/src/solid/devices/backends/fstab/fstabhandling.cpp b/src/solid/devices/backends/fstab/fstabhandling.cpp
index 63130c6..3632b52 100644
--- a/src/solid/devices/backends/fstab/fstabhandling.cpp
+++ b/src/solid/devices/backends/fstab/fstabhandling.cpp
@@ -121,6 +121,11 @@ bool _k_isFstabNetworkFileSystem(const QString &fstype, const QString &devName)
     return false;
 }
 
+bool _k_isFstabFuseFileSystem(const QString &fstype)
+{
+    return fstype.startsWith(QLatin1String("fuse."));
+}
+
 void Solid::Backends::Fstab::FstabHandling::_k_updateFstabMountPointsCache()
 {
     if (globalFstabCache->m_fstabCacheValid) {
@@ -288,7 +293,7 @@ void Solid::Backends::Fstab::FstabHandling::_k_updateMtabMountPointsCache()
     STRUCT_MNTENT fe;
     while (GETMNTENT(mnttab, fe)) {
         QString type = QFile::decodeName(MOUNTTYPE(fe));
-        if (_k_isFstabNetworkFileSystem(type, QString())) {
+        if (_k_isFstabNetworkFileSystem(type, QString()) || _k_isFstabFuseFileSystem(type)) {
             const QString device = QFile::decodeName(FSNAME(fe));
             const QString mountpoint = QFile::decodeName(MOUNTPOINT(fe));
             globalFstabCache->m_mtabCache.insert(device, mountpoint);

the only caveat is that the mount points shows up in the "Remote" list, but that is probably easy to fix.

@bruns - please post the code you have when you get a chance to find it, otherwise I will continue to pursue this direction.

In D20938#460029, @hallas wrote:

I have tried to modify the fstab backend to also show fuse mounts and a very simple prototype is this:

diff --git a/src/solid/devices/backends/fstab/fstabhandling.cpp b/src/solid/devices/backends/fstab/fstabhandling.cpp
index 63130c6..3632b52 100644
--- a/src/solid/devices/backends/fstab/fstabhandling.cpp
+++ b/src/solid/devices/backends/fstab/fstabhandling.cpp
@@ -121,6 +121,11 @@ bool _k_isFstabNetworkFileSystem(const QString &fstype, const QString &devName)
     return false;
 }
 
+bool _k_isFstabFuseFileSystem(const QString &fstype)
+{
+    return fstype.startsWith(QLatin1String("fuse."));
+}
+
 void Solid::Backends::Fstab::FstabHandling::_k_updateFstabMountPointsCache()
 {
     if (globalFstabCache->m_fstabCacheValid) {
@@ -288,7 +293,7 @@ void Solid::Backends::Fstab::FstabHandling::_k_updateMtabMountPointsCache()
     STRUCT_MNTENT fe;
     while (GETMNTENT(mnttab, fe)) {
         QString type = QFile::decodeName(MOUNTTYPE(fe));
-        if (_k_isFstabNetworkFileSystem(type, QString())) {
+        if (_k_isFstabNetworkFileSystem(type, QString()) || _k_isFstabFuseFileSystem(type)) {
             const QString device = QFile::decodeName(FSNAME(fe));
             const QString mountpoint = QFile::decodeName(MOUNTPOINT(fe));
             globalFstabCache->m_mtabCache.insert(device, mountpoint);

the only caveat is that the mount points shows up in the "Remote" list, but that is probably easy to fix.

Yeah, that'll be fixable in https://cgit.kde.org/kio.git/tree/src/filewidgets/kfileplacesmodel.cpp

This patch has been superseeded by the wotk @bruns has done in D20995 - so closing this one :D