On Fri, Jul 10, 2026 at 1:32 PM Alexander Mikhalitsyn
<[email protected]> wrote:
>
> 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.

Makes sense, thanks for the explanation.

> >
> > > 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

-- 
Manos Pitsidianakis
Emulation and Virtualization Engineer at Linaro Ltd

Reply via email to