Conversation
| })?; | ||
| let id = Uuid::from_str(s).map_err(|e| e.to_string())?; | ||
| Ok(ZpoolName(id)) | ||
| if let Some(s) = s.strip_prefix(ZPOOL_EXTERNAL_PREFIX) { |
There was a problem hiding this comment.
nit: this looks duplicated from above. I think you could just call one impl from the other?
There was a problem hiding this comment.
Sure, updated to have the serde impl call from_str.
|
|
||
| /* TODO: Could also store physical disk FK here */ | ||
| /* FK into the Physical Disk table */ | ||
| physical_disk_id UUID NOT NULL, |
There was a problem hiding this comment.
It seems like we're officially baking in the assumption that each zpool is backed by exactly one physical disk here. I guess we should update the comment above.
There was a problem hiding this comment.
Sure, updated (from "Typically these are backed by a single physical disk" to simply "These are backed by a single physical disk").
| .filter(dsl::serial.eq(serial.clone())) | ||
| .filter(dsl::model.eq(model.clone())) | ||
| .select(PhysicalDisk::as_select()) | ||
| .first_async(self.pool()) |
There was a problem hiding this comment.
nit: pool_authorized()?
| .map_err(|e| public_error_from_diesel_pool(e, ErrorHandler::Server)) | ||
| } | ||
|
|
||
| pub async fn physical_disk_lookup( |
There was a problem hiding this comment.
Should we have a physical_disk LookupPath type?
There was a problem hiding this comment.
I believe we already do:
omicron/nexus/db-queries/src/db/lookup.rs
Lines 678 to 685 in e4a5dd0
The primary key is already an ID, but I'm not sure how to say "this object can also be uniquely looked-up by vendor/serial/model".
There was a problem hiding this comment.
Went ahead and updated the PK columns here: 1563a70 -- seems to work
There was a problem hiding this comment.
Okay, actually, this required some follow-up changes in d997cc3 too
| pub size: ByteCount, | ||
|
|
||
| // Information to identify the disk to which this zpool belongs | ||
| pub vendor: String, |
There was a problem hiding this comment.
nit: prefix here, like "disk_vendor"? (it seems a little confusing to call these the "vendor", "serial", and "model" of the zpool)
There was a problem hiding this comment.
Updated for vendor, serial, and model.
| // NOTE: This relies on the name being a UUID exactly. | ||
| // We could be more flexible... | ||
| Ok(Pool { id: name.id(), info, zones: HashMap::new(), disk_path }) | ||
| Ok(Pool { name, info, zones: HashMap::new(), parent }) |
There was a problem hiding this comment.
The comment above this line seems not quite true?
Sled Agent (and
illumos-utils,sled-hardware)oxi_prefix, in addition to those with theoxp_prefixNexus
This is the first part of #2688