Re: [PATCH v8 02/12] Add virtio-sound-pci device

[Manos Pitsidianakis <manos.pitsidianakis@linaro.org>]

On Mon, 04 Sep 2023 09:32, Volker Rümelin <vr_qemu@t-online.de> wrote:
>> +static Property virtio_snd_pci_properties[] = {
>> +    DEFINE_AUDIO_PROPERTIES(VirtIOSoundPCI, vdev.card),
>
>I think DEFINE_AUDIO_PROPERTIES should be moved back to virtio-snd.c. 
>The audiodev property is a virtio-sound property and not a 
>virtio-sound-pci property.

Hm, is it? Can you instantiate a virtio-sound device without the PCI 
wrapper? Under hw/audio, DEFINE_AUDIO_PROPERTIES is set in PCI devices 
as well (e.g. ac97)

>
>> +    DEFINE_PROP_BIT("ioeventfd", VirtIOPCIProxy, flags,
>> +                    VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT, true),
>> +    DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors,
>> +                       DEV_NVECTORS_UNSPECIFIED),
>> +    DEFINE_PROP_END_OF_LIST(),
>> +};
>> +
>> +static void virtio_snd_pci_realize(VirtIOPCIProxy *vpci_dev, Error **errp)
>> +{
>> +    VirtIOSoundPCI *dev = VIRTIO_SND_PCI(vpci_dev);
>> +    DeviceState *vdev = DEVICE(&dev->vdev);
>> +
>> +    if (vpci_dev->nvectors == DEV_NVECTORS_UNSPECIFIED) {
>> +        vpci_dev->nvectors = 2;
>> +    }
>
>Why do you need that intermediate step with DEV_NVECTORS_UNSPECIFIED? 
>Unlike e.g. virtio-scsi-pci and virtio-net-pci devices, the default 
>value of nvectors is already known at compile time and can be specified 
>in the property definition.

I did not think this through properly, you are correct. Thank you!

Manos