Differential D20301

Ensure to add a trailing / at the end of fstab mounted devices.
AbandonedPublic
Actions

Authored by meven on Apr 6 2019, 12:25 PM.

Details

Reviewers
bruns
lukas
broulik
Summary

The nfs mounted device are listed without a trailing / according to my testing, preventing them to be grouped with the unmounted device.

BUG: 406242
CCBUG: 390691
FIXED-IN: 5.58

Test Plan

You need an fstab configured nfs drive.

  1. List the hardware "solid-hardware list | grep fstab" note the relevant udi for the nfs drive
  2. solid-hardware mount '/org/kde/fstab/192....' (<- replace with actual udi)
  3. List the hardware : there is only one device for this drive listed.

I have tested in Dolphin as well.

Diff Detail

Repository
R245 Solid
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 10513
Build 10531: arc lint + arc unit
meven created this revision.Apr 6 2019, 12:25 PM
Restricted Application added a project: Frameworks. · View Herald TranscriptApr 6 2019, 12:25 PM
Restricted Application added a subscriber: kde-frameworks-devel. · View Herald Transcript
meven requested review of this revision.Apr 6 2019, 12:25 PM
meven added a reviewer: lukas.Apr 6 2019, 12:25 PM

The patch is currently quite naive.
Should I add a check if (!device.endsWith('/') ?
Is it sound enough ?

meven edited the summary of this revision. (Show Details)Apr 6 2019, 12:29 PM
meven edited the summary of this revision. (Show Details)Apr 7 2019, 9:56 AM
meven edited the summary of this revision. (Show Details)Apr 12 2019, 9:07 AM
meven added a comment.Apr 13 2019, 4:36 PM

I believe QFile::decodeName returns fileNames without a trailing '/'
It could be that it is the unmounted device that would need to remove its ending '/'

meven added a comment.Apr 19 2019, 7:52 AM

Friendly ping

ngraham added a subscriber: ngraham.Apr 19 2019, 2:39 PM

I've been told that @bruns can review this sometime next week. :)

meven added a comment.EditedApr 26 2019, 3:29 PM
In D20301#453031, @ngraham wrote:

I've been told that @bruns can review this sometime next week. :)

Can't wait, this bug annoys me.

meven added a comment.May 5 2019, 3:45 PM

What do you think @bruns ?

bruns added a comment.May 5 2019, 6:03 PM

I think this is the wrong way round:

man mount.cifs:

The mount.cifs utility attaches the UNC name (exported network resource) specified as service (using //server/share syntax, where "server" is the server name or IP address and "share" is the name of the share) to the local directory mount-point.

man mount.nfs

remotetarget is a server share usually in the form of servername:/path/to/share. dir is the directory on which the file system is to be mounted.

In general, the "device" has no trailing slash. Apparently the kernel (or mount?) strips it for CIFS, but not for NFS.

The correct approach, to avoid double entries in case the user has added a '/' to the device, is to compare the entries slash-insensitive.

meven abandoned this revision.May 14 2019, 7:44 AM

Abandoned n favor of D21204

Revision Contents
Changeset List

Diff 55547

View Options

src/solid/devices/backends/fstab/fstabhandling.cpp

Loading...