New issue
Advanced search Search tips

Issue 773873 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: Oct 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug

Blocking:
issue 773859



Sign in to add a comment

shill's device storage identifier generation is unnecessarily complicated

Project Member Reported by benchan@chromium.org, Oct 11 2017

Issue description

The device storage identifier generation in shill is unnecessarily complicated:

- It starts with the RPC Identifier, e.g.                      => /device/eth0
- Strips off the leading '/'                                   => device/eth0
- Replaces remaining '/' with '_'                              => device_eth0
- Replaces the substring after the first '_' with MAC address  => device_aabbccddeeff

In the end, the storage identifier is essentially in the form of 'device_<mac>', which is loosely based on the RPC identifier for no good reasons. We could change the RPC identifier format over time but the storage identifier format (without proper migration), which is used to look up device entries in the shill profile.  For that, we should decouple the storage identifier from the RPC identifier, and further simplify the storage identifier generation.


 
Status: Started (was: Assigned)
Blocking: 773859
Status: Fixed (was: Started)
Project Member

Comment 4 by bugdroid1@chromium.org, Oct 13 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/aosp/platform/system/connectivity/shill/+/2091f4c74a128d0ff8a444f4d6333094f034a989

commit 2091f4c74a128d0ff8a444f4d6333094f034a989
Author: Ben Chan <benchan@chromium.org>
Date: Fri Oct 13 07:26:58 2017

shill: simplify device storage identifier generation

The device storage identifier is unnecessarily coupled with the RPC
identifier, which they share nothing but a similar prefix (e.g. storage
identifier: device_0011223344 vs RPC identifier: /device/eth0). While
it's ok to change the format of device RPC identifier in the future, we
need to ensure the format of device storage identifier remains unchanged
(unless we migrate device storage identifiers from the old to new format
in shill profiles). This CL decouples the device storage identifier
generation from the RPC identifier, which in turn simplifies the device
storage identifier generation. The format of device storage identifier
remains unchanged to ensure backward compatibility.

BUG= chromium:773873 
TEST=Run unit tests.
TEST=Manually verify that shill finds matching device entries in an
     existing profile.

Change-Id: I2571e83679008aa5f38385e86c6445b45e737380
Reviewed-on: https://chromium-review.googlesource.com/715178
Commit-Ready: Ben Chan <benchan@chromium.org>
Tested-by: Ben Chan <benchan@chromium.org>
Reviewed-by: Eric Caruso <ejcaruso@chromium.org>

[modify] https://crrev.com/2091f4c74a128d0ff8a444f4d6333094f034a989/control_interface.h
[modify] https://crrev.com/2091f4c74a128d0ff8a444f4d6333094f034a989/device_unittest.cc
[modify] https://crrev.com/2091f4c74a128d0ff8a444f4d6333094f034a989/device.cc

Comment 5 by dchan@chromium.org, Jan 22 2018

Status: archived (was: Fixed)

Comment 6 by dchan@chromium.org, Jan 23 2018

Status: Fixed (was: Archived)
Status: Verified (was: Fixed)

Sign in to add a comment