From db79b5a06e416d6ee70cbb97919962c427a64ea8 Mon Sep 17 00:00:00 2001 From: Alex Lyn Date: Fri, 1 Aug 2025 16:38:56 +0800 Subject: [PATCH 1/3] runtime-rs: move get_scsi_id_lun upper within hotplug_block_device Move the closure get_scsi_id_lun upper within hotplug_block_device and make it more helpful. Signed-off-by: Alex Lyn --- .../crates/hypervisor/src/qemu/qmp.rs | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/runtime-rs/crates/hypervisor/src/qemu/qmp.rs b/src/runtime-rs/crates/hypervisor/src/qemu/qmp.rs index a3fb754433..8683d61428 100644 --- a/src/runtime-rs/crates/hypervisor/src/qemu/qmp.rs +++ b/src/runtime-rs/crates/hypervisor/src/qemu/qmp.rs @@ -541,6 +541,15 @@ impl Qmp { is_readonly: bool, no_drop: bool, ) -> Result<(Option, Option)> { + // Helper closure to decode a flattened u16 SCSI index into an (ID, LUN) pair. + let get_scsi_id_lun = |index_u16: u16| -> Result<(u8, u8)> { + // Uses bitwise operations for efficient and clear conversion. + let scsi_id = (index_u16 >> 8) as u8; // Equivalent to index_u16 / 256 + let lun = (index_u16 & 0xFF) as u8; // Equivalent to index_u16 % 256 + + Ok((scsi_id, lun)) + }; + // `blockdev-add` let node_name = format!("drive-{index}"); @@ -618,15 +627,6 @@ impl Qmp { blkdev_add_args.insert("drive".to_owned(), node_name.clone().into()); if block_driver == VIRTIO_SCSI { - // Helper closure to decode a flattened u16 SCSI index into an (ID, LUN) pair. - let get_scsi_id_lun = |index_u16: u16| -> Result<(u8, u8)> { - // Uses bitwise operations for efficient and clear conversion. - let scsi_id = (index_u16 >> 8) as u8; // Equivalent to index_u16 / 256 - let lun = (index_u16 & 0xFF) as u8; // Equivalent to index_u16 % 256 - - Ok((scsi_id, lun)) - }; - // Safely convert the u64 index to u16, ensuring it does not exceed `u16::MAX` (65535). let (scsi_id, lun) = get_scsi_id_lun(u16::try_from(index)?)?; let scsi_addr = format!("{}:{}", scsi_id, lun); From 5b40a7927e31d95fa1df18e93b89e5225d2762b9 Mon Sep 17 00:00:00 2001 From: Alex Lyn Date: Fri, 1 Aug 2025 16:44:26 +0800 Subject: [PATCH 2/3] runtime-rs: Add idempotency to hotplug block device operations Due to the lack of atomicity in the operation, a partial failure can lead to an inconsistent QEMU state, which pollutes subsequent operations. This can easily trigger a "Duplicate nodes" error. To prevent this, we should query the state before performing the operation. ee should ensure its validation and idempotency when making the function idempotent allows it to be safely retried. Fixes #11649 Signed-off-by: Alex Lyn --- .../crates/hypervisor/src/qemu/qmp.rs | 49 +++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/src/runtime-rs/crates/hypervisor/src/qemu/qmp.rs b/src/runtime-rs/crates/hypervisor/src/qemu/qmp.rs index 8683d61428..4952629641 100644 --- a/src/runtime-rs/crates/hypervisor/src/qemu/qmp.rs +++ b/src/runtime-rs/crates/hypervisor/src/qemu/qmp.rs @@ -553,6 +553,55 @@ impl Qmp { // `blockdev-add` let node_name = format!("drive-{index}"); + // Pre-check block drive and block device with qapi + { + let node_exists = self + .qmp + .execute(&qapi_qmp::query_named_block_nodes { flat: Some(true) })? + .into_iter() + .any(|d| d.node_name == Some(node_name.clone())); + let device_exists = self + .qmp + .execute(&qapi_qmp::query_block {})? + .into_iter() + .any(|d| match d.inserted { + Some(node) => node.node_name == Some(node_name.clone()), + None => false, + }); + + if node_exists && device_exists { + if block_driver == VIRTIO_SCSI { + // Safely convert the u64 index to u16, ensuring it does not exceed `u16::MAX` (65535). + let (scsi_id, lun) = get_scsi_id_lun(u16::try_from(index)?)?; + let scsi_addr = format!("{}:{}", scsi_id, lun); + + return Ok((None, Some(scsi_addr))); + } else { + let pci_path = self + .get_device_by_qdev_id(&node_name) + .context("get device by qdev_id failed")?; + info!( + sl!(), + "hotplug block device return pci path: {:?}", &pci_path + ); + + return Ok((Some(pci_path), None)); + } + } + + if node_exists && !device_exists { + warn!( + sl!(), + "Found orphaned backend node {:?}, do cleanup before retry.", &node_name + ); + self.qmp + .execute(&qapi_qmp::blockdev_del { + node_name: node_name.clone(), + }) + .ok(); + } + } + let create_base_options = || qapi_qmp::BlockdevOptionsBase { auto_read_only: None, cache: if is_direct.is_none() { From c66a4be41ef8a8bdc5ad41865951cbbc6a243b48 Mon Sep 17 00:00:00 2001 From: Alex Lyn Date: Thu, 7 Aug 2025 18:58:09 +0800 Subject: [PATCH 3/3] runtime-rs: Support share-rw=true when hotplug block device within qemu Support for the share-rw=true parameter has been added. While this parameter is essential for maintaining data consistency across multiple QEMU instances sharing a backend disk image, its implementation also serves to standardize parameters with the block device hotplug functionality in kata-runtime/qemu. Signed-off-by: Alex Lyn --- src/runtime-rs/crates/hypervisor/src/qemu/qmp.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/runtime-rs/crates/hypervisor/src/qemu/qmp.rs b/src/runtime-rs/crates/hypervisor/src/qemu/qmp.rs index 4952629641..65976d41b3 100644 --- a/src/runtime-rs/crates/hypervisor/src/qemu/qmp.rs +++ b/src/runtime-rs/crates/hypervisor/src/qemu/qmp.rs @@ -683,6 +683,7 @@ impl Qmp { // add SCSI frontend device blkdev_add_args.insert("scsi-id".to_string(), scsi_id.into()); blkdev_add_args.insert("lun".to_string(), lun.into()); + blkdev_add_args.insert("share-rw".to_string(), true.into()); self.qmp .execute(&qmp::device_add { @@ -703,6 +704,7 @@ impl Qmp { } else { let (bus, slot) = self.find_free_slot()?; blkdev_add_args.insert("addr".to_owned(), format!("{:02}", slot).into()); + blkdev_add_args.insert("share-rw".to_string(), true.into()); self.qmp .execute(&qmp::device_add {