D5784: Add support for FreeBSD in FSUtils::getDirectoryFileSystem().

2018-04-12 Thread Tobias C . Berner 
tcberner abandoned this revision.

REPOSITORY
  R293 Baloo

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

To: tcberner, #freebsd, poboiko, bruns
Cc: bruns, adridg, kfunk, #frameworks, ashaposhnikov, michaelh, astippich, 
spoorun, ngraham, alexeymin

D5784: Add support for FreeBSD in FSUtils::getDirectoryFileSystem().

2018-04-12 Thread Adriaan de Groot 
adridg added a comment.


  Given the other reviews (which just drop this function), this one should be 
abandoned. tcberner@?

REPOSITORY
  R293 Baloo

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

To: tcberner, #freebsd, poboiko, bruns
Cc: bruns, adridg, kfunk, #frameworks, ashaposhnikov, michaelh, astippich, 
spoorun, ngraham, alexeymin

D5784: Add support for FreeBSD in FSUtils::getDirectoryFileSystem().

2018-04-11 Thread Stefan Brüns 
bruns added a comment.


  See D12136 

REPOSITORY
  R293 Baloo

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

To: tcberner, #freebsd, poboiko, bruns
Cc: bruns, adridg, kfunk, #frameworks, ashaposhnikov, michaelh, astippich, 
spoorun, ngraham, alexeymin

D5784: Add support for FreeBSD in FSUtils::getDirectoryFileSystem().

2018-04-09 Thread Stefan Brüns 
bruns added a comment.


  In D5784#243432 , @adridg wrote:
  
  > The question is mostly: does the existing (complicated) code do anything 
that QStorageInfo doesn't? Because switching to QStorageInfo gives us the 
functionality on FreeBSD for free (even if it's not useful because we'll never 
have btrfs).
  
  
  My opinion is to scrap the function completely, on *BSD **and** on Linux. 
//Iff// there ever is a need, one can add it back, or just call QStorageInfo 
directly wherever it is used.

REPOSITORY
  R293 Baloo

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

To: tcberner, #freebsd, poboiko, bruns
Cc: bruns, adridg, kfunk, #frameworks, ashaposhnikov, michaelh, astippich, 
spoorun, ngraham, alexeymin

D5784: Add support for FreeBSD in FSUtils::getDirectoryFileSystem().

2018-04-09 Thread Adriaan de Groot 
adridg added a comment.


  The question is mostly: does the existing (complicated) code do anything that 
QStorageInfo doesn't? Because switching to QStorageInfo gives us the 
functionality on FreeBSD for free (even if it's not useful because we'll never 
have btrfs).

REPOSITORY
  R293 Baloo

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

To: tcberner, #freebsd, poboiko, bruns
Cc: bruns, adridg, kfunk, #frameworks, ashaposhnikov, michaelh, astippich, 
spoorun, ngraham, alexeymin

D5784: Add support for FreeBSD in FSUtils::getDirectoryFileSystem().

2018-04-09 Thread Kevin Funk 
kfunk resigned from this revision.

REPOSITORY
  R293 Baloo

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

To: tcberner, #freebsd, poboiko, bruns
Cc: bruns, adridg, kfunk, #frameworks, ashaposhnikov, michaelh, astippich, 
spoorun, ngraham, alexeymin

D5784: Add support for FreeBSD in FSUtils::getDirectoryFileSystem().

2018-04-09 Thread Stefan Brüns 
bruns added a reviewer: bruns.

REPOSITORY
  R293 Baloo

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

To: tcberner, #freebsd, kfunk, poboiko, bruns
Cc: bruns, adridg, kfunk, #frameworks, ashaposhnikov, michaelh, astippich, 
spoorun, ngraham, alexeymin

D5784: Add support for FreeBSD in FSUtils::getDirectoryFileSystem().

2018-04-09 Thread Stefan Brüns 
bruns added a comment.
Restricted Application added a project: Baloo.


  The whole function as currently used is pointless.
  
  Currently, it sets the NO_COW flag on BTRFS, but this is insufficient:
  
  - XFS - https://lwn.net/Articles/747633/
  - F2FS
  - Hammer2 ?
  
  At least on Linux, the correct thing is to *try* to set the no_cow flag, and 
silently ignore any `EOPNOTSUPP` errors.
  
  Thats what I get on Linux for:
  
  - File without permission to write: `ioctl(3, FS_IOC_SETFLAGS, 
0x7ffee879084c) = -1 EPERM (Operation not permitted)`
  - On an old XFS: `ioctl(3, FS_IOC_SETFLAGS, 0x7fff8e52467c) = -1 EOPNOTSUPP 
(Operation not supported)`
  - BTRFS: `ioctl(3, FS_IOC_SETFLAGS, 0x7fff06817a2c) = 0`
  
  This way the code is future proof even when a new FS appears or an old one 
gains new features.

REPOSITORY
  R293 Baloo

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

