D23420: Use solid to check if a KFileItem is located on a network mount

[Méven Car]

meven abandoned this revision.
meven added a comment.


  Abandoned in favor of D26407  using 
KMountPoint which does close to the same thing as solid.

REPOSITORY
  R241 KIO

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

To: meven, dfaure, #frameworks
Cc: bruns, broulik, anthonyfieroni, kde-frameworks-devel, LeGast00n, GB_2, 
michaelh, ngraham

D23420: Use solid to check if a KFileItem is located on a network mount

[Stefan Brüns]

bruns added a comment.


  Solid should be quite fine. When queried for devices providing 
'Solid::DeviceInterface::NetworkShare', it will only use the fstab backend, 
which does not block and only reparses fstab/mtab when these change.
  
  There are some unnecessary QString constructions in the fstab backend, but 
these are easily solvable.
  
  For better performance, a global static cache with the network filepath()s 
would be an option.

REPOSITORY
  R241 KIO

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

To: meven, dfaure, #frameworks
Cc: bruns, broulik, anthonyfieroni, kde-frameworks-devel, LeGast00n, GB_2, 
michaelh, ngraham

D23420: Use solid to check if a KFileItem is located on a network mount

[David Faure]

dfaure added a comment.


  Heh, yeah, we have 3 APIs about mounted filesystems. Solid, KMountPoint, and 
KFileSystemType.
  
  KFileSystemType uses the blocking statvfs, while KMountPoint relies on a 
properly filled in /etc/fstab -- which breaks on the FreeBSD CI these days, 
some weird magic with containers not needing anything in /etc/fstab.
  
  It seems to me that we're still looking for the silver bullet solution in 
this area

REPOSITORY
  R241 KIO

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

To: meven, dfaure, #frameworks
Cc: broulik, anthonyfieroni, kde-frameworks-devel, LeGast00n, GB_2, michaelh, 
ngraham, bruns

D23420: Use solid to check if a KFileItem is located on a network mount

[Méven Car]

meven added a comment.


  I didn't know about `KMount`
  
  Gwenview uses it as we are trying here :
  
bool urlIsFastLocalFile(const QUrl )
{
if (!url.isLocalFile()) {
return false;
}

KMountPoint::List list = KMountPoint::currentMountPoints();
KMountPoint::Ptr mountPoint = list.findByPath(url.toLocalFile());
if (!mountPoint) {
// We couldn't find a mount point for the url. We are probably in a
// chroot. Assume everything is fast then.
return true;
}

return !mountPoint->probablySlow();
}

REPOSITORY
  R241 KIO

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

To: meven, dfaure, #frameworks
Cc: broulik, anthonyfieroni, kde-frameworks-devel, LeGast00n, GB_2, michaelh, 
ngraham, bruns

D23420: Use solid to check if a KFileItem is located on a network mount

[David Faure]

dfaure requested changes to this revision.
dfaure added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> broulik wrote in kfileitem.cpp:766
> If you have a location `/home/foo/foobar` and a slow mount `/home/foo/foo`, 
> it will consider it slow because "starts with"

But going via QUrl to fix this seems overkill to me.

It would be much faster to do this with the usual "location==mount or 
location.startsWith(mount+'/')".

I'm afraid this code will be called often (many KFileItems) and has to iterate 
over all mounts every time, so this is N*M.

REPOSITORY
  R241 KIO

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

To: meven, dfaure, #frameworks
Cc: broulik, anthonyfieroni, kde-frameworks-devel, LeGast00n, GB_2, michaelh, 
ngraham, bruns

D23420: Use solid to check if a KFileItem is located on a network mount

[Kai Uwe Broulik]

broulik added a comment.


  > isSlow() didn't check linkDest before.
  
  Even more reason to do it properly now :P
  
  `KFileSystemType` uses `statfs`. I didn't find any documentation on whether 
it follows symlinks, but it is documented to return an `ELOOP` error code "Too 
many symbolic links were encountered in translating path.", so maybe it does so 
implicitly, which we lost now.

