Re: [PATCH 02/10] hw/audio/virtio-sound: allocate all streams in advance

[Marc-André Lureau]

Hi

On Fri, Jan 5, 2024 at 12:34 AM Volker Rümelin <vr_qemu@t-online.de> wrote:
>
> 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, where some pointers can also be NULL.
>
> For this reason, the audio streams are already allocated during
> the realization phase and all stream variables that are constant
> at runtime are initialised immediately after allocation. This is
> a step towards being able 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>
> ---
>  hw/audio/virtio-snd.c         | 35 ++++++++++++++++++++++-------------
>  include/hw/audio/virtio-snd.h |  1 +
>  2 files changed, 23 insertions(+), 13 deletions(-)
>
> diff --git a/hw/audio/virtio-snd.c b/hw/audio/virtio-snd.c
> index 8344f61c64..36b1bb502c 100644
> --- a/hw/audio/virtio-snd.c
> +++ b/hw/audio/virtio-snd.c
> @@ -447,11 +447,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->pcm = s->pcm;
> -        stream->s = s;
>          QSIMPLEQ_INIT(&stream->queue);
>          QSIMPLEQ_INIT(&stream->invalid);

note: I can't find where s->pcm->streams[stream_id] is reset to NULL
on pcm_release...

>
> @@ -463,14 +461,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;
> @@ -1074,6 +1064,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->id = i;
> +        stream->s = vsnd;
> +        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;

Fixed max channels set to 2.. ? before this was set to
MIN(AUDIO_MAX_CHANNELS, params->channels)

> +    }
> +
>      vsnd->pcm = g_new0(VirtIOSoundPCM, 1);
>      vsnd->pcm->snd = vsnd;
>      vsnd->pcm->streams =
> @@ -1314,14 +1322,13 @@ static void virtio_snd_unrealize(DeviceState *dev)
>      qemu_del_vm_change_state_handler(vsnd->vmstate);
>      trace_virtio_snd_unrealize(vsnd);
>
> -    if (vsnd->pcm) {
> +    if (vsnd->streams) {
>          if (vsnd->pcm->streams) {
>              for (uint32_t i = 0; i < vsnd->snd_conf.streams; i++) {
>                  stream = vsnd->pcm->streams[i];
>                  if (stream) {
>                      virtio_snd_process_cmdq(stream->s);
>                      virtio_snd_pcm_close(stream);
> -                    g_free(stream);
>                  }
>              }
>              g_free(vsnd->pcm->streams);
> @@ -1329,6 +1336,8 @@ static void virtio_snd_unrealize(DeviceState *dev)
>          g_free(vsnd->pcm->pcm_params);
>          g_free(vsnd->pcm);
>          vsnd->pcm = NULL;
> +        g_free(vsnd->streams);
> +        vsnd->streams = NULL;
>      }
>      AUD_remove_card(&vsnd->card);
>      virtio_delete_queue(vsnd->queues[VIRTIO_SND_VQ_CONTROL]);
> diff --git a/include/hw/audio/virtio-snd.h b/include/hw/audio/virtio-snd.h
> index ea6315f59b..05b4490488 100644
> --- a/include/hw/audio/virtio-snd.h
> +++ b/include/hw/audio/virtio-snd.h
> @@ -216,6 +216,7 @@ struct VirtIOSound {
>      VirtQueue *queues[VIRTIO_SND_VQ_MAX];
>      uint64_t features;
>      VirtIOSoundPCM *pcm;
> +    VirtIOSoundPCMStream *streams;
>      QEMUSoundCard card;
>      VMChangeStateEntry *vmstate;
>      virtio_snd_config snd_conf;
> --
> 2.35.3
>