nvme: Pass controller instance ID to NvmeDriver for better tracing #1661
+176
−43
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -175,7 +175,7 @@ impl NvmeManager { | |
| enum Request { | ||
| Inspect(inspect::Deferred), | ||
| ForceLoadDriver(inspect::DeferredUpdate), | ||
| GetNamespace(Rpc<(String, String, u32), Result<nvme_driver::Namespace, NamespaceError>>), | ||
| Save(Rpc<(), Result<NvmeManagerSavedState, anyhow::Error>>), | ||
| Shutdown { | ||
| span: tracing::Span, | ||
|
|
@@ -191,13 +191,22 @@ pub struct NvmeManagerClient { | |
| impl NvmeManagerClient { | ||
| pub async fn get_namespace( | ||
| &self, | ||
| debug_id: String, | ||
| pci_id: String, | ||
| nsid: u32, | ||
| ) -> anyhow::Result<nvme_driver::Namespace> { | ||
| Ok(self | ||
| .sender | ||
| .call( | ||
| Request::GetNamespace, | ||
| (debug_id.clone(), pci_id.clone(), nsid), | ||
| ) | ||
| .instrument(tracing::info_span!( | ||
| "nvme_get_namespace", | ||
| debug_id, | ||
| pci_id, | ||
| nsid | ||
| )) | ||
| .await | ||
| .context("nvme manager is shut down")??) | ||
| } | ||
|
|
@@ -235,7 +244,10 @@ impl NvmeManagerWorker { | |
| match req { | ||
| Request::Inspect(deferred) => deferred.inspect(&self), | ||
| Request::ForceLoadDriver(update) => { | ||
| match self | ||
| .get_driver("force-load".to_string(), update.new_value().to_owned()) | ||
| .await | ||
| { | ||
| Ok(_) => { | ||
| let pci_id = update.new_value().to_string(); | ||
| update.succeed(pci_id); | ||
|
|
@@ -246,8 +258,8 @@ impl NvmeManagerWorker { | |
| } | ||
| } | ||
| Request::GetNamespace(rpc) => { | ||
| rpc.handle(async |(debug_id, pci_id, nsid)| { | ||
| self.get_namespace(debug_id, pci_id.clone(), nsid) | ||
| .map_err(|source| NamespaceError { pci_id, source }) | ||
| .await | ||
| }) | ||
|
|
@@ -283,9 +295,12 @@ impl NvmeManagerWorker { | |
| if !nvme_keepalive || !self.save_restore_supported { | ||
| async { | ||
| join_all(self.devices.drain().map(|(pci_id, driver)| { | ||
| let debug_id = driver.debug_id(); | ||
| driver.shutdown().instrument(tracing::info_span!( | ||
| "shutdown_nvme_driver", | ||
| pci_id, | ||
| debug_id | ||
| )) | ||
| })) | ||
| .await | ||
| } | ||
|
|
@@ -296,6 +311,7 @@ impl NvmeManagerWorker { | |
|
|
||
| async fn get_driver( | ||
| &mut self, | ||
| debug_id: String, | ||
| pci_id: String, | ||
| ) -> Result<&mut nvme_driver::NvmeDriver<VfioDevice>, InnerError> { | ||
| let driver = match self.devices.entry(pci_id.to_owned()) { | ||
|
|
@@ -328,6 +344,7 @@ impl NvmeManagerWorker { | |
| self.vp_count, | ||
| device, | ||
| self.is_isolated, | ||
| debug_id, | ||
| ) | ||
| .instrument(tracing::info_span!( | ||
| "nvme_driver_init", | ||
|
|
@@ -344,10 +361,11 @@ impl NvmeManagerWorker { | |
|
|
||
| async fn get_namespace( | ||
| &mut self, | ||
| debug_id: String, | ||
| pci_id: String, | ||
| nsid: u32, | ||
| ) -> Result<nvme_driver::Namespace, InnerError> { | ||
| let driver = self.get_driver(debug_id, pci_id.to_owned()).await?; | ||
| driver | ||
| .namespace(nsid) | ||
| .await | ||
|
|
@@ -407,6 +425,7 @@ impl NvmeManagerWorker { | |
| vfio_device, | ||
| &disk.driver_state, | ||
| self.is_isolated, | ||
| None, // controller_instance_id is not persisted in saved state | ||
|
||
| ) | ||
| .instrument(tracing::info_span!("nvme_driver_restore")) | ||
| .await?; | ||
|
|
@@ -440,20 +459,31 @@ impl AsyncResolveResource<DiskHandleKind, NvmeDiskConfig> for NvmeDiskResolver { | |
| ) -> Result<Self::Output, Self::Error> { | ||
| let namespace = self | ||
| .manager | ||
| .get_namespace(rsrc.debug_id, rsrc.pci_id, rsrc.nsid) | ||
| .await | ||
| .context("could not open nvme namespace")?; | ||
|
|
||
| Ok(ResolvedDisk::new(disk_nvme::NvmeDisk::new(namespace)).context("invalid disk")?) | ||
| } | ||
| } | ||
|
|
||
| #[derive(MeshPayload)] | ||
| pub struct NvmeDiskConfig { | ||
| pub debug_id: String, | ||
| pub pci_id: String, | ||
| pub nsid: u32, | ||
| } | ||
|
|
||
| impl Default for NvmeDiskConfig { | ||
| fn default() -> Self { | ||
| Self { | ||
| debug_id: "force-load".to_string(), | ||
|
||
| pci_id: String::new(), | ||
| nsid: 0, | ||
| } | ||
| } | ||
| } | ||
|
|
||
| impl ResourceId<DiskHandleKind> for NvmeDiskConfig { | ||
| const ID: &'static str = "nvme"; | ||
| } | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is prone for errors. Make each String strongly typed, so that the caller can't confuse the
debug_idand thepci_id.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Created strongly typed
NvmeDeviceNameandPciIdwrapper types withGetNamespaceParamsstruct to prevent parameter confusion in RPC calls. This eliminates the risk of mixing up the name and pci_id parameters. (commit 02fbdbc)