From 572519253ddc0ef6c27bb465d880420a29466e37 Mon Sep 17 00:00:00 2001 From: Grzegorz Jaszczyk Date: Tue, 20 Sep 2022 23:00:36 +0000 Subject: [PATCH] audio: fix VirtioSnd re-activation Hitherto the activation could succeed only once since take() function on Option "Takes the value out of the option, leaving a None in its place." Above resulted with crosvm crash in guest suspend/resume scenario when the activate was called more than once (for the second time snd_data and stream_source_generators was None). To overcome above issue use clone for snd_data. For stream_source_generator the clone can't be easily used due to missing Clone support for stream_source_generators embed data types (e.g. 369 | struct AudioBuffer<'a> { 370 | buffer: &'a mut [u8], | ^^^^^^^^^^^^^^^^^^^^ the trait `Clone` is not implemented for `&mut [u8]` and some other). Therefore for stream_source_generator move create_stream_source_generators from new() to activate. BUG=b:246728970 TEST=Perform multiple suspend/resume s2idle cycle for borealis VM and make sure that 1) crosvm doesn't crash anymore 2) audio from borealis VM is still working after VM suspend/resume cycles. Change-Id: I8b27042e4cc0e5efb1d92756ac3b71a5a744f705 Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3904649 Commit-Queue: Norman Bintang Reviewed-by: Norman Bintang Reviewed-by: Daniel Verkamp Reviewed-by: Chih-Yang Hsia --- devices/src/virtio/snd/common_backend/mod.rs | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/devices/src/virtio/snd/common_backend/mod.rs b/devices/src/virtio/snd/common_backend/mod.rs index b13b54c1e7..f9b0e0f572 100644 --- a/devices/src/virtio/snd/common_backend/mod.rs +++ b/devices/src/virtio/snd/common_backend/mod.rs @@ -133,6 +133,7 @@ pub enum WorkerStatus { } // Stores constant data +#[derive(Clone)] pub struct SndData { jack_info: Vec, pcm_info: Vec, @@ -167,13 +168,13 @@ pub struct PcmResponse { pub struct VirtioSnd { cfg: virtio_snd_config, - snd_data: Option, + snd_data: SndData, avail_features: u64, acked_features: u64, queue_sizes: Box<[u16]>, worker_threads: Vec>, kill_evt: Option, - stream_source_generators: Option>>, + params: Parameters, } impl VirtioSnd { @@ -181,17 +182,16 @@ impl VirtioSnd { let cfg = hardcoded_virtio_snd_config(¶ms); let snd_data = hardcoded_snd_data(¶ms); let avail_features = base_features; - let stream_source_generators = create_stream_source_generators(¶ms, &snd_data); Ok(VirtioSnd { cfg, - snd_data: Some(snd_data), + snd_data, avail_features, acked_features: 0, queue_sizes: vec![MAX_VRING_LEN; MAX_QUEUE_NUM].into_boxed_slice(), worker_threads: Vec::new(), kill_evt: None, - stream_source_generators: Some(stream_source_generators), + params, }) } } @@ -371,12 +371,8 @@ impl VirtioDevice for VirtioSnd { }; self.kill_evt = Some(self_kill_evt); - let snd_data = self.snd_data.take().expect("snd_data is none"); - let stream_source_generators = self - .stream_source_generators - .take() - .expect("stream_source_generators is none"); - + let snd_data = self.snd_data.clone(); + let stream_source_generators = create_stream_source_generators(&self.params, &snd_data); let worker_result = thread::Builder::new() .name("virtio_snd w".to_string()) .spawn(move || {