Re: [PATCH 07/18] Introduce machine's default-audiodev property

["Daniel P. Berrangé" <berrange@redhat.com>]

On Mon, Apr 25, 2022 at 10:21:50AM +0200, Martin Kletzander wrote:
> Many machine types have default audio devices with no way to set the underlying
> audiodev.  Instead of adding an option for each and every one of them this new
> property can be used as a default during machine initialisation when creating
> such devices.
> 
> Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
> ---
>  hw/core/machine.c   | 23 +++++++++++++++++++++++
>  include/hw/boards.h |  1 +
>  2 files changed, 24 insertions(+)
> 
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index cb9bbc844d24..d055a126d398 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -596,6 +596,22 @@ static void machine_set_memdev(Object *obj, const char *value, Error **errp)
>      ms->ram_memdev_id = g_strdup(value);
>  }
>  
> +static char *machine_get_default_audiodev(Object *obj, Error **errp)
> +{
> +    MachineState *ms = MACHINE(obj);
> +
> +    return g_strdup(ms->default_audiodev);
> +}
> +
> +static void machine_set_default_audiodev(Object *obj, const char *value,
> +                                         Error **errp)
> +{
> +    MachineState *ms = MACHINE(obj);
> +
> +    g_free(ms->default_audiodev);
> +    ms->default_audiodev = g_strdup(value);
> +}
> +
>  HotpluggableCPUList *machine_query_hotpluggable_cpus(MachineState *machine)
>  {
>      int i;
> @@ -867,6 +883,12 @@ static void machine_class_init(ObjectClass *oc, void *data)
>      object_class_property_set_description(oc, "confidential-guest-support",
>                                            "Set confidential guest scheme to support");
>  
> +    object_class_property_add_str(oc, "default-audiodev",
> +                                  machine_get_default_audiodev,
> +                                  machine_set_default_audiodev);
> +    object_class_property_set_description(oc, "default-audiodev",
> +                                          "Audiodev to use for default machine devices");
> +

Hmm, if we add this, people might reasonably question why this default
audiodev can't be used for everything, instead of making other 'audiodev'
parameter mandatory.

For the x86 machines we have a property tied specifically to the PC
speaker.

  -machine ...,pcspk-audiodev=<name>


If we assume that's the desired pattern, then every machine which has
a built-in audio device should gain some  "{$device}-audiodev" proprerty
where '$device' is some reasonable name for the built-in audio device
of the machine.  This would work better if a machine ended up with
two built-in audio devices and needed separate audiodevs for them.


>      /* For compatibility */
>      object_class_property_add_str(oc, "memory-encryption",
>          machine_get_memory_encryption, machine_set_memory_encryption);
> @@ -961,6 +983,7 @@ static void machine_finalize(Object *obj)
>      g_free(ms->device_memory);
>      g_free(ms->nvdimms_state);
>      g_free(ms->numa_state);
> +    g_free(ms->default_audiodev);
>  }
>  
>  bool machine_usb(MachineState *machine)
> diff --git a/include/hw/boards.h b/include/hw/boards.h
> index d64b5481e834..5be1de50af03 100644
> --- a/include/hw/boards.h
> +++ b/include/hw/boards.h
> @@ -346,6 +346,7 @@ struct MachineState {
>       */
>      MemoryRegion *ram;
>      DeviceMemoryState *device_memory;
> +    char *default_audiodev;
>  
>      ram_addr_t ram_size;
>      ram_addr_t maxram_size;
> -- 
> 2.35.1
> 

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|