Am Fr., 10. Juli 2026 um 12:17 Uhr schrieb Manos Pitsidianakis
<[email protected]>:
>
> On Fri, Jul 10, 2026 at 12:43 PM Alexander Mikhalitsyn
> <[email protected]> wrote:
> >
> > Am Mo., 29. Juni 2026 um 10:28 Uhr schrieb Manos Pitsidianakis
> > <[email protected]>:
> > >
> > > On Fri, 26 Jun 2026 15:35, Alexander Mikhalitsyn
> > > <[email protected]> wrote:
> > > >From: Volker Rümelin <[email protected]>
> > > >
> > > >The virtio-sound device is currently not migratable. Add the
> > > >missing VMSTATE fields, enable migration and reconnect the audio
> > > >streams after migration.
> > > >
> > > >The queue_inuse[] array variables mimic the inuse variable in
> > > >struct VirtQueue which is private. They are needed to restart
> > > >the virtio queues after migration.
> > > >
> > > >Signed-off-by: Volker Rümelin <[email protected]>
> > > >[AM: trivial rebase changes]
> > > >Signed-off-by: Alexander Mikhalitsyn
> > > ><[email protected]>
> > > >---
> > > >v3:
> > > > - added latency_bytes field to VMStateDescription
> > > >
> > > > As suggested by Marc-André Lureau:
> > > > - removed the "rc" variable from virtio_snd_post_load()
> > > > - dropped info field from VMStateDescription, because
> > > > it can't be modified by guest and initialized only from realize
> > > > - added minimum_version_id/version_id so we can extend
> > > > VMStateDescription
> > > > without breaking compatibility in the future
> > > >---
> > > > hw/audio/virtio-snd.c | 80 +++++++++++++++++++++++++++++++----
> > > > include/hw/audio/virtio-snd.h | 1 +
> > > > 2 files changed, 73 insertions(+), 8 deletions(-)
> > > >
> > > >diff --git a/hw/audio/virtio-snd.c b/hw/audio/virtio-snd.c
> > > >index 81ba1e1a277..7af0b63c03b 100644
> > > >--- a/hw/audio/virtio-snd.c
> > > >+++ b/hw/audio/virtio-snd.c
> > > >@@ -24,7 +24,6 @@
> > > > #include "qapi/error.h"
> > > > #include "hw/audio/virtio-snd.h"
> > > >
> > > >-#define VIRTIO_SOUND_VM_VERSION 1
> > > > #define VIRTIO_SOUND_JACK_DEFAULT 0
> > > > #define VIRTIO_SOUND_STREAM_DEFAULT 2
> > > > #define VIRTIO_SOUND_CHMAP_DEFAULT 0
> > > >@@ -78,17 +77,40 @@ static uint32_t supported_rates =
> > > >BIT(VIRTIO_SND_PCM_RATE_5512)
> > > > | BIT(VIRTIO_SND_PCM_RATE_192000)
> > > > | BIT(VIRTIO_SND_PCM_RATE_384000);
> > > >
> > > >+static const VMStateDescription vmstate_virtio_snd_stream = {
> > > >+ .name = "virtio-sound-stream",
> > > >+ .version_id = 1,
> > > >+ .minimum_version_id = 1,
> > > >+ .fields = (const VMStateField[]) {
> > > >+ VMSTATE_UINT32(state, VirtIOSoundPCMStream),
> > > >+ VMSTATE_UINT32(params.buffer_bytes, VirtIOSoundPCMStream),
> > > >+ VMSTATE_UINT32(params.period_bytes, VirtIOSoundPCMStream),
> > > >+ VMSTATE_UINT32(params.features, VirtIOSoundPCMStream),
> > > >+ VMSTATE_UINT8(params.channels, VirtIOSoundPCMStream),
> > > >+ VMSTATE_UINT8(params.format, VirtIOSoundPCMStream),
> > > >+ VMSTATE_UINT8(params.rate, VirtIOSoundPCMStream),
> > > >+ VMSTATE_UINT32(latency_bytes, VirtIOSoundPCMStream),
> > > >+ VMSTATE_END_OF_LIST()
> > > >+ },
> > > >+};
> > > >+
> > > > static const VMStateDescription vmstate_virtio_snd_device = {
> > > >- .name = TYPE_VIRTIO_SND,
> > > >- .version_id = VIRTIO_SOUND_VM_VERSION,
> > > >- .minimum_version_id = VIRTIO_SOUND_VM_VERSION,
> > > >+ .name = "virtio-sound-device",
> > > >+ .version_id = 1,
> > > >+ .minimum_version_id = 1,
> > > >+ .fields = (const VMStateField[]) {
> > > >+ VMSTATE_UINT32_ARRAY(queue_inuse, VirtIOSound,
> > > >VIRTIO_SND_VQ_MAX),
> > > >+ VMSTATE_STRUCT_VARRAY_POINTER_UINT32(streams, VirtIOSound,
> > > >+ snd_conf.streams,
> > > >+ vmstate_virtio_snd_stream, VirtIOSoundPCMStream),
> > > >+ VMSTATE_END_OF_LIST()
> > > >+ },
> > > > };
> > > >
> > > > static const VMStateDescription vmstate_virtio_snd = {
> > > >- .name = TYPE_VIRTIO_SND,
> > > >- .unmigratable = 1,
> > > >- .minimum_version_id = VIRTIO_SOUND_VM_VERSION,
> > > >- .version_id = VIRTIO_SOUND_VM_VERSION,
> > > >+ .name = "virtio-sound",
> >
> > Dear Manos,
> >
> > >
> > > Why change the name? It differentiates between the device impl and
> > > virtio-sound-pci wrapper device.
> >
> > I'm not sure that I understand this point. Before this patch both
> > vmstate_virtio_snd and vmstate_virtio_snd_device
> > had the same name TYPE_VIRTIO_SND, we are trying to give the
> > distinctive names here. Please, elaborate on this.
>
> Ah, I misunderstood the change then. The name must be unique for the
> vmstate to be migrateable, right?
I don't think that it is a strict requirement, because, basically
vmstate_virtio_snd_device is implicitly nested in vmstate_virtio_snd.
In vmstate_virtio_snd we only have VMSTATE_VIRTIO_DEVICE as a VMStateField,
which, in turn leads migration engine to virtio_save() and then to
vmstate_save_state(f,
vdc->vmsd (= vmstate_virtio_snd_device), ...).
But, when and if somebody will analyze a migration stream (dump as
JSON, or just use tracepoints in migration subsystem), then
it is just more convenient to have distinctive names, IMHO.
>
> > Kind regards,
> > Alex
> >
> > >
> > > >+ .version_id = 1,
> > > >+ .minimum_version_id = 1,
> > > > .fields = (const VMStateField[]) {
> > > > VMSTATE_VIRTIO_DEVICE,
> > > > VMSTATE_END_OF_LIST()
> > > >@@ -799,6 +821,7 @@ process_cmd(VirtIOSound *s, virtio_snd_ctrl_command
> > > >*cmd)
> > > > sizeof(virtio_snd_hdr));
> > > > virtqueue_push(cmd->vq, cmd->elem,
> > > > sizeof(virtio_snd_hdr) + cmd->payload_size);
> > > >+ s->queue_inuse[VIRTIO_SND_VQ_CONTROL] -= 1;
> > >
> > > Let's do a g_assert that this is > 0 before decrementing it.
> >
> > Great idea, I will address this in v4.
> >
> > >
> > > > virtio_notify(VIRTIO_DEVICE(s), cmd->vq);
> > > > }
> > > >
> > > >@@ -845,6 +868,7 @@ static void virtio_snd_handle_ctrl(VirtIODevice
> > > >*vdev, VirtQueue *vq)
> > > >
> > > > elem = virtqueue_pop(vq, sizeof(VirtQueueElement));
> > > > while (elem) {
> > > >+ s->queue_inuse[VIRTIO_SND_VQ_CONTROL] += 1;
> > > > cmd = g_new0(virtio_snd_ctrl_command, 1);
> > > > cmd->elem = elem;
> > > > cmd->vq = vq;
> > > >@@ -955,6 +979,7 @@ static void virtio_snd_handle_tx_xfer(VirtIODevice
> > > >*vdev, VirtQueue *vq)
> > > > goto tx_err;
> > > > }
> > > >
> > > >+ vsnd->queue_inuse[VIRTIO_SND_VQ_TX] += 1;
> > > > size = iov_size(elem->out_sg, elem->out_num) - msg_sz;
> > > >
> > > > buffer = g_malloc0(sizeof(VirtIOSoundPCMBuffer) + size);
> > > >@@ -1034,6 +1059,7 @@ static void virtio_snd_handle_rx_xfer(VirtIODevice
> > > >*vdev, VirtQueue *vq)
> > > > goto rx_err;
> > > > }
> > > >
> > > >+ vsnd->queue_inuse[VIRTIO_SND_VQ_RX] += 1;
> > > > size = iov_size(elem->in_sg, elem->in_num) -
> > > > sizeof(virtio_snd_pcm_status);
> > > > buffer = g_malloc0(sizeof(VirtIOSoundPCMBuffer) + size);
> > > >@@ -1175,6 +1201,7 @@ static inline void
> > > >return_tx_buffer(VirtIOSoundPCMStream *stream,
> > > > virtqueue_push(buffer->vq,
> > > > buffer->elem,
> > > > sizeof(virtio_snd_pcm_status));
> > > >+ stream->s->queue_inuse[VIRTIO_SND_VQ_TX] -= 1;
> > >
> > > Ditto
> >
> > +
> >
> > >
> > > > virtio_notify(VIRTIO_DEVICE(stream->s), buffer->vq);
> > > > QSIMPLEQ_REMOVE(&stream->queue,
> > > > buffer,
> > > >@@ -1268,6 +1295,7 @@ static inline void
> > > >return_rx_buffer(VirtIOSoundPCMStream *stream,
> > > > virtqueue_push(buffer->vq,
> > > > buffer->elem,
> > > > sizeof(virtio_snd_pcm_status) + buffer->size);
> > > >+ stream->s->queue_inuse[VIRTIO_SND_VQ_RX] -= 1;
> > >
> > > Ditto
> >
> > +
> >
> > >
> > > > virtio_notify(VIRTIO_DEVICE(stream->s), buffer->vq);
> > > > QSIMPLEQ_REMOVE(&stream->queue,
> > > > buffer,
> > > >@@ -1386,6 +1414,37 @@ static void virtio_snd_unrealize(DeviceState *dev)
> > > > virtio_cleanup(vdev);
> > > > }
> > > >
> > > >+static int virtio_snd_post_load(VirtIODevice *vdev)
> > > >+{
> > > >+ VirtIOSound *s = VIRTIO_SND(vdev);
> > > >+ uint32_t i;
> > > >+
> > > >+ for (i = 0; i < s->snd_conf.streams; i++) {
> > > >+ struct VirtIOSoundPCMStream *stream;
> > > >+
> > > >+ stream = virtio_snd_pcm_get_stream(s, i);
> > > >+ if (stream->state & VSND_PCMSTREAM_STATE_F_PREPARED) {
> > > >+ virtio_snd_pcm_open(stream);
> > > >+
> > > >+ if (stream->state & VSND_PCMSTREAM_STATE_F_ACTIVE) {
> > > >+ virtio_snd_pcm_set_active(stream, true);
> > > >+ }
> > > >+ }
> > > >+ }
> > > >+
> > > >+ for (i = 0; i < VIRTIO_SND_VQ_MAX; i++) {
> > > >+ if (s->queue_inuse[i]) {
> > > >+ if (!virtqueue_rewind(s->queues[i], s->queue_inuse[i])) {
> > > >+ error_report(
> > > >+ "virtio-snd: could not rewind %u elements in queue
> > > >%u",
> > > >+ s->queue_inuse[i], i);
> > > >+ }
> > > >+ s->queue_inuse[i] = 0;
> > > >+ }
> > > >+ }
> > > >+
> > > >+ return 0;
> > > >+}
> > > >
> > > > static void virtio_snd_reset(VirtIODevice *vdev)
> > > > {
> > > >@@ -1418,6 +1477,10 @@ static void virtio_snd_reset(VirtIODevice *vdev)
> > > > virtio_snd_pcm_buffer_free(buffer);
> > > > }
> > > > }
> > > >+
> > > >+ for (i = 0; i < VIRTIO_SND_VQ_MAX; i++) {
> > > >+ vsnd->queue_inuse[i] = 0;
> > > >+ }
> > > > }
> > > >
> > > > static void virtio_snd_class_init(ObjectClass *klass, const void *data)
> > > >@@ -1431,6 +1494,7 @@ static void virtio_snd_class_init(ObjectClass
> > > >*klass, const void *data)
> > > >
> > > > dc->vmsd = &vmstate_virtio_snd;
> > > > vdc->vmsd = &vmstate_virtio_snd_device;
> > > >+ vdc->post_load = virtio_snd_post_load;
> > > > vdc->realize = virtio_snd_realize;
> > > > vdc->unrealize = virtio_snd_unrealize;
> > > > vdc->get_config = virtio_snd_get_config;
> > > >diff --git a/include/hw/audio/virtio-snd.h
> > > >b/include/hw/audio/virtio-snd.h
> > > >index 85d5d7c8619..384d2868c19 100644
> > > >--- a/include/hw/audio/virtio-snd.h
> > > >+++ b/include/hw/audio/virtio-snd.h
> > > >@@ -194,6 +194,7 @@ struct VirtIOSound {
> > > > VirtIODevice parent_obj;
> > > >
> > > > VirtQueue *queues[VIRTIO_SND_VQ_MAX];
> > > >+ uint32_t queue_inuse[VIRTIO_SND_VQ_MAX];
> > > > uint64_t features;
> > > > VirtIOSoundPCMStream *streams;
> > > > AudioBackend *audio_be;
> > > >--
> > > >2.47.3
> > > >
>
> --
> Manos Pitsidianakis
> Emulation and Virtualization Engineer at Linaro Ltd
