From a11b4cee63efec5b3947be521f5d2e0851cec4fa Mon Sep 17 00:00:00 2001 From: Manos Pitsidianakis Date: Fri, 24 Nov 2023 09:22:29 +0200 Subject: [PATCH 1/6] sound: upgrade to 2021 Rust edition There's no reason to use 2018, 2021 is the default nowadays. Switch the versions in the rustfmt files of sound and video while at it. Signed-off-by: Manos Pitsidianakis --- staging/vhost-device-sound/Cargo.toml | 2 +- staging/vhost-device-sound/rustfmt.toml | 2 +- staging/vhost-device-video/rustfmt.toml | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/staging/vhost-device-sound/Cargo.toml b/staging/vhost-device-sound/Cargo.toml index 9bf2b7014..d896ba5cb 100644 --- a/staging/vhost-device-sound/Cargo.toml +++ b/staging/vhost-device-sound/Cargo.toml @@ -8,7 +8,7 @@ readme = "README.md" keywords = ["vhost", "sound", "virtio-sound", "virtio-snd", "virtio"] categories = ["multimedia::audio", "virtualization"] license = "Apache-2.0 OR BSD-3-Clause" -edition = "2018" +edition = "2021" [features] xen = ["vm-memory/xen", "vhost/xen", "vhost-user-backend/xen"] diff --git a/staging/vhost-device-sound/rustfmt.toml b/staging/vhost-device-sound/rustfmt.toml index c6f0942d7..ebecb99fe 100644 --- a/staging/vhost-device-sound/rustfmt.toml +++ b/staging/vhost-device-sound/rustfmt.toml @@ -1,4 +1,4 @@ -edition = "2018" +edition = "2021" format_generated_files = false format_code_in_doc_comments = true format_strings = true diff --git a/staging/vhost-device-video/rustfmt.toml b/staging/vhost-device-video/rustfmt.toml index c6f0942d7..ebecb99fe 100644 --- a/staging/vhost-device-video/rustfmt.toml +++ b/staging/vhost-device-video/rustfmt.toml @@ -1,4 +1,4 @@ -edition = "2018" +edition = "2021" format_generated_files = false format_code_in_doc_comments = true format_strings = true From e576905274f797af1d087f4fd5121215f3990bb3 Mon Sep 17 00:00:00 2001 From: Manos Pitsidianakis Date: Fri, 24 Nov 2023 09:48:02 +0200 Subject: [PATCH 2/6] staging: move rustfmt.toml to workspace directory Both staging crates had the same rustfmt.toml file, let's make it workspace default. Signed-off-by: Manos Pitsidianakis --- staging/{vhost-device-sound => }/rustfmt.toml | 0 staging/vhost-device-video/rustfmt.toml | 7 ------- 2 files changed, 7 deletions(-) rename staging/{vhost-device-sound => }/rustfmt.toml (100%) delete mode 100644 staging/vhost-device-video/rustfmt.toml diff --git a/staging/vhost-device-sound/rustfmt.toml b/staging/rustfmt.toml similarity index 100% rename from staging/vhost-device-sound/rustfmt.toml rename to staging/rustfmt.toml diff --git a/staging/vhost-device-video/rustfmt.toml b/staging/vhost-device-video/rustfmt.toml deleted file mode 100644 index ebecb99fe..000000000 --- a/staging/vhost-device-video/rustfmt.toml +++ /dev/null @@ -1,7 +0,0 @@ -edition = "2021" -format_generated_files = false -format_code_in_doc_comments = true -format_strings = true -imports_granularity = "Crate" -group_imports = "StdExternalCrate" -wrap_comments = true From 261a881357d84a0d353c6aed4f6c4bbe40ed5083 Mon Sep 17 00:00:00 2001 From: Manos Pitsidianakis Date: Fri, 24 Nov 2023 09:48:58 +0200 Subject: [PATCH 3/6] .buildkite: add nightly rustfmt check in staging Add a new test json file with a cargo-fmt check with the nightly toolchain. This allows to use nightly rustfmt rules in rustfmt.toml. Signed-off-by: Manos Pitsidianakis --- .buildkite/cargo-rustfmt.json | 9 +++++++++ 1 file changed, 9 insertions(+) create mode 100644 .buildkite/cargo-rustfmt.json diff --git a/.buildkite/cargo-rustfmt.json b/.buildkite/cargo-rustfmt.json new file mode 100644 index 000000000..039c5ebe4 --- /dev/null +++ b/.buildkite/cargo-rustfmt.json @@ -0,0 +1,9 @@ +{ + "tests": [ + { + "test_name": "staging: style", + "command": "cd staging && cargo +nightly fmt --all -- --config-path rustfmt.toml --check" + } + ] +} + From 25ef9dacd0e883e6acd709b48f37d2e33e6a4eef Mon Sep 17 00:00:00 2001 From: Manos Pitsidianakis Date: Fri, 24 Nov 2023 09:50:34 +0200 Subject: [PATCH 4/6] staging/rustfmt.toml: update Add doc comments for each rule and a bunch of new rules. Signed-off-by: Manos Pitsidianakis --- staging/rustfmt.toml | 71 +++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 70 insertions(+), 1 deletion(-) diff --git a/staging/rustfmt.toml b/staging/rustfmt.toml index ebecb99fe..35d818818 100644 --- a/staging/rustfmt.toml +++ b/staging/rustfmt.toml @@ -1,7 +1,76 @@ edition = "2021" +# Format generated files. A file is considered generated if any of the first +# five lines contain a @generated comment marker. (default value is true) format_generated_files = false +# Format code snippet included in doc comments. (default value is false) format_code_in_doc_comments = true -format_strings = true +# Format the metavariable matching patterns in macros. (default value is false) +format_macro_matchers = true +# Control the case of the letters in hexadecimal literal values (default value is Preserve) +hex_literal_case = "Upper" +match_block_trailing_comma = false +# Controls how imports are structured in use statements. Imports will be merged +# or split to the configured level of granularity. (default value is Preserve) imports_granularity = "Crate" +# Unix or Windows line endings (default value is Auto) +newline_style = "Unix" +# Convert /* */ comments to // comments where possible. (default value is false) +normalize_comments = true +# Convert #![doc] and #[doc] attributes to //! and /// doc comments. (default value is false) +normalize_doc_attributes = true +# Reorder impl items. type and const are put first, then macros and methods. (default value is false) +reorder_impl_items = true +# Format string literals where necessary. (default value is false) +format_strings = true +# Discard existing import groups, and create three groups for: +# +# std, core and alloc, +# external crates, +# self, super and crate imports. group_imports = "StdExternalCrate" +# Break comments to fit on the line (default value is false) wrap_comments = true +# Minimum number of blank lines which must be put between items. If two items +# have fewer blank lines between them, additional blank lines are inserted. +# (default value is 0) +blank_lines_upper_bound = 1 +# Replace strings of _ wildcards by a single .. in tuple patterns (default value is false) +condense_wildcard_suffixes = true +# Use field initialize shorthand if possible. (default value is false) +use_field_init_shorthand = true +# Which version of the formatting rules to use. Version::One is +# backwards-compatible with Rustfmt 1.0. Other versions are only backwards +# compatible within a major version number. (default value is "One" +version = "Two" + +## Default values +# +## Max width for code snippets included in doc comments. Only used if +## format_code_in_doc_comments is true. (default value) +#doc_comment_code_block_width = 100 +## Format the bodies of macros. (default value) +#format_macro_bodies = true +## Use tab characters for indentation, spaces for alignment. (default value) +#hard_tabs = false +## Indent style of imports (default value) +#imports_indent = "Block" +## Item layout inside a imports block. (default value) +#imports_layout = "Mixed" +## Indent on expressions or items. (default value) +#indent_style = "Block" +## (default value) +#max_width = 100 +## Merge multiple derives into a single one. (default value) +#merge_derives = true +## Remove nested parens. (default value) +#remove_nested_parens = true +## Number of spaces per tab (default value) +#tab_spaces = 4 +## How to handle trailing commas for lists (default value) +#trailing_comma = "Vertical" +## Add trailing semicolon after break, continue and return. (default value) +#trailing_semicolon = true +## Reorder import and extern crate statements alphabetically in groups (a group is separated by a newline). (default value) +#reorder_imports = true +## Reorder mod declarations alphabetically in group. (default value) +#reorder_modules = true From 61d211d1069631ed185d5799f92ae0498af7b9a3 Mon Sep 17 00:00:00 2001 From: Manos Pitsidianakis Date: Fri, 24 Nov 2023 09:53:28 +0200 Subject: [PATCH 5/6] staging: fix rustfmt lints from previous commit Signed-off-by: Manos Pitsidianakis --- .../src/audio_backends/alsa.rs | 11 +++---- .../src/audio_backends/pipewire.rs | 2 +- staging/vhost-device-sound/src/device.rs | 4 +-- staging/vhost-device-sound/src/stream.rs | 8 ++--- .../vhost-device-sound/src/virtio_sound.rs | 12 +++---- staging/vhost-device-video/src/stream.rs | 8 +++-- staging/vhost-device-video/src/vhu_video.rs | 2 +- .../src/vhu_video_thread.rs | 26 +++++++++------ staging/vhost-device-video/src/video.rs | 4 +-- .../src/video_backends/v4l2_decoder.rs | 32 +++++++++---------- 10 files changed, 57 insertions(+), 52 deletions(-) diff --git a/staging/vhost-device-sound/src/audio_backends/alsa.rs b/staging/vhost-device-sound/src/audio_backends/alsa.rs index d219fda1f..ae94db528 100644 --- a/staging/vhost-device-sound/src/audio_backends/alsa.rs +++ b/staging/vhost-device-sound/src/audio_backends/alsa.rs @@ -1,5 +1,4 @@ /// Alsa backend -// // Manos Pitsidianakis // SPDX-License-Identifier: Apache-2.0 or BSD-3-Clause use std::{ @@ -734,6 +733,11 @@ impl AudioBackend for AlsaBackend { start Start, } + send_action! { + ctrl set_parameters SetParameters, + ctrl release Release, + } + fn stop(&self, id: u32) -> CrateResult<()> { if let Some(Err(err)) = self .streams @@ -746,9 +750,4 @@ impl AudioBackend for AlsaBackend { } Ok(()) } - - send_action! { - ctrl set_parameters SetParameters, - ctrl release Release, - } } diff --git a/staging/vhost-device-sound/src/audio_backends/pipewire.rs b/staging/vhost-device-sound/src/audio_backends/pipewire.rs index 1f472cded..12ca6f106 100644 --- a/staging/vhost-device-sound/src/audio_backends/pipewire.rs +++ b/staging/vhost-device-sound/src/audio_backends/pipewire.rs @@ -336,7 +336,7 @@ impl AudioBackend for PwBackend { } let mut param = [Pod::from_bytes(&value_clone).unwrap()]; - //callback to negotiate new set of streams + // callback to negotiate new set of streams stream .update_params(&mut param) .expect("could not update params"); diff --git a/staging/vhost-device-sound/src/device.rs b/staging/vhost-device-sound/src/device.rs index 5dff291ec..b4a7eae05 100644 --- a/staging/vhost-device-sound/src/device.rs +++ b/staging/vhost-device-sound/src/device.rs @@ -698,8 +698,8 @@ impl VhostUserSoundBackend { } impl VhostUserBackend for VhostUserSoundBackend { - type Vring = VringRwLock; type Bitmap = (); + type Vring = VringRwLock; fn num_queues(&self) -> usize { NUM_QUEUES as usize @@ -1104,7 +1104,7 @@ mod tests { let queues_per_thread = backend.queues_per_thread(); assert_eq!(queues_per_thread.len(), 1); - assert_eq!(queues_per_thread[0], 0xf); + assert_eq!(queues_per_thread[0], 0xF); let config = backend.get_config(0, 8); assert_eq!(config.len(), 8); diff --git a/staging/vhost-device-sound/src/stream.rs b/staging/vhost-device-sound/src/stream.rs index 8dfb4ccfd..38006fc1c 100644 --- a/staging/vhost-device-sound/src/stream.rs +++ b/staging/vhost-device-sound/src/stream.rs @@ -121,10 +121,6 @@ macro_rules! set_new_state { } impl PCMState { - pub fn new() -> Self { - Self::default() - } - set_new_state!( set_parameters, Self::SetParameters, @@ -142,6 +138,10 @@ impl PCMState { set_new_state!(stop, Self::Stop, Self::Start); set_new_state!(release, Self::Release, Self::Prepare | Self::Stop); + + pub fn new() -> Self { + Self::default() + } } impl std::fmt::Display for PCMState { diff --git a/staging/vhost-device-sound/src/virtio_sound.rs b/staging/vhost-device-sound/src/virtio_sound.rs index bb9744cf9..8a062331f 100644 --- a/staging/vhost-device-sound/src/virtio_sound.rs +++ b/staging/vhost-device-sound/src/virtio_sound.rs @@ -250,12 +250,12 @@ pub struct VirtioSoundJackInfo { // reading its content from byte array. unsafe impl ByteValued for VirtioSoundJackInfo {} -///If the VIRTIO_SND_JACK_F_REMAP feature bit is set in the jack information +/// If the VIRTIO_SND_JACK_F_REMAP feature bit is set in the jack information /// Remap control request #[derive(Copy, Clone, Debug, Default, PartialEq, Eq)] #[repr(C)] pub struct VirtioSoundJackRemap { - pub hdr: VirtioSoundJackHeader, /* .code = VIRTIO_SND_R_JACK_REMAP */ + pub hdr: VirtioSoundJackHeader, // .code = VIRTIO_SND_R_JACK_REMAP pub association: Le32, pub sequence: Le32, } @@ -279,9 +279,9 @@ unsafe impl ByteValued for VirtioSoundPcmHeader {} #[repr(C)] pub struct VirtioSoundPcmInfo { pub hdr: VirtioSoundInfo, - pub features: Le32, /* 1 << VIRTIO_SND_PCM_F_XXX */ - pub formats: Le64, /* 1 << VIRTIO_SND_PCM_FMT_XXX */ - pub rates: Le64, /* 1 << VIRTIO_SND_PCM_RATE_XXX */ + pub features: Le32, // 1 << VIRTIO_SND_PCM_F_XXX + pub formats: Le64, // 1 << VIRTIO_SND_PCM_FMT_XXX + pub rates: Le64, // 1 << VIRTIO_SND_PCM_RATE_XXX pub direction: u8, pub channels_min: u8, pub channels_max: u8, @@ -299,7 +299,7 @@ pub struct VirtioSndPcmSetParams { pub hdr: VirtioSoundPcmHeader, pub buffer_bytes: Le32, pub period_bytes: Le32, - pub features: Le32, /* 1 << VIRTIO_SND_PCM_F_XXX */ + pub features: Le32, // 1 << VIRTIO_SND_PCM_F_XXX pub channels: u8, pub format: u8, pub rate: u8, diff --git a/staging/vhost-device-video/src/stream.rs b/staging/vhost-device-video/src/stream.rs index 88398b303..405bdf0e3 100644 --- a/staging/vhost-device-video/src/stream.rs +++ b/staging/vhost-device-video/src/stream.rs @@ -442,9 +442,11 @@ mod tests { assert!(stream.is_queue_streaming(QueueType::InputQueue)); assert!(stream.all_resources_state(QueueType::InputQueue, ResourceState::Queued)); // Resource can be found by index - assert!(stream - .find_resource_mut_by_index(0, QueueType::InputQueue) - .is_some()); + assert!( + stream + .find_resource_mut_by_index(0, QueueType::InputQueue) + .is_some() + ); { let res = stream .find_resource_mut(resource_id, QueueType::InputQueue) diff --git a/staging/vhost-device-video/src/vhu_video.rs b/staging/vhost-device-video/src/vhu_video.rs index 98903a20d..92d986a30 100644 --- a/staging/vhost-device-video/src/vhu_video.rs +++ b/staging/vhost-device-video/src/vhu_video.rs @@ -152,8 +152,8 @@ impl VuVideoBackend { /// VhostUserBackend trait methods impl VhostUserBackendMut for VuVideoBackend { - type Vring = VringRwLock; type Bitmap = (); + type Vring = VringRwLock; fn num_queues(&self) -> usize { NUM_QUEUES diff --git a/staging/vhost-device-video/src/vhu_video_thread.rs b/staging/vhost-device-video/src/vhu_video_thread.rs index 96018d426..674d27a53 100644 --- a/staging/vhost-device-video/src/vhu_video_thread.rs +++ b/staging/vhost-device-video/src/vhu_video_thread.rs @@ -632,12 +632,16 @@ mod tests { #[rstest] fn test_video_poller(dummy_fd: EventFd) { let poller = VideoPoller::new().unwrap(); - assert!(poller - .add(dummy_fd.as_raw_fd(), PollerEvent::new(EventType::Write, 1)) - .is_ok()); - assert!(poller - .modify(dummy_fd.as_raw_fd(), PollerEvent::new(EventType::Read, 1)) - .is_ok()); + assert!( + poller + .add(dummy_fd.as_raw_fd(), PollerEvent::new(EventType::Write, 1)) + .is_ok() + ); + assert!( + poller + .modify(dummy_fd.as_raw_fd(), PollerEvent::new(EventType::Read, 1)) + .is_ok() + ); // Poller captures a read event. dummy_fd.write(1).unwrap(); @@ -786,10 +790,12 @@ mod tests { ); thread.mem = Some(mem.clone()); - assert!(thread - .poller - .add(dummy_fd.as_raw_fd(), PollerEvent::new(EventType::Read, 1)) - .is_ok()); + assert!( + thread + .poller + .add(dummy_fd.as_raw_fd(), PollerEvent::new(EventType::Read, 1)) + .is_ok() + ); let vring = VringRwLock::new(mem, 0x1000).unwrap(); vring.set_queue_info(0x100, 0x200, 0x300).unwrap(); diff --git a/staging/vhost-device-video/src/video.rs b/staging/vhost-device-video/src/video.rs index f3e4543b6..c48501e5b 100644 --- a/staging/vhost-device-video/src/video.rs +++ b/staging/vhost-device-video/src/video.rs @@ -1,5 +1,5 @@ // SPDX-License-Identifier: Apache-2.0 or BSD-3-Clause -#![allow(dead_code)] //TODO: remove +#![allow(dead_code)] // TODO: remove // Struct definitions use the kernel-style naming for consistency #![allow(non_camel_case_types)] @@ -311,7 +311,7 @@ impl VideoCmd { ) -> vhu_video::Result { use self::VideoCmd::*; macro_rules! read_body { - ($a: expr) => { + ($a:expr) => { desc_chain.read_body(0, $a) }; } diff --git a/staging/vhost-device-video/src/video_backends/v4l2_decoder.rs b/staging/vhost-device-video/src/video_backends/v4l2_decoder.rs index db84f751c..df860c096 100644 --- a/staging/vhost-device-video/src/video_backends/v4l2_decoder.rs +++ b/staging/vhost-device-video/src/video_backends/v4l2_decoder.rs @@ -398,18 +398,16 @@ impl VideoBackend for V4L2Decoder { for resource in stream.queued_resources_mut(queue_type) { resource.ready_with(video::BufferFlags::ERR, 0); } - /* - * QUEUE_CLEAR behaviour from virtio-video spec - * Return already queued buffers back from the input or the output queue - * of the device. The device SHOULD return all of the buffers from the - * respective queue as soon as possible without pushing the buffers through - * the processing pipeline. - * - * From v4l2 PoV we issue a VIDIOC_STREAMOFF on the queue which will abort - * or finish any DMA in progress, unlocks any user pointer buffers locked - * in physical memory, and it removes all buffers from the incoming and - * outgoing queues. - */ + // QUEUE_CLEAR behaviour from virtio-video spec + // Return already queued buffers back from the input or the output queue + // of the device. The device SHOULD return all of the buffers from the + // respective queue as soon as possible without pushing the buffers through + // the processing pipeline. + // + // From v4l2 PoV we issue a VIDIOC_STREAMOFF on the queue which will abort + // or finish any DMA in progress, unlocks any user pointer buffers locked + // in physical memory, and it removes all buffers from the incoming and + // outgoing queues. if let Err(e) = v4l2r::ioctl::streamoff(stream, queue) { warn!("streamoff failed: {}", e); return Sync(video::CmdResponse::Error(InvalidParameter)); @@ -492,9 +490,9 @@ impl VideoBackend for V4L2Decoder { return Sync(video::CmdResponse::Error(InvalidParameter)); }; - /*if queue_type.direction() == v4l2r::QueueDirection::Capture { - todo!("compose on CAPTURE"); - }*/ + // if queue_type.direction() == v4l2r::QueueDirection::Capture { + // todo!("compose on CAPTURE"); + // } Sync(video::CmdResponse::OkNoData) } @@ -891,7 +889,7 @@ mod tests { Some(v4l2_fmtdesc { index, type_: v4l2r::bindings::v4l2_buf_type_V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE, - pixelformat: 0x3231564e, // NV12 + pixelformat: 0x3231564E, // NV12 // SAFETY: test environment only. ..unsafe { mem::zeroed() } }) @@ -1106,7 +1104,7 @@ mod tests { fn test_v4l2_backend_helpers(test_dir: TempDir) { let v4l2_device = VideoDeviceMock::new(&test_dir); let decoder = V4L2Decoder::new(Path::new(&v4l2_device.path)).unwrap(); - let nv12 = u32::from_le(0x3231564e); + let nv12 = u32::from_le(0x3231564E); assert_matches!( V4L2Decoder::v4l2_get_selection(&decoder.video_device, video::QueueType::InputQueue), None From 58ab80477ced825aeb75de5c282b84542d9b716f Mon Sep 17 00:00:00 2001 From: Manos Pitsidianakis Date: Fri, 24 Nov 2023 10:02:17 +0200 Subject: [PATCH 6/6] Add .git-blame-ignore-revs file Add previous formatting commit SHA to .git-blame-ignore-revs The filename does not seem to be standardised, but must be configured explicitly. Note: to use this file always, you must set: git config --local blame.ignoreRevsFile .git-blame-ignore-revs (or --global) or use git blame with `--ignore-revs-file .git-blame-ignore-revs` Signed-off-by: Manos Pitsidianakis --- .git-blame-ignore-revs | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 .git-blame-ignore-revs diff --git a/.git-blame-ignore-revs b/.git-blame-ignore-revs new file mode 100644 index 000000000..3cde3281a --- /dev/null +++ b/.git-blame-ignore-revs @@ -0,0 +1,2 @@ +# staging: fix rustfmt lints from previous commit +61d211d1069631ed185d5799f92ae0498af7b9a3