Re: [PATCH v1 2/4] virtio-snd: factor card setup out of realize func

[Philippe Mathieu-Daudé]

On 22/4/24 14:52, Manos Pitsidianakis wrote:
> Extract audio card setup logic out of the device realize callback so
> that it can be re-used in follow up commits.
>
> Signed-off-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
> ---
>   hw/audio/virtio-snd.c | 72 ++++++++++++++++++++++++-------------------
>   1 file changed, 41 insertions(+), 31 deletions(-)


> +static void virtio_snd_realize(DeviceState *dev, Error **errp)
> +{
> +    ERRP_GUARD();
> +    VirtIOSound *vsnd = VIRTIO_SND(dev);
> +    VirtIODevice *vdev = VIRTIO_DEVICE(dev);
> +
> +    trace_virtio_snd_realize(vsnd);
> +
> +    vsnd->vmstate =
> +        qemu_add_vm_change_state_handler(virtio_snd_vm_state_change, vsnd);
> +
> +    virtio_init(vdev, VIRTIO_ID_SOUND, sizeof(virtio_snd_config));
> +    virtio_add_feature(&vsnd->features, VIRTIO_F_VERSION_1);
> +
>       vsnd->queues[VIRTIO_SND_VQ_CONTROL] =
>           virtio_add_queue(vdev, 64, virtio_snd_handle_ctrl);
>       vsnd->queues[VIRTIO_SND_VQ_EVENT] =
> @@ -1123,26 +1149,10 @@ static void virtio_snd_realize(DeviceState *dev, Error 
> **errp)
>       QTAILQ_INIT(&vsnd->cmdq);
>       QSIMPLEQ_INIT(&vsnd->invalid);
>   
> -    for (uint32_t i = 0; i < vsnd->snd_conf.streams; i++) {
> -        status = virtio_snd_set_pcm_params(vsnd, i, &default_params);
> -        if (status != cpu_to_le32(VIRTIO_SND_S_OK)) {
> -            error_setg(errp,
> -                       "Can't initialize stream params, device responded with 
> %s.",
> -                       print_code(status));
> -            goto error_cleanup;
> -        }
> -        status = virtio_snd_pcm_prepare(vsnd, i);
> -        if (status != cpu_to_le32(VIRTIO_SND_S_OK)) {
> -            error_setg(errp,
> -                       "Can't prepare streams, device responded with %s.",
> -                       print_code(status));
> -            goto error_cleanup;
> -        }
> +    if (virtio_snd_setup(vsnd, errp)) {
> +        return;
>       }
>   
> -    return;
> -
> -error_cleanup:
>       virtio_snd_unrealize(dev);

We usually handle failure as:

  if (!virtio_snd_setup(vsnd, errp)) {
    virtio_snd_unrealize(dev);
  }

>   }
>   

Otherwise LGTM.