Invalid are usually those mounted on a filesystem we can't access:
$ df /run/user/0/gvfs
df: /run/user/0/gvfs: Permission denied
$ ./qstorageinfo /run/user/0/gvfs
Could not get info on /run/user/0/gvfs
df already doesn't include it by default:
$ df | grep -c gvfs
0
But we were:
$./qstorageinfo | sed -n '1p;/gvfs/p'
Filesystem (Type) Size Available BSize Label Mounted on
gvfsd-fuse (fuse.gvfsd-fuse) RW 0 0 0 /run/user/0/gvfs
Note also how this is showing a total size of 0, which is usually the
type of filesystem we exclude. It's actually -1 but got rounded down to
0 when we divided by 1024.
Pick-to: 6.6
Change-Id: I8f3ce163ccc5408cac39fffd178d7e4f9dc13b24
Reviewed-by: Edward Welbourne <edward.welbourne@qt.io>
Reviewed-by: Ahmad Samir <a.samirh78@gmail.com>
This function is only called with the name of a file coming from
QFileInfo::fileName() so it's usually already detached anyway. And if
there's nothing to decode, pass the string through without even
attempting to modify it.
Pick-to: 6.6
Change-Id: I9d43e5b91eb142d6945cfffd1787651437074d35
Reviewed-by: Ahmad Samir <a.samirh78@gmail.com>
Reviewed-by: Axel Spoerl <axel.spoerl@qt.io>
Instead, create a (flat) map of the entries that we can seek on while
creating the list of mountedVolumes(). On my machine, that went down
from 14 times to 1.
Pick-to: 6.6
Change-Id: I9d43e5b91eb142d6945cfffd17875458541f28f9
Reviewed-by: Ahmad Samir <a.samirh78@gmail.com>
Instead of the previous realpath() comparison resulting from the symlink
processing. parseMountInfo() was extracting the device number from
/proc, so this information was already readily available.
We must take care of anonymous block devices (major == 0). Certain
filesystems, such as btrfs, always use them, so we must still stat() the
device path to get the real block device.
This implementation assumes that udev only creates entries in the
/dev/disks/by-label directory that are symlinks to real devices, but
that must already be the case because they are in /dev in the first
place. An alternative implementation would be to compare the inode and
host device (st_dev) of the entry, if different /dev entries could have
different labels. I don't think that's possible. But multiple /dev
entries for the same device is definitely possible.
Pick-to: 6.6
Change-Id: I9d43e5b91eb142d6945cfffd1787552af3d09676
Reviewed-by: Ahmad Samir <a.samirh78@gmail.com>
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
This fixes a similar problem as the previous commit. Because Linux
allows one to mount over non-empty paths, it's possible to make
mountpoints unreachable, and yet they will still be present in the
/proc/self/mountinfo listing. Because we have the device ID from the
scan, we can confirm that the mountpoint matches the MountInfo.
# mkdir -p /tmp/foo/bar
# mount -t tmpfs /tmp/foo/bar
# mount -t tmpfs -o size=1M /tmp/foo
# mkdir /tmp/foo/bar
$ ./tests/manual/qstorageinfo/qstorageinfo | awk 'NR==1 || /tmp\/foo/'
Filesystem (Type) Size Available BSize Label Mounted on
tmpfs RW 1024 1024 4096 /tmp/foo
Change-Id: I79e700614d034281bf55fffd178f8b99b10a8b69
Reviewed-by: Ahmad Samir <a.samirh78@gmail.com>
Reviewed-by: Edward Welbourne <edward.welbourne@qt.io>
Linux allows admins to mount new filesystems over non-empty paths
(though I managed to do so on FreeBSD and macOS too), so we must find
the most recent mount that applies to this path instead of the longest
matching mountpoint. We do that by scanning the /proc/self/mountinfo
list backwards and thus any matching isParentOf() must be the correct
one.
# mkdir -p /tmp/foo/bar
# mount -t tmpfs tmpfs /tmp/foo/bar
# mount -t tmpfs -o size=1M tmpfs /tmp/foo
$ ./tests/manual/qstorageinfo/qstorageinfo /tmp/foo/bar
Filesystem (Type) Size Available BSize Label Mounted on
tmpfs RW 1024 1024 4096 /tmp/foo
But we must guard against an earlier mount still being (somehow)
accessible. We've seen this in the CI, where /run is earlier than / but
still somehow accessible -- I guess this is one or a pair of mount
--move.
An additional benefit is that don't even attempt to compare to the
virtual filesystems mounted by the system early after boot, if what
we're looking for isn't the root.
See next commit for a fix for QStorageInfo::mountedVolumes().
Change-Id: I79e700614d034281bf55fffd178f8befc5e80edb
Reviewed-by: Ahmad Samir <a.samirh78@gmail.com>
There's nothing wrong with device labels starting with a dot. Whether
udev would encode those as \x2e is unknown, but we may as well not tempt
fate in case it has changed or changes in the future.
Also including QDir::System in case udev places the actual device nodes
in /dev/disks/by-label instead of a symlink.
As a nice and intentional side-effect, QDirIterator no longer performs a
stat() in each of the entries, removing the double stat'ing that started
happening with the previous commit.
Pick-to: 6.6
Change-Id: I9d43e5b91eb142d6945cfffd1787681b4cf2bb58
Reviewed-by: Ahmad Samir <a.samirh78@gmail.com>
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
In addition to what parseMountInfo() filtered, we also filter entries
with zero total bytes (other than the root filesystem). This avoids
creating yet another QStorageInfoPrivate that may not be used, but most
importantly it avoids calling root() for that check, which would call
parseMountInfo() again.
Pick-to: 6.6
Change-Id: I9d43e5b91eb142d6945cfffd1787538dd3f285d0
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
Reviewed-by: Ahmad Samir <a.samirh78@gmail.com>
Reviewed-by: Edward Welbourne <edward.welbourne@qt.io>
Mine has 41 lines, of which 22 are returned by parseMountInfo with
filtering. That meant the file was parsed once to get the listing, then
22 times more to create a QStorageInfo for each entry. Now
QStorageInfo::mountedVolumes() opens the file and parses it only once.
Pick-to: 6.6
Change-Id: I9d43e5b91eb142d6945cfffd178752ef6c2122f2
Reviewed-by: Edward Welbourne <edward.welbourne@qt.io>
Reviewed-by: Ahmad Samir <a.samirh78@gmail.com>
Amends da95ad91b3. Caught by CodeChecker:
std::move of the const expression has no effect; remove std::move()
I'll instead remove the const.
Pick-to: 6.6
Change-Id: I8f3ce163ccc5408cac39fffd178ccec9fcc38e9c
Reviewed-by: Ahmad Samir <a.samirh78@gmail.com>
Execute the cheaper test first, so we loop over the entries more
quickly.
Pick-to: 6.6
Change-Id: I9d43e5b91eb142d6945cfffd178749a12c966739
Reviewed-by: Ahmad Samir <a.samirh78@gmail.com>
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
Copying QStrings and QByteArrays is reasonably cheap, but moving is
cheaper.
Pick-to: 6.6
Change-Id: I9d43e5b91eb142d6945cfffd1787498ead687da2
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
Reviewed-by: Ahmad Samir <a.samirh78@gmail.com>
All these where somewhat cheap to copy (QStrings and QByteArrays), but
why copy multiple times at all? Just copy at the end.
Pick-to: 6.6
Change-Id: I9d43e5b91eb142d6945cfffd1787497434632dd4
Reviewed-by: Ahmad Samir <a.samirh78@gmail.com>
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
symlinkTarget() suffices, because if the candidate is not a symlink, it
will return an empty string. Plus, /dev/disks/by-label is a udev-managed
directory, so everything should be a symlink.
This doesn't change the number of statx() calls because QDirIterator
needs some information on file types to decide how to filter and,
unfortunately, that information is missing for symlinks (we know it's a
symlink, but we don't know what it points to). Moreover, due to
QDirIterator's design, we always statx() one entry past the one we
wanted.
Pick-to: 6.6
Change-Id: I9d43e5b91eb142d6945cfffd1786ce1bd3398d1b
Reviewed-by: Ahmad Samir <a.samirh78@gmail.com>
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
Much less crowded.
Inline QStorageInfoPrivate::root() to ease the split; the Windows
specific code is one extra line.
Change-Id: Icec6822cf436e2b4aa1b3a04184fbfa40e508078
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
Thiago Macieira
Ahmad Samir