commit 572519253ddc0ef6c27bb465d880420a29466e37
author Grzegorz Jaszczyk <jaz@semihalf.com> Tue Sep 20 23:00:36 2022 +0000
committer crosvm LUCI <crosvm-scoped@luci-project-accounts.iam.gserviceaccount.com> Thu Sep 22 09:14:19 2022 +0000
tree df7655ed3c0d61b34b911a7b856f7c3d8af9c8d7
parent aea39132e8cc58697397ac8773062719496b9a5a

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 <normanbt@chromium.org>
Reviewed-by: Norman Bintang <normanbt@chromium.org>
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Reviewed-by: Chih-Yang Hsia <paulhsia@chromium.org>

devices/src/virtio/snd/common_backend/mod.rs

diff --git a/devices/src/virtio/snd/common_backend/mod.rs b/devices/src/virtio/snd/common_backend/mod.rs
index b13b54c..f9b0e0f 100644
--- a/devices/src/virtio/snd/common_backend/mod.rs
+++ b/devices/src/virtio/snd/common_backend/mod.rs
@@ -133,6 +133,7 @@
 }
 
 // Stores constant data
+#[derive(Clone)]
 pub struct SndData {
     jack_info: Vec<virtio_snd_jack_info>,
     pcm_info: Vec<virtio_snd_pcm_info>,
@@ -167,13 +168,13 @@
 
 pub struct VirtioSnd {
     cfg: virtio_snd_config,
-    snd_data: Option<SndData>,
+    snd_data: SndData,
     avail_features: u64,
     acked_features: u64,
     queue_sizes: Box<[u16]>,
     worker_threads: Vec<thread::JoinHandle<()>>,
     kill_evt: Option<Event>,
-    stream_source_generators: Option<Vec<Box<dyn StreamSourceGenerator>>>,
+    params: Parameters,
 }
 
 impl VirtioSnd {
@@ -181,17 +182,16 @@
         let cfg = hardcoded_virtio_snd_config(&params);
         let snd_data = hardcoded_snd_data(&params);
         let avail_features = base_features;
-        let stream_source_generators = create_stream_source_generators(&params, &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 @@
             };
         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 || {