Re: [PATCH v3 2/9] hw/audio/virtio-sound: allocate an array of streams

[Manos Pitsidianakis <manos.pitsidianakis@linaro.org>]

On Fri, 26 Jun 2026 15:35, Alexander Mikhalitsyn <alexander@mihalicyn.com> wrote:
>From: Volker Rümelin <vr_qemu@t-online.de>
>
>It is much easier to migrate an array of structs than individual
>structs that are accessed via a pointer to a pointer to an array
>of pointers to struct.
>
>For this reason, allocate an array of streams in
>virtio_snd_realize() and initialise all stream variables that
>are constant at runtime immediately after allocation.
>
>This makes it easier to remove the virtio_snd_set_pcm_params()
>and virtio_snd_pcm_prepare() calls in the realisation phase and
>to migrate the audio streams of the virtio sound device after
>the next few patches.
>
>Signed-off-by: Volker Rümelin <vr_qemu@t-online.de>
>[AM: there were too many conflicts, I did `git checkout --ours -- <.>`
>     and then reimplemented the patch idea
>/AM]
>Signed-off-by: Alexander Mikhalitsyn <aleksandr.mikhalitsyn@futurfusion.io>
>Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>---
>v3:
>	As suggested by Marc-André Lureau:
>	- drop VirtIOSoundPCMStream's id field
>---
> hw/audio/virtio-snd.c         | 34 +++++++++++++++++++++-------------
> include/hw/audio/virtio-snd.h |  2 +-
> 2 files changed, 22 insertions(+), 14 deletions(-)
>
>diff --git a/hw/audio/virtio-snd.c b/hw/audio/virtio-snd.c
>index 6eb31e2838e..859c2770195 100644
>--- a/hw/audio/virtio-snd.c
>+++ b/hw/audio/virtio-snd.c
>@@ -436,12 +436,9 @@ static uint32_t virtio_snd_pcm_prepare(VirtIOSound *s, uint32_t stream_id)
> 
>     stream = virtio_snd_pcm_get_stream(s, stream_id);
>     if (stream == NULL) {
>-        stream = g_new0(VirtIOSoundPCMStream, 1);
>+        stream = &s->streams[stream_id];
>         stream->active = false;
>-        stream->id = stream_id;
>-        stream->s = s;
>         stream->latency_bytes = 0;
>-        QSIMPLEQ_INIT(&stream->queue);
> 
>         /*
>          * stream_id >= s->snd_conf.streams was checked before so this is
>@@ -451,14 +448,6 @@ static uint32_t virtio_snd_pcm_prepare(VirtIOSound *s, uint32_t stream_id)
>     }
> 
>     virtio_snd_get_qemu_audsettings(&as, params);
>-    stream->info.direction = stream_id < s->snd_conf.streams / 2 +
>-        (s->snd_conf.streams & 1) ? VIRTIO_SND_D_OUTPUT : VIRTIO_SND_D_INPUT;
>-    stream->info.hdr.hda_fn_nid = VIRTIO_SOUND_HDA_FN_NID;
>-    stream->info.features = 0;
>-    stream->info.channels_min = 1;
>-    stream->info.channels_max = as.nchannels;
>-    stream->info.formats = supported_formats;
>-    stream->info.rates = supported_rates;
>     stream->params = *params;
> 
>     stream->positions[0] = VIRTIO_SND_CHMAP_FL;
>@@ -1046,6 +1035,24 @@ static void virtio_snd_realize(DeviceState *dev, Error **errp)
>     vsnd->vmstate =
>         qemu_add_vm_change_state_handler(virtio_snd_vm_state_change, vsnd);
> 
>+    vsnd->streams = g_new0(VirtIOSoundPCMStream, vsnd->snd_conf.streams);
>+
>+    for (uint32_t i = 0; i < vsnd->snd_conf.streams; i++) {
>+        VirtIOSoundPCMStream *stream = &vsnd->streams[i];
>+
>+        stream->s = vsnd;
>+        QSIMPLEQ_INIT(&stream->queue);
>+        stream->info.hdr.hda_fn_nid = VIRTIO_SOUND_HDA_FN_NID;
>+        stream->info.features = 0;
>+        stream->info.formats = supported_formats;
>+        stream->info.rates = supported_rates;
>+        stream->info.direction =
>+            i < vsnd->snd_conf.streams / 2 + (vsnd->snd_conf.streams & 1)
>+            ? VIRTIO_SND_D_OUTPUT : VIRTIO_SND_D_INPUT;
>+        stream->info.channels_min = 1;
>+        stream->info.channels_max = 2;


Why 2 instead of as.nchannels? Does this break support for more than 2 
channels?

>+    }
>+
>     vsnd->pcm.streams =
>         g_new0(VirtIOSoundPCMStream *, vsnd->snd_conf.streams);
>     vsnd->pcm.pcm_params =
>@@ -1314,12 +1321,13 @@ static void virtio_snd_unrealize(DeviceState *dev)
>             if (stream) {
>                 virtio_snd_process_cmdq(stream->s);
>                 virtio_snd_pcm_close(stream);
>-                g_free(stream);
>             }
>         }
>         g_free(vsnd->pcm.streams);
>     }
>     g_free(vsnd->pcm.pcm_params);
>+    g_free(vsnd->streams);
>+    vsnd->streams = NULL;
>     virtio_delete_queue(vsnd->queues[VIRTIO_SND_VQ_CONTROL]);
>     virtio_delete_queue(vsnd->queues[VIRTIO_SND_VQ_EVENT]);
>     virtio_delete_queue(vsnd->queues[VIRTIO_SND_VQ_TX]);
>diff --git a/include/hw/audio/virtio-snd.h b/include/hw/audio/virtio-snd.h
>index fce7d1feea2..41c63b3f23b 100644
>--- a/include/hw/audio/virtio-snd.h
>+++ b/include/hw/audio/virtio-snd.h
>@@ -136,7 +136,6 @@ struct VirtIOSoundPCM {
> struct VirtIOSoundPCMStream {
>     virtio_snd_pcm_info info;
>     virtio_snd_pcm_set_params params;
>-    uint32_t id;
>     /* channel position values (VIRTIO_SND_CHMAP_XXX) */
>     uint8_t positions[VIRTIO_SND_CHMAP_MAX_SIZE];
>     VirtIOSound *s;
>@@ -214,6 +213,7 @@ struct VirtIOSound {
>     VirtQueue *queues[VIRTIO_SND_VQ_MAX];
>     uint64_t features;
>     VirtIOSoundPCM pcm;
>+    VirtIOSoundPCMStream *streams;
>     AudioBackend *audio_be;
>     VMChangeStateEntry *vmstate;
>     virtio_snd_config snd_conf;
>-- 
>2.47.3
>