To: tcberner, #freebsd, kfunk, poboiko
Cc: bruns, adridg, kfunk, #frameworks, ashaposhnikov, michaelh, astippich, 
spoorun, ngraham, alexeymin

D5784: Add support for FreeBSD in FSUtils::getDirectoryFileSystem().

2017-06-24 Thread Adriaan de Groot 
adridg added reviewers: kfunk, poboiko.
adridg added a comment.


  Add reviewers who have commented, or who have committed to baloo recently.

REPOSITORY
  R293 Baloo

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

To: tcberner, #freebsd, kfunk, poboiko
Cc: adridg, kfunk, #frameworks

D5784: Add support for FreeBSD in FSUtils::getDirectoryFileSystem().

2017-06-24 Thread Adriaan de Groot 
adridg added a comment.


  So this patch looks ready to land to me: it simplifies the code, defers 
fstype-detection to QStorageInfo (for FreeBSD, we should fix the original issue 
there), and is otherwise non-intrusive.

REPOSITORY
  R293 Baloo

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

To: tcberner, #freebsd
Cc: adridg, kfunk, #frameworks

D5784: Add support for FreeBSD in FSUtils::getDirectoryFileSystem().

2017-05-30 Thread Tobias C. Berner 
tcberner updated this revision to Diff 14965.
tcberner added a comment.


  Use QStorageInfo.

REPOSITORY
  R293 Baloo

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D5784?vs=14334=14965

BRANCH
  master

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

AFFECTED FILES
  src/engine/fsutils.cpp

To: tcberner, #freebsd
Cc: kfunk, #frameworks

D5784: Add support for FreeBSD in FSUtils::getDirectoryFileSystem().

2017-05-29 Thread Tobias C. Berner 
tcberner added a comment.


  Ok, I think I found the issue in QStorageInfo -- it calls getmntinfo() with 
`flags=0` [1] . Instead one of the valid arguments 1-4 [2].
  
  [1] 
https://github.com/qt/qtbase/blob/dev/src/corelib/io/qstorageinfo_unix.cpp#L199
  [2] 
https://svnweb.freebsd.org/base/head/sys/sys/mount.h?revision=318736=markup#l441

REPOSITORY
  R293 Baloo

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

To: tcberner, #freebsd
Cc: kfunk, #frameworks

D5784: Add support for FreeBSD in FSUtils::getDirectoryFileSystem().

2017-05-09 Thread Tobias C. Berner 
tcberner planned changes to this revision.
tcberner added a comment.


  I agree. I'll try and figure out whats wrong inside QStorageInfo, and then 
update the review here to be using it.

REPOSITORY
  R293 Baloo

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

To: tcberner, #freebsd
Cc: kfunk, #frameworks

D5784: Add support for FreeBSD in FSUtils::getDirectoryFileSystem().

2017-05-09 Thread Kevin Funk 
kfunk added a comment.


  Okay, then maybe support for the current FreeBSD version is not implemented? 
I have no idea about FreeBSD, to be honest. But it looks like using this API, 
and implementing the proper support in `QStorageInfo` internals would be the 
appropriate place.
  
  Anyhow, if Qt support is lacking, this patch hier makes sense then -- at 
least the hunks for FreeBSD. I'll let others review this patch in detail.

REPOSITORY
  R293 Baloo

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

To: tcberner, #freebsd
Cc: kfunk, #frameworks

D5784: Add support for FreeBSD in FSUtils::getDirectoryFileSystem().

2017-05-09 Thread Tobias C. Berner 
tcberner added a comment.


  Hm, QStorageInfo does not seem to return anything sensible on FreeBSD:
  
""
name: ""
fileSystemType: ""
size: 0 MB
availableSize: 0 MB
  
  This is the output from the `QStorageInfo::root()` example.

REPOSITORY
  R293 Baloo

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

To: tcberner, #freebsd
Cc: kfunk, #frameworks

D5784: Add support for FreeBSD in FSUtils::getDirectoryFileSystem().

2017-05-09 Thread Kevin Funk 
kfunk added a comment.


  What about http://doc.qt.io/qt-5/qstorageinfo.html#fileSystemType?

REPOSITORY
  R293 Baloo

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

To: tcberner, #freebsd
Cc: kfunk, #frameworks

D5784: Add support for FreeBSD in FSUtils::getDirectoryFileSystem().

2017-05-09 Thread Tobias C. Berner 
tcberner created this revision.
Restricted Application added a project: Frameworks.
Restricted Application added a subscriber: Frameworks.

REVISION SUMMARY
  This is not really needed at the moment, as it only seems to be needed to 
check if the filesystem is btrfs.
  
  I'm however not sure, if the simpler code used for FreeBSD would not also 
work on linux :)

REPOSITORY
  R293 Baloo

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

AFFECTED FILES
  src/engine/fsutils.cpp

To: tcberner, #freebsd
Cc: #frameworks