bump Propolis, Crucible, do not set NVMe VWC for local volumes

Cargo.toml

@@ -747,11 +747,11 @@ progenitor-extras = "0.2.0"
 # NOTE: if you change the pinned revision of the `bhyve_api` and propolis
 # dependencies, you must also update the references in package-manifest.toml to
 # match the new revision.
-bhyve_api = { git = "https://github.com/oxidecomputer/propolis", rev = "979b72896bc39a876c98fb32fe706da37a4fc408" }
-propolis-api-types-versions = { git = "https://github.com/oxidecomputer/propolis", rev = "979b72896bc39a876c98fb32fe706da37a4fc408" }
-propolis_api_types = { git = "https://github.com/oxidecomputer/propolis", rev = "979b72896bc39a876c98fb32fe706da37a4fc408" }
-propolis-client = { git = "https://github.com/oxidecomputer/propolis", rev = "979b72896bc39a876c98fb32fe706da37a4fc408" }
-propolis-mock-server = { git = "https://github.com/oxidecomputer/propolis", rev = "979b72896bc39a876c98fb32fe706da37a4fc408" }
+bhyve_api = { git = "https://github.com/oxidecomputer/propolis", rev = "65417b83f31b63ec1f7a981aeec78d304cec3bd0" }
+propolis-api-types-versions = { git = "https://github.com/oxidecomputer/propolis", rev = "65417b83f31b63ec1f7a981aeec78d304cec3bd0" }
+propolis_api_types = { git = "https://github.com/oxidecomputer/propolis", rev = "65417b83f31b63ec1f7a981aeec78d304cec3bd0" }
+propolis-client = { git = "https://github.com/oxidecomputer/propolis", rev = "65417b83f31b63ec1f7a981aeec78d304cec3bd0" }
+propolis-mock-server = { git = "https://github.com/oxidecomputer/propolis", rev = "65417b83f31b63ec1f7a981aeec78d304cec3bd0" }
 # NOTE: see above!
 proptest = "1.7.0"
 qorb = "0.4.1"

nexus/src/app/instance_platform/mod.rs

@@ -225,7 +225,7 @@ impl DisksByIdBuilder {
         Self { map: BTreeMap::new(), slot_usage: BTreeSet::new() }
     }
 
-    fn add_generic_disk(
+    fn add_nvme_disk(
         &mut self,
         disk: &Disk,
         backend: Component,
@@ -246,12 +246,58 @@ impl DisksByIdBuilder {
 
         let pci_path = slot_to_pci_bdf(slot, PciDeviceKind::Disk)?;
 
+        // Generally we report that storage devices have volatile write cache
+        // semantics. This is a conservative default that matches most system
+        // behaviors. Crucible fast-acks writes and requires flushes to persist
+        // writes to non-volatile storage, and - theoretically - file-backed
+        // disks could be *any file* which may include normal POSIX "you must
+        // fdatasync() for writes to not be lost" semantics.
+        //
+        // This being anything other than "true" must be carefully considered;
+        // incorrectly claiming there is no write cache while the backing
+        // storage has volatile write cache semantics risks guest data loss in
+        // the event of power loss or crashes.
+        //
+        // On the other hand, when we can avoid claiming volatile write cache
+        // semantics, guest OSes know to not send spurious flushes. This can
+        // have important performance consequences from avoided VM exits,
+        // Propolis syscalls, interrupts, etc.
+        let volatile_write_cache = match &backend {
+            // We match on all fields so that if FileStorageBackend changes,
+            // those changes must consider if volatile-write-cache semantics are
+            // correctly captured here.
+            Component::FileStorageBackend(FileStorageBackend {
+                path,
+                readonly: _,
+                block_size: _,
+                workers: _,
+            }) => {
+                // In the product, for the forseeable future, local storage raw
+                // zvols are on enterprise U.2s which do not report volatile
+                // write caches. "/rdsk/" here refers to the character device
+                // for that raw volume with unbuffered semantics (versus
+                // `/dsk/`, the block device, which can buffer writes when not
+                // opened O_DIRECT - see spec_write(), vpm_data_copy(), and
+                // vpm_sync_pages()).
+                //
+                // XXX: In development and non-product environments where
+                // storage may be commodity M.2s or worse, this can claim "no
+                // VWC semantics" when the underlying storage actually does.
+                // This could be improved. See Omicron#10933.
+                let vwc_semantics = !path.starts_with("/dev/zvol/rdsk/");
+
+                vwc_semantics
+            }
+            _ => true,
+        };
+
         let device = Component::NvmeDisk(NvmeDisk {
             backend_id: SpecKey::Uuid(disk.id()),
             pci_path,
             serial_number: zero_padded_nvme_serial_from_str(
                 disk.name().as_str(),
             ),
+            has_write_cache: volatile_write_cache,
         });
 
         let device_name = component_names::device_name_from_id(&disk.id());
@@ -279,10 +325,10 @@ impl DisksByIdBuilder {
                 request_json: volume.data().to_owned(),
             });
 
-        self.add_generic_disk(disk, backend)
+        self.add_nvme_disk(disk, backend)
     }
 
-    fn add_file_backed_disk(
+    fn add_local_disk(
         &mut self,
         disk: &Disk,
         path: String,
@@ -296,7 +342,7 @@ impl DisksByIdBuilder {
             workers: Some(LOCAL_STORAGE_WORKERS),
         });
 
-        self.add_generic_disk(disk, backend)
+        self.add_nvme_disk(disk, backend)
     }
 }
 