REPOSITORY
  R241 KIO

BRANCH
  arcpatch-D23420

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

To: meven, dfaure, #frameworks
Cc: broulik, anthonyfieroni, kde-frameworks-devel, LeGast00n, GB_2, michaelh, 
ngraham, bruns

D23420: Use solid to check if a KFileItem is located on a network mount

[Méven Car]

meven marked an inline comment as done.
meven added a comment.


  In D23420#519949 , @broulik wrote:
  
  > Now we still need the `linkDest` check :)
  
  
  isSlow() didn't check linkDest before.

REPOSITORY
  R241 KIO

BRANCH
  arcpatch-D23420

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

To: meven, dfaure, #frameworks
Cc: broulik, anthonyfieroni, kde-frameworks-devel, LeGast00n, GB_2, michaelh, 
ngraham, bruns

D23420: Use solid to check if a KFileItem is located on a network mount

[Méven Car]

meven updated this revision to Diff 64716.
meven added a comment.


  Add a const& clean unused variable

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D23420?vs=64715=64716

BRANCH
  arcpatch-D23420

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

AFFECTED FILES
  src/core/CMakeLists.txt
  src/core/kfileitem.cpp

To: meven, dfaure, #frameworks
Cc: broulik, anthonyfieroni, kde-frameworks-devel, LeGast00n, GB_2, michaelh, 
ngraham, bruns

D23420: Use solid to check if a KFileItem is located on a network mount

[Anthony Fieroni]

anthonyfieroni added inline comments.

INLINE COMMENTS

> anthonyfieroni wrote in kfileitem.cpp:764
> Get it as `const Solid::StorageAccess`, you can use `auto storageAccess = ...`

Let clarify what David mean

  for (const Solid::Device& device : devices) {
  auto storageAccess = device.as();

REPOSITORY
  R241 KIO

BRANCH
  arcpatch-D23420

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

To: meven, dfaure, #frameworks
Cc: broulik, anthonyfieroni, kde-frameworks-devel, LeGast00n, GB_2, michaelh, 
ngraham, bruns

D23420: Use solid to check if a KFileItem is located on a network mount

[Kai Uwe Broulik]

broulik added a comment.


  Now we still need the `linkDest` check :)

REPOSITORY
  R241 KIO

BRANCH
  arcpatch-D23420

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

To: meven, dfaure, #frameworks
Cc: broulik, anthonyfieroni, kde-frameworks-devel, LeGast00n, GB_2, michaelh, 
ngraham, bruns

D23420: Use solid to check if a KFileItem is located on a network mount

[Méven Car]

meven updated this revision to Diff 64715.
meven added a comment.


  better check file path

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D23420?vs=64645=64715

BRANCH
  arcpatch-D23420

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

AFFECTED FILES
  src/core/CMakeLists.txt
  src/core/kfileitem.cpp

To: meven, dfaure, #frameworks
Cc: broulik, anthonyfieroni, kde-frameworks-devel, LeGast00n, GB_2, michaelh, 
ngraham, bruns

D23420: Use solid to check if a KFileItem is located on a network mount

[Méven Car]

meven marked an inline comment as done.

REPOSITORY
  R241 KIO

BRANCH
  arcpatch-D23420

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

To: meven, dfaure, #frameworks
Cc: broulik, anthonyfieroni, kde-frameworks-devel, LeGast00n, GB_2, michaelh, 
ngraham, bruns

D23420: Use solid to check if a KFileItem is located on a network mount

[Nathaniel Graham]

ngraham retitled this revision from "Use solid to check if a a KFileItem is 
located on a network mount" to "Use solid to check if a KFileItem is located on 
a network mount".

REPOSITORY
  R241 KIO

BRANCH
  solid-network-fs-check

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

To: meven, dfaure, #frameworks
Cc: broulik, anthonyfieroni, kde-frameworks-devel, LeGast00n, GB_2, michaelh, 
ngraham, bruns