Fix various clippy nits

Author: pfmooney

bin/propolis-server/src/lib/mock_server.rs

@@ -126,7 +126,7 @@ impl InstanceContext {
                     self.state_watcher_tx
                         .send(api::InstanceStateMonitorResponse {
                             gen: self.generation,
-                            state: self.state.clone(),
+                            state: self.state,
                             migration: None,
                         })
                         .map_err(|_| Error::TransitionSendFail)
@@ -223,7 +223,7 @@ async fn instance_ensure(
             ))
         })?;
         base64::engine::general_purpose::STANDARD
-            .decode(&cloud_init_bytes)
+            .decode(cloud_init_bytes)
             .map_err(|e| {
                 let err = IoError::new(ErrorKind::InvalidInput, e.to_string());
                 HttpError::for_internal_error(format!(
@@ -253,7 +253,7 @@ async fn instance_get(
     })?;
     let instance_info = api::Instance {
         properties: instance.properties.clone(),
-        state: instance.state.clone(),
+        state: instance.state,
         disks: vec![],
         nics: vec![],
     };

bin/propolis-server/src/lib/server.rs

@@ -196,8 +196,10 @@ impl ServiceProviders {
             );
         }
         if let Some(serial_task) = self.serial_task.lock().await.take() {
-            let _ =
-                serial_task.control_ch.send(SerialTaskControlMessage::Stopping);
+            let _ = serial_task
+                .control_ch
+                .send(SerialTaskControlMessage::Stopping)
+                .await;
             // Wait for the serial task to exit
             let _ = serial_task.task.await;
         }

bin/propolis-server/src/main.rs

@@ -66,6 +66,77 @@ pub fn run_openapi() -> Result<(), String> {
         .map_err(|e| e.to_string())
 }
 
+#[cfg(not(feature = "mock-only"))]
+async fn run_server(
+    config_app: config::Config,
+    config_dropshot: dropshot::ConfigDropshot,
+    metrics_addr: Option<SocketAddr>,
+    vnc_addr: SocketAddr,
+    log: slog::Logger,
+) -> anyhow::Result<()> {
+    // Check that devices conform to expected API version
+    propolis::api_version::check().context("API version checks")?;
+
+    let vnc_server = setup_vnc(&log, vnc_addr);
+    let vnc_server_hdl = vnc_server.clone();
+    let use_reservoir = config::reservoir_decide(&log);
+
+    let config_metrics = metrics_addr.map(|addr| {
+        let imc =
+            MetricsEndpointConfig::new(config_dropshot.bind_address, addr);
+        info!(log, "Metrics server will use {:?}", imc);
+        imc
+    });
+
+    let context = server::DropshotEndpointContext::new(
+        config_app,
+        vnc_server,
+        use_reservoir,
+        log.new(slog::o!()),
+        config_metrics,
+    );
+
+    info!(log, "Starting server...");
+
+    let server = HttpServerStarter::new(
+        &config_dropshot,
+        server::api(),
+        Arc::new(context),
+        &log,
+    )
+    .map_err(|error| anyhow!("Failed to start server: {}", error))?
+    .start();
+
+    let server_res = join!(server, vnc_server_hdl.start()).0;
+
+    server_res.map_err(|e| anyhow!("Server exited with an error: {}", e))
+}
+
+#[cfg(feature = "mock-only")]
+async fn run_server(
+    config_app: config::Config,
+    config_dropshot: dropshot::ConfigDropshot,
+    _metrics_addr: Option<SocketAddr>,
+    _vnc_addr: SocketAddr,
+    log: slog::Logger,
+) -> anyhow::Result<()> {
+    let context = server::Context::new(config_app, log.new(slog::o!()));
+
+    info!(log, "Starting server...");
+
+    let server = HttpServerStarter::new(
+        &config_dropshot,
+        server::api(),
+        Arc::new(context),
+        &log,
+    )
+    .map_err(|error| anyhow!("Failed to start server: {}", error))?
+    .start();
+
+    let server_res = server.await;
+    server_res.map_err(|e| anyhow!("Server exited with an error: {}", e))
+}
+
 #[tokio::main]
 async fn main() -> anyhow::Result<()> {
     // Ensure proper setup of USDT probes
@@ -93,54 +164,8 @@ async fn main() -> anyhow::Result<()> {
                 |error| anyhow!("failed to create logger: {}", error),
             )?;
 
-            let context;
-            cfg_if::cfg_if! {
-                if #[cfg(not(feature = "mock-only"))] {
-                    // Check that devices conform to expected API version
-                    propolis::api_version::check().context("API version checks")?;
-
-                    let vnc_server = setup_vnc(&log, vnc_addr);
-                    let vnc_server_hdl = vnc_server.clone();
-                    let use_reservoir = config::reservoir_decide(&log);
-
-                    let metric_config = metric_addr.map(|addr| {
-                        let imc = MetricsEndpointConfig::new(propolis_addr, addr);
-                        info!(log, "Metrics server will use {:?}", imc);
-                        imc
-                    });
-                    context = server::DropshotEndpointContext::new(
-                        config,
-                        vnc_server,
-                        use_reservoir,
-                        log.new(slog::o!()),
-                        metric_config,
-                    );
-                } else {
-                    context = server::Context::new(config, log.new(slog::o!()));
-                }
-            }
-
-            info!(log, "Starting server...");
-
-            let server = HttpServerStarter::new(
-                &config_dropshot,
-                server::api(),
-                Arc::new(context),
-                &log,
-            )
-            .map_err(|error| anyhow!("Failed to start server: {}", error))?
-            .start();
-
-            let server_res;
-            cfg_if::cfg_if! {
-                if #[cfg(not(feature = "mock-only"))] {
-                    server_res = join!(server, vnc_server_hdl.start()).0;
-                } else {
-                    server_res = server.await;
-                }
-            };
-            server_res
-                .map_err(|e| anyhow!("Server exited with an error: {}", e))
+            run_server(config, config_dropshot, metric_addr, vnc_addr, log)
+                .await
         }
     }
 }