@@ -526,7 +572,7 @@ impl super::Nexus {
                 }
 
                 db::datastore::Disk::LocalStorage(local_storage_disk) => {
-                    builder.add_file_backed_disk(
+                    builder.add_local_disk(
                         disk,
                         // Use the delegated zvol as the target for the file
                         // backed disk

openapi/sled-agent/sled-agent-42.0.0-c9afad.json.gitstub

@@ -0,0 +1 @@
+02d41eee19979824212b3963a7edc425d1bd73fa:openapi/sled-agent/sled-agent-42.0.0-c9afad.json

openapi/sled-agent/sled-agent-42.0.0-c9afad.json → openapi/sled-agent/sled-agent-43.0.0-9cba68.json

@@ -7,7 +7,7 @@
       "url": "https://oxide.computer",
       "email": "api@oxide.computer"
     },
-    "version": "42.0.0"
+    "version": "43.0.0"
   },
   "paths": {
     "/artifacts": {
@@ -7461,6 +7461,10 @@
               }
             ]
           },
+          "has_write_cache": {
+            "description": "Control if the NVMe disk reports the presence of a volatile write cache.\n\nThis generally should be configured in consideration of the storage backend for the NVMe device. \"true\" is a safe default, and was historically the only configurable value. If the storage backend will not lose data once writes are accepted, even in the face of unplanned crashes or power loss (or, if you really want to lie to guests), setting this to \"false\" can advise guests they may skip issuing flushes to the device.",
+            "type": "boolean"
+          },
           "pci_path": {
             "description": "The PCI bus/device/function at which this disk should be attached.",
             "allOf": [
@@ -7483,6 +7487,7 @@
         },
         "required": [
           "backend_id",
+          "has_write_cache",
           "pci_path",
           "serial_number"
         ],

openapi/sled-agent/sled-agent-latest.json

@@ -1 +1 @@
-sled-agent-42.0.0-c9afad.json
+sled-agent-43.0.0-9cba68.json

package-manifest.toml

@@ -667,10 +667,10 @@ service_name = "propolis-server"
 only_for_targets.image = "standard"
 source.type = "prebuilt"
 source.repo = "propolis"
-source.commit = "979b72896bc39a876c98fb32fe706da37a4fc408"
+source.commit = "65417b83f31b63ec1f7a981aeec78d304cec3bd0"
 # The SHA256 digest is automatically posted to:
 # https://buildomat.eng.oxide.computer/public/file/oxidecomputer/propolis/image/<commit>/propolis-server.sha256.txt
-source.sha256 = "b950c8da9e2bb61eeb3b7087c27b2edab6c1b341ae8e7db6e88e7c172af58aff"
+source.sha256 = "a22a130cb625c57ce7d4b3468d5ab47b22d6f9529ee49bea98a8a2033851a965"
 output.type = "zone"
 
 [package.mg-ddm-gz]

sled-agent/api/src/lib.rs

@@ -21,7 +21,7 @@ use omicron_common::api::internal::{
 };
 use sled_agent_types_versions::{
     latest, v1, v4, v6, v7, v9, v10, v11, v12, v14, v16, v17, v18, v20, v22,
-    v24, v25, v26, v28, v29, v30, v31, v32, v33, v34, v37, v39, v42,
+    v24, v25, v26, v28, v29, v30, v31, v32, v33, v34, v37, v39, v41, v42,
 };
 use sled_diagnostics::SledDiagnosticsQueryOutput;
 use slog_error_chain::InlineErrorChain;
@@ -38,6 +38,7 @@ api_versions!([
     // |  example for the next person.
     // v
     // (next_int, IDENT),
+    (43, PROPOLIS_NVME_VWC),
     (42, NON_EMPTY_UPLINK_PORTS),
     (41, ADD_INSTANCE_PRIMARY_NIC_MTU),
     (40, ADD_FMD_TO_INVENTORY),
@@ -446,14 +447,28 @@ pub trait SledAgentApi {
         operation_id = "vmm_register",
         method = PUT,
         path = "/vmms/{propolis_id}",
-        versions = VERSION_ADD_INSTANCE_PRIMARY_NIC_MTU..
+        versions = VERSION_PROPOLIS_NVME_VWC..
     }]
     async fn vmm_register(
         rqctx: RequestContext<Self::Context>,
         path_params: Path<latest::instance::VmmPathParam>,
         body: TypedBody<latest::instance::InstanceEnsureBody>,
     ) -> Result<HttpResponseOk<latest::instance::SledVmmState>, HttpError>;
 
+    #[endpoint {
+        operation_id = "vmm_register",
+        method = PUT,
+        path = "/vmms/{propolis_id}",
+        versions = VERSION_ADD_INSTANCE_PRIMARY_NIC_MTU..VERSION_PROPOLIS_NVME_VWC
+    }]
+    async fn vmm_register_v41(
+        rqctx: RequestContext<Self::Context>,
+        path_params: Path<latest::instance::VmmPathParam>,
+        body: TypedBody<v41::instance::InstanceEnsureBody>,
+    ) -> Result<HttpResponseOk<latest::instance::SledVmmState>, HttpError> {
+        Self::vmm_register(rqctx, path_params, body.map(Into::into)).await
+    }
+
     #[endpoint {
         operation_id = "vmm_register",
         method = PUT,
