review feedback

illumos-utils/src/zfs.rs

@@ -260,9 +260,27 @@ impl Zfs {
         Ok(())
     }
 
-    /// Creates a new ZFS filesystem named `name`, unless one already exists.
+    /// Creates a new ZFS filesystem unless one already exists.
     ///
-    /// Applies an optional quota, provided _in bytes_.
+    /// - `name`: the full path to the zfs dataset
+    /// - `mountpoint`: The expected mountpoint of this filesystem.
+    /// If the filesystem already exists, and is not mounted here, and error is
+    /// returned.
+    /// - `zoned`: identifies whether or not this filesystem should be
+    /// used in a zone. Only used when creating a new filesystem - ignored
+    /// if the filesystem already exists.
+    /// - `do_format`: if "false", prevents a new filesystem from being created,
+    /// and returns an error if it is not found.
+    /// - `encryption_details`: Ensures a filesystem as an encryption root.
+    /// For new filesystems, this supplies the key, and all datasets within this
+    /// root are implicitly encrypted. For existing filesystems, ensures that
+    /// they are mounted (and that keys are loaded), but does not verify the
+    /// input details.
+    /// - `size_details`: If supplied, sets size-related information. These
+    /// values are set on both new filesystem creation as well as when loading
+    /// existing filesystems.
+    /// - `additional_options`: Additional ZFS options, which are only set when
+    /// creating new filesystems.
     #[allow(clippy::too_many_arguments)]
     pub fn ensure_filesystem(
         name: &str,

openapi/sled-agent.json

@@ -179,6 +179,7 @@
     },
     "/datasets": {
       "get": {
+        "summary": "Lists the datasets that this sled is configured to use",
         "operationId": "datasets_get",
         "responses": {
           "200": {
@@ -200,6 +201,7 @@
         }
       },
       "put": {
+        "summary": "Configures datasets to be used on this sled",
         "operationId": "datasets_put",
         "requestBody": {
           "content": {

sled-agent/src/http_entrypoints.rs

@@ -352,6 +352,7 @@ async fn omicron_zones_get(
     Ok(HttpResponseOk(sa.omicron_zones_list().await?))
 }
 
+/// Configures datasets to be used on this sled
 #[endpoint {
     method = PUT,
     path = "/datasets",
@@ -366,6 +367,7 @@ async fn datasets_put(
     Ok(HttpResponseOk(result))
 }
 
+/// Lists the datasets that this sled is configured to use
 #[endpoint {
     method = GET,
     path = "/datasets",
@@ -374,7 +376,7 @@ async fn datasets_get(
     rqctx: RequestContext<SledAgent>,
 ) -> Result<HttpResponseOk<DatasetsConfig>, HttpError> {
     let sa = rqctx.context();
-    Ok(HttpResponseOk(sa.datasets_list().await?))
+    Ok(HttpResponseOk(sa.datasets_config_list().await?))
 }
 
 #[endpoint {

sled-agent/src/sled_agent.rs

@@ -810,8 +810,8 @@ impl SledAgent {
         self.inner.zone_bundler.cleanup().await.map_err(Error::from)
     }
 
-    pub async fn datasets_list(&self) -> Result<DatasetsConfig, Error> {
-        Ok(self.storage().datasets_list().await?)
+    pub async fn datasets_config_list(&self) -> Result<DatasetsConfig, Error> {
+        Ok(self.storage().datasets_config_list().await?)
     }
 
     pub async fn datasets_ensure(

sled-storage/src/manager.rs

@@ -273,7 +273,7 @@ impl StorageHandle {
 
     /// Reads the last value written to storage by
     /// [Self::datasets_ensure].
-    pub async fn datasets_list(&self) -> Result<DatasetsConfig, Error> {
+    pub async fn datasets_config_list(&self) -> Result<DatasetsConfig, Error> {
         let (tx, rx) = oneshot::channel();
         self.tx
             .send(StorageRequest::DatasetsList { tx: tx.into() })
@@ -479,7 +479,7 @@ impl StorageManager {
                 let _ = tx.0.send(self.datasets_ensure(config).await);
             }
             StorageRequest::DatasetsList { tx } => {
-                let _ = tx.0.send(self.datasets_list().await);
+                let _ = tx.0.send(self.datasets_config_list().await);
             }
             StorageRequest::OmicronPhysicalDisksEnsure { config, tx } => {
                 let _ =
@@ -790,8 +790,9 @@ impl StorageManager {
         status
     }
 
-    async fn datasets_list(&mut self) -> Result<DatasetsConfig, Error> {
-        let log = self.log.new(o!("request" => "datasets_list"));
+    // Lists datasets that this sled is configured to use.
+    async fn datasets_config_list(&mut self) -> Result<DatasetsConfig, Error> {
+        let log = self.log.new(o!("request" => "datasets_config_list"));
 
         let ledger_paths = self.all_omicron_dataset_ledgers().await;
         let maybe_ledger =
@@ -1637,7 +1638,8 @@ mod tests {
         assert!(!status.has_error());
 
         // List datasets, expect to see what we just created
-        let observed_config = harness.handle().datasets_list().await.unwrap();
+        let observed_config =
+            harness.handle().datasets_config_list().await.unwrap();
         assert_eq!(config, observed_config);
 
         // Calling "datasets_ensure" with the same input should succeed.