Add sled slot to disks in externals /hardware/disks endpoint - #11221
fakemonster wants to merge 4 commits into
Conversation
Using an IdOrdMap means less scanning for disk slots on calls to the /system/disks API, but it has a few side effects, all of which I believe are benign: 1. While the DiskIdentity strikes me as a reasonable key, it doesn't match the primary key `inv_physical_disk` uses, and so a DB lookup could in principle report the same DiskIdentity for two slots, and we'll drop one of the duplicates now. This deduplication also applies at collection time, meaning if prior to this commit we were recording two disks with the same identity into crdb, that'll no longer happen (which may then hide an unhealthy case we'd want to catch!). But I don't believe this is a problem in principle, because the `physical_disk` table itself has a unique index on the components of DiskIdentity, so a duplicate disk would be unadoptable anyways. 2. Relatedly, since that case seems unlikely, I opted for `from_iter` instead of `from_iter_unique`. It would perhaps be more rigorous to attempt `from_iter_unique`, and loudly fall back to `from_iter`? 3. When omdb lists disks, they'll now be ordered by their identity (and omit duplicates, which as mentioned before, I'm not so sure happens in practice). I figure no one is bothered by this, but it's a difference!
hawkw
left a comment
There was a problem hiding this comment.
Thanks for taking this on! I had some relatively minor notes. It would be nice to test this on a live system such as a racklette and makes sure that the API returns what we would expect.
| collection | ||
| .sled_agents | ||
| .get(&sled_id) | ||
| .and_then(|a| a.disks.get(&disk_identity).map(|d| d.slot)) |
There was a problem hiding this comment.
nitpicky: since all of this is inside of an and_then closure, if we use ? here, we'll just bail out of that closure with a None, instead of bailing out of to_external_api. So, I think this might read a little better if we rewrote it like:
| collection | |
| .sled_agents | |
| .get(&sled_id) | |
| .and_then(|a| a.disks.get(&disk_identity).map(|d| d.slot)) | |
| let disk = collection | |
| .sled_agents | |
| .get(&sled_id)? | |
| .disks | |
| .get(&disk_identity)?; | |
| Some(disk.slot) |
or similar
| /// The physical slot in the sled where this disk was last observed to be | ||
| /// located, or null if its location is not known at this time. | ||
| pub slot: Option<i64>, |
There was a problem hiding this comment.
I don't love that this is an i64 in the API. Is that really how it's represented in the inventory? That seems a bit unfortunate. This might be necessary because it's a u64 or something and CockroachDB doesn't have unsigned numbers, but if that's the case, we should probably be converting this into an unsigned number...somewhere...between here and the DB, because we probably don't want the OpenAPI document to tell callers that this could be a negative number, since it won't be.
We may also want to think about using a much smaller number; on current generation Oxide sleds, there won't be more than 2 M.2s and 10 U.2s. I don't know if we want to state that in the API document in case we eventually do a system with 24 E1.S SSDs or something, but I can't really imagine any reason we would actually need 64 bits of slot number unless a sled gives us back something weird during inventory collection?
| /// The physical slot in the sled where this disk was last observed to be | ||
| /// located, or null if its location is not known at this time. |
There was a problem hiding this comment.
It would be nice if this comment had some more detail about what this value means, since it's going to end up in the API docs. In particular, I think we should:
- make sure it's clear that these correspond to the physical slot numbers marked on the chassis
- document how the slot numbers of M.2 and U.2 devices work --- I don't know off the top of my head whether the sled-agent will return slots like N0, N1, N2,... for U.2s and also use numbers 0 and 1 for M.2s or if there is one set of slot numbers that has the M.2s cast as 10 and 11, so we should double check this and make sure we document it here.
There was a problem hiding this comment.
Update: I did some poking around on the dogfood rack with omdb, and it looks like the M.2 devices actually end up in slots 17 and 18, which surprised me a bit:
[dogfood]root@oxz_switch0:~# omdb db inventory physical-disks
note: database URL not specified. Will search DNS.
note: (override with --db-url or OMDB_DB_URL)
note: using DNS from system config (typically /etc/resolv.conf)
note: (if this is not right, use --dns-server to specify an alternate DNS server)
note: using database URL postgresql://root@[fd00:1122:3344:109::3]:32221,[fd00:1122:3344:105::3]:32221,[fd00:1122:3344:10b::3]:32221,[fd00:1122:3344:107::3]:32221,[fd00:1122:3344:108::3]:32221/omicron?sslmode=disable
note: database schema version matches expected (297.0.0)
INV_COLLECTION_ID SLED_ID SLOT VENDOR MODEL SERIAL VARIANT FIRMWARE NEXT_FIRMWARE
1532212c-ef05-4b64-a25e-2551a22176a5 0c7011f7-a4bf-4daf-90cc-1c2410103300 0 1b96 WUS4C6432DSP3X3 A084A704 U2 R2210010 R2210010
1532212c-ef05-4b64-a25e-2551a22176a5 0c7011f7-a4bf-4daf-90cc-1c2410103300 1 1b96 WUS4C6432DSP3X3 A084A5DA U2 R2210010 R2210010
1532212c-ef05-4b64-a25e-2551a22176a5 0c7011f7-a4bf-4daf-90cc-1c2410103300 2 1b96 WUS4C6432DSP3X3 A084A7DF U2 R2210010 R2210010
1532212c-ef05-4b64-a25e-2551a22176a5 0c7011f7-a4bf-4daf-90cc-1c2410103300 3 1b96 WUS4C6432DSP3X3 A084A5EE U2 R2210010 R2210010
1532212c-ef05-4b64-a25e-2551a22176a5 0c7011f7-a4bf-4daf-90cc-1c2410103300 4 1b96 WUS4C6432DSP3X3 A084A617 U2 R2210010 R2210010
1532212c-ef05-4b64-a25e-2551a22176a5 0c7011f7-a4bf-4daf-90cc-1c2410103300 5 1b96 WUS4C6432DSP3X3 A084A6C9 U2 R2210010 R2210010
1532212c-ef05-4b64-a25e-2551a22176a5 0c7011f7-a4bf-4daf-90cc-1c2410103300 6 1b96 WUS4C6432DSP3X3 A084A723 U2 R2210010 R2210010
1532212c-ef05-4b64-a25e-2551a22176a5 0c7011f7-a4bf-4daf-90cc-1c2410103300 7 1b96 WUS4C6432DSP3X3 A084A643 U2 R2210010 R2210010
1532212c-ef05-4b64-a25e-2551a22176a5 0c7011f7-a4bf-4daf-90cc-1c2410103300 8 1b96 WUS4C6432DSP3X3 A084A820 U2 R2210010 R2210010
1532212c-ef05-4b64-a25e-2551a22176a5 0c7011f7-a4bf-4daf-90cc-1c2410103300 9 1b96 WUS4C6432DSP3X3 A084A60F U2 R2210010 R2210010
1532212c-ef05-4b64-a25e-2551a22176a5 0c7011f7-a4bf-4daf-90cc-1c2410103300 17 1344 Micron_7300_MTFDHBG1T9TDF 21413275374B M2 95420280
1532212c-ef05-4b64-a25e-2551a22176a5 0c7011f7-a4bf-4daf-90cc-1c2410103300 18 1344 Micron_7300_MTFDHBG1T9TDF 214132748192 M2 95420280
...I guess these slot numbers are being produced based on the PCIe bus topology or something, which makes sense...but we should make sure returning these numbers is correct based on how the devices are actually labeled in the product. If they are labeled differently, this change is going to get a bit more complicated, since the goal here is really to make sure we are returning the actual physical label on the slot (see RFD 200), rather than passing along whatever value the sled gives us.
|
On a related note to #11221 (comment), I also saw some sleds on [dogfood]root@oxz_switch0:~# omdb db inventory physical-disks
note: database URL not specified. Will search DNS.
note: (override with --db-url or OMDB_DB_URL)
note: using DNS from system config (typically /etc/resolv.conf)
note: (if this is not right, use --dns-server to specify an alternate DNS server)
note: using database URL postgresql://root@[fd00:1122:3344:109::3]:32221,[fd00:1122:3344:105::3]:32221,[fd00:1122:3344:10b::3]:32221,[fd00:1122:3344:107::3]:32221,[fd00:1122:3344:108::3]:32221/omicron?sslmode=disable
note: database schema version matches expected (297.0.0)
INV_COLLECTION_ID SLED_ID SLOT VENDOR MODEL SERIAL VARIANT FIRMWARE NEXT_FIRMWARE
1532212c-ef05-4b64-a25e-2551a22176a5 0c7011f7-a4bf-4daf-90cc-1c2410103300 0 1b96 WUS4C6432DSP3X3 A084A704 U2 R2210010 R2210010
1532212c-ef05-4b64-a25e-2551a22176a5 0c7011f7-a4bf-4daf-90cc-1c2410103300 1 1b96 WUS4C6432DSP3X3 A084A5DA U2 R2210010 R2210010
1532212c-ef05-4b64-a25e-2551a22176a5 0c7011f7-a4bf-4daf-90cc-1c2410103300 2 1b96 WUS4C6432DSP3X3 A084A7DF U2 R2210010 R2210010
1532212c-ef05-4b64-a25e-2551a22176a5 0c7011f7-a4bf-4daf-90cc-1c2410103300 3 1b96 WUS4C6432DSP3X3 A084A5EE U2 R2210010 R2210010
1532212c-ef05-4b64-a25e-2551a22176a5 0c7011f7-a4bf-4daf-90cc-1c2410103300 4 1b96 WUS4C6432DSP3X3 A084A617 U2 R2210010 R2210010
1532212c-ef05-4b64-a25e-2551a22176a5 0c7011f7-a4bf-4daf-90cc-1c2410103300 5 1b96 WUS4C6432DSP3X3 A084A6C9 U2 R2210010 R2210010
1532212c-ef05-4b64-a25e-2551a22176a5 0c7011f7-a4bf-4daf-90cc-1c2410103300 6 1b96 WUS4C6432DSP3X3 A084A723 U2 R2210010 R2210010
1532212c-ef05-4b64-a25e-2551a22176a5 0c7011f7-a4bf-4daf-90cc-1c2410103300 7 1b96 WUS4C6432DSP3X3 A084A643 U2 R2210010 R2210010
1532212c-ef05-4b64-a25e-2551a22176a5 0c7011f7-a4bf-4daf-90cc-1c2410103300 8 1b96 WUS4C6432DSP3X3 A084A820 U2 R2210010 R2210010
1532212c-ef05-4b64-a25e-2551a22176a5 0c7011f7-a4bf-4daf-90cc-1c2410103300 9 1b96 WUS4C6432DSP3X3 A084A60F U2 R2210010 R2210010
1532212c-ef05-4b64-a25e-2551a22176a5 0c7011f7-a4bf-4daf-90cc-1c2410103300 17 1344 Micron_7300_MTFDHBG1T9TDF 21413275374B M2 95420280
1532212c-ef05-4b64-a25e-2551a22176a5 0c7011f7-a4bf-4daf-90cc-1c2410103300 18 1344 Micron_7300_MTFDHBG1T9TDF 214132748192 M2 95420280
...
f6c0c0ab-1f8e-49ae-b009-4b24e1c504b3 4d0e70d5-a8ad-40b1-aa7b-209e2165bc75 32 1b96 WUS4C6432DSP3X3 A084A5EC U2 R2210010 R2210010
f6c0c0ab-1f8e-49ae-b009-4b24e1c504b3 4d0e70d5-a8ad-40b1-aa7b-209e2165bc75 33 1b96 WUS4C6432DSP3X3 A084A7E5 U2 R2210010 R2210010
f6c0c0ab-1f8e-49ae-b009-4b24e1c504b3 4d0e70d5-a8ad-40b1-aa7b-209e2165bc75 34 1b96 WUS4C6432DSP3X3 A084A7CC U2 R2210010 R2210010
f6c0c0ab-1f8e-49ae-b009-4b24e1c504b3 4d0e70d5-a8ad-40b1-aa7b-209e2165bc75 35 1b96 WUS4C6432DSP3X3 A084A68F U2 R2210010 R2210010
f6c0c0ab-1f8e-49ae-b009-4b24e1c504b3 4d0e70d5-a8ad-40b1-aa7b-209e2165bc75 36 1b96 WUS4C6432DSP3X3 A084A73E U2 R2210010 R2210010
f6c0c0ab-1f8e-49ae-b009-4b24e1c504b3 4d0e70d5-a8ad-40b1-aa7b-209e2165bc75 37 1b96 WUS4C6432DSP3X3 A084A77D U2 R2210010 R2210010
f6c0c0ab-1f8e-49ae-b009-4b24e1c504b3 4d0e70d5-a8ad-40b1-aa7b-209e2165bc75 38 1b96 WUS4C6432DSP3X3 A084A5BE U2 R2210010 R2210010
f6c0c0ab-1f8e-49ae-b009-4b24e1c504b3 4d0e70d5-a8ad-40b1-aa7b-209e2165bc75 39 1b96 WUS4C6432DSP3X3 A079E73D U2 R2210010 R2210010
f6c0c0ab-1f8e-49ae-b009-4b24e1c504b3 4d0e70d5-a8ad-40b1-aa7b-209e2165bc75 40 1b96 WUS4C6432DSP3X3 A084A5F8 U2 R2210010 R2210010
f6c0c0ab-1f8e-49ae-b009-4b24e1c504b3 4d0e70d5-a8ad-40b1-aa7b-209e2165bc75 41 1b96 WUS4C6432DSP3X3 A079E2CD U2 R2210010 R2210010
...
ab34fa8b-482b-4eea-a928-3c547b4a9002 781ab3c9-ff3c-428a-9f3e-895a1e86a111 17 1344 Micron_7450_MTFDKBG1T9TFR 22314564B5D8 M2 E2MU300
ab34fa8b-482b-4eea-a928-3c547b4a9002 781ab3c9-ff3c-428a-9f3e-895a1e86a111 18 1344 Micron_7450_MTFDKBG1T9TFR 25034E674499 M2 E2MU300
ab34fa8b-482b-4eea-a928-3c547b4a9002 781ab3c9-ff3c-428a-9f3e-895a1e86a111 32 1b96 WUS4C6432DSP3X3 A084A804 U2 R2210010 R2210010
ab34fa8b-482b-4eea-a928-3c547b4a9002 781ab3c9-ff3c-428a-9f3e-895a1e86a111 33 1b96 WUS4C6432DSP3X3 A079DF0A U2 R2210010 R2210010
ab34fa8b-482b-4eea-a928-3c547b4a9002 781ab3c9-ff3c-428a-9f3e-895a1e86a111 34 1b96 WUS4C6432DSP3X3 A084A7E3 U2 R2210010 R2210010
ab34fa8b-482b-4eea-a928-3c547b4a9002 781ab3c9-ff3c-428a-9f3e-895a1e86a111 35 1b96 WUS4C6432DSP3X3 A079E760 U2 R2210010 R2210010
ab34fa8b-482b-4eea-a928-3c547b4a9002 781ab3c9-ff3c-428a-9f3e-895a1e86a111 36 1b96 WUS4C6432DSP3X3 A079E752 U2 R2210010 R2210010
ab34fa8b-482b-4eea-a928-3c547b4a9002 781ab3c9-ff3c-428a-9f3e-895a1e86a111 37 1b96 WUS4C6432DSP3X3 A079E7A5 U2 R2210010 R2210010
ab34fa8b-482b-4eea-a928-3c547b4a9002 781ab3c9-ff3c-428a-9f3e-895a1e86a111 38 1b96 WUS4C6432DSP3X3 A079E425 U2 R2210010 R2210010
ab34fa8b-482b-4eea-a928-3c547b4a9002 781ab3c9-ff3c-428a-9f3e-895a1e86a111 39 1b96 WUS4C6432DSP3X3 A079DFC5 U2 R2210010 R2210010
ab34fa8b-482b-4eea-a928-3c547b4a9002 781ab3c9-ff3c-428a-9f3e-895a1e86a111 40 1b96 WUS4C6432DSP3X3 A079E74E U2 R2210010 R2210010
ab34fa8b-482b-4eea-a928-3c547b4a9002 781ab3c9-ff3c-428a-9f3e-895a1e86a111 41 1b96 WUS4C6432DSP3X3 A079E707 U2 R2210010 R2210010
...This worried me a bit since, again, as I said in #11221 (comment), the goal here is to make sure the information in the customer-facing API is reporting slot numbers that represent the slot marking on the physical chassis, and not internal details of PCIe bus topology or similar. I think I know what's going on here but will do some more digging to confirm. |
[dogfood]root@oxz_switch0:~# omdb db sleds
note: database URL not specified. Will search DNS.
note: (override with --db-url or OMDB_DB_URL)
note: using DNS from system config (typically /etc/resolv.conf)
note: (if this is not right, use --dns-server to specify an alternate DNS server)
note: using database URL postgresql://root@[fd00:1122:3344:109::3]:32221,[fd00:1122:3344:105::3]:32221,[fd00:1122:3344:10b::3]:32221,[fd00:1122:3344:107::3]:32221,[fd00:1122:3344:108::3]:32221/omicron?sslmode=disable
note: database schema version matches expected (297.0.0)
note: listing all commissioned sleds (use -F to filter, e.g. -F in-service)
SERIAL IP ROLE POLICY STATE BP AVAIL ID
BRM42220057 [fd00:1122:3344:104::1]:12345 - in service active available 0c7011f7-a4bf-4daf-90cc-1c2410103300
BRM44220011 [fd00:1122:3344:103::1]:12345 - in service active available 2707b587-9c7f-4fb0-a7af-37c3b7a9a0fa
BRM22250001 [fd00:1122:3344:128::1]:12345 - in service active available 4d0e70d5-a8ad-40b1-aa7b-209e2165bc75
BRM42220017 [fd00:1122:3344:109::1]:12345 - in service active available 5f6720b8-8a31-45f8-8c94-8e699218f28b
BRM42220051 [fd00:1122:3344:10b::1]:12345 scrimlet in service active available 71def415-55ad-46b4-ba88-3ca55d7fb287
BRM13250012 [fd00:1122:3344:129::1]:12345 - in service active available 781ab3c9-ff3c-428a-9f3e-895a1e86a111
BRM27230045 [fd00:1122:3344:127::1]:12345 - in service active available 7b473a3b-4ec2-4b58-8376-9b3cb68d1392
BRM44220010 [fd00:1122:3344:101::1]:12345 - in service active available 7b862eb6-7f50-4c2f-b9a6-0d12ac913d3c
BRM42220014 [fd00:1122:3344:108::1]:12345 scrimlet in service active available 87c2c4fc-b0c7-4fef-a305-78f0ed265bbc
BRM42220031 [fd00:1122:3344:102::1]:12345 - in service active available a2adea92-b56e-44fc-8a0d-7d63b5fd3b93
BRM42220006 [fd00:1122:3344:106::1]:12345 - in service active available b886b58a-1e3f-4be1-b9f2-0c2e66c6bc88
BRM42220009 [fd00:1122:3344:107::1]:12345 - in service active available db183874-65b5-4263-a1c1-ddb2737ae0e9
BRM42220016 [fd00:1122:3344:10a::1]:12345 - in service active available dd83e75a-1edf-4aa1-89a0-cd8b2091a7cd
BRM44220005 [fd00:1122:3344:105::1]:12345 - in service active available f15774c1-b8e5-434f-a493-ec43f96cba06Okay, the sleds whose numbering starts at 32 rather than 0 are [dogfood]root@oxz_switch0:~# pilot sp ls
CUBBY SERIAL TYPE IMAGE IP
0 - - - -
1 - - - -
2 BRM22250001 cosmo c6936cdddad8d869 fe80::aa40:25ff:fe04:c86
3 BRM13250012 cosmo c6936cdddad8d869 fe80::aa40:25ff:fe04:402
4 - - - -
5 - - - -
6 - - - -
7 BRM27230045 gimlet aa915b62f157d678 fe80::aa40:25ff:fe04:6c6
8 BRM44220011 gimlet 35c296036a3feb4b fe80::aa40:25ff:fe04:3c5
9 BRM44220005 gimlet 35c296036a3feb4b fe80::aa40:25ff:fe04:347
10 BRM42220009 gimlet 35c296036a3feb4b fe80::aa40:25ff:fe04:3c4
11 BRM42220006 gimlet 35c296036a3feb4b fe80::aa40:25ff:fe04:181
12 BRM42220057 gimlet 35c296036a3feb4b fe80::aa40:25ff:fe04:383
13 BRM42220018 gimlet b8cf06f5c2c228cd fe80::aa40:25ff:fe04:1c1
14 BRM42220051 gimlet 35c296036a3feb4b fe80::aa40:25ff:fe04:185
15 - - - -
16 BRM42220014 gimlet 35c296036a3feb4b fe80::aa40:25ff:fe04:342
17 BRM42220017 gimlet 35c296036a3feb4b fe80::aa40:25ff:fe04:182
18 - - - -
19 - - - -
20 - - - -
21 BRM42220031 gimlet 35c296036a3feb4b fe80::aa40:25ff:fe04:343
22 - - - -
23 BRM42220016 gimlet 35c296036a3feb4b fe80::aa40:25ff:fe04:385
24 - - - -
25 BRM44220010 gimlet 35c296036a3feb4b fe80::aa40:25ff:fe04:344
26 - - - -
27 - - - -
28 - - - -
29 - - - -
30 - - - -
31 - - - -Yup, I thought so! I'm glad to have realized this now, instead of later, but unfortunately, that's kinda bad news for this PR (sorry Joe!). This is going to end up getting a lot more complicated in order to make sure we are returning the physical slot number, and I'm not totally sure what the best way to solve this is yet. |
There was a problem hiding this comment.
Okay, sorry to really throw a wrench in the works here, but I think the approach we're using here is not really going to work (through no fault of yours!). As I discovered in #11221 (comment) and #11221 (comment), on Cosmo, the "slot" number from the sled-agent's inventory response does not correspond to the physical marking on the chassis slot. I believe this is because the values for "slots" that sled-agent returns is determined based on the topology of the PCIe bus, which is different between Cosmo and Gimlet (but I'd love confirmation from someone who knows more about illumos devfs). This means that on Cosmos, these numbers aren't going to match the label on the U.2 slot on the sled, which is what this API needs to be returning, since it's how an operator or technician would actually locate the U.2.
I can think of a couple approaches to handling this, but I'm not really sure of the details, and we'll need to do more research or get input from others. Basically, I'm thinking that at a high level, the potential directions we could go are:
- We could just have the code you added in this PR also check if the sled is a Gimlet or Cosmo, and map the slot numbers in the inventory to the physical slot numbers based on the sled family. I don't know if I love this, especially because the representation of those slot numbers in the inventory will then be different from what we return in the public API, which feels bad and potentially confusing. If we want to, later on, add APIs for things like looking up a disk by its location in the sled, we would have to remember to apply the same re-mapping there too.
- The more complex, but probably better, option, would be to change the inventory to include the physical slot number. I think that if the PCIe-based slot numbers are being used (and they probably are?) we may want to do that in addition to these numbers rather than by replacing them. Either the sled-agent or Nexus could maintain a mapping of which slot corresponds to what physical location based on the sled model, and we could apply that during inventory collection. Or, perhaps this information could be retrieved by the sled-agent using
libtopowhen it reports its inventory? We'd have to do additional poking around to figure out how that would work, but I think this is the more correct direction to go in.
Poking around a bit in the sled-agent code, it looks like this code in sled_hardware::illumos is where we actually determine the slot property to report:
omicron/sled-hardware/src/illumos/mod.rs
Lines 453 to 461 in df990b0
Then, there's a somewhat complex path this takes from the HardwareManager's disks view to the ConfigReconciler's inventory which is what makes it into the sled-agent's actual response to inventory requests. I'm not really sure where in here the right place to add additional information about the slot-as-marked-on-the-chassis would be, and I'm also not really sure if we would have to do this by just matching on the sled type and rewriting these numbers if it's a Cosmo, or if there is a more authoritative way to ask the host OS for the slot-number-as-marked-on-the-chassis (which would be ideal). Perhaps @jgallagher or @smklein know more about this?
|
For the record, @fakemonster, please don't feel like it is your responsibility to run the Cosmo slot-numbering stuff I discussed in #11221 (review) to ground. The API changes here look pretty good, and I'm going to make a separate issue to discuss what we should do about inventory disk slot numbers. If you want to just set this PR down until we get that figured out, and then come back and update this a little bit once there's a way to access the information we actually want here, I think that would make sense. Of course, you want to pull the thread and go on an adventure into other layers of the system, though, it could be fun and educational! If that's the case, I'd be happy to help out, though I think we're gonna want some advice from others on this. |
|
Okay, I opened #11258 to discuss figuring out what to do about Cosmo's slot numbers. |
Kind of like #11077, but for disks! Particularly it's similar in that it's grafting the slot number on from an inventory check. The hope in the linked issue is that it'd be unnecessary in this case because
physical_diskgets recreated when disks are moved between sleds, so we could potentially add aslotcolumn tophsyical_diskand skip the inventory check. But as far as I can tell, a disk readopted to the same sled won't go through the same flow, so we'd need bigger changes to accomplish that.Additionally, this refactors the inventory's disk
Vecinto anIdOrdMapfor a bit of convenience. This is done as its own commit to be easily revertable, in case it's undesirable, but I believe it's a generally reasonable improvement, and the side effects justified (discussed in that commit's message body).Resolves #11088