From d144c7b2d80acaea189aa435da45d4f6014d0218 Mon Sep 17 00:00:00 2001 From: Fan Xuan Zhe Date: Tue, 20 May 2025 16:15:36 +0800 Subject: [PATCH 01/20] address_space: Refactor AddressRange ordering and clarify file open behavior Reimplemented `PartialOrd` and `Ord` for `AddressRange` to avoid relying on `unwrap()` and improve correctness and clarity of ordering logic. Adjusted doc comments formatting in `AddressSpace::addr_cache_init` for better readability. Explicitly set `truncate(false)` in `FileBackend` to prevent unintended file truncation during mmap-backed file operations. Signed-off-by: Fan Xuan Zhe --- address_space/src/address.rs | 12 ++++++------ address_space/src/address_space.rs | 1 + address_space/src/host_mmap.rs | 4 ++-- address_space/src/region.rs | 2 +- 4 files changed, 10 insertions(+), 9 deletions(-) diff --git a/address_space/src/address.rs b/address_space/src/address.rs index 5d1d02b67..658fdc973 100644 --- a/address_space/src/address.rs +++ b/address_space/src/address.rs @@ -141,18 +141,18 @@ impl From<(u64, u64)> for AddressRange { /// Implement PartialOrd trait for AddressRange. impl PartialOrd for AddressRange { fn partial_cmp(&self, other: &AddressRange) -> Option { - if self.base != other.base { - self.base.partial_cmp(&other.base) - } else { - self.size.partial_cmp(&other.size) - } + Some(self.cmp(other)) } } /// Implement Ord trait for AddressRange. impl Ord for AddressRange { fn cmp(&self, other: &AddressRange) -> Ordering { - self.partial_cmp(other).unwrap() + if self.base != other.base { + self.base.cmp(&other.base) + } else { + self.size.cmp(&other.size) + } } } diff --git a/address_space/src/address_space.rs b/address_space/src/address_space.rs index 95cad2ac9..a3b49eea5 100644 --- a/address_space/src/address_space.rs +++ b/address_space/src/address_space.rs @@ -529,6 +529,7 @@ impl AddressSpace { /// # Arguments /// /// * `addr` - Guest address. + /// /// Return Error if the `addr` is not mapped. /// or return the HVA address and available mem length pub fn addr_cache_init(&self, addr: GuestAddress, attr: AddressAttr) -> Option<(u64, u64)> { diff --git a/address_space/src/host_mmap.rs b/address_space/src/host_mmap.rs index d31d4fd34..a89aa4adf 100644 --- a/address_space/src/host_mmap.rs +++ b/address_space/src/host_mmap.rs @@ -112,6 +112,7 @@ impl FileBackend { .read(true) .write(true) .create(true) + .truncate(false) .open(path) .with_context(|| format!("Failed to open file: {}", file_path))? }; @@ -351,9 +352,8 @@ fn set_host_memory_policy(mem_mappings: &Arc, zone: &MemZoneConf let nodes = zone.host_numa_nodes.as_ref().unwrap(); let mut max_node = nodes[nodes.len() - 1] as usize; - let mut nmask: Vec = Vec::new(); // Upper limit of max_node is MAX_NODES. - nmask.resize(max_node / 64 + 1, 0); + let mut nmask: Vec = vec![0; max_node / 64 + 1]; for node in nodes.iter() { nmask[(*node / 64) as usize] |= 1_u64 << (*node % 64); } diff --git a/address_space/src/region.rs b/address_space/src/region.rs index 48c70302a..6c1be857e 100644 --- a/address_space/src/region.rs +++ b/address_space/src/region.rs @@ -696,7 +696,7 @@ impl Region { .with_context(|| "Failed to write buffer to Ram")?; } RegionType::RomDevice | RegionType::IO => { - if count >= std::usize::MAX as u64 { + if count >= usize::MAX as u64 { return Err(anyhow!(AddressSpaceError::Overflow(count))); } let mut slice = vec![0_u8; count as usize]; -- Gitee From fa8411fa4b3fd4abe9c453843dcb6339486a80cb Mon Sep 17 00:00:00 2001 From: Fan Xuan Zhe Date: Tue, 20 May 2025 16:44:23 +0800 Subject: [PATCH 02/20] block_backend: Refactor error handling with `inspect_err` for clarity Replaced `map_err(...e...)` patterns with `inspect_err(...)` in `file.rs` and `qcow2/mod.rs` Improves readability and makes intent clearer by avoiding unnecessary error remapping Fix typos Signed-off-by: Fan Xuan Zhe --- block_backend/src/file.rs | 3 +-- block_backend/src/qcow2/mod.rs | 11 ++++------- block_backend/src/qcow2/refcount.rs | 8 ++++---- 3 files changed, 9 insertions(+), 13 deletions(-) diff --git a/block_backend/src/file.rs b/block_backend/src/file.rs index 3acef0381..e1a07389c 100644 --- a/block_backend/src/file.rs +++ b/block_backend/src/file.rs @@ -235,9 +235,8 @@ impl FileIoHandler { fn aio_complete_handler(&mut self) -> Result { let error_cb = self.error_cb.clone(); - self.aio.borrow_mut().handle_complete().map_err(|e| { + self.aio.borrow_mut().handle_complete().inspect_err(|_e| { error_cb(); - e }) } } diff --git a/block_backend/src/qcow2/mod.rs b/block_backend/src/qcow2/mod.rs index 1151798d7..ddb3f7e1e 100644 --- a/block_backend/src/qcow2/mod.rs +++ b/block_backend/src/qcow2/mod.rs @@ -1431,9 +1431,8 @@ impl InternalSnapshotOps for Qcow2Driver { // when creating snapshot failed. self.flush()?; self.qcow2_create_snapshot(name, vm_clock_nsec) - .map_err(|e| { + .inspect_err(|_e| { self.drop_dirty_caches(); - e }) } @@ -1441,17 +1440,15 @@ impl InternalSnapshotOps for Qcow2Driver { // Flush the dirty metadata first, so it can drop dirty caches for reverting // when deleting snapshot failed. self.flush()?; - self.qcow2_delete_snapshot(name).map_err(|e| { + self.qcow2_delete_snapshot(name).inspect_err(|_e| { self.drop_dirty_caches(); - e }) } fn apply_snapshot(&mut self, name: String) -> Result<()> { self.flush()?; - self.qcow2_apply_snapshot(name).map_err(|e| { + self.qcow2_apply_snapshot(name).inspect_err(|_e| { self.drop_dirty_caches(); - e }) } @@ -1880,7 +1877,7 @@ impl BlockDriverOps for Qcow2Driver { let offset_end = std::cmp::min(offset_start + nbytes, file_size); let mut total_bytes = offset_end.checked_sub(offset_start).with_context(|| { format!( - "Write zeroes: ofset: {} nbytes: {} out of range", + "Write zeroes: offset: {} nbytes: {} out of range", offset, nbytes ) })?; diff --git a/block_backend/src/qcow2/refcount.rs b/block_backend/src/qcow2/refcount.rs index 1c7ff10e2..ffb37cba1 100644 --- a/block_backend/src/qcow2/refcount.rs +++ b/block_backend/src/qcow2/refcount.rs @@ -199,7 +199,7 @@ impl RefCount { /// * `start_idx` - alloc space for the new refcount table starting from the start index. /// * `new_table_clusters` - number of clusters for new refcount table. /// * `new_block_clusters` - number of clusters for refcount block, the size of refcount blocks - /// should be guaranteed to record all newly added clusters. + /// should be guaranteed to record all newly added clusters. fn extend_refcount_table( &mut self, header: &mut QcowHeader, @@ -304,9 +304,8 @@ impl RefCount { let rb_addr = self.refcount_table[rt_idx as usize]; if rb_addr == 0 { - self.alloc_refcount_block(rt_idx).map_err(|e| { + self.alloc_refcount_block(rt_idx).inspect_err(|_e| { self.refcount_table[rt_idx as usize] = 0; - e })?; } } @@ -649,7 +648,8 @@ impl RefCount { /// * `cluster_size` - size of cluster in bytes. /// * `refcount_order` - refcount bits power of 2 exponent. /// * `is_reserve` - if this parameter set true, the refcount table size -/// will have 50% more entries than necessary, which can avoid growing again soon. +/// will have 50% more entries than necessary, which can avoid growing again soon. +/// /// Returns: (clusters of new refcount table, clusters of refcount block) pub fn refcount_metadata_size( nb_clusters: u64, -- Gitee From 6b3e414a675806468b911041d022638345d6d4bc Mon Sep 17 00:00:00 2001 From: Fan Xuan Zhe Date: Tue, 20 May 2025 16:47:49 +0800 Subject: [PATCH 03/20] boot_loader: Refactor GDT loop and unify doc comment formatting Removed unused index variable in GDT table write loop for cleaner code Adjusted comment indentation in bzImage boot process documentation for consistency and readability Signed-off-by: Fan Xuan Zhe --- boot_loader/src/x86_64/direct_boot/gdt.rs | 2 +- boot_loader/src/x86_64/direct_boot/mod.rs | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/boot_loader/src/x86_64/direct_boot/gdt.rs b/boot_loader/src/x86_64/direct_boot/gdt.rs index 07995a061..227387c3a 100644 --- a/boot_loader/src/x86_64/direct_boot/gdt.rs +++ b/boot_loader/src/x86_64/direct_boot/gdt.rs @@ -92,7 +92,7 @@ impl From for u64 { fn write_gdt_table(table: &[u64], guest_mem: &Arc) -> Result<()> { let mut boot_gdt_addr = BOOT_GDT_OFFSET; - for (_, entry) in table.iter().enumerate() { + for entry in table.iter() { guest_mem .write_object(entry, GuestAddress(boot_gdt_addr), AddressAttr::Ram) .with_context(|| format!("Failed to load gdt to 0x{:x}", boot_gdt_addr))?; diff --git a/boot_loader/src/x86_64/direct_boot/mod.rs b/boot_loader/src/x86_64/direct_boot/mod.rs index c910d8225..07fb1645a 100644 --- a/boot_loader/src/x86_64/direct_boot/mod.rs +++ b/boot_loader/src/x86_64/direct_boot/mod.rs @@ -38,11 +38,11 @@ use util::byte_code::ByteCode; /// According to Linux `Documentation/x86/boot.txt`, bzImage includes two parts: /// * the setup /// * the compressed kernel -/// The setup `RealModeKernelHeader` can be load at offset `0x01f1` in bzImage kernel image. -/// The compressed kernel will be loaded into guest memory at `code32_start` in -/// `RealModeKernelHeader`. -/// The start address of compressed kernel is the loader address + 0x200. It will be -/// set in `kernel_start` in `BootLoader` structure set. +/// The setup `RealModeKernelHeader` can be load at offset `0x01f1` in bzImage kernel image. +/// The compressed kernel will be loaded into guest memory at `code32_start` in +/// `RealModeKernelHeader`. +/// The start address of compressed kernel is the loader address + 0x200. It will be +/// set in `kernel_start` in `BootLoader` structure set. /// /// # Arguments /// -- Gitee From 0e6ac44464061833ff6dc300aece5851702e6da6 Mon Sep 17 00:00:00 2001 From: Fan Xuan Zhe Date: Tue, 20 May 2025 16:51:21 +0800 Subject: [PATCH 04/20] chardev_ackend: Clarify file open behavior Explicitly set `truncate(false)` to prevent unintended file truncation Signed-off-by: Fan Xuan Zhe --- chardev_backend/src/chardev.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/chardev_backend/src/chardev.rs b/chardev_backend/src/chardev.rs index 3f9e58b06..71f685770 100644 --- a/chardev_backend/src/chardev.rs +++ b/chardev_backend/src/chardev.rs @@ -177,6 +177,7 @@ impl Chardev { .read(true) .write(true) .create(true) + .truncate(false) .open(path)?, )); self.output = Some(file); -- Gitee From 98725d873d76c74112c21100d39f2f1055ed2a69 Mon Sep 17 00:00:00 2001 From: Fan Xuan Zhe Date: Tue, 20 May 2025 16:54:33 +0800 Subject: [PATCH 05/20] cpu: suppress clippy::missing_const_for_thread_local warning for LOCAL_THREAD_VCPU Added `#[allow(clippy::missing_const_for_thread_local)]` to silence lint warning Switched `thread_local!` declaration to block form for improved readability and maintainability Signed-off-by: Fan Xuan Zhe --- cpu/src/lib.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/cpu/src/lib.rs b/cpu/src/lib.rs index 21f6bb014..59ee794f3 100644 --- a/cpu/src/lib.rs +++ b/cpu/src/lib.rs @@ -465,7 +465,10 @@ pub struct CPUThreadWorker { } impl CPUThreadWorker { - thread_local!(static LOCAL_THREAD_VCPU: RefCell> = const { RefCell::new(None) }); + thread_local! { + #[allow(clippy::missing_const_for_thread_local)] + static LOCAL_THREAD_VCPU: RefCell> = const { RefCell::new(None) }; + } /// Allocates a new `CPUThreadWorker`. fn new(thread_cpu: Arc) -> Self { -- Gitee From 2c5af214fe4206e84aadac506dd2aa7fa67ee89e Mon Sep 17 00:00:00 2001 From: Fan Xuan Zhe Date: Tue, 20 May 2025 17:00:33 +0800 Subject: [PATCH 06/20] devices: code cleanup and minor fixes across multiple modules MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Simplified Vec initialization using `vec![value; len]` idiom in `acpi::cpu_controller` Used slice (`&[&str]`) instead of `&Vec<&str>` for better flexibility in `acpi::power` Replaced `std::u32::MAX` and `u64::max_value()` with `u32::MAX` and `u64::MAX` for modern syntax Fixed typos and improved comments in several files: - "calculted" → "calculated" - "unpluged" → "unplugged" - "granulatity" → "granularity" - "notifing" → "notifying" Moved `#[cfg(feature = "usb_uas")]` attributes directly onto `set_capability_descriptors` for better control Corrected `null_mut` type casts in `usbhost::host_usblib` using `` notation Reordered `Sync` and `Send` impls for `UsbHostRequest` for logical grouping Signed-off-by: Fan Xuan Zhe --- devices/src/acpi/cpu_controller.rs | 3 +-- devices/src/acpi/power.rs | 2 +- devices/src/legacy/fwcfg.rs | 6 +++--- devices/src/misc/scream/mod.rs | 2 +- devices/src/misc/scream/pulseaudio.rs | 6 +++--- devices/src/pci/demo_device/mod.rs | 1 + devices/src/pci/intx.rs | 6 +++--- devices/src/pci/root_port.rs | 4 ++-- devices/src/scsi/bus.rs | 2 +- devices/src/sysbus/mod.rs | 2 +- devices/src/usb/descriptor.rs | 2 ++ devices/src/usb/usbhost/host_usblib.rs | 4 ++-- devices/src/usb/usbhost/mod.rs | 14 +++++++------- devices/src/usb/xhci/xhci_controller.rs | 9 +++------ 14 files changed, 31 insertions(+), 32 deletions(-) diff --git a/devices/src/acpi/cpu_controller.rs b/devices/src/acpi/cpu_controller.rs index 3f85e288d..f400738aa 100644 --- a/devices/src/acpi/cpu_controller.rs +++ b/devices/src/acpi/cpu_controller.rs @@ -411,8 +411,7 @@ impl AmlCpu { flags: 1 << MADT_CPU_ENABLE_FLAG, }; - let mut mat_data: Vec = Vec::new(); - mat_data.resize(std::mem::size_of_val(&lapic), 0); + let mut mat_data: Vec = vec![0; std::mem::size_of_val(&lapic)]; // SAFETY: mat_data is large enough to hold lapic. unsafe { *(mat_data.as_mut_ptr() as *mut AcpiLocalApic) = lapic }; diff --git a/devices/src/acpi/power.rs b/devices/src/acpi/power.rs index ae3d0bf61..83e662528 100644 --- a/devices/src/acpi/power.rs +++ b/devices/src/acpi/power.rs @@ -106,7 +106,7 @@ impl PowerDev { fn read_sysfs_power_props( &self, dir_name: &str, - sysfs_props: &Vec<&str>, + sysfs_props: &[&str], pdev_props: &mut [u32], ) -> Result<()> { for i in 0..sysfs_props.len() { diff --git a/devices/src/legacy/fwcfg.rs b/devices/src/legacy/fwcfg.rs index 59b2d250d..bcdbc7e98 100644 --- a/devices/src/legacy/fwcfg.rs +++ b/devices/src/legacy/fwcfg.rs @@ -394,7 +394,7 @@ impl FwCfgCommon { ) -> Result<()> { let key = (key as u16) & FW_CFG_ENTRY_MASK; - if key >= self.max_entry() || data.len() >= std::u32::MAX as usize { + if key >= self.max_entry() || data.len() >= u32::MAX as usize { return Err(anyhow!(LegacyError::InvalidFwCfgEntry(key))); } @@ -424,7 +424,7 @@ impl FwCfgCommon { // Update a FwCfgEntry fn update_entry_data(&mut self, key: u16, mut data: Vec) -> Result<()> { - if key >= self.max_entry() || data.len() >= std::u32::MAX as usize { + if key >= self.max_entry() || data.len() >= u32::MAX as usize { return Err(anyhow!(LegacyError::InvalidFwCfgEntry(key))); } @@ -761,7 +761,7 @@ impl FwCfgCommon { fn common_realize(&mut self) -> Result<()> { // Firmware configurations add Signature item - let sig = &[b'Q', b'E', b'M', b'U']; + let sig = b"QEMU"; self.add_entry(FwCfgEntryType::Signature, None, None, sig.to_vec(), false)?; self.add_entry( diff --git a/devices/src/misc/scream/mod.rs b/devices/src/misc/scream/mod.rs index 9dc1b3966..73905abe6 100644 --- a/devices/src/misc/scream/mod.rs +++ b/devices/src/misc/scream/mod.rs @@ -367,7 +367,7 @@ impl StreamData { let mut last_end = 0_u64; // The recording buffer is behind the playback buffer. Thereforce, the end position of - // the playback buffer must be calculted to determine whether the two buffers overlap. + // the playback buffer must be calculated to determine whether the two buffers overlap. if dir == ScreamDirection::Record && header.play.is_started != 0 { last_end = u64::from(header.play.offset) + u64::from(header.play.chunk_size) * u64::from(header.play.max_chunks); diff --git a/devices/src/misc/scream/pulseaudio.rs b/devices/src/misc/scream/pulseaudio.rs index e0d1dc265..f731a5ee0 100644 --- a/devices/src/misc/scream/pulseaudio.rs +++ b/devices/src/misc/scream/pulseaudio.rs @@ -86,9 +86,9 @@ impl PulseStreamData { let buffer_attr = BufferAttr { maxlength: ss.usec_to_bytes(MicroSeconds(u64::from(MAX_LATENCY_MS) * 1000)) as u32, tlength: ss.usec_to_bytes(MicroSeconds(u64::from(TARGET_LATENCY_MS) * 1000)) as u32, - prebuf: std::u32::MAX, - minreq: std::u32::MAX, - fragsize: std::u32::MAX, + prebuf: u32::MAX, + minreq: u32::MAX, + fragsize: u32::MAX, }; let pa_dir = dir.transform(); diff --git a/devices/src/pci/demo_device/mod.rs b/devices/src/pci/demo_device/mod.rs index 8f6fe6022..620471510 100644 --- a/devices/src/pci/demo_device/mod.rs +++ b/devices/src/pci/demo_device/mod.rs @@ -18,6 +18,7 @@ /// 2. After r/w, it sends back a msix interrupt to the guest, which means that it has also /// msix capability. We assume msix bar is in bar 0. /// 3. Finally, it supports hotplug/hotunplug. +/// /// As that it has device memory, it means it has a bar space, we assume the /// bar size is 4KB in bar 1. /// As that it has device memory, it means it has a bar space other than the msix one.( diff --git a/devices/src/pci/intx.rs b/devices/src/pci/intx.rs index 9b62e0bda..8cfb91c71 100644 --- a/devices/src/pci/intx.rs +++ b/devices/src/pci/intx.rs @@ -125,7 +125,7 @@ pub fn init_intx( devfn: u8, ) -> Result<()> { if config.config[INTERRUPT_PIN as usize] == 0 { - let (irq, intx_state) = (std::u32::MAX, None); + let (irq, intx_state) = (u32::MAX, None); let intx = Arc::new(Mutex::new(Intx::new(name, irq, intx_state))); config.intx = Some(intx); return Ok(()); @@ -151,13 +151,13 @@ pub fn init_intx( Some(pci_bus.intx_state.as_ref().unwrap().clone()), ) } else { - (std::u32::MAX, None) + (u32::MAX, None) } } }; (irq, intx_state) } else { - (std::u32::MAX, None) + (u32::MAX, None) }; let intx = Arc::new(Mutex::new(Intx::new(name, irq, intx_state))); diff --git a/devices/src/pci/root_port.rs b/devices/src/pci/root_port.rs index fdf4ef47f..e531f84b7 100644 --- a/devices/src/pci/root_port.rs +++ b/devices/src/pci/root_port.rs @@ -113,7 +113,7 @@ impl RootPort { let devfn = cfg.addr.0 << 3 | cfg.addr.1; #[cfg(target_arch = "x86_64")] let io_region = Region::init_container_region(1 << 16, "RootPortIo"); - let mem_region = Region::init_container_region(u64::max_value(), "RootPortMem"); + let mem_region = Region::init_container_region(u64::MAX, "RootPortMem"); let child_bus = Arc::new(Mutex::new(PciBus::new( cfg.id.clone(), #[cfg(target_arch = "x86_64")] @@ -637,7 +637,7 @@ impl HotplugOps for RootPort { if ((sltctl & PCI_EXP_SLTCTL_PIC) == PCI_EXP_SLTCTL_PWR_IND_OFF) && ((sltctl & PCI_EXP_SLTCTL_PCC) == PCI_EXP_SLTCTL_PWR_OFF) { - // if the slot has already been unpluged, skip notifing the guest. + // if the slot has already been unpluged, skip notifying the guest. return Ok(()); } diff --git a/devices/src/scsi/bus.rs b/devices/src/scsi/bus.rs index ceaa1e5bb..035ffc31d 100644 --- a/devices/src/scsi/bus.rs +++ b/devices/src/scsi/bus.rs @@ -983,7 +983,7 @@ fn scsi_command_emulate_vpd_page( // Byte[16-19] = 0: Maximum Prefetch Length. // Byte[20-23]: Maximum unmap lba count. // Byte[24-27]: Maximum unmap block descriptor count. - // Byte[28-31]: Optimal unmap granulatity. + // Byte[28-31]: Optimal unmap granularity. // Byte[32-35] = 0: Unmap Granularity alignment. // Byte[36-43]: Maximum write same length. // Bytes[44-47] = 0: Maximum atomic Transfer length. diff --git a/devices/src/sysbus/mod.rs b/devices/src/sysbus/mod.rs index 01de827dd..e472541ef 100644 --- a/devices/src/sysbus/mod.rs +++ b/devices/src/sysbus/mod.rs @@ -402,7 +402,7 @@ pub fn devices_register_sysbusdevops_type() -> Result<()> { #[cfg(target_arch = "aarch64")] { register_sysbusdevops_type::()?; - #[cfg(all(feature = "ramfb"))] + #[cfg(feature = "ramfb")] register_sysbusdevops_type::()?; register_sysbusdevops_type::()?; register_sysbusdevops_type::()?; diff --git a/devices/src/usb/descriptor.rs b/devices/src/usb/descriptor.rs index 55858cd4d..0e7424dbf 100644 --- a/devices/src/usb/descriptor.rs +++ b/devices/src/usb/descriptor.rs @@ -453,6 +453,7 @@ pub trait UsbDescriptorOps { fn set_interface_descriptor(&mut self, index: u32, v: u32) -> Result<()>; /// Set super speed capability descriptors. + #[cfg(feature = "usb_uas")] fn set_capability_descriptors(&mut self, caps: Vec); /// Init all endpoint descriptors and reset the USB endpoint. @@ -529,6 +530,7 @@ impl UsbDescriptorOps for UsbDeviceBase { Ok(()) } + #[cfg(feature = "usb_uas")] fn set_capability_descriptors(&mut self, caps: Vec) { self.descriptor.capabilities = caps; } diff --git a/devices/src/usb/usbhost/host_usblib.rs b/devices/src/usb/usbhost/host_usblib.rs index 8c5542afb..0962c5094 100644 --- a/devices/src/usb/usbhost/host_usblib.rs +++ b/devices/src/usb/usbhost/host_usblib.rs @@ -379,9 +379,9 @@ pub fn set_option(opt: u32) -> Result<()> { // SAFETY: This function will only configure a specific option within libusb, null for ctx is valid. let err = unsafe { libusb_set_option( - std::ptr::null_mut() as *mut libusb_context, + std::ptr::null_mut::(), opt, - std::ptr::null_mut() as *mut c_void, + std::ptr::null_mut::(), ) }; if err != LIBUSB_SUCCESS { diff --git a/devices/src/usb/usbhost/mod.rs b/devices/src/usb/usbhost/mod.rs index 3c94ceb14..e345c1027 100644 --- a/devices/src/usb/usbhost/mod.rs +++ b/devices/src/usb/usbhost/mod.rs @@ -84,6 +84,13 @@ pub struct UsbHostRequest { pub is_control: bool, } +// SAFETY: The UsbHostRequest is created in main thread and then be passed to the +// libUSB thread. Once this data is processed, it is cleaned up. So there will be +// no problem with data sharing or synchronization. +unsafe impl Sync for UsbHostRequest {} +// SAFETY: The reason is same as above. +unsafe impl Send for UsbHostRequest {} + impl UsbHostRequest { pub fn new( hostbus: u8, @@ -168,13 +175,6 @@ impl UsbHostRequest { } } -// SAFETY: The UsbHostRequest is created in main thread and then be passed to the -// libUSB thread. Once this data is processed, it is cleaned up. So there will be -// no problem with data sharing or synchronization. -unsafe impl Sync for UsbHostRequest {} -// SAFETY: The reason is same as above. -unsafe impl Send for UsbHostRequest {} - pub struct IsoTransfer { host_transfer: *mut libusb_transfer, copy_completed: bool, diff --git a/devices/src/usb/xhci/xhci_controller.rs b/devices/src/usb/xhci/xhci_controller.rs index e437283b1..9bb2a55aa 100644 --- a/devices/src/usb/xhci/xhci_controller.rs +++ b/devices/src/usb/xhci/xhci_controller.rs @@ -1131,9 +1131,8 @@ impl XhciDevice { if !self.running() { return Ok(()); } - self.check_bme_valid().map_err(|e| { + self.check_bme_valid().inspect_err(|_e| { self.host_controller_error(); - e })?; let mut evt = XhciEvent::new(TRBType::ErPortStatusChange, TRBCCode::Success); @@ -1837,9 +1836,8 @@ impl XhciDevice { return Ok(()); } - self.check_bme_valid().map_err(|e| { + self.check_bme_valid().inspect_err(|_e| { self.host_controller_error(); - e })?; trace::usb_xhci_ep_kick(&slot_id, &ep_id, &ring.get_dequeue_ptr()); @@ -2376,9 +2374,8 @@ impl XhciDevice { return Ok(()); } - self.check_bme_valid().map_err(|e| { + self.check_bme_valid().inspect_err(|_e| { self.host_controller_error(); - e })?; let mut seg = XhciEventRingSeg::new(&self.mem_space); -- Gitee From 8728633c59cf041b746ff559dfdf60868105d022 Mon Sep 17 00:00:00 2001 From: Fan Xuan Zhe Date: Tue, 20 May 2025 17:05:48 +0800 Subject: [PATCH 07/20] docs: Fix typos Change `offerred` to `offered` Signed-off-by: Fan Xuan Zhe --- docs/quickstart.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/quickstart.md b/docs/quickstart.md index 80275cef5..9fe8874b6 100644 --- a/docs/quickstart.md +++ b/docs/quickstart.md @@ -19,7 +19,7 @@ have KVM module on your platform. ## 2. Get the StratoVirt Binary -StratoVirt is offerred by openEuler 20.09 or later. You can install by yum directly. +StratoVirt is offered by openEuler 20.09 or later. You can install by yum directly. ```shell $ sudo yum install stratovirt -- Gitee From 12c10fafa34bfad3d66a86132300a3c44f2c6e63 Mon Sep 17 00:00:00 2001 From: Fan Xuan Zhe Date: Tue, 20 May 2025 17:10:38 +0800 Subject: [PATCH 08/20] hypervisor: Optimize code by replacing redundant patterns and improving readability Replaced `.get(0).unwrap()` with `.first().unwrap()` for better semantics when accessing the first element of `redist_regions`. Used `.inspect_err()` instead of `.map_err()` in `register_irqfd` and `unregister_irqfd` to log errors without altering the result flow. Simplified test code by replacing redundant `drop(cpu_state)` calls with `let _ = cpu_state` to explicitly silence unused variable warnings. Signed-off-by: Fan Xuan Zhe --- hypervisor/src/kvm/aarch64/gicv3.rs | 2 +- hypervisor/src/kvm/mod.rs | 22 +++++++++------------- 2 files changed, 10 insertions(+), 14 deletions(-) diff --git a/hypervisor/src/kvm/aarch64/gicv3.rs b/hypervisor/src/kvm/aarch64/gicv3.rs index 23d86c0df..7a0ec8382 100644 --- a/hypervisor/src/kvm/aarch64/gicv3.rs +++ b/hypervisor/src/kvm/aarch64/gicv3.rs @@ -68,7 +68,7 @@ impl GICv3Access for KvmGICv3 { &self.fd, kvm_bindings::KVM_DEV_ARM_VGIC_GRP_ADDR, u64::from(kvm_bindings::KVM_VGIC_V3_ADDR_TYPE_REDIST), - &redist_regions.get(0).unwrap().base as *const u64 as u64, + &redist_regions.first().unwrap().base as *const u64 as u64, true, ) .with_context(|| "Failed to set GICv3 attribute: redistributor address")?; diff --git a/hypervisor/src/kvm/mod.rs b/hypervisor/src/kvm/mod.rs index 16e7a479a..019376ade 100644 --- a/hypervisor/src/kvm/mod.rs +++ b/hypervisor/src/kvm/mod.rs @@ -776,9 +776,8 @@ impl LineIrqManager for KVMInterruptManager { match trigger_mode { TriggerMode::Edge => { - self.vm_fd.register_irqfd(&irq_fd, irq).map_err(|e| { + self.vm_fd.register_irqfd(&irq_fd, irq).inspect_err(|e| { error!("Failed to register irq, error is {:?}", e); - e })?; } _ => { @@ -790,9 +789,8 @@ impl LineIrqManager for KVMInterruptManager { } fn unregister_irqfd(&self, irq_fd: Arc, irq: u32) -> Result<()> { - self.vm_fd.unregister_irqfd(&irq_fd, irq).map_err(|e| { + self.vm_fd.unregister_irqfd(&irq_fd, irq).inspect_err(|e| { error!("Failed to unregister irq, error is {:?}", e); - e })?; Ok(()) @@ -863,9 +861,8 @@ impl MsiIrqManager for KVMInterruptManager { fn register_irqfd(&self, irq_fd: Arc, irq: u32) -> Result<()> { trace::kvm_register_irqfd(&irq_fd, irq); - self.vm_fd.register_irqfd(&irq_fd, irq).map_err(|e| { + self.vm_fd.register_irqfd(&irq_fd, irq).inspect_err(|e| { error!("Failed to register irq, error is {:?}", e); - e })?; Ok(()) @@ -873,9 +870,8 @@ impl MsiIrqManager for KVMInterruptManager { fn unregister_irqfd(&self, irq_fd: Arc, irq: u32) -> Result<()> { trace::kvm_unregister_irqfd(&irq_fd, irq); - self.vm_fd.unregister_irqfd(&irq_fd, irq).map_err(|e| { + self.vm_fd.unregister_irqfd(&irq_fd, irq).inspect_err(|e| { error!("Failed to unregister irq, error is {:?}", e); - e })?; Ok(()) @@ -1134,7 +1130,7 @@ mod test { ); let (cpu_state, _) = &*cpu.state; assert_eq!(*cpu_state.lock().unwrap(), CpuLifecycleState::Created); - drop(cpu_state); + let _ = cpu_state; let cpus_thread_barrier = Arc::new(Barrier::new(2)); let cpu_thread_barrier = cpus_thread_barrier.clone(); @@ -1163,7 +1159,7 @@ mod test { cpus_thread_barrier.wait(); let (cpu_state, _) = &*cpu_arc.state; assert_eq!(*cpu_state.lock().unwrap(), CpuLifecycleState::Paused); - drop(cpu_state); + let _ = cpu_state; assert!(cpu_arc.resume().is_ok()); @@ -1171,7 +1167,7 @@ mod test { std::thread::sleep(Duration::from_millis(50)); let (cpu_state, _) = &*cpu_arc.state; assert_eq!(*cpu_state.lock().unwrap(), CpuLifecycleState::Running); - drop(cpu_state); + let _ = cpu_state; assert!(cpu_arc.pause().is_ok()); @@ -1179,7 +1175,7 @@ mod test { std::thread::sleep(Duration::from_millis(50)); let (cpu_state, _) = &*cpu_arc.state; assert_eq!(*cpu_state.lock().unwrap(), CpuLifecycleState::Paused); - drop(cpu_state); + let _ = cpu_state; assert!(cpu_arc.resume().is_ok()); // Wait for CPU finish state change. @@ -1191,6 +1187,6 @@ mod test { std::thread::sleep(Duration::from_millis(50)); let (cpu_state, _) = &*cpu_arc.state; assert_eq!(*cpu_state.lock().unwrap(), CpuLifecycleState::Stopped); - drop(cpu_state); + let _ = cpu_state; } } -- Gitee From dadf5ae638f00dd285d8a533f0b42f7cce02ced3 Mon Sep 17 00:00:00 2001 From: Fan Xuan Zhe Date: Tue, 20 May 2025 17:17:59 +0800 Subject: [PATCH 09/20] image: clean up test code by removing redundant `.clone()` and silencing dead_code warning Removed unnecessary `.clone()` calls when passing string literals to `remove_file`, improving clarity and avoiding redundant allocations. Marked `TestQcow2Image` struct with `#[allow(dead_code)]` to silence compiler warnings in test context. Signed-off-by: Fan Xuan Zhe --- image/src/img.rs | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/image/src/img.rs b/image/src/img.rs index bc5b58ebc..9dbb4675d 100644 --- a/image/src/img.rs +++ b/image/src/img.rs @@ -849,6 +849,7 @@ mod test { const M: u64 = 1024 * 1024; const G: u64 = 1024 * 1024 * 1024; + #[allow(dead_code)] pub struct TestQcow2Image { pub header: QcowHeader, pub cluster_bits: u64, @@ -2383,8 +2384,8 @@ mod test { fn test_image_convert_from_qcow2_to_raw() { let src_path = "/tmp/test_image_convert_src.qcow2"; let dst_path = "/tmp/test_image_convert_dst.raw"; - let _ = remove_file(src_path.clone()); - let _ = remove_file(dst_path.clone()); + let _ = remove_file(src_path); + let _ = remove_file(dst_path); let test_image = TestQcow2Image::create(16, 16, src_path, "10G"); let mut src_driver = test_image.create_driver(); @@ -2438,7 +2439,7 @@ mod test { assert_eq!(img_info.actual_size, 3 * M); // Clean. - assert!(remove_file(dst_path.clone()).is_ok()); + assert!(remove_file(dst_path).is_ok()); } /// Test image convert parameters parsing. @@ -2453,8 +2454,8 @@ mod test { fn test_image_convert_parameters_parsing() { let src_path = "/tmp/test_image_convert_paring_src.qcow2"; let dst_path = "/tmp/test_image_convert_paring_dst.raw"; - let _ = remove_file(src_path.clone()); - let _ = remove_file(dst_path.clone()); + let _ = remove_file(src_path); + let _ = remove_file(dst_path); let test_image = TestQcow2Image::create(16, 16, src_path, "1G"); let mut src_driver = test_image.create_driver(); @@ -2510,7 +2511,7 @@ mod test { // detecting the alignment length. assert_eq!(img_info.actual_size, 4 * K + 8 * K); // 4K allocated filled first part + 8K data. drop(dst_driver); - assert!(remove_file(dst_path.clone()).is_ok()); + assert!(remove_file(dst_path).is_ok()); // test2: The destination file already exists. let existed_raw = TestRawImage::create(dst_path.to_string(), "1G".to_string()); @@ -2545,6 +2546,6 @@ mod test { // Will not create holes here. assert_eq!(img_info.actual_size, 4 * K + 16 * K); drop(dst_driver); - assert!(remove_file(dst_path.clone()).is_ok()); + assert!(remove_file(dst_path).is_ok()); } } -- Gitee From 6cac3cb5b7af4952aca914650313940ddf5a361a Mon Sep 17 00:00:00 2001 From: Fan Xuan Zhe Date: Tue, 20 May 2025 17:21:07 +0800 Subject: [PATCH 10/20] machine: Remove unused import and fix typos Remove unused std::u64 import and fix some typos Signed-off-by: Fan Xuan Zhe --- machine/src/aarch64/micro.rs | 17 +++++++---------- machine/src/aarch64/standard.rs | 21 +++++++++------------ machine/src/lib.rs | 1 - machine/src/standard_common/mod.rs | 3 +-- machine/src/x86_64/standard.rs | 2 +- 5 files changed, 18 insertions(+), 26 deletions(-) diff --git a/machine/src/aarch64/micro.rs b/machine/src/aarch64/micro.rs index b79f13b3e..54e7b1352 100644 --- a/machine/src/aarch64/micro.rs +++ b/machine/src/aarch64/micro.rs @@ -302,16 +302,13 @@ impl CompileFDTHelper for LightMachine { format!("/pl011@{:x}", MEM_LAYOUT[LayoutEntryType::Uart as usize].0); fdt.set_property_string("stdout-path", &pl011_property_string)?; - match &boot_source.initrd { - Some(initrd) => { - fdt.set_property_u64("linux,initrd-start", initrd.initrd_addr)?; - let initrd_end = initrd - .initrd_addr - .checked_add(initrd.initrd_size) - .with_context(|| "initrd end overflow")?; - fdt.set_property_u64("linux,initrd-end", initrd_end)?; - } - None => {} + if let Some(initrd) = &boot_source.initrd { + fdt.set_property_u64("linux,initrd-start", initrd.initrd_addr)?; + let initrd_end = initrd + .initrd_addr + .checked_add(initrd.initrd_size) + .with_context(|| "initrd end overflow")?; + fdt.set_property_u64("linux,initrd-end", initrd_end)?; } fdt.end_node(chosen_node_dep) } diff --git a/machine/src/aarch64/standard.rs b/machine/src/aarch64/standard.rs index 8612b3f12..34394e1b7 100644 --- a/machine/src/aarch64/standard.rs +++ b/machine/src/aarch64/standard.rs @@ -210,7 +210,7 @@ impl StdMachine { .with_context(|| "Fail to reset all devices")?; locked_vm .reset_fwcfg_boot_order() - .with_context(|| "Fail to update boot order imformation to FwCfg device")?; + .with_context(|| "Fail to update boot order information to FwCfg device")?; if QmpChannel::is_connected() { let reset_msg = qmp_schema::Reset { guest: true }; @@ -624,7 +624,7 @@ impl MachineOps for StdMachine { locked_vm .reset_fwcfg_boot_order() - .with_context(|| "Fail to update boot order imformation to FwCfg device")?; + .with_context(|| "Fail to update boot order information to FwCfg device")?; locked_vm .display_init(vm_config) @@ -1299,16 +1299,13 @@ impl CompileFDTHelper for StdMachine { format!("/pl011@{:x}", MEM_LAYOUT[LayoutEntryType::Uart as usize].0); fdt.set_property_string("stdout-path", &pl011_property_string)?; - match &boot_source.initrd { - Some(initrd) => { - fdt.set_property_u64("linux,initrd-start", initrd.initrd_addr)?; - let initrd_end = initrd - .initrd_addr - .checked_add(initrd.initrd_size) - .with_context(|| "initrd end overflow")?; - fdt.set_property_u64("linux,initrd-end", initrd_end)?; - } - None => {} + if let Some(initrd) = &boot_source.initrd { + fdt.set_property_u64("linux,initrd-start", initrd.initrd_addr)?; + let initrd_end = initrd + .initrd_addr + .checked_add(initrd.initrd_size) + .with_context(|| "initrd end overflow")?; + fdt.set_property_u64("linux,initrd-end", initrd_end)?; } fdt.end_node(chosen_node_dep) } diff --git a/machine/src/lib.rs b/machine/src/lib.rs index bda1c9cc8..7b37ac7ca 100644 --- a/machine/src/lib.rs +++ b/machine/src/lib.rs @@ -42,7 +42,6 @@ use std::sync::{Arc, Barrier, Condvar, Mutex, RwLock, Weak}; use std::thread; #[cfg(feature = "windows_emu_pid")] use std::time::Duration; -use std::u64; use anyhow::{anyhow, bail, Context, Result}; use clap::Parser; diff --git a/machine/src/standard_common/mod.rs b/machine/src/standard_common/mod.rs index 7ac6bcbbc..a94090122 100644 --- a/machine/src/standard_common/mod.rs +++ b/machine/src/standard_common/mod.rs @@ -21,7 +21,6 @@ use std::os::unix::prelude::AsRawFd; use std::rc::Rc; use std::string::String; use std::sync::{Arc, Mutex}; -use std::u64; use anyhow::{bail, Context, Result}; use log::{error, warn}; @@ -665,7 +664,7 @@ pub(crate) trait AcpiBuilder { fadt.set_field(244, 0x01_u8); fadt.set_field(245, 0x08_u8); fadt.set_field(248, u64::from(SLEEP_CTRL_OFFSET)); - // Sleep status tegister, offset is 256. + // Sleep status register, offset is 256. fadt.set_field(256, 0x01_u8); fadt.set_field(257, 0x08_u8); fadt.set_field(260, u64::from(SLEEP_CTRL_OFFSET)); diff --git a/machine/src/x86_64/standard.rs b/machine/src/x86_64/standard.rs index e85042bf2..6ab59ec37 100644 --- a/machine/src/x86_64/standard.rs +++ b/machine/src/x86_64/standard.rs @@ -560,7 +560,7 @@ impl MachineOps for StdMachine { locked_vm .reset_fwcfg_boot_order() - .with_context(|| "Fail to update boot order imformation to FwCfg device")?; + .with_context(|| "Fail to update boot order information to FwCfg device")?; locked_vm .display_init(vm_config) -- Gitee From 8a926fcf235f1ce744c1372127622bc8b121a837 Mon Sep 17 00:00:00 2001 From: Fan Xuan Zhe Date: Tue, 20 May 2025 17:26:09 +0800 Subject: [PATCH 11/20] machine_manager: Fix typos, modernize global init, and add build script Replaced all instances of "Unknow" with the correct spelling "Unknown". Improved indentation in comments to enhance clarity and consistency. Replaced unsafe static global `QMP_CHANNEL` with `OnceLock` for thread-safe lazy initialization. Added a `build.rs` script to register custom `cfg` flags for conditional compilation checks. Added missing `Sync` and `Send` implementations to `Socket` with safety annotations. Fixed minor logic issues in socket message writing using `write_all` for better robustness. Signed-off-by: Fan Xuan Zhe --- machine_manager/build.rs | 18 +++++++++++++++++ machine_manager/src/config/drive.rs | 2 +- machine_manager/src/config/mod.rs | 14 ++++++------- machine_manager/src/config/smbios.rs | 2 +- machine_manager/src/machine.rs | 6 +++--- machine_manager/src/qmp/mod.rs | 10 +++++----- machine_manager/src/qmp/qmp_channel.rs | 27 +++++++++----------------- machine_manager/src/qmp/qmp_schema.rs | 15 +++++++------- machine_manager/src/qmp/qmp_socket.rs | 6 ++++++ machine_manager/src/socket.rs | 4 ++-- machine_manager/src/test_server.rs | 7 +++++++ 11 files changed, 67 insertions(+), 44 deletions(-) create mode 100644 machine_manager/build.rs diff --git a/machine_manager/build.rs b/machine_manager/build.rs new file mode 100644 index 000000000..a74e50b05 --- /dev/null +++ b/machine_manager/build.rs @@ -0,0 +1,18 @@ +// Copyright (c) 2025 Huawei Technologies Co.,Ltd. All rights reserved. +// +// StratoVirt is licensed under Mulan PSL v2. +// You can use this software according to the terms and conditions of the Mulan +// PSL v2. +// You may obtain a copy of Mulan PSL v2 at: +// http://license.coscl.org.cn/MulanPSL2 +// THIS SOFTWARE IS PROVIDED ON AN "AS IS" BASIS, WITHOUT WARRANTIES OF ANY +// KIND, EITHER EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO +// NON-INFRINGEMENT, MERCHANTABILITY OR FIT FOR A PARTICULAR PURPOSE. +// See the Mulan PSL v2 for more details. + +fn main() { + println!("cargo::rustc-check-cfg=cfg(TRUE)"); + println!("cargo::rustc-check-cfg=cfg(FALSE)"); + + println!("cargo::rustc-cfg=TRUE"); +} diff --git a/machine_manager/src/config/drive.rs b/machine_manager/src/config/drive.rs index a9a132eb2..118d198b1 100644 --- a/machine_manager/src/config/drive.rs +++ b/machine_manager/src/config/drive.rs @@ -278,7 +278,7 @@ impl VmConfig { self.add_flashdev(drive_cfg.clone())?; } _ => { - bail!("Unknow 'if' argument: {:?}", &drive_cfg.drive_type); + bail!("Unknown 'if' argument: {:?}", &drive_cfg.drive_type); } } diff --git a/machine_manager/src/config/mod.rs b/machine_manager/src/config/mod.rs index 6f793d4e6..a3f96b04a 100644 --- a/machine_manager/src/config/mod.rs +++ b/machine_manager/src/config/mod.rs @@ -234,7 +234,7 @@ impl VmConfig { self.add_saslauth(object_args)?; } _ => { - bail!("Unknow object type: {:?}", &object_type); + bail!("Unknown object type: {:?}", &object_type); } } @@ -446,7 +446,7 @@ pub fn parse_bool(s: &str) -> Result { match s { "true" | "on" | "yes" | "unmap" => Ok(true), "false" | "off" | "no" | "ignore" => Ok(false), - _ => Err(anyhow!("Unknow bool value {s}")), + _ => Err(anyhow!("Unknown bool value {s}")), } } @@ -626,16 +626,16 @@ fn concat_classtype(args: &str, concat: bool) -> String { /// Stratovirt command line may use the first parameter as class type. /// Eg: /// 1. drive config: "-drive file=,if=pflash,unit=0" -/// This cmdline has no class type. +/// This cmdline has no class type. /// 2. device config: "-device virtio-balloon-pci,id=,bus=,addr=<0x4>" -/// This cmdline sets device type `virtio-balloon-pci` as the first parameter. +/// This cmdline sets device type `virtio-balloon-pci` as the first parameter. /// /// Use first_pos_is_type to indicate whether the first parameter is a type class which needs a separate analysis. /// Eg: /// 1. drive config: "-drive file=,if=pflash,unit=0" -/// Set first_pos_is_type false for this cmdline has no class type. +/// Set first_pos_is_type false for this cmdline has no class type. /// 2. device config: "-device virtio-balloon-pci,id=,bus=,addr=<0x4>" -/// Set first_pos_is_type true for this cmdline has device type "virtio-balloon-pci" as the first parameter. +/// Set first_pos_is_type true for this cmdline has device type "virtio-balloon-pci" as the first parameter. /// /// Use first_pos_is_subcommand to indicate whether the first parameter is a subclass. /// Eg: @@ -643,7 +643,7 @@ fn concat_classtype(args: &str, concat: bool) -> String { /// in the same `ChardevConfig` structure by using `enum`. So, we will use class type as a subcommand to indicate which subtype /// will be used to store the configuration in enumeration type. Subcommand in `clap` doesn't need `--` in parameter. /// 1. -serial file,path= -/// Set first_pos_is_subcommand true for first parameter `file` is the subclass type for chardev. +/// Set first_pos_is_subcommand true for first parameter `file` is the subclass type for chardev. pub fn str_slip_to_clap( args: &str, first_pos_is_type: bool, diff --git a/machine_manager/src/config/smbios.rs b/machine_manager/src/config/smbios.rs index 75220f456..56f8778f0 100644 --- a/machine_manager/src/config/smbios.rs +++ b/machine_manager/src/config/smbios.rs @@ -330,7 +330,7 @@ impl VmConfig { self.add_smbios_type17(smbios_args)?; } _ => { - bail!("Unknow smbios type: {:?}", &smbios_type); + bail!("Unknown smbios type: {:?}", &smbios_type); } } diff --git a/machine_manager/src/machine.rs b/machine_manager/src/machine.rs index bc4a38ec9..f23acb51a 100644 --- a/machine_manager/src/machine.rs +++ b/machine_manager/src/machine.rs @@ -81,13 +81,13 @@ impl FromStr for HypervisorType { /// /// **Notice**: /// 1. Migrate state(`Migrated` and `InMigrating`), -/// not include in Life cycle, both migrate state should deal like `PAUSED` -/// state. +/// not include in Life cycle, both migrate state should deal like `PAUSED` +/// state. /// /// 2. Snapshot state deal with `PAUSED` state. /// /// 3. every one concern with VM or Device state need to implement this trait, -/// will be notified when VM state changed through `lifecycle_notify` hook. +/// will be notified when VM state changed through `lifecycle_notify` hook. pub trait MachineLifecycle { /// Start VM or Device, VM or Device enter running state after this call return. fn start(&self) -> bool { diff --git a/machine_manager/src/qmp/mod.rs b/machine_manager/src/qmp/mod.rs index 53f87e252..f2a88d078 100644 --- a/machine_manager/src/qmp/mod.rs +++ b/machine_manager/src/qmp/mod.rs @@ -18,13 +18,13 @@ //! which allows applications to control a VM instance. //! It has three feature: //! 1. Qmp server is no-async service as well as Qemu's. -//! Command + events can replace asynchronous command. +//! Command + events can replace asynchronous command. //! 2. Qmp server can only be connected a client at one time. -//! It's no situation where be communicated with many clients. -//! When it must use, can use other communication way not QMP. +//! It's no situation where be communicated with many clients. +//! When it must use, can use other communication way not QMP. //! 3. Qmp's message structure base is transformed by scripts from Qemu's -//! `qmp-schema.json`. It's can be compatible by Qemu's zoology. Those -//! transformed structures can be found in `machine_manager/src/qmp/qmp_schema.rs` +//! `qmp-schema.json`. It's can be compatible by Qemu's zoology. Those +//! transformed structures can be found in `machine_manager/src/qmp/qmp_schema.rs` pub mod qmp_channel; pub mod qmp_response; diff --git a/machine_manager/src/qmp/qmp_channel.rs b/machine_manager/src/qmp/qmp_channel.rs index 51063331f..c975f7320 100644 --- a/machine_manager/src/qmp/qmp_channel.rs +++ b/machine_manager/src/qmp/qmp_channel.rs @@ -13,7 +13,7 @@ use std::collections::BTreeMap; use std::io::Write; use std::os::unix::io::RawFd; -use std::sync::{Arc, RwLock}; +use std::sync::{Arc, OnceLock, RwLock}; use std::time::{SystemTime, UNIX_EPOCH}; use log::{error, info, warn}; @@ -23,7 +23,7 @@ use super::qmp_schema::{self as schema}; use crate::socket::SocketRWHandler; use util::time::NANOSECONDS_PER_SECOND; -static mut QMP_CHANNEL: Option> = None; +static QMP_CHANNEL: OnceLock> = OnceLock::new(); /// Macro `event!`: send event to qmp-client. /// @@ -96,14 +96,12 @@ impl QmpChannel { pub fn object_init() { // SAFETY: Global variable QMP_CHANNEL is only used in the main thread, // so there are no competition or synchronization. - unsafe { - if QMP_CHANNEL.is_none() { - QMP_CHANNEL = Some(Arc::new(QmpChannel { - event_writer: RwLock::new(None), - fds: Arc::new(RwLock::new(BTreeMap::new())), - })); - } - } + QMP_CHANNEL.get_or_init(|| { + Arc::new(QmpChannel { + event_writer: RwLock::new(None), + fds: Arc::new(RwLock::new(BTreeMap::new())), + }) + }); } /// Bind a `SocketRWHandler` to `QMP_CHANNEL`. @@ -171,14 +169,7 @@ impl QmpChannel { fn inner() -> &'static std::sync::Arc { // SAFETY: Global variable QMP_CHANNEL is only used in the main thread, // so there are no competition or synchronization. - unsafe { - match &QMP_CHANNEL { - Some(channel) => channel, - None => { - panic!("Qmp channel not initialized"); - } - } - } + QMP_CHANNEL.get().expect("Qmp channel not initialized") } } diff --git a/machine_manager/src/qmp/qmp_schema.rs b/machine_manager/src/qmp/qmp_schema.rs index 5d5558d98..00fa664ca 100644 --- a/machine_manager/src/qmp/qmp_schema.rs +++ b/machine_manager/src/qmp/qmp_schema.rs @@ -142,6 +142,7 @@ define_qmp_command_enum!( ); /// Command trait for Deserialize and find back Response. +#[allow(dead_code)] trait Command: Serialize { type Res: DeserializeOwned; fn back(self) -> Self::Res; @@ -1025,7 +1026,7 @@ generate_command_impl!(switch_audio_record, Empty); /// /// # Returns /// -/// `BalloonInfo` includs the actual size of memory. +/// `BalloonInfo` includes the actual size of memory. /// /// # Examples /// @@ -2257,7 +2258,7 @@ mod tests { let ret_msg = r#"ok"#; assert!(err_msg.contains(ret_msg)); - // unknow arguments for device_add. + // unknown arguments for device_add. let json_msg = r#" { "execute": "device_add" , @@ -2265,7 +2266,7 @@ mod tests { "id":"net-0", "driver":"virtio-net-mmio", "addr":"0x0", - "UnknowArg": "should go to error" + "UnknownArg": "should go to error" } } "#; @@ -2273,7 +2274,7 @@ mod tests { Ok(_) => "ok".to_string(), Err(e) => e.to_string(), }; - let ret_msg = r#"unknown field `UnknowArg`"#; + let ret_msg = r#"unknown field `UnknownArg`"#; assert!(err_msg.contains(ret_msg)); // wrong spelling arguments for device_add. @@ -2323,9 +2324,9 @@ mod tests { Ok(_) => "ok".to_string(), Err(e) => e.to_string(), }; - let unknow_msg = r#"unknown field `value`"#; + let unknown_msg = r#"unknown field `value`"#; let expect_msg = r#"expected `id`"#; - assert!(err_msg.contains(unknow_msg)); + assert!(err_msg.contains(unknown_msg)); assert!(err_msg.contains(expect_msg)); // missing arguments for getfd. @@ -2429,7 +2430,7 @@ mod tests { let part_msg = r#"unknown variant `hello-world`"#; assert!(err_msg.contains(part_msg)); - // unsupported qmp command, and unknow field. + // unsupported qmp command, and unknown field. let json_msg = r#" { "execute": "hello-world" , diff --git a/machine_manager/src/qmp/qmp_socket.rs b/machine_manager/src/qmp/qmp_socket.rs index aa6ee53f9..a3187884e 100644 --- a/machine_manager/src/qmp/qmp_socket.rs +++ b/machine_manager/src/qmp/qmp_socket.rs @@ -104,6 +104,12 @@ pub struct Socket { performer: Option>>, } +// SAFETY: Implementing Send and Sync is safe for Socket +// because only locked access(r/w) is permitted +unsafe impl Sync for Socket {} +// SAFETY: Same as above +unsafe impl Send for Socket {} + impl Socket { /// Allocates a new `Socket` with `SocketListener`. /// diff --git a/machine_manager/src/socket.rs b/machine_manager/src/socket.rs index a2c0f3369..5dc703cff 100644 --- a/machine_manager/src/socket.rs +++ b/machine_manager/src/socket.rs @@ -366,9 +366,9 @@ impl SocketHandler { pub fn send_str(&mut self, s: &str) -> std::io::Result<()> { self.stream.flush().unwrap(); let msg = s.to_string() + "\r"; - match self.stream.write(msg.as_bytes()) { + match self.stream.write_all(msg.as_bytes()) { Ok(_) => { - let _ = self.stream.write(&[b'\n'])?; + let _ = self.stream.write(b"\n")?; Ok(()) } Err(_) => Err(Error::new( diff --git a/machine_manager/src/test_server.rs b/machine_manager/src/test_server.rs index f126ede07..2a393974c 100644 --- a/machine_manager/src/test_server.rs +++ b/machine_manager/src/test_server.rs @@ -30,6 +30,13 @@ pub struct TestSock { controller: Arc>, } +// SAFETY: Send and Sync is not auto-implemented for dynamic trait type, +// implementing them is safe because field of controller won't change +// once initialized, only locked access(r/w) is permitted +unsafe impl Sync for TestSock {} +// SAFETY: Same as above +unsafe impl Send for TestSock {} + impl TestSock { pub fn new(path: &str, controller: Arc>) -> Self { let stream = match UnixStream::connect(path) { -- Gitee From 7f81bf2318cc92fd3aecbe9783f14f5d0978576c Mon Sep 17 00:00:00 2001 From: Fan Xuan Zhe Date: Sat, 24 May 2025 10:54:20 +0800 Subject: [PATCH 12/20] QMP: bugfix the error in the return result of query_commands There is no need to use conditional judgment for enumeration variable aliases; otherwise, the names of each variable in the enumeration will be wrongly obtained. Signed-off-by: Mingwang Li --- machine_manager/build.rs | 18 ---- machine_manager/src/qmp/qmp_schema.rs | 130 +++++++++++++------------- 2 files changed, 65 insertions(+), 83 deletions(-) delete mode 100644 machine_manager/build.rs diff --git a/machine_manager/build.rs b/machine_manager/build.rs deleted file mode 100644 index a74e50b05..000000000 --- a/machine_manager/build.rs +++ /dev/null @@ -1,18 +0,0 @@ -// Copyright (c) 2025 Huawei Technologies Co.,Ltd. All rights reserved. -// -// StratoVirt is licensed under Mulan PSL v2. -// You can use this software according to the terms and conditions of the Mulan -// PSL v2. -// You may obtain a copy of Mulan PSL v2 at: -// http://license.coscl.org.cn/MulanPSL2 -// THIS SOFTWARE IS PROVIDED ON AN "AS IS" BASIS, WITHOUT WARRANTIES OF ANY -// KIND, EITHER EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO -// NON-INFRINGEMENT, MERCHANTABILITY OR FIT FOR A PARTICULAR PURPOSE. -// See the Mulan PSL v2 for more details. - -fn main() { - println!("cargo::rustc-check-cfg=cfg(TRUE)"); - println!("cargo::rustc-check-cfg=cfg(FALSE)"); - - println!("cargo::rustc-cfg=TRUE"); -} diff --git a/machine_manager/src/qmp/qmp_schema.rs b/machine_manager/src/qmp/qmp_schema.rs index 00fa664ca..2342c0beb 100644 --- a/machine_manager/src/qmp/qmp_schema.rs +++ b/machine_manager/src/qmp/qmp_schema.rs @@ -54,7 +54,7 @@ impl QmpErrorClass { } macro_rules! define_qmp_command_enum { - ($($command:ident($name:expr, $args_type:ty, $need_strum:ident $(, $serde_default:ident)?)),*) => { + ($($command:ident($name:expr, $args_type:ty $(, $serde_default:ident)?)),*) => { /// A enum to store all command struct #[derive(Debug, Clone, Serialize, Deserialize, EnumIter, EnumVariantNames, EnumString)] #[serde(tag = "execute")] @@ -62,7 +62,7 @@ macro_rules! define_qmp_command_enum { pub enum QmpCommand { $( #[serde(rename = $name)] - #[cfg_attr($need_strum, strum(serialize = $name))] + #[strum(serialize = $name)] $command { $(#[serde($serde_default)])? arguments: $args_type, @@ -76,69 +76,69 @@ macro_rules! define_qmp_command_enum { // QMP command enum definition example: command("name", arguments, ..) define_qmp_command_enum!( - qmp_capabilities("qmp_capabilities", qmp_capabilities, FALSE, default), - quit("quit", quit, FALSE, default), - stop("stop", stop, FALSE, default), - cont("cont", cont, FALSE, default), - system_powerdown("system_powerdown", system_powerdown, FALSE, default), - system_reset("system_reset", system_reset, FALSE, default), - device_add("device_add", Box, FALSE), - device_del("device_del", device_del, FALSE), - chardev_add("chardev-add", chardev_add, FALSE), - chardev_remove("chardev-remove", chardev_remove, FALSE), - netdev_add("netdev_add", Box, FALSE), - netdev_del("netdev_del", netdev_del, FALSE), - cameradev_add("cameradev_add", cameradev_add, FALSE), - cameradev_del("cameradev_del", cameradev_del, FALSE), - query_hotpluggable_cpus("query-hotpluggable-cpus", query_hotpluggable_cpus, TRUE, default), - query_cpus("query-cpus", query_cpus, TRUE, default), - query_status("query-status", query_status, FALSE, default), - getfd("getfd", getfd, FALSE), - blockdev_add("blockdev-add", Box, FALSE), - blockdev_del("blockdev-del", blockdev_del, FALSE), - balloon("balloon", balloon, FALSE, default), - query_mem("query-mem", query_mem, FALSE, default), - query_mem_gpa("query-mem-gpa", query_mem_gpa, FALSE, default), - query_balloon("query-balloon", query_balloon, FALSE, default), - query_vnc("query-vnc", query_vnc, TRUE, default), - query_display_image("query-display-image", query_display_image, FALSE, default), - switch_audio_record("switch-audio-record", switch_audio_record, FALSE), - migrate("migrate", migrate, FALSE), - query_migrate("query-migrate", query_migrate, FALSE, default), - cancel_migrate("migrate_cancel", cancel_migrate, FALSE, default), - query_version("query-version", query_version, FALSE, default), - query_commands("query-commands", query_commands, FALSE, default), - query_target("query-target", query_target, FALSE, default), - query_kvm("query-kvm", query_kvm, FALSE, default), - query_machines("query-machines", query_machines, FALSE, default), - query_events("query-events", query_events, TRUE, default), - list_type("qom-list-types", list_type, FALSE, default), - device_list_properties("device-list-properties", device_list_properties, FALSE, default), - block_commit("block-commit", block_commit, TRUE, default), - query_tpm_models("query-tpm-models", query_tpm_models, FALSE, default), - query_tpm_types("query-tpm-types", query_tpm_types, FALSE, default), - query_command_line_options("query-command-line-options", query_command_line_options, FALSE, default), - query_migrate_capabilities("query-migrate-capabilities", query_migrate_capabilities, FALSE, default), - query_qmp_schema("query-qmp-schema", query_qmp_schema, FALSE, default), - query_sev_capabilities("query-sev-capabilities", query_sev_capabilities, FALSE, default), - query_chardev("query-chardev", query_chardev, TRUE, default), - qom_list("qom-list", qom_list, TRUE, default), - qom_get("qom-get", qom_get, TRUE, default), - query_block("query-block", query_block, TRUE, default), - query_named_block_nodes("query-named-block-nodes", query_named_block_nodes, TRUE, default), - query_blockstats("query-blockstats", query_blockstats, TRUE, default), - query_block_jobs("query-block-jobs", query_block_jobs, TRUE, default), - query_gic_capabilities("query-gic-capabilities", query_gic_capabilities, TRUE, default), - query_iothreads("query-iothreads", query_iothreads, TRUE, default), - update_region("update_region", update_region, TRUE, default), - input_event("input_event", input_event, FALSE, default), - human_monitor_command("human-monitor-command", human_monitor_command, FALSE), - blockdev_snapshot_internal_sync("blockdev-snapshot-internal-sync", blockdev_snapshot_internal, FALSE), - blockdev_snapshot_delete_internal_sync("blockdev-snapshot-delete-internal-sync", blockdev_snapshot_internal, FALSE), - query_vcpu_reg("query-vcpu-reg", query_vcpu_reg, FALSE), - trace_get_state("trace-get-state", trace_get_state, FALSE), - trace_set_state("trace-set-state", trace_set_state, FALSE), - query_workloads("query-workloads", query_workloads, FALSE) + qmp_capabilities("qmp_capabilities", qmp_capabilities, default), + quit("quit", quit, default), + stop("stop", stop, default), + cont("cont", cont, default), + system_powerdown("system_powerdown", system_powerdown, default), + system_reset("system_reset", system_reset, default), + device_add("device_add", Box), + device_del("device_del", device_del), + chardev_add("chardev-add", chardev_add), + chardev_remove("chardev-remove", chardev_remove), + netdev_add("netdev_add", Box), + netdev_del("netdev_del", netdev_del), + cameradev_add("cameradev_add", cameradev_add), + cameradev_del("cameradev_del", cameradev_del), + query_hotpluggable_cpus("query-hotpluggable-cpus", query_hotpluggable_cpus, default), + query_cpus("query-cpus", query_cpus, default), + query_status("query-status", query_status, default), + getfd("getfd", getfd), + blockdev_add("blockdev-add", Box), + blockdev_del("blockdev-del", blockdev_del), + balloon("balloon", balloon, default), + query_mem("query-mem", query_mem, default), + query_mem_gpa("query-mem-gpa", query_mem_gpa, default), + query_balloon("query-balloon", query_balloon, default), + query_vnc("query-vnc", query_vnc, default), + query_display_image("query-display-image", query_display_image, default), + switch_audio_record("switch-audio-record", switch_audio_record), + migrate("migrate", migrate), + query_migrate("query-migrate", query_migrate, default), + cancel_migrate("migrate_cancel", cancel_migrate, default), + query_version("query-version", query_version, default), + query_commands("query-commands", query_commands, default), + query_target("query-target", query_target, default), + query_kvm("query-kvm", query_kvm, default), + query_machines("query-machines", query_machines, default), + query_events("query-events", query_events, default), + list_type("qom-list-types", list_type, default), + device_list_properties("device-list-properties", device_list_properties, default), + block_commit("block-commit", block_commit, default), + query_tpm_models("query-tpm-models", query_tpm_models, default), + query_tpm_types("query-tpm-types", query_tpm_types, default), + query_command_line_options("query-command-line-options", query_command_line_options, default), + query_migrate_capabilities("query-migrate-capabilities", query_migrate_capabilities, default), + query_qmp_schema("query-qmp-schema", query_qmp_schema, default), + query_sev_capabilities("query-sev-capabilities", query_sev_capabilities, default), + query_chardev("query-chardev", query_chardev, default), + qom_list("qom-list", qom_list, default), + qom_get("qom-get", qom_get, default), + query_block("query-block", query_block, default), + query_named_block_nodes("query-named-block-nodes", query_named_block_nodes, default), + query_blockstats("query-blockstats", query_blockstats, default), + query_block_jobs("query-block-jobs", query_block_jobs, default), + query_gic_capabilities("query-gic-capabilities", query_gic_capabilities, default), + query_iothreads("query-iothreads", query_iothreads, default), + update_region("update_region", update_region, default), + input_event("input_event", input_event, default), + human_monitor_command("human-monitor-command", human_monitor_command), + blockdev_snapshot_internal_sync("blockdev-snapshot-internal-sync", blockdev_snapshot_internal), + blockdev_snapshot_delete_internal_sync("blockdev-snapshot-delete-internal-sync", blockdev_snapshot_internal), + query_vcpu_reg("query-vcpu-reg", query_vcpu_reg), + trace_get_state("trace-get-state", trace_get_state), + trace_set_state("trace-set-state", trace_set_state), + query_workloads("query-workloads", query_workloads) ); /// Command trait for Deserialize and find back Response. -- Gitee From 80f26b7688a382560dee9e0aaf698250646a4455 Mon Sep 17 00:00:00 2001 From: Fan Xuan Zhe Date: Tue, 20 May 2025 17:32:10 +0800 Subject: [PATCH 13/20] migration: Refactor buffer initialization and improve documentation formatting Replaced manual Vec allocation with `vec![0; len]` for cleaner and safer buffer initialization. Adjusted and aligned doc comments for better readability and consistency in `migration.rs` and `migration_derive`. Simplified unsafe memory slice casting logic for memory blocks. Signed-off-by: Fan Xuan Zhe --- migration/migration_derive/src/lib.rs | 2 +- migration/src/general.rs | 16 ++++++---------- migration/src/migration.rs | 12 +++++------- 3 files changed, 12 insertions(+), 18 deletions(-) diff --git a/migration/migration_derive/src/lib.rs b/migration/migration_derive/src/lib.rs index 11736fc1a..f0ac73f78 100644 --- a/migration/migration_derive/src/lib.rs +++ b/migration/migration_derive/src/lib.rs @@ -48,7 +48,7 @@ //! ``` //! //! 2. The `ByteCode` derive to auto add `ByteCode` trait and its relying trait for -//! struct, such as `Default`, `Sync`, `Send`. +//! struct, such as `Default`, `Sync`, `Send`. mod attr_parser; mod field_parser; diff --git a/migration/src/general.rs b/migration/src/general.rs index d0c7c65dc..cfe6909cb 100644 --- a/migration/src/general.rs +++ b/migration/src/general.rs @@ -83,9 +83,8 @@ impl MigrationManager { } // 2. read header according to its length - let mut header_bytes = Vec::new(); - // SAFETY: upper limit of header_len is HEADER_LENGTH - 8. - header_bytes.resize(header_len as usize, 0); + // upper limit of header_len is HEADER_LENGTH - 8. + let mut header_bytes = vec![0; header_len as usize]; fd.read_exact(&mut header_bytes)?; // 3. change the binary format header into struct @@ -113,9 +112,8 @@ impl MigrationManager { /// Write all `DeviceStateDesc` in `desc_db` hashmap to `Write` trait object. pub fn save_desc_db(fd: &mut dyn Write) -> Result<()> { let length = Self::desc_db_len()?; - let mut buffer = Vec::new(); - // SAFETY: desc db length is under control. - buffer.resize(length, 0); + // desc db length is under control. + let mut buffer = vec![0; length]; let mut start = 0; let desc_db = MIGRATION_MANAGER.desc_db.read().unwrap(); @@ -136,9 +134,8 @@ impl MigrationManager { fd: &mut dyn Read, desc_length: usize, ) -> Result> { - let mut desc_buffer = Vec::new(); // SAFETY: desc_length has been checked in check_header(). - desc_buffer.resize(desc_length, 0); + let mut desc_buffer = vec![0; desc_length]; fd.read_exact(&mut desc_buffer)?; let mut snapshot_desc_db = HashMap::::new(); @@ -187,9 +184,8 @@ impl MigrationManager { .get(&snap_desc.name) .with_context(|| "Failed to get snap_desc name")?; - let mut state_data = Vec::new(); // SAFETY: size has been checked in restore_desc_db(). - state_data.resize(snap_desc.size as usize, 0); + let mut state_data = vec![0; snap_desc.size as usize]; fd.read_exact(&mut state_data)?; match current_desc.check_version(snap_desc) { diff --git a/migration/src/migration.rs b/migration/src/migration.rs index 2782d9cd8..ee96d1fe5 100644 --- a/migration/src/migration.rs +++ b/migration/src/migration.rs @@ -33,8 +33,8 @@ impl MigrationManager { /// # Arguments /// /// * `fd` - The fd implements `Read` and `Write` trait object. it - /// will send source VM memory data and devices state to destination VM. - /// And, it will receive confirmation from destination VM. + /// will send source VM memory data and devices state to destination VM. + /// And, it will receive confirmation from destination VM. pub fn send_migration(fd: &mut T) -> Result<()> where T: Read + Write, @@ -97,8 +97,8 @@ impl MigrationManager { /// # Arguments /// /// * `fd` - The fd implements `Read` and `Write` trait object. it - /// will receive source VM memory data and devices state. And, - /// it will send confirmation to source VM. + /// will receive source VM memory data and devices state. And, + /// it will send confirmation to source VM. pub fn recv_migration(fd: &mut T) -> Result<()> where T: Read + Write, @@ -375,9 +375,7 @@ impl MigrationManager { // SAFETY: // 1. The pointer of blocks can be guaranteed not null. // 2. The len is constant. - unsafe { - std::slice::from_raw_parts(blocks.as_ptr() as *const MemBlock as *const u8, len) - }, + unsafe { std::slice::from_raw_parts(blocks.as_ptr() as *const u8, len) }, )?; if let Some(locked_memory) = &MIGRATION_MANAGER.vmm.read().unwrap().memory { -- Gitee From 7de67bfc27e5e9a601c1fbab3373a3f3a5e688e0 Mon Sep 17 00:00:00 2001 From: Fan Xuan Zhe Date: Tue, 20 May 2025 17:35:33 +0800 Subject: [PATCH 14/20] ozonec: Refactor status formatting, improve defaults, and clean up unused code Replaced the `ToString` impl for `ContainerStatus` with a `Display` implementation for better integration with formatting macros. Simplified handling of `annotations` by using `unwrap_or_default()` instead of manual `Option` matching. Minor cleanup: - Removed unused `HashMap` import. - Simplified `CString::new` usage for command arguments. - Removed redundant `.write(true)` flag in log file opening since `.append(true)` implies write access. Signed-off-by: Fan Xuan Zhe --- ozonec/oci_spec/src/state.rs | 12 ++++++------ ozonec/src/linux/container.rs | 8 ++------ ozonec/src/linux/process.rs | 2 +- ozonec/src/utils/logger.rs | 1 - 4 files changed, 9 insertions(+), 14 deletions(-) diff --git a/ozonec/oci_spec/src/state.rs b/ozonec/oci_spec/src/state.rs index 105f128ef..6e0f0f476 100644 --- a/ozonec/oci_spec/src/state.rs +++ b/ozonec/oci_spec/src/state.rs @@ -25,13 +25,13 @@ pub enum ContainerStatus { Stopped, } -impl ToString for ContainerStatus { - fn to_string(&self) -> String { +impl std::fmt::Display for ContainerStatus { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match *self { - ContainerStatus::Creating => String::from("creating"), - ContainerStatus::Created => String::from("created"), - ContainerStatus::Running => String::from("running"), - ContainerStatus::Stopped => String::from("stopped"), + ContainerStatus::Creating => write!(f, "creating"), + ContainerStatus::Created => write!(f, "created"), + ContainerStatus::Running => write!(f, "running"), + ContainerStatus::Stopped => write!(f, "stopped"), } } } diff --git a/ozonec/src/linux/container.rs b/ozonec/src/linux/container.rs index 776a65e7a..25f57634d 100644 --- a/ozonec/src/linux/container.rs +++ b/ozonec/src/linux/container.rs @@ -11,7 +11,6 @@ // See the Mulan PSL v2 for more details. use std::{ - collections::HashMap, fs::{self, canonicalize, create_dir_all, OpenOptions}, io::Write, os::unix::net::UnixStream, @@ -627,11 +626,7 @@ impl Container for LinuxContainer { .to_string(), None => bail!("Failed to get bundle directory"), }; - let annotations = if let Some(a) = self.config.annotations.clone() { - a - } else { - HashMap::new() - }; + let annotations = self.config.annotations.clone().unwrap_or_default(); Ok(OciState { ociVersion: self.config.ociVersion.clone(), id: self.id.clone(), @@ -795,6 +790,7 @@ impl Container for LinuxContainer { #[cfg(test)] pub mod tests { + use std::collections::HashMap; use std::ffi::CStr; use chrono::DateTime; diff --git a/ozonec/src/linux/process.rs b/ozonec/src/linux/process.rs index 20bd35d6a..1b7da254b 100644 --- a/ozonec/src/linux/process.rs +++ b/ozonec/src/linux/process.rs @@ -290,7 +290,7 @@ impl Process { // It has been make sure that args is not None in validate_config(). let args = &self.oci.args.as_ref().unwrap(); // args don't have 0 byte in the middle such as "hello\0world". - let exec_bin = CString::new(args[0].as_str().as_bytes()).unwrap(); + let exec_bin = CString::new(args[0].as_bytes()).unwrap(); let args: Vec = args .iter() .map(|s| CString::new(s.as_bytes()).unwrap_or_default()) diff --git a/ozonec/src/utils/logger.rs b/ozonec/src/utils/logger.rs index 33ecd86bc..899b721d0 100644 --- a/ozonec/src/utils/logger.rs +++ b/ozonec/src/utils/logger.rs @@ -88,7 +88,6 @@ impl LogRotate { fn open_log_file(path: &PathBuf) -> Result { OpenOptions::new() .read(false) - .write(true) .append(true) .create(true) .mode(0o640) -- Gitee From 93b59a23995099a33d7b5dfdb3ba67d83390d7d9 Mon Sep 17 00:00:00 2001 From: Fan Xuan Zhe Date: Tue, 20 May 2025 17:37:38 +0800 Subject: [PATCH 15/20] tests: Fix typos Fix some typos like `repeat` and `config`. Signed-off-by: Fan Xuan Zhe --- tests/mod_test/tests/pci_test.rs | 2 +- tests/mod_test/tests/serial_test.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/mod_test/tests/pci_test.rs b/tests/mod_test/tests/pci_test.rs index dcad2ea4a..0102f8506 100644 --- a/tests/mod_test/tests/pci_test.rs +++ b/tests/mod_test/tests/pci_test.rs @@ -2209,7 +2209,7 @@ fn test_pci_hotunplug_004() { tear_down(None, test_state, alloc, None, Some(image_paths)); } -/// Repeate hotunplug the same device. +/// Repeat hotunplug the same device. #[test] fn test_pci_hotunplug_005() { let blk_nums = 1; diff --git a/tests/mod_test/tests/serial_test.rs b/tests/mod_test/tests/serial_test.rs index 8009f8f20..a24640524 100644 --- a/tests/mod_test/tests/serial_test.rs +++ b/tests/mod_test/tests/serial_test.rs @@ -487,7 +487,7 @@ fn verify_input_data(input: &mut dyn Read, test_data: &String) { /// Expect: /// 1/2/3/4: success. #[test] -fn serial_config_rw_conifg() { +fn serial_config_rw_config() { let port = PortConfig { chardev_type: ChardevType::Pty, nr: 0, -- Gitee From c4b59804ada7bb866f0e0cb909280d0d31541541 Mon Sep 17 00:00:00 2001 From: Fan Xuan Zhe Date: Tue, 20 May 2025 17:47:31 +0800 Subject: [PATCH 16/20] trace: Optimize static global implementation Replaced static global `TRACE_SCOPE_COUNTER` with `lazy_static!` for thread-safe lazy initialization. Signed-off-by: Fan Xuan Zhe --- trace/src/trace_scope.rs | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/trace/src/trace_scope.rs b/trace/src/trace_scope.rs index a860ef8d7..b5f7e5ef1 100644 --- a/trace/src/trace_scope.rs +++ b/trace/src/trace_scope.rs @@ -10,6 +10,7 @@ // NON-INFRINGEMENT, MERCHANTABILITY OR FIT FOR A PARTICULAR PURPOSE. // See the Mulan PSL v2 for more details. +use lazy_static::lazy_static; use std::sync::atomic::{AtomicI32, Ordering}; #[cfg(all(target_env = "ohos", feature = "trace_to_hitrace"))] @@ -18,7 +19,9 @@ use crate::hitrace::{finish_trace, finish_trace_async, start_trace, start_trace_ #[cfg(feature = "trace_to_ftrace")] use crate::ftrace::write_trace_marker; -static mut TRACE_SCOPE_COUNTER: AtomicI32 = AtomicI32::new(i32::MIN); +lazy_static! { + static ref TRACE_SCOPE_COUNTER: AtomicI32 = AtomicI32::new(i32::MIN); +} #[derive(Clone)] pub enum Scope { @@ -75,13 +78,11 @@ impl TraceScopeAsyn { #[allow(unused_variables)] pub fn new(value: String) -> Self { // SAFETY: AtomicI32 can be safely shared between threads. - let id = unsafe { - TRACE_SCOPE_COUNTER - .fetch_update(Ordering::SeqCst, Ordering::SeqCst, |x| { - Some(x.wrapping_add(1)) - }) - .unwrap() - }; + let id = TRACE_SCOPE_COUNTER + .fetch_update(Ordering::SeqCst, Ordering::SeqCst, |x| { + Some(x.wrapping_add(1)) + }) + .unwrap(); #[cfg(feature = "trace_to_logger")] { log::trace!("[SCOPE_START(id={})]{}", id, value); -- Gitee From b9e9868b4cebf7a82ec0c3291aa3132fb7f7eb7f Mon Sep 17 00:00:00 2001 From: Fan Xuan Zhe Date: Tue, 20 May 2025 17:51:58 +0800 Subject: [PATCH 17/20] ui: Fix typos, cleanup code, and implement Sync + Send for thread safety MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Implemented `unsafe impl Sync` and `Send` for several structs (`DisplayChangeListener`, `DisplayConsole`, `RectInfo`, `ConnState`, `ClientState`, `ClientIoHandler`) to enable thread-safe usage. `// SAFETY:` comments were added as placeholders for safety justifications. Fixed minor typos and grammar in comments and docstrings (e.g., “returen” → “return”, “includs” → “includes”, “capcity” → “capacity”). Improved formatting of multi-line doc comments for better readability and consistent indentation. Simplified some expressions using `.unwrap_or_default()` to streamline logic. Removed unnecessary `to_string()` calls when removing keys from `HashMap`, using the string slice directly. Clarified unsafe pointer usage in `pixman.rs` and avoided unnecessary casts. Added `Align32` wrapper for properly aligned image data in test code, and adjusted references to use `.0` field accordingly. Signed-off-by: Fan Xuan Zhe --- ui/src/console.rs | 14 +++++++++- ui/src/gtk/menu.rs | 1 + ui/src/gtk/mod.rs | 10 ++++--- ui/src/input.rs | 9 +++---- ui/src/ohui_srv/msg_handle.rs | 2 +- ui/src/pixman.rs | 5 ++-- ui/src/vnc/client_io.rs | 26 ++++++++++++++++++- ui/src/vnc/encoding/enc_hextile.rs | 6 ++--- .../vnc/encoding/test_hextile_image_data.rs | 15 ++++++----- ui/src/vnc/mod.rs | 3 ++- ui/src/vnc/server_io.rs | 6 +++++ 11 files changed, 72 insertions(+), 25 deletions(-) diff --git a/ui/src/console.rs b/ui/src/console.rs index 7d0e27bc0..80dbbc49a 100644 --- a/ui/src/console.rs +++ b/ui/src/console.rs @@ -178,6 +178,12 @@ pub struct DisplayChangeListener { pub dpy_opts: Arc, } +// SAFETY: Implementing Send and Sync is safe for DisplayChangeListener +// because only locked access(r/w) is permitted +unsafe impl Sync for DisplayChangeListener {} +// SAFETY: Same as above +unsafe impl Send for DisplayChangeListener {} + impl DisplayChangeListener { pub fn new(con_id: Option, dpy_opts: Arc) -> Self { Self { @@ -206,6 +212,12 @@ pub struct DisplayConsole { pub active: bool, } +// SAFETY: Implementing Send and Sync is safe for DisplayConsole +// because only locked access(r/w) is permitted +unsafe impl Sync for DisplayConsole {} +// SAFETY: Same as above +unsafe impl Send for DisplayConsole {} + impl DisplayConsole { pub fn new( con_id: usize, @@ -645,7 +657,7 @@ pub fn unregister_display(dcl: &Option>>) -> R Ok(()) } -/// Create a console and add into a global list. Then returen a console id +/// Create a console and add into a global list. Then return a console id /// for later finding the assigned console. pub fn console_init( dev_name: String, diff --git a/ui/src/gtk/menu.rs b/ui/src/gtk/menu.rs index c1e4b6b64..05aad1879 100644 --- a/ui/src/gtk/menu.rs +++ b/ui/src/gtk/menu.rs @@ -83,6 +83,7 @@ impl GtkMenu { /// 1. Setting callback function for button. /// 2. Set shortcut keys for buttons. + /// /// Button shortcut key /// shutdown_item: Ctrl + Alt + S. /// full_screen_item Ctrl + Alt + F diff --git a/ui/src/gtk/mod.rs b/ui/src/gtk/mod.rs index 9c3fdcaa8..11a081aba 100644 --- a/ui/src/gtk/mod.rs +++ b/ui/src/gtk/mod.rs @@ -253,7 +253,7 @@ impl GtkDisplay { fn get_current_display(&self) -> Result>> { let page_num = self.gtk_menu.note_book.current_page(); let gs = match page_num { - Some(num) if self.pagenum2ds.get(&num).is_some() => self.pagenum2ds.get(&num).unwrap(), + Some(num) if self.pagenum2ds.contains_key(&num) => self.pagenum2ds.get(&num).unwrap(), _ => bail!("No active display"), }; Ok(gs.clone()) @@ -445,6 +445,7 @@ impl GtkDisplayScreen { /// In some situation: /// 1. Image is scaled. /// 2. There may be unfilled areas between the window and the image. + /// /// Input: relative coordinates of window. /// Output: relative coordinates of images. fn convert_coord(&mut self, mut x: f64, mut y: f64) -> Result<(f64, f64)> { @@ -683,8 +684,9 @@ fn gs_show_menu_callback( /// There is a situation: /// 1. Switch operation 1, the gtk display should change the image from a to b. /// 2. Switch operation 2, the gtk display should change the image from b to c, but -/// the channel between stratovirt mainloop and gtk mainloop lost the event. +/// the channel between stratovirt mainloop and gtk mainloop lost the event. /// 3. The gtk display always show the image. +/// /// So, the refresh operation will always check if the image has been switched, if /// the result is yes, then use the switch operation to switch the latest image. fn do_refresh_event(gs: &Rc>) -> Result<()> { @@ -901,7 +903,7 @@ fn do_switch_event(gs: &Rc>) -> Result<()> { // 2. The copy range will not exceed the image data. unsafe { ImageSurface::create_for_data_unsafe( - data as *mut u8, + data, Format::Rgb24, surface_width, surface_height, @@ -925,7 +927,7 @@ fn do_switch_event(gs: &Rc>) -> Result<()> { // 2. The copy range will not exceed the image data. unsafe { ImageSurface::create_for_data_unsafe( - data as *mut u8, + data, Format::Rgb24, surface_width, surface_height, diff --git a/ui/src/input.rs b/ui/src/input.rs index c59926be7..6f1e10c77 100644 --- a/ui/src/input.rs +++ b/ui/src/input.rs @@ -159,10 +159,7 @@ impl Default for KeyBoardState { impl KeyBoardState { /// Get the corresponding keyboard modifier. fn keyboard_modifier_get(&self, key_mod: KeyboardModifier) -> bool { - match self.keymods.contain(key_mod as usize) { - Ok(res) => res, - Err(_e) => false, - } + self.keymods.contain(key_mod as usize).unwrap_or_default() } /// Reset all keyboard modifier state. @@ -272,7 +269,7 @@ impl Inputs { } fn unregister_kbd(&mut self, device: &str) { - self.kbd_lists.remove(&device.to_string()); + self.kbd_lists.remove(device); let len = self.kbd_ids.len(); for i in 0..len { if self.kbd_ids[i] == device { @@ -288,7 +285,7 @@ impl Inputs { } fn unregister_mouse(&mut self, device: &str) { - self.tablet_lists.remove(&device.to_string()); + self.tablet_lists.remove(device); let len = self.tablet_ids.len(); for i in 0..len { if self.tablet_ids[i] == device { diff --git a/ui/src/ohui_srv/msg_handle.rs b/ui/src/ohui_srv/msg_handle.rs index 47d2939f3..18ec699c0 100755 --- a/ui/src/ohui_srv/msg_handle.rs +++ b/ui/src/ohui_srv/msg_handle.rs @@ -424,7 +424,7 @@ impl MsgReader { } let buf = self.body.as_mut().unwrap(); // SAFETY: 1. we guarantee new message has new body, so - // buf's capcity is equal to body_size. 2. buf has 'u8' + // buf's capacity is equal to body_size. 2. buf has 'u8' // type elements, it will be initialized by zero. unsafe { buf.set_len(body_size); diff --git a/ui/src/pixman.rs b/ui/src/pixman.rs index 216321065..be6643d1e 100644 --- a/ui/src/pixman.rs +++ b/ui/src/pixman.rs @@ -148,7 +148,7 @@ pub fn get_image_stride(image: *mut pixman_image_t) -> i32 { pub fn get_image_data(image: *mut pixman_image_t) -> *mut u32 { if image.is_null() { - return ptr::null_mut() as *mut u32; + return ptr::null_mut(); } // SAFETY: The reason is the same as above. @@ -174,9 +174,10 @@ pub fn create_pixman_image( if !(0..MAX_IMAGE_SIZE).contains(&width) || !(0..MAX_IMAGE_SIZE).contains(&height) { return ptr::null_mut() as *mut pixman_image_t; } + let image_data_local = image_data; // SAFETY: The reason is the same as above. - unsafe { pixman_image_create_bits(image_format, width, height, image_data as *mut u32, stride) } + unsafe { pixman_image_create_bits(image_format, width, height, image_data_local, stride) } } /// Bpp: bit per pixel diff --git a/ui/src/vnc/client_io.rs b/ui/src/vnc/client_io.rs index 7ffa89968..9866b2d8b 100644 --- a/ui/src/vnc/client_io.rs +++ b/ui/src/vnc/client_io.rs @@ -235,6 +235,12 @@ pub struct RectInfo { pub rects: Vec, } +// SAFETY: Implementing Send and Sync is safe for RectInfo +// because only locked access(r/w) is permitted +unsafe impl Sync for RectInfo {} +// SAFETY: Same as above +unsafe impl Send for RectInfo {} + impl RectInfo { pub fn new(client: &Arc, rects: Vec) -> Self { RectInfo { @@ -332,6 +338,12 @@ pub struct ConnState { pub client_io: Option>>, } +// SAFETY: Implementing Send and Sync is safe for ConnState +// because only locked access(r/w) is permitted +unsafe impl Sync for ConnState {} +// SAFETY: Same as above +unsafe impl Send for ConnState {} + impl Default for ConnState { fn default() -> Self { ConnState { @@ -388,6 +400,12 @@ pub struct ClientState { pub dirty_bitmap: Arc>>, } +// SAFETY: Implementing Send and Sync is safe for ClientState +// because only locked access(r/w) is permitted +unsafe impl Sync for ClientState {} +// SAFETY: Same as above +unsafe impl Send for ClientState {} + impl ClientState { pub fn new(addr: String) -> Self { ClientState { @@ -427,6 +445,12 @@ pub struct ClientIoHandler { pub server: Arc, } +// SAFETY: Implementing Send and Sync is safe for ClientIoHandler +// because only locked access(r/w) is permitted +unsafe impl Sync for ClientIoHandler {} +// SAFETY: Same as above +unsafe impl Send for ClientIoHandler {} + impl ClientIoHandler { pub fn new( stream: TcpStream, @@ -449,7 +473,7 @@ impl ClientIoHandler { } impl ClientIoHandler { - /// This function interacts with the client interface, it includs several + /// This function interacts with the client interface, it includes several /// steps: Read the data stream from the fd, save the data in buffer, /// and then process the data by io handle function. fn client_handle_read(&mut self) -> Result<(), anyhow::Error> { diff --git a/ui/src/vnc/encoding/enc_hextile.rs b/ui/src/vnc/encoding/enc_hextile.rs index f41a4d8c1..50e7f39f0 100644 --- a/ui/src/vnc/encoding/enc_hextile.rs +++ b/ui/src/vnc/encoding/enc_hextile.rs @@ -389,7 +389,7 @@ mod tests { let client_be = false; let enc = ENCODING_HEXTILE; let client_dpm = DisplayMode::new(enc, client_be, convert, pf); - let image_data = IMAGE_DATA_SINGLE_PIXEL; + let image_data = &IMAGE_DATA_SINGLE_PIXEL.0; let target_data = TARGET_DATA_SINGLE_PIXEL; let image_width: i32 = 32; let image_height: i32 = 32; @@ -420,7 +420,7 @@ mod tests { let client_be = false; let enc = ENCODING_HEXTILE; let client_dpm = DisplayMode::new(enc, client_be, convert, pf); - let image_data = IMAGE_DATA_TWO_PIXEL; + let image_data = &IMAGE_DATA_TWO_PIXEL.0; let target_data = TARGET_DATA_TWO_PIXEL; let image_width: i32 = 40; let image_height: i32 = 40; @@ -451,7 +451,7 @@ mod tests { let client_be = false; let enc = ENCODING_HEXTILE; let client_dpm = DisplayMode::new(enc, client_be, convert, pf); - let image_data = IMAGE_DATA_MULTI_PIXELS; + let image_data = &IMAGE_DATA_MULTI_PIXELS.0; let target_data = TARGET_DATA_MULTI_PIXELS; let image_width: i32 = 40; let image_height: i32 = 40; diff --git a/ui/src/vnc/encoding/test_hextile_image_data.rs b/ui/src/vnc/encoding/test_hextile_image_data.rs index 4aeadf1e4..4e8ed1b3e 100644 --- a/ui/src/vnc/encoding/test_hextile_image_data.rs +++ b/ui/src/vnc/encoding/test_hextile_image_data.rs @@ -10,12 +10,15 @@ // NON-INFRINGEMENT, MERCHANTABILITY OR FIT FOR A PARTICULAR PURPOSE. // See the Mulan PSL v2 for more details. +#[repr(align(32))] +pub struct Align32(pub T); + /// Image data: Each tile contains only one pixel. /// Width of image = 32 /// Height of image = 32 /// Stride of image = 128 /// Total length is 4096 Byte. -pub const IMAGE_DATA_SINGLE_PIXEL: [u8; 4096] = [ +pub const IMAGE_DATA_SINGLE_PIXEL: Align32<[u8; 4096]> = Align32([ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, @@ -272,7 +275,7 @@ pub const IMAGE_DATA_SINGLE_PIXEL: [u8; 4096] = [ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, -]; +]); /// The data stream obtained after the IMAGE_DATA_2 is compressed using the Hextile algorithm. /// Total length is equal to 12. @@ -285,7 +288,7 @@ pub const TARGET_DATA_SINGLE_PIXEL: [u8; 12] = [ /// Height of image = 40 /// Stride of image = 160 /// Total length is 6400 Byte. -pub const IMAGE_DATA_TWO_PIXEL: [u8; 6400] = [ +pub const IMAGE_DATA_TWO_PIXEL: Align32<[u8; 6400]> = Align32([ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, @@ -686,7 +689,7 @@ pub const IMAGE_DATA_TWO_PIXEL: [u8; 6400] = [ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xaa, 0xaa, 0xaa, 0x00, 0xaa, 0xaa, 0xaa, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xaa, 0xaa, 0xaa, 0x00, 0xaa, 0xaa, 0xaa, 0x00, -]; +]); pub const TARGET_DATA_TWO_PIXEL: [u8; 348] = [ 0x0e, 0x00, 0x00, 0x00, 0x00, 0xaa, 0xaa, 0xaa, 0x00, 0x19, 0x02, 0x70, 0x03, 0x10, 0x33, 0x10, @@ -718,7 +721,7 @@ pub const TARGET_DATA_TWO_PIXEL: [u8; 348] = [ /// Height of image = 40 /// Stride of image = 160 /// Total length is 6400 Byte. -pub const IMAGE_DATA_MULTI_PIXELS: [u8; 6400] = [ +pub const IMAGE_DATA_MULTI_PIXELS: Align32<[u8; 6400]> = Align32([ 0x8a, 0x72, 0x7b, 0xff, 0x8a, 0x72, 0x7b, 0xff, 0x8a, 0x72, 0x7b, 0xff, 0x8a, 0x72, 0x7b, 0xff, 0x8a, 0x72, 0x7b, 0xff, 0x8a, 0x72, 0x7b, 0xff, 0x8a, 0x71, 0x7b, 0xff, 0x8a, 0x71, 0x7b, 0xff, 0x8a, 0x71, 0x7b, 0xff, 0x8a, 0x71, 0x7b, 0xff, 0x8a, 0x71, 0x7b, 0xff, 0x8a, 0x71, 0x7b, 0xff, @@ -1119,7 +1122,7 @@ pub const IMAGE_DATA_MULTI_PIXELS: [u8; 6400] = [ 0x8a, 0x71, 0x7a, 0xff, 0x8a, 0x71, 0x7a, 0xff, 0x8a, 0x71, 0x7a, 0xff, 0x8a, 0x71, 0x7a, 0xff, 0x8a, 0x71, 0x7a, 0xff, 0x8a, 0x71, 0x7a, 0xff, 0x8a, 0x70, 0x7a, 0xff, 0x8a, 0x70, 0x7a, 0xff, 0x8a, 0x70, 0x7a, 0xff, 0x8a, 0x70, 0x7a, 0xff, 0x8a, 0x70, 0x7a, 0xff, 0x8a, 0x70, 0x7a, 0xff, -]; +]); pub const TARGET_DATA_MULTI_PIXELS: [u8; 557] = [ 0x1a, 0x8a, 0x71, 0x7b, 0xff, 0x20, 0x8a, 0x72, 0x7b, 0xff, 0x00, 0x50, 0x8a, 0x71, 0x7a, 0xff, diff --git a/ui/src/vnc/mod.rs b/ui/src/vnc/mod.rs index a760ae3b3..466ce59af 100644 --- a/ui/src/vnc/mod.rs +++ b/ui/src/vnc/mod.rs @@ -557,10 +557,11 @@ pub fn write_pixel( buf: &mut Vec, ) { if !client_dpm.convert { + let data_ptr_local = data_ptr; let mut con = vec![0; copy_bytes]; // SAFETY: Tt can be ensure the raw pointer will not exceed the range. unsafe { - ptr::copy(data_ptr as *mut u8, con.as_mut_ptr(), copy_bytes); + ptr::copy(data_ptr_local, con.as_mut_ptr(), copy_bytes); } buf.append(&mut con); } else if client_dpm.convert && bytes_per_pixel() == 4 { diff --git a/ui/src/vnc/server_io.rs b/ui/src/vnc/server_io.rs index 2b2903559..d3118670d 100644 --- a/ui/src/vnc/server_io.rs +++ b/ui/src/vnc/server_io.rs @@ -303,6 +303,12 @@ pub struct VncSurface { pub guest_format: pixman_format_code_t, } +// SAFETY: Implementing Send and Sync is safe for VncSurface +// because only locked access(r/w) is permitted +unsafe impl Sync for VncSurface {} +// SAFETY: Same as above +unsafe impl Send for VncSurface {} + impl VncSurface { fn new(guest_image: *mut pixman_image_t) -> Self { VncSurface { -- Gitee From f9e6402e730a0feab5e142caafddfdd498df6446 Mon Sep 17 00:00:00 2001 From: Fan Xuan Zhe Date: Tue, 20 May 2025 17:59:45 +0800 Subject: [PATCH 18/20] util: Cleanup and Add Sync/Send Safety Annotations in util Module Improved comment alignment and readability in `bitmap.rs` and `daemonize.rs`. Fixed typos in seccomp documentation comments ("programe" -> "program"). Removed unused `std::{fmt, i32}` import from `loop_context.rs`. Added `unsafe impl Sync/Send` for thread safety: - `List` in `link_list.rs` - `EventNotifier` and `Timer` in `loop_context.rs` Minor cosmetic cleanup in `pixman.rs` (removed trailing newline). Signed-off-by: Fan Xuan Zhe --- util/src/bitmap.rs | 2 +- util/src/daemonize.rs | 6 +++--- util/src/link_list.rs | 6 ++++++ util/src/loop_context.rs | 14 +++++++++++++- util/src/seccomp.rs | 4 ++-- 5 files changed, 25 insertions(+), 7 deletions(-) diff --git a/util/src/bitmap.rs b/util/src/bitmap.rs index 3d8111c67..b2d5b71d6 100644 --- a/util/src/bitmap.rs +++ b/util/src/bitmap.rs @@ -29,7 +29,7 @@ impl Bitmap { /// # Arguments /// /// * `size` - The size of bitmap is the number of bit unit. If you want - /// to restore a `length` with bitmap. The `size` would be `length/bit_unit_size+1`. + /// to restore a `length` with bitmap. The `size` would be `length/bit_unit_size+1`. pub fn new(size: usize) -> Self { Bitmap:: { data: [T::zero()].repeat(size), diff --git a/util/src/daemonize.rs b/util/src/daemonize.rs index f693abb4e..5c93858b9 100644 --- a/util/src/daemonize.rs +++ b/util/src/daemonize.rs @@ -20,15 +20,15 @@ //! waiting for some event to occur, or waiting to perform some specified task //! on a periodic basis. A typical daemon program will: //! 1. Close all open file descriptors(especially standard input, standard -//! output and standard error). +//! output and standard error). //! 2. Change its working directory to the root filesystem, to ensure that it -//! doesn't tie up another filesystem and prevent it from being unmounted. +//! doesn't tie up another filesystem and prevent it from being unmounted. //! 3. Reset its umask value. //! 4. Run in the background(i.e., fork). //! 5. Ignore all terminal I/O signals. //! 6. Disassociate from the control terminal. //! 7. Disassociate from its process group, to insulate itself from signals -//! sent to the process group. +//! sent to the process group. //! 8. Handle any `SIGCLD` signals. use std::cmp::Ordering; diff --git a/util/src/link_list.rs b/util/src/link_list.rs index a779bce17..fbcba646d 100644 --- a/util/src/link_list.rs +++ b/util/src/link_list.rs @@ -27,6 +27,12 @@ pub struct List { marker: PhantomData>>, } +// SAFETY: Implementing Send and Sync is safe for List +// because only locked access(r/w) is permitted +unsafe impl Sync for List {} +// SAFETY: Same as above +unsafe impl Send for List {} + impl Drop for List { fn drop(&mut self) { while self.pop_head().is_some() {} diff --git a/util/src/loop_context.rs b/util/src/loop_context.rs index d1a3f9bf4..0a0af54f2 100644 --- a/util/src/loop_context.rs +++ b/util/src/loop_context.rs @@ -11,6 +11,7 @@ // See the Mulan PSL v2 for more details. use std::collections::BTreeMap; +use std::fmt; use std::fmt::Debug; use std::io::Error; use std::os::unix::io::{AsRawFd, RawFd}; @@ -18,7 +19,6 @@ use std::rc::Rc; use std::sync::atomic::{AtomicBool, AtomicU64, Ordering}; use std::sync::{Arc, Barrier, Mutex, RwLock}; use std::time::{Duration, Instant}; -use std::{fmt, i32}; use anyhow::{anyhow, Context, Result}; use libc::{c_void, read, EFD_CLOEXEC, EFD_NONBLOCK}; @@ -93,6 +93,12 @@ pub struct EventNotifier { status: Arc>, } +// SAFETY: Logically the EventNotifier structure will not be used +// in multiple threads at the same time +unsafe impl Sync for EventNotifier {} +// SAFETY: Same as above +unsafe impl Send for EventNotifier {} + impl fmt::Debug for EventNotifier { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { f.debug_struct("EventNotifier") @@ -179,6 +185,12 @@ struct Timer { id: u64, } +// SAFETY: Logically the Timer structure will not be used +// in multiple threads at the same time +unsafe impl Sync for Timer {} +// SAFETY: Same as above +unsafe impl Send for Timer {} + impl Timer { /// Construct function /// diff --git a/util/src/seccomp.rs b/util/src/seccomp.rs index cce0facd2..5282a0489 100644 --- a/util/src/seccomp.rs +++ b/util/src/seccomp.rs @@ -76,7 +76,7 @@ //! f.read(&mut buffer).unwrap(); //! println!("{}", String::from_utf8_lossy(&buffer)); //! ``` -//! This programe will be trapped. +//! This program will be trapped. use anyhow::{bail, Result}; @@ -188,7 +188,7 @@ impl From for u32 { } } -/// The format of BPF programe executes over. +/// The format of BPF program executes over. /// /// See: https://elixir.bootlin.com/linux/v4.19.123/source/include/uapi/linux/seccomp.h#L56 #[repr(C, packed)] -- Gitee From e3d3c1dd6d4501038c3b124a7b1bfb92d346b7ff Mon Sep 17 00:00:00 2001 From: Fan Xuan Zhe Date: Tue, 20 May 2025 18:02:42 +0800 Subject: [PATCH 19/20] virtio: Refactor and improve thread safety and code clarity across Virtio devices Marked `GpuIoHandler` as `Sync` and `Send` to explicitly declare thread safety. Simplified match expressions using `if let` for better readability in: - `GpuIoHandler::change_run_stage` - `InputIoHandler` event dispatch logic Replaced `.map(...).map(|v| { ...; v })` with `.inspect(...)` in `GpuIoHandler` for cleaner intent without altering behavior. Minor cleanup: removed redundant casts and clarified pointer declarations in GPU memory handling. Removed unused imports (e.g., `usize`) and reordered standard imports for consistency in `serial.rs`. Replaced a verbose `ToString` implementation with a more idiomatic `Display` impl for `VhostBackendType`. Avoided unnecessary `enumerate()` usage in `net.rs` when only values were needed in loop. Signed-off-by: Fan Xuan Zhe --- virtio/src/device/gpu.rs | 25 +++++++++++++------------ virtio/src/device/input.rs | 7 ++----- virtio/src/device/net.rs | 2 +- virtio/src/device/serial.rs | 2 +- virtio/src/vhost/user/client.rs | 10 +++++----- 5 files changed, 22 insertions(+), 24 deletions(-) diff --git a/virtio/src/device/gpu.rs b/virtio/src/device/gpu.rs index 9cc884485..e0eff9870 100644 --- a/virtio/src/device/gpu.rs +++ b/virtio/src/device/gpu.rs @@ -521,6 +521,12 @@ struct GpuIoHandler { used_hostmem: u64, } +// SAFETY: Logically the GpuIoHandler structure will not be used +// in multiple threads at the same time +unsafe impl Sync for GpuIoHandler {} +// SAFETY: Same as above +unsafe impl Send for GpuIoHandler {} + fn create_surface( scanout: &mut GpuScanout, info_set_scanout: VirtioGpuSetScanout, @@ -702,13 +708,10 @@ fn is_rect_in_resource(rect: &VirtioGpuRect, res: &GpuResource) -> bool { impl GpuIoHandler { fn change_run_stage(&self) -> Result<()> { if get_run_stage() == VmRunningStage::Bios && !self.scanouts.is_empty() { - match &self.scanouts[0].con.as_ref().and_then(|c| c.upgrade()) { - Some(con) => { - let dev_name = con.lock().unwrap().dev_name.clone(); - display_set_major_screen(&dev_name)?; - set_run_stage(VmRunningStage::Os); - } - None => {} + if let Some(con) = &self.scanouts[0].con.as_ref().and_then(|c| c.upgrade()) { + let dev_name = con.lock().unwrap().dev_name.clone(); + display_set_major_screen(&dev_name)?; + set_run_stage(VmRunningStage::Os); }; } Ok(()) @@ -1369,7 +1372,7 @@ impl GpuIoHandler { let width = get_image_width(res.pixman_image) as u32; let bpp = (u32::from(pixman_format_bpp(pixman_format as u32)) + 8 - 1) / 8; let stride = get_image_stride(res.pixman_image) as u32; - let data = get_image_data(res.pixman_image).cast() as *mut u8; + let data: *mut u8 = get_image_data(res.pixman_image).cast(); if res.format == VIRTIO_GPU_FORMAT_MONOCHROME { // SAFETY: iov is generated by address_space. @@ -1387,11 +1390,10 @@ impl GpuIoHandler { // SAFETY: offset_dst and trans_size do not exceeds data size. let dst = unsafe { from_raw_parts_mut(data.add(offset_dst), trans_size) }; // SAFETY: iov is generated by address_space. - unsafe { iov_to_buf_direct(&res.iov, trans_info.offset, dst) }.map(|v| { + unsafe { iov_to_buf_direct(&res.iov, trans_info.offset, dst) }.inspect(|&v| { if v < trans_size { warn!("No enough data is copied for transfer_to_host_2d"); } - v })?; return Ok(()); } @@ -1405,11 +1407,10 @@ impl GpuIoHandler { // SAFETY: offset_dst and line_size do not exceeds data size. let dst = unsafe { from_raw_parts_mut(data.add(offset_dst), line_size) }; // SAFETY: iov is generated by address_space. - unsafe { iov_to_buf_direct(&res.iov, offset_src as u64, dst) }.map(|v| { + unsafe { iov_to_buf_direct(&res.iov, offset_src as u64, dst) }.inspect(|&v| { if v < line_size { warn!("No enough data is copied for transfer_to_host_2d"); } - v })?; offset_src += stride as usize; offset_dst += stride as usize; diff --git a/virtio/src/device/input.rs b/virtio/src/device/input.rs index 721b4e3df..eb2896de5 100644 --- a/virtio/src/device/input.rs +++ b/virtio/src/device/input.rs @@ -303,11 +303,8 @@ impl InputIoHandler { locked_status_queue.vring.get_cache(), )? .to_evt(); - match &self.evdev_fd.clone() { - Some(evdev_fd) => { - let _ = evdev_fd.as_ref().write(evt.as_bytes()); - } - None => {} + if let Some(evdev_fd) = &self.evdev_fd.clone() { + let _ = evdev_fd.as_ref().write(evt.as_bytes()); } locked_status_queue .vring diff --git a/virtio/src/device/net.rs b/virtio/src/device/net.rs index 4d7450897..2ab39e5eb 100644 --- a/virtio/src/device/net.rs +++ b/virtio/src/device/net.rs @@ -1695,7 +1695,7 @@ impl VirtioDevice for Net { let features = self.driver_features(0_u32); let flags = get_tap_offload_flags(u64::from(features)); if let Some(taps) = &self.taps { - for (_, tap) in taps.iter().enumerate() { + for tap in taps.iter() { tap.set_offload(flags) .with_context(|| "Failed to set tap offload")?; } diff --git a/virtio/src/device/serial.rs b/virtio/src/device/serial.rs index 3141672bb..6f5a1bada 100644 --- a/virtio/src/device/serial.rs +++ b/virtio/src/device/serial.rs @@ -10,12 +10,12 @@ // NON-INFRINGEMENT, MERCHANTABILITY OR FIT FOR A PARTICULAR PURPOSE. // See the Mulan PSL v2 for more details. +use std::cmp; use std::mem::size_of; use std::os::unix::io::{AsRawFd, RawFd}; use std::rc::Rc; use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::{Arc, Mutex, Weak}; -use std::{cmp, usize}; use anyhow::{anyhow, bail, Context, Result}; use byteorder::{ByteOrder, LittleEndian}; diff --git a/virtio/src/vhost/user/client.rs b/virtio/src/vhost/user/client.rs index d4c86a780..15386398b 100644 --- a/virtio/src/vhost/user/client.rs +++ b/virtio/src/vhost/user/client.rs @@ -390,12 +390,12 @@ pub enum VhostBackendType { TypeFs, } -impl ToString for VhostBackendType { - fn to_string(&self) -> String { +impl std::fmt::Display for VhostBackendType { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { - VhostBackendType::TypeNet => String::from("net"), - VhostBackendType::TypeBlock => String::from("block"), - VhostBackendType::TypeFs => String::from("fs"), + VhostBackendType::TypeNet => write!(f, "net"), + VhostBackendType::TypeBlock => write!(f, "block"), + VhostBackendType::TypeFs => write!(f, "fs"), } } } -- Gitee From b63bc0a9e4e1ab329b6e4f7fa778753f145bef8f Mon Sep 17 00:00:00 2001 From: Fan Xuan Zhe Date: Tue, 20 May 2025 18:04:56 +0800 Subject: [PATCH 20/20] typos: Add new typo extend words thr = "thr" mch = "mch" Mch = "Mch" MCH = "MCH" ANC = "ANC" DBE = "DBE" AER = "AER" Pn = "Pn" PN = "PN" These words are intentional to be what it looks like, thus make them valid in typos rules. Signed-off-by: Fan Xuan Zhe --- _typos.toml | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/_typos.toml b/_typos.toml index 9bbf0e694..3f88a669c 100644 --- a/_typos.toml +++ b/_typos.toml @@ -32,3 +32,12 @@ inout = "inout" IST = "IST" NCE = "NCE" parm = "parm" +thr = "thr" +mch = "mch" +Mch = "Mch" +MCH = "MCH" +ANC = "ANC" +DBE = "DBE" +AER = "AER" +Pn = "Pn" +PN = "PN" -- Gitee