@@ -465,7 +480,7 @@ pub trait SledAgentApi {
         path_params: Path<latest::instance::VmmPathParam>,
         body: TypedBody<v32::instance::InstanceEnsureBody>,
     ) -> Result<HttpResponseOk<latest::instance::SledVmmState>, HttpError> {
-        Self::vmm_register(rqctx, path_params, body.map(Into::into)).await
+        Self::vmm_register_v41(rqctx, path_params, body.map(Into::into)).await
     }
 
     #[endpoint {

sled-agent/types/versions/src/add_vsock_component/instance.rs

@@ -5,7 +5,7 @@
 use std::net::SocketAddr;
 
 use omicron_uuid_kinds::InstanceUuid;
-use propolis_api_types::instance_spec::InstanceSpec;
+use propolis_api_types_versions::v3::instance_spec::InstanceSpec;
 use schemars::JsonSchema;
 use serde::{Deserialize, Serialize};
 use uuid::Uuid;

sled-agent/types/versions/src/impls/instance.rs

@@ -10,11 +10,12 @@ use crate::latest::instance::VmmSpec;
 use crate::latest::instance::VmmState;
 use crate::latest::instance::VmmStateRequested;
 use crate::latest::inventory::SourceNatConfig;
-use propolis_api_types::instance_spec::{
-    Component, SpecKey,
+use propolis_api_types::instance_spec::SpecKey;
+use propolis_api_types_versions::latest::{
     components::backends::{
         CrucibleStorageBackend, FileStorageBackend, VirtioNetworkBackend,
     },
+    instance_spec::Component,
 };
 use std::collections::BTreeSet;
 use std::net::Ipv4Addr;

sled-agent/types/versions/src/latest.rs

@@ -114,15 +114,15 @@ pub mod instance {
     pub use crate::v7::instance::InstanceMulticastBody;
     pub use crate::v7::instance::InstanceMulticastMembership;
 
-    pub use crate::v29::instance::VmmSpec;
-
     pub use crate::v31::instance::ResolvedVpcFirewallRule;
     pub use crate::v32::instance::ExternalIpConfig;
     pub use crate::v32::instance::ExternalIps;
     pub use crate::v32::instance::ExternalIpv4Config;
     pub use crate::v32::instance::ExternalIpv6Config;
-    pub use crate::v41::instance::InstanceEnsureBody;
     pub use crate::v41::instance::InstanceSledLocalConfig;
+
+    pub use crate::v43::instance::InstanceEnsureBody;
+    pub use crate::v43::instance::VmmSpec;
 }
 
 pub mod inventory {

sled-agent/types/versions/src/lib.rs

@@ -91,6 +91,8 @@ pub mod v40;
 pub mod v41;
 #[path = "non_empty_uplink_ports/mod.rs"]
 pub mod v42;
+#[path = "propolis_nvme_vwc/mod.rs"]
+pub mod v43;
 #[path = "add_probe_put_endpoint/mod.rs"]
 pub mod v6;
 #[path = "multicast_support/mod.rs"]