lib/propolis-client/src/lib.rs

@@ -128,7 +128,7 @@ pub mod support {
         > {
             let value = self.ws_stream.next().await;
             if let Some(Ok(WSMessage::Text(json))) = &value {
-                match serde_json::from_str(&json) {
+                match serde_json::from_str(json) {
                     Ok(InstanceSerialConsoleControlMessage::Migrating {
                         destination,
                         from_start,

phd-tests/framework/src/disk/file.rs

@@ -41,6 +41,11 @@ impl FileBackedDisk {
         // read-only).
         let disk_file = std::fs::File::open(&disk_path)?;
         let mut permissions = disk_file.metadata()?.permissions();
+
+        // TODO: Clippy is upset that `set_readonly(false)` results in
+        // world-writable files on UNIX-like OSes.  Suppress the lint for now
+        // until someone gets around to a more specific solution.
+        #[allow(clippy::permissions_set_readonly_false)]
         permissions.set_readonly(false);
         disk_file.set_permissions(permissions)?;
 

phd-tests/framework/src/test_vm/mod.rs

@@ -349,7 +349,7 @@ impl TestVm {
                     .map_err(|e| anyhow!("error during migration: {}", e))
             }
             VmState::Ensured { .. } => {
-                return Err(VmStateError::InstanceAlreadyEnsured.into());
+                Err(VmStateError::InstanceAlreadyEnsured.into())
             }
         }
     }
@@ -396,11 +396,8 @@ impl TestVm {
         );
 
         let wait_fn = || -> Result<(), backoff::Error<anyhow::Error>> {
-            let current = self
-                .get()
-                .map_err(|e| backoff::Error::Permanent(e.into()))?
-                .instance
-                .state;
+            let current =
+                self.get().map_err(backoff::Error::Permanent)?.instance.state;
             if current == target {
                 Ok(())
             } else {

phd-tests/tests/src/migrate.rs

@@ -22,7 +22,7 @@ fn smoke_test(ctx: &TestContext) {
         .new_vm_from_cloned_config("migration_smoke_target", &source)?;
 
     let serial_hist_pre = source.get_serial_console_history(0)?;
-    assert!(serial_hist_pre.data.len() > 0);
+    assert!(!serial_hist_pre.data.is_empty());
 
     let migration_id = Uuid::new_v4();
     target.migrate_from(&source, migration_id, Duration::from_secs(60))?;