D28745: Skip caching thumbnails on encrypted filesystems

[Marcin Gurtowski]

marcingu added a comment.


  In D28745#676321 <https://phabricator.kde.org/D28745#676321>, @bruns wrote:
  
  > In D28745#676320 <https://phabricator.kde.org/D28745#676320>, @marcingu wrote:
  >
  > > In D28745#676317 <https://phabricator.kde.org/D28745#676317>, @bruns wrote:
  > >
  > > > In D28745#676313 <https://phabricator.kde.org/D28745#676313>, @marcingu wrote:
  > > >
  > > > > Ping!
  > > > >  I'm remanding about question early, because I could do much more work if I get to do it on weekend.
  > > > >
  > > > > Question:
  > > > >  This code won't save thumbnail for file on any device that isn't `StorageVolume` or is `StorageVolume` with `usage` `UsageType::Encrypded`.
  > > >
  > > >
  > > > The whole block can never return true, so it should just be removed, along with all its dependencies.
  > >
  > >
  > > I tested it once more and it returns true when it should, as expected. What makes you think it doesn't?
  >
  >
  > Even worse, its almost random:
  >
  >   udi = '/org/kde/fstab///pebbles/foo:/mnt'
  >     parent = '/org/kde/fstab'  (string)
  >     vendor = 'pebbles'  (string)
  >     product = 'foo:/mnt'  (string)
  >     description = 'foo:/mnt on pebbles'  (string)
  >     icon = 'network-server'  (string)
  >     StorageAccess.accessible = false  (bool)
  >     StorageAccess.filePath = '/mnt'  (string)
  >     StorageAccess.ignored = false  (bool)
  >     NetworkShare.type = 'Cifs'  (0x2)  (enum)
  >     NetworkShare.url = 'smb://pebbles/foo:/mnt'  (string)
  >
  >
  > `if (device.is<Solid::StorageVolume>())` -> false, though it should be cached
  >
  >   udi = '/org/freedesktop/UDisks2/block_devices/dm_2d2'
  >     parent = '/'  (string)
  >     vendor = ''  (string)
  >     product = ''  (string)
  >     description = '100,0 GiB Hard Drive'  (string)
  >     icon = 'drive-harddisk-root'  (string)
  >     Block.major = 254  (0xfe)  (int)
  >     Block.minor = 2  (0x2)  (int)
  >     Block.device = '/dev/dm-2'  (string)
  >     StorageAccess.accessible = true  (bool)
  >     StorageAccess.filePath = '/'  (string)
  >     StorageAccess.ignored = true  (bool)
  >     StorageVolume.ignored = false  (bool)
  >     StorageVolume.usage = 'FileSystem'  (0x2)  (enum)
  >     StorageVolume.fsType = 'btrfs'  (string)
  >     StorageVolume.label = ''  (string)
  >     StorageVolume.uuid = '5832ebfa-bf02-40d2-bdc7-90403b207b62'  (string)
  >     StorageVolume.size = 107374182400  (0x1900000000)  (qulonglong)
  >
  >
  > This is an LUKS encrypted volume so should not be cached ...
  
  
  Ok, so once more, let's go back to my question, but in more detail.
  
  Currently I get device as StorageAccess, because storage volumes don't include all of the mount points, such as mounted encfs encrypted data.
  This could be a problem as StorageAccess doesn't have an information about encryption (as far as I can tell) . As mentioned before, from lack of better solution, I assume false for storage devices that aren't StorageVolume, but it's not ideal.
  
  For the rest of devices I was relaying on `usage` from `StorageVolume`, but as it was pointed out, it holds `FileSystem` for LUKS encrypted volume, so it's either a bug in Solid or I'm using it wrong and I should get information about encryption elsewhere. I wasn't able to find anything else that could help though.
  
  When it comes to NetworkShare, I tested it with ssh and MTP. KIO won't even attempt to make previews for directories, so catching is out of question. I don't think we should change this behavior as a part of this story, but it would be good for code to work for them anyway, because we might decide to make those previews in the future or  use 
   the same mechanism for catching rest of the files.
  
  Do we want to store caches for NetworkShare devices and external storage? Currently we do, with exception of directories on NetworkShare and I see no need to change it, but it might be good to make sure.
  Are there any other corner cases we're forgetting?
  
  I also have a personal request for @bruns . Your responses tend to be laconic, often missing suggested solution for given issue and sometimes even pointing out what the problem is. This leads to me wasting a lot of time to find out something that would take you few moments to communicate.
  I'm willing to spend as much time as it takes to make this code as good as possible and I'm thankful for all responses so far, but you need to remember I'm only starting work with KDE code, so things that are obvious to you, often won't be as clear to me.
  
  In D28745#676322 <https://phabricator.kde.org/D28745#676322>, @thiago wrote:
  
  > BTW, have you checked if the thumbnails are still generated for non-removable but encrypted filesystems? My whole system is encrypted (except for /boot), so it would be a performance loss if no thumbnails were ever cached.
  
  
  Non-removable – not. Currently I do cache encrypted files when they are on the same device as thumbnails directory (~/.cache/thumbnails/). If you think we should cover some different cases as well, it's open for discussion.

REPOSITORY
  R320 KIO Extras

REVISION DETAIL
  https://phabricator.kde.org/D28745

To: marcingu, ivan, broulik, #dolphin, ngraham, meven, bruns, dfaure
Cc: dfaure, thiago, bruns, meven, ngraham, kde-frameworks-devel, kfm-devel, waitquietly, azyx, nikolaik, pberestov, iasensio, aprcela, fprice, LeGast00n, cblack, fbampaloukas, alexde, Codezela, feverfew, michaelh, spoorun, navarromorales, firef, andrebarros, emmanuelp, rdieter, mikesomov
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20200905/81fb4654/attachment.htm>