Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(245)

Issue 7493063: Get the proper serial number for device enrollment. (Closed)

Created:
9 years, 4 months ago by Mattias Nissler (ping if slow)
Modified:
9 years, 4 months ago
Reviewers:
Louis, pastarmovj
CC:
chromium-reviews, Glenn Wilson
Visibility:
Public.

Description

Get the proper serial number for device enrollment. BUG=chromium-os:18198 TEST=Enroll a device and check the serial number it sends. Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=94697

Patch Set 1 #

Total comments: 4
Unified diffs Side-by-side diffs Delta from patch set Stats (+23 lines, -4 lines) Patch
M chrome/browser/policy/device_policy_identity_strategy.cc View 2 chunks +23 lines, -4 lines 4 comments Download

Messages

Total messages: 7 (0 generated)
Mattias Nissler (ping if slow)
This is the backported serial number fix for M13. Please review!
9 years, 4 months ago (2011-07-28 22:12:32 UTC) #1
Louis
Thanks for doing this. See my 2 cents inline. Sorry that I didn't find the ...
9 years, 4 months ago (2011-07-29 02:52:05 UTC) #2
Mattias Nissler (ping if slow)
http://codereview.chromium.org/7493063/diff/1/chrome/browser/policy/device_policy_identity_strategy.cc File chrome/browser/policy/device_policy_identity_strategy.cc (right): http://codereview.chromium.org/7493063/diff/1/chrome/browser/policy/device_policy_identity_strategy.cc#newcode35 chrome/browser/policy/device_policy_identity_strategy.cc:35: "serial_number" // VPD v2+ devices On 2011/07/29 02:52:05, Louis ...
9 years, 4 months ago (2011-07-29 09:19:52 UTC) #3
Mattias Nissler (ping if slow)
Julian, can I get a quote on this?
9 years, 4 months ago (2011-07-29 14:36:15 UTC) #4
pastarmovj
LGTM.
9 years, 4 months ago (2011-07-29 15:08:55 UTC) #5
Louis
http://codereview.chromium.org/7493063/diff/1/chrome/browser/policy/device_policy_identity_strategy.cc File chrome/browser/policy/device_policy_identity_strategy.cc (right): http://codereview.chromium.org/7493063/diff/1/chrome/browser/policy/device_policy_identity_strategy.cc#newcode35 chrome/browser/policy/device_policy_identity_strategy.cc:35: "serial_number" // VPD v2+ devices On 2011/07/29 09:19:52, Mattias ...
9 years, 4 months ago (2011-08-01 08:18:11 UTC) #6
Mattias Nissler (ping if slow)
9 years, 4 months ago (2011-08-01 08:31:17 UTC) #7
http://codereview.chromium.org/7493063/diff/1/chrome/browser/policy/device_po...
File chrome/browser/policy/device_policy_identity_strategy.cc (right):

http://codereview.chromium.org/7493063/diff/1/chrome/browser/policy/device_po...
chrome/browser/policy/device_policy_identity_strategy.cc:35: "serial_number"  //
VPD v2+ devices
On 2011/08/01 08:18:11, Louis wrote:
> On 2011/07/29 09:19:52, Mattias Nissler wrote:
> > On 2011/07/29 02:52:05, Louis wrote:
> > > I'd like to suggest to re-order the list. Try "serial_number" first
(because
> > it
> > > is the official name in the future), then "sn" and "Product_S/N".
> > 
> > That's unfortunately not possible due to the name clash mentioned in the
> comment
> > above. The information we access through GetMachineStatistic not only comes
> from
> > VPD, but also other sources. One of them is the mosys output, which also
> > includes a "serial_number" field. Making "serial_number" the first choice
> would
> > result in the value from mosys being used in case the VPD doesn't give us
one,
> > which is not what we want.
> What argument do you use for mosys? Does its serial_number mean the same
thing?

/sbin/chromeos_startup does this:

mosys -k smbios info system

Which dumps a serial_number that is different from the one we get from VPD, and,
what's worse, not actually machine-specific (all the Alex devices I tested on
returned the same serial number string).

> > 
> > > 
> > > By the way, we are going to show VPD 1.0 structure in the vpd_2.0.txt too
> > > (http://gerrit.chromium.org/gerrit/4552
> > > ) so that you may add "Product_SN" for CR-48.
> > 
> > OK, I guess this is only relevant once that support has landed? Let's make
> that
> > change on ToT then.
> FYI. I've committed in that code.
> 

Thanks, I'll add "Product_SN" to the list then.

Powered by Google App Engine
This is Rietveld 408576698