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);
@@ -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;
+ }
+
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
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
>
Am 05.01.24 um 11:54 schrieb 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...
Hi Marc-André,
right, I'll have to remove the subordinate clause from the commit
message where I claim some pointers can be NULL. All streams get
allocated in virtio_snd_realize() with calls of virtio_snd_pcm_prepare()
and get freed in virtio_snd_unrealize().
>> @@ -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)
Before my patch params->channels and stream->info.max_channels were also
2 at the time the guest driver queried stream info. They got initialized
in function virtio_snd_realize().
default_params.channels = 2;
...
status = virtio_snd_set_pcm_params(vsnd, i, &default_params);
...
status = virtio_snd_pcm_prepare(vsnd, i);
The guest may not update stream->info variables. They are configuration
information set when QEMU starts. This was wrong in
virtio_snd_pcm_prepare().
>
>> + }
>> +
>> 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
>>
© 2016 - 2024 Red Hat, Inc.