New issue
Advanced search Search tips

Issue 878498 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Sep 28
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

Refactor Mount tracking from MountManager

Project Member Reported by zentaro@chromium.org, Aug 28

Issue description

There are 3 data structures in MountManager that track the relationships between mount_ids, SambaInterfaceId's and share roots.

Refactor these out to a separate class with the necessary API, then use this new class in MountManager. New class should DCHECK where needed to ensure the data structures are in sync.

Possibly/Likely? depending on design the MountInfo class will be either move to a private type in this new class or possible made public in its own file.

Write up a one pager to describe the new API before imeplementing. We can have a sync up with Bailey and I to give more context before you start.
 
Status: Started (was: Assigned)
Project Member

Comment 2 by bugdroid1@chromium.org, Sep 11

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/ccba6f59c31ae2d833c1867f1aac7dab0cc06af6

commit ccba6f59c31ae2d833c1867f1aac7dab0cc06af6
Author: Jimmy Gong <jimmyxgong@google.com>
Date: Tue Sep 11 00:17:37 2018

smbprovider: Add boilerplate code for MountTracker and MountTrackerTest

- Add boilerplate code for MountTracker and its tester.
- No functionality, future CL's will implement functions to both MountTracker
  and MountTrackerTest.

BUG= chromium:878498 
TEST=emerges

Change-Id: I3e39afb7aa0dc19d50aea85f2568c0117dd12e7b
Reviewed-on: https://chromium-review.googlesource.com/1208890
Commit-Ready: jimmy gong <jimmyxgong@chromium.org>
Tested-by: jimmy gong <jimmyxgong@chromium.org>
Reviewed-by: Zentaro Kavanagh <zentaro@chromium.org>
Reviewed-by: Bailey Berro <baileyberro@chromium.org>

[add] https://crrev.com/ccba6f59c31ae2d833c1867f1aac7dab0cc06af6/smbprovider/mount_tracker_test.cc
[modify] https://crrev.com/ccba6f59c31ae2d833c1867f1aac7dab0cc06af6/smbprovider/mount_manager_test.cc
[modify] https://crrev.com/ccba6f59c31ae2d833c1867f1aac7dab0cc06af6/smbprovider/smbprovider.gyp
[add] https://crrev.com/ccba6f59c31ae2d833c1867f1aac7dab0cc06af6/smbprovider/mount_tracker.cc
[add] https://crrev.com/ccba6f59c31ae2d833c1867f1aac7dab0cc06af6/smbprovider/mount_tracker.h

Project Member

Comment 3 by bugdroid1@chromium.org, Sep 11

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/2b6b536e281dd0f91ed5e9dc58801c1cd627f93a

commit 2b6b536e281dd0f91ed5e9dc58801c1cd627f93a
Author: Jimmy Gong <jimmyxgong@google.com>
Date: Tue Sep 11 00:17:37 2018

smbprovider: Add MountInfo to MountTracker

- MountInfo will be moved from MountManager to MountTracker.
- MountTracker is responsible for managing data pertaining to a mount's
  credentials and status. MountInfo holds data of a mount's credentials.

BUG= chromium:878498 
TEST=emerges

Change-Id: I0bb308a3a72d30c3ad8096f599e58d0705f6419f
Reviewed-on: https://chromium-review.googlesource.com/1208891
Commit-Ready: jimmy gong <jimmyxgong@chromium.org>
Tested-by: jimmy gong <jimmyxgong@chromium.org>
Reviewed-by: Zentaro Kavanagh <zentaro@chromium.org>
Reviewed-by: Bailey Berro <baileyberro@chromium.org>

[modify] https://crrev.com/2b6b536e281dd0f91ed5e9dc58801c1cd627f93a/smbprovider/mount_tracker.h

Project Member

Comment 4 by bugdroid1@chromium.org, Sep 11

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/f4d31d4db21e412b2bd38b808feb8f5f5d829fec

commit f4d31d4db21e412b2bd38b808feb8f5f5d829fec
Author: Jimmy Gong <jimmyxgong@google.com>
Date: Tue Sep 11 00:17:38 2018

smbprovider: Add mount data structures to MountTracker

- The three main data structures that used by MountTracker are |mounts_|,
  |samba_interface_map_|, and |mounted_share_paths_|.
- These data structures work in conjunction with each other to store mount
  data and status.

BUG= chromium:878498 
TEST=emerges

Change-Id: Id9f67b292bd1687ed1c7340ee937f357e99a8cf9
Reviewed-on: https://chromium-review.googlesource.com/1208894
Commit-Ready: jimmy gong <jimmyxgong@chromium.org>
Tested-by: jimmy gong <jimmyxgong@chromium.org>
Reviewed-by: Zentaro Kavanagh <zentaro@chromium.org>

[modify] https://crrev.com/f4d31d4db21e412b2bd38b808feb8f5f5d829fec/smbprovider/mount_tracker.h

Project Member

Comment 5 by bugdroid1@chromium.org, Sep 11

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/1a7e5de5dff742f2f18f42a42ec15dc79a1ee4ed

commit 1a7e5de5dff742f2f18f42a42ec15dc79a1ee4ed
Author: Jimmy Gong <jimmyxgong@google.com>
Date: Tue Sep 11 00:17:38 2018

smbprovider: Add IsAlreadyMounted and ExistsInMount to MountTracker

- |IsAlreadyMounted| checks that either MountId or SharePath are mounted.
- |ExistsInMount| checks to ensure that |mounts_| and |mounted_share_path_|
  are in sync.
- Added negative test cases to MountTrackerTest to test functionality of
  |IsAlreadyMounted|.

BUG= chromium:878498 
TEST=emerges

Change-Id: Ia7bca405422bbc547de4893039c0965ad63d9502
Reviewed-on: https://chromium-review.googlesource.com/1208895
Commit-Ready: jimmy gong <jimmyxgong@chromium.org>
Tested-by: jimmy gong <jimmyxgong@chromium.org>
Reviewed-by: Zentaro Kavanagh <zentaro@chromium.org>

[modify] https://crrev.com/1a7e5de5dff742f2f18f42a42ec15dc79a1ee4ed/smbprovider/mount_tracker_test.cc
[modify] https://crrev.com/1a7e5de5dff742f2f18f42a42ec15dc79a1ee4ed/smbprovider/mount_tracker.h
[modify] https://crrev.com/1a7e5de5dff742f2f18f42a42ec15dc79a1ee4ed/smbprovider/mount_tracker.cc

Project Member

Comment 6 by bugdroid1@chromium.org, Sep 15

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/fa84e0313dfa1d0c2d2fd1d0a4c1f18efbf9d3a9

commit fa84e0313dfa1d0c2d2fd1d0a4c1f18efbf9d3a9
Author: Jimmy Gong <jimmyxgong@google.com>
Date: Sat Sep 15 00:05:44 2018

smbprovider: Add AddMount to MountTracker

- Adds AddMount and simple test cases in MountTrackerTest.
- Adds private helper functions in MountTracker for AddMount's use.
- Adds FakeSambaProxy and FakeSambaInterface to MounTrackerTest.

BUG= chromium:878498 
TEST=unittests

Change-Id: Ib95b893f05ef6ce8f5726930af32a373f24db7a1
Reviewed-on: https://chromium-review.googlesource.com/1212323
Commit-Ready: jimmy gong <jimmyxgong@chromium.org>
Tested-by: jimmy gong <jimmyxgong@chromium.org>
Reviewed-by: Zentaro Kavanagh <zentaro@chromium.org>

[modify] https://crrev.com/fa84e0313dfa1d0c2d2fd1d0a4c1f18efbf9d3a9/smbprovider/mount_tracker_test.cc
[modify] https://crrev.com/fa84e0313dfa1d0c2d2fd1d0a4c1f18efbf9d3a9/smbprovider/mount_tracker.h
[modify] https://crrev.com/fa84e0313dfa1d0c2d2fd1d0a4c1f18efbf9d3a9/smbprovider/mount_tracker.cc

Project Member

Comment 7 by bugdroid1@chromium.org, Sep 15

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/bd0266ec6935a102d4489cbd1c3580666c025fa5

commit bd0266ec6935a102d4489cbd1c3580666c025fa5
Author: Jimmy Gong <jimmyxgong@google.com>
Date: Sat Sep 15 00:05:44 2018

smbprovider: Add MountCount to MountTracker

- MountCount is a simple function that returns the number of mounts within
  MountTracker.

BUG= chromium:878498 
TEST=unittests

Change-Id: I670a3c0d0795070f9674ef01931423ed07214faa
Reviewed-on: https://chromium-review.googlesource.com/1213321
Commit-Ready: jimmy gong <jimmyxgong@chromium.org>
Tested-by: jimmy gong <jimmyxgong@chromium.org>
Reviewed-by: Zentaro Kavanagh <zentaro@chromium.org>

[modify] https://crrev.com/bd0266ec6935a102d4489cbd1c3580666c025fa5/smbprovider/mount_tracker_test.cc
[modify] https://crrev.com/bd0266ec6935a102d4489cbd1c3580666c025fa5/smbprovider/mount_tracker.h

Project Member

Comment 8 by bugdroid1@chromium.org, Sep 15

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/7049debca80f8240e2958bc758b4720ad73bdd96

commit 7049debca80f8240e2958bc758b4720ad73bdd96
Author: Jimmy Gong <jimmyxgong@google.com>
Date: Sat Sep 15 00:05:43 2018

smbprovider: Add At function to IdMap

- IdMap can now call on an At function, which returns the value when given a
  key.

BUG= chromium:878498 
TEST=emerges

Change-Id: Icf3c888e5f85e8c0473fb32c0cba0b81094a7f3b
Reviewed-on: https://chromium-review.googlesource.com/1214113
Commit-Ready: jimmy gong <jimmyxgong@chromium.org>
Tested-by: jimmy gong <jimmyxgong@chromium.org>
Reviewed-by: Zentaro Kavanagh <zentaro@chromium.org>
Reviewed-by: Bailey Berro <baileyberro@chromium.org>

[modify] https://crrev.com/7049debca80f8240e2958bc758b4720ad73bdd96/smbprovider/id_map.h
[modify] https://crrev.com/7049debca80f8240e2958bc758b4720ad73bdd96/smbprovider/id_map_test.cc

Project Member

Comment 9 by bugdroid1@chromium.org, Sep 19

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/1fbf2f73599a2093e6f140e92bf74f28123f9227

commit 1fbf2f73599a2093e6f140e92bf74f28123f9227
Author: Jimmy Gong <jimmyxgong@google.com>
Date: Wed Sep 19 22:40:01 2018

smbprovider: Add AddMountWithId to MountTracker

- Add AddMountWithId and its tests to MountTrackerTest.
- AddMountWithId was previously known as Remount. Changed name to be more
  descriptive.

BUG= chromium:878498 
TEST=unittests

Change-Id: I0198a84b30cf85245aca74e488785f33a76ac4be
Reviewed-on: https://chromium-review.googlesource.com/1214062
Commit-Ready: jimmy gong <jimmyxgong@chromium.org>
Tested-by: jimmy gong <jimmyxgong@chromium.org>
Reviewed-by: Zentaro Kavanagh <zentaro@chromium.org>

[modify] https://crrev.com/1fbf2f73599a2093e6f140e92bf74f28123f9227/smbprovider/mount_tracker_test.cc
[modify] https://crrev.com/1fbf2f73599a2093e6f140e92bf74f28123f9227/smbprovider/mount_tracker.h
[modify] https://crrev.com/1fbf2f73599a2093e6f140e92bf74f28123f9227/smbprovider/mount_tracker.cc

Project Member

Comment 10 by bugdroid1@chromium.org, Sep 19

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/35944f31f542b148e51954ec4ab7c9f47e439cd2

commit 35944f31f542b148e51954ec4ab7c9f47e439cd2
Author: Jimmy Gong <jimmyxgong@google.com>
Date: Wed Sep 19 22:40:02 2018

smbprovider: Add RemoveMount to MountTracker

- Adds RemoveMount to MountTracker and its tests to MountTrackerTest.

BUG= chromium:878498 
TEST=unit test

Change-Id: Ief4e8055957f86b3d3112741ff785530dc30a34d
Reviewed-on: https://chromium-review.googlesource.com/1217828
Commit-Ready: jimmy gong <jimmyxgong@chromium.org>
Tested-by: jimmy gong <jimmyxgong@chromium.org>
Reviewed-by: Zentaro Kavanagh <zentaro@chromium.org>

[modify] https://crrev.com/35944f31f542b148e51954ec4ab7c9f47e439cd2/smbprovider/mount_tracker_test.cc
[modify] https://crrev.com/35944f31f542b148e51954ec4ab7c9f47e439cd2/smbprovider/mount_tracker.h
[modify] https://crrev.com/35944f31f542b148e51954ec4ab7c9f47e439cd2/smbprovider/mount_tracker.cc

Project Member

Comment 11 by bugdroid1@chromium.org, Sep 19

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/6d92f14e2719bcab9f38e80a4a90121614723e1e

commit 6d92f14e2719bcab9f38e80a4a90121614723e1e
Author: Jimmy Gong <jimmyxgong@google.com>
Date: Wed Sep 19 22:40:02 2018

smbprovider: Add GetFullPath to MountTracker

- Adds GetFullPath and its associated unit tests to MountTrackerTest.

BUG= chromium:878498 
TEST=unit test

Change-Id: I4677eab5a84fe07008cfceaa1acea88a4493902f
Reviewed-on: https://chromium-review.googlesource.com/1219973
Commit-Ready: jimmy gong <jimmyxgong@chromium.org>
Tested-by: jimmy gong <jimmyxgong@chromium.org>
Reviewed-by: Zentaro Kavanagh <zentaro@chromium.org>
Reviewed-by: Bailey Berro <baileyberro@chromium.org>

[modify] https://crrev.com/6d92f14e2719bcab9f38e80a4a90121614723e1e/smbprovider/mount_tracker_test.cc
[modify] https://crrev.com/6d92f14e2719bcab9f38e80a4a90121614723e1e/smbprovider/mount_tracker.h
[modify] https://crrev.com/6d92f14e2719bcab9f38e80a4a90121614723e1e/smbprovider/mount_tracker.cc

Project Member

Comment 12 by bugdroid1@chromium.org, Sep 19

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/316f31a8886dfd3650d091613eb0f2e005f51e0f

commit 316f31a8886dfd3650d091613eb0f2e005f51e0f
Author: Jimmy Gong <jimmyxgong@google.com>
Date: Wed Sep 19 22:40:03 2018

smbprovider: Add GetRelativePath to MountTracker

- Adds GetRelativePath and its associated unit tests in MountTrackerTest.

BUG= chromium:878498 
TEST=unit test

Change-Id: I6986c303b7877a34973390405bd5ae930786bd93
Reviewed-on: https://chromium-review.googlesource.com/1219974
Commit-Ready: jimmy gong <jimmyxgong@chromium.org>
Tested-by: jimmy gong <jimmyxgong@chromium.org>
Reviewed-by: Zentaro Kavanagh <zentaro@chromium.org>
Reviewed-by: Bailey Berro <baileyberro@chromium.org>

[modify] https://crrev.com/316f31a8886dfd3650d091613eb0f2e005f51e0f/smbprovider/mount_tracker_test.cc
[modify] https://crrev.com/316f31a8886dfd3650d091613eb0f2e005f51e0f/smbprovider/mount_tracker.h
[modify] https://crrev.com/316f31a8886dfd3650d091613eb0f2e005f51e0f/smbprovider/mount_tracker.cc

Project Member

Comment 13 by bugdroid1@chromium.org, Sep 19

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/f48449626cc5d77c043c36e0e9424616cf8e6d5b

commit f48449626cc5d77c043c36e0e9424616cf8e6d5b
Author: Jimmy Gong <jimmyxgong@google.com>
Date: Wed Sep 19 22:40:03 2018

smbprovider: Add GetCredential to MountTracker

- Adds GetCredential and its associated tests to MountTrackerTest.
- Adds public GetSambaInterface to MountTracker.

BUG= chromium:878498 
TEST=unit tests

Change-Id: Ibf2ba940ff7e8b9cc8f59d58aefaf86fcfc81daf
Reviewed-on: https://chromium-review.googlesource.com/1219979
Commit-Ready: jimmy gong <jimmyxgong@chromium.org>
Tested-by: jimmy gong <jimmyxgong@chromium.org>
Reviewed-by: Zentaro Kavanagh <zentaro@chromium.org>

[modify] https://crrev.com/f48449626cc5d77c043c36e0e9424616cf8e6d5b/smbprovider/mount_tracker_test.cc
[modify] https://crrev.com/f48449626cc5d77c043c36e0e9424616cf8e6d5b/smbprovider/mount_tracker.h
[modify] https://crrev.com/f48449626cc5d77c043c36e0e9424616cf8e6d5b/smbprovider/mount_tracker.cc

Project Member

Comment 14 by bugdroid1@chromium.org, Sep 19

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/be0c7837fd1f8b21f78861872dccb7961fbc43b0

commit be0c7837fd1f8b21f78861872dccb7961fbc43b0
Author: Jimmy Gong <jimmyxgong@google.com>
Date: Wed Sep 19 22:40:04 2018

smbprovider: Add IsAlreadyMounted for SambaInterfaceId to MountTracker

- Add IsAlreadyMounted for SambaInterfaceId and its tests to MountTrackerTest.

BUG= chromium:878498 
TEST=unit tests

Change-Id: Ib85a195f540b3e6280d732f4973870ac9836b394
Reviewed-on: https://chromium-review.googlesource.com/1226219
Commit-Ready: jimmy gong <jimmyxgong@chromium.org>
Tested-by: jimmy gong <jimmyxgong@chromium.org>
Reviewed-by: Zentaro Kavanagh <zentaro@chromium.org>

[modify] https://crrev.com/be0c7837fd1f8b21f78861872dccb7961fbc43b0/smbprovider/mount_tracker_test.cc
[modify] https://crrev.com/be0c7837fd1f8b21f78861872dccb7961fbc43b0/smbprovider/mount_tracker.h
[modify] https://crrev.com/be0c7837fd1f8b21f78861872dccb7961fbc43b0/smbprovider/mount_tracker.cc

Project Member

Comment 15 by bugdroid1@chromium.org, Sep 19

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/176b5368ee3acb53fb7148e7970330aa932cac5d

commit 176b5368ee3acb53fb7148e7970330aa932cac5d
Author: Jimmy Gong <jimmyxgong@google.com>
Date: Wed Sep 19 22:40:04 2018

smbprovider: Add GetMetadataCache to MountTracker

- Adds GetMetadataCache and its tests to MountTrackerTest.

BUG= chromium:878498 
TEST=unit tests

Change-Id: I97abe14df7fec6d154637bb3cd58f00b2be6f901
Reviewed-on: https://chromium-review.googlesource.com/1226220
Commit-Ready: jimmy gong <jimmyxgong@chromium.org>
Tested-by: jimmy gong <jimmyxgong@chromium.org>
Reviewed-by: Zentaro Kavanagh <zentaro@chromium.org>
Reviewed-by: Bailey Berro <baileyberro@chromium.org>

[modify] https://crrev.com/176b5368ee3acb53fb7148e7970330aa932cac5d/smbprovider/mount_tracker_test.cc
[modify] https://crrev.com/176b5368ee3acb53fb7148e7970330aa932cac5d/smbprovider/mount_tracker.h
[modify] https://crrev.com/176b5368ee3acb53fb7148e7970330aa932cac5d/smbprovider/mount_tracker.cc

Project Member

Comment 16 by bugdroid1@chromium.org, Sep 21

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/79d5281f3b7bb6495a1003afa66d4023a9b7feb5

commit 79d5281f3b7bb6495a1003afa66d4023a9b7feb5
Author: Jimmy Gong <jimmyxgong@google.com>
Date: Fri Sep 21 07:51:26 2018

smbprovider: Add MountTracker to MountManager

- This change will revamp MountManager's responsibilities of managing mounts.
  MountManager now owns a MountTracker that is responsible for managing
  adding/remounting/removing mounts.

BUG= chromium:878498 
TEST=unit tests

Change-Id: I43fa5084b8ca83d2bb1e65cdf6edc0a1375e827c
Reviewed-on: https://chromium-review.googlesource.com/1226221
Commit-Ready: jimmy gong <jimmyxgong@chromium.org>
Tested-by: jimmy gong <jimmyxgong@chromium.org>
Reviewed-by: Zentaro Kavanagh <zentaro@chromium.org>
Reviewed-by: Bailey Berro <baileyberro@chromium.org>

[modify] https://crrev.com/79d5281f3b7bb6495a1003afa66d4023a9b7feb5/smbprovider/mount_manager.cc
[modify] https://crrev.com/79d5281f3b7bb6495a1003afa66d4023a9b7feb5/smbprovider/mount_manager.h
[modify] https://crrev.com/79d5281f3b7bb6495a1003afa66d4023a9b7feb5/smbprovider/mount_manager_test.cc
[modify] https://crrev.com/79d5281f3b7bb6495a1003afa66d4023a9b7feb5/smbprovider/smbprovider_main.cc
[modify] https://crrev.com/79d5281f3b7bb6495a1003afa66d4023a9b7feb5/smbprovider/smbprovider_test.cc

Status: Fixed (was: Started)
This is completed now.
Cc: baileyberro@chromium.org zentaro@chromium.org

Sign in to add a comment