* [RFC PATCH 1/6] pc: remove -soundhw pcspk
2022-04-27 11:32 [RFC PATCH 0/6] replace -soundhw with -audio Paolo Bonzini
@ 2022-04-27 11:32 ` Paolo Bonzini
2022-04-29 13:37 ` Paolo Bonzini
2022-04-27 11:32 ` [RFC PATCH 2/6] soundhw: remove ability to create multiple soundcards Paolo Bonzini
` (4 subsequent siblings)
5 siblings, 1 reply; 12+ messages in thread
From: Paolo Bonzini @ 2022-04-27 11:32 UTC (permalink / raw)
To: qemu-devel; +Cc: mkletzan, berrange, kraxel
The pcspk device is the only user of isa_register_soundhw, and the only
-soundhw option which does not create a new device (it hacks into the
PCSpkState by hand). Remove it, since it was deprecated.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
hw/audio/pcspk.c | 10 ----------
hw/audio/soundhw.c | 27 ++++-----------------------
include/hw/audio/soundhw.h | 3 ---
3 files changed, 4 insertions(+), 36 deletions(-)
diff --git a/hw/audio/pcspk.c b/hw/audio/pcspk.c
index dfc7ebca4e..daf92a4ce1 100644
--- a/hw/audio/pcspk.c
+++ b/hw/audio/pcspk.c
@@ -245,18 +245,8 @@ static const TypeInfo pcspk_info = {
.class_init = pcspk_class_initfn,
};
-static int pcspk_audio_init_soundhw(ISABus *bus)
-{
- PCSpkState *s = pcspk_state;
-
- warn_report("'-soundhw pcspk' is deprecated, "
- "please set a backend using '-machine pcspk-audiodev=<name>' instead");
- return pcspk_audio_init(s);
-}
-
static void pcspk_register(void)
{
type_register_static(&pcspk_info);
- isa_register_soundhw("pcspk", "PC speaker", pcspk_audio_init_soundhw);
}
type_init(pcspk_register)
diff --git a/hw/audio/soundhw.c b/hw/audio/soundhw.c
index 173b674ff5..f7d94d7dfa 100644
--- a/hw/audio/soundhw.c
+++ b/hw/audio/soundhw.c
@@ -36,26 +36,12 @@ struct soundhw {
const char *typename;
int enabled;
int isa;
- union {
- int (*init_isa) (ISABus *bus);
- int (*init_pci) (PCIBus *bus);
- } init;
+ int (*init_pci) (PCIBus *bus);
};
static struct soundhw soundhw[9];
static int soundhw_count;
-void isa_register_soundhw(const char *name, const char *descr,
- int (*init_isa)(ISABus *bus))
-{
- assert(soundhw_count < ARRAY_SIZE(soundhw) - 1);
- soundhw[soundhw_count].name = name;
- soundhw[soundhw_count].descr = descr;
- soundhw[soundhw_count].isa = 1;
- soundhw[soundhw_count].init.init_isa = init_isa;
- soundhw_count++;
-}
-
void pci_register_soundhw(const char *name, const char *descr,
int (*init_pci)(PCIBus *bus))
{
@@ -63,7 +49,7 @@ void pci_register_soundhw(const char *name, const char *descr,
soundhw[soundhw_count].name = name;
soundhw[soundhw_count].descr = descr;
soundhw[soundhw_count].isa = 0;
- soundhw[soundhw_count].init.init_pci = init_pci;
+ soundhw[soundhw_count].init_pci = init_pci;
soundhw_count++;
}
@@ -158,18 +144,13 @@ void soundhw_init(void)
} else {
pci_create_simple(pci_bus, -1, c->typename);
}
- } else if (c->isa) {
- if (!isa_bus) {
- error_report("ISA bus not available for %s", c->name);
- exit(1);
- }
- c->init.init_isa(isa_bus);
} else {
+ assert(!c->isa);
if (!pci_bus) {
error_report("PCI bus not available for %s", c->name);
exit(1);
}
- c->init.init_pci(pci_bus);
+ c->init_pci(pci_bus);
}
}
}
diff --git a/include/hw/audio/soundhw.h b/include/hw/audio/soundhw.h
index f09a297854..e68685fcda 100644
--- a/include/hw/audio/soundhw.h
+++ b/include/hw/audio/soundhw.h
@@ -1,9 +1,6 @@
#ifndef HW_SOUNDHW_H
#define HW_SOUNDHW_H
-void isa_register_soundhw(const char *name, const char *descr,
- int (*init_isa)(ISABus *bus));
-
void pci_register_soundhw(const char *name, const char *descr,
int (*init_pci)(PCIBus *bus));
void deprecated_register_soundhw(const char *name, const char *descr,
--
2.35.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [RFC PATCH 1/6] pc: remove -soundhw pcspk
2022-04-27 11:32 ` [RFC PATCH 1/6] pc: remove -soundhw pcspk Paolo Bonzini
@ 2022-04-29 13:37 ` Paolo Bonzini
0 siblings, 0 replies; 12+ messages in thread
From: Paolo Bonzini @ 2022-04-29 13:37 UTC (permalink / raw)
To: qemu-devel; +Cc: mkletzan, berrange, kraxel
On 4/27/22 13:32, Paolo Bonzini wrote:
> The pcspk device is the only user of isa_register_soundhw, and the only
> -soundhw option which does not create a new device (it hacks into the
> PCSpkState by hand). Remove it, since it was deprecated.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> hw/audio/pcspk.c | 10 ----------
> hw/audio/soundhw.c | 27 ++++-----------------------
> include/hw/audio/soundhw.h | 3 ---
> 3 files changed, 4 insertions(+), 36 deletions(-)
I'll queue this patch, in the meanwhile feedback on especially 6/6 is
welcome.
Paolo
> diff --git a/hw/audio/pcspk.c b/hw/audio/pcspk.c
> index dfc7ebca4e..daf92a4ce1 100644
> --- a/hw/audio/pcspk.c
> +++ b/hw/audio/pcspk.c
> @@ -245,18 +245,8 @@ static const TypeInfo pcspk_info = {
> .class_init = pcspk_class_initfn,
> };
>
> -static int pcspk_audio_init_soundhw(ISABus *bus)
> -{
> - PCSpkState *s = pcspk_state;
> -
> - warn_report("'-soundhw pcspk' is deprecated, "
> - "please set a backend using '-machine pcspk-audiodev=<name>' instead");
> - return pcspk_audio_init(s);
> -}
> -
> static void pcspk_register(void)
> {
> type_register_static(&pcspk_info);
> - isa_register_soundhw("pcspk", "PC speaker", pcspk_audio_init_soundhw);
> }
> type_init(pcspk_register)
> diff --git a/hw/audio/soundhw.c b/hw/audio/soundhw.c
> index 173b674ff5..f7d94d7dfa 100644
> --- a/hw/audio/soundhw.c
> +++ b/hw/audio/soundhw.c
> @@ -36,26 +36,12 @@ struct soundhw {
> const char *typename;
> int enabled;
> int isa;
> - union {
> - int (*init_isa) (ISABus *bus);
> - int (*init_pci) (PCIBus *bus);
> - } init;
> + int (*init_pci) (PCIBus *bus);
> };
>
> static struct soundhw soundhw[9];
> static int soundhw_count;
>
> -void isa_register_soundhw(const char *name, const char *descr,
> - int (*init_isa)(ISABus *bus))
> -{
> - assert(soundhw_count < ARRAY_SIZE(soundhw) - 1);
> - soundhw[soundhw_count].name = name;
> - soundhw[soundhw_count].descr = descr;
> - soundhw[soundhw_count].isa = 1;
> - soundhw[soundhw_count].init.init_isa = init_isa;
> - soundhw_count++;
> -}
> -
> void pci_register_soundhw(const char *name, const char *descr,
> int (*init_pci)(PCIBus *bus))
> {
> @@ -63,7 +49,7 @@ void pci_register_soundhw(const char *name, const char *descr,
> soundhw[soundhw_count].name = name;
> soundhw[soundhw_count].descr = descr;
> soundhw[soundhw_count].isa = 0;
> - soundhw[soundhw_count].init.init_pci = init_pci;
> + soundhw[soundhw_count].init_pci = init_pci;
> soundhw_count++;
> }
>
> @@ -158,18 +144,13 @@ void soundhw_init(void)
> } else {
> pci_create_simple(pci_bus, -1, c->typename);
> }
> - } else if (c->isa) {
> - if (!isa_bus) {
> - error_report("ISA bus not available for %s", c->name);
> - exit(1);
> - }
> - c->init.init_isa(isa_bus);
> } else {
> + assert(!c->isa);
> if (!pci_bus) {
> error_report("PCI bus not available for %s", c->name);
> exit(1);
> }
> - c->init.init_pci(pci_bus);
> + c->init_pci(pci_bus);
> }
> }
> }
> diff --git a/include/hw/audio/soundhw.h b/include/hw/audio/soundhw.h
> index f09a297854..e68685fcda 100644
> --- a/include/hw/audio/soundhw.h
> +++ b/include/hw/audio/soundhw.h
> @@ -1,9 +1,6 @@
> #ifndef HW_SOUNDHW_H
> #define HW_SOUNDHW_H
>
> -void isa_register_soundhw(const char *name, const char *descr,
> - int (*init_isa)(ISABus *bus));
> -
> void pci_register_soundhw(const char *name, const char *descr,
> int (*init_pci)(PCIBus *bus));
> void deprecated_register_soundhw(const char *name, const char *descr,
^ permalink raw reply [flat|nested] 12+ messages in thread
* [RFC PATCH 2/6] soundhw: remove ability to create multiple soundcards
2022-04-27 11:32 [RFC PATCH 0/6] replace -soundhw with -audio Paolo Bonzini
2022-04-27 11:32 ` [RFC PATCH 1/6] pc: remove -soundhw pcspk Paolo Bonzini
@ 2022-04-27 11:32 ` Paolo Bonzini
2022-04-27 11:32 ` [RFC PATCH 3/6] soundhw: extract soundhw help to a separate function Paolo Bonzini
` (3 subsequent siblings)
5 siblings, 0 replies; 12+ messages in thread
From: Paolo Bonzini @ 2022-04-27 11:32 UTC (permalink / raw)
To: qemu-devel; +Cc: mkletzan, berrange, kraxel
The usefulness of enabling a dozen soundcards is dubious. Simplify the
code by allowing a single instance of -soundhw, with no support for
parsing either comma-separated values or 'soundhw all'.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
hw/audio/soundhw.c | 88 +++++++++++++++++-----------------------------
1 file changed, 32 insertions(+), 56 deletions(-)
diff --git a/hw/audio/soundhw.c b/hw/audio/soundhw.c
index f7d94d7dfa..097501fee1 100644
--- a/hw/audio/soundhw.c
+++ b/hw/audio/soundhw.c
@@ -25,6 +25,7 @@
#include "qemu/option.h"
#include "qemu/help_option.h"
#include "qemu/error-report.h"
+#include "qapi/error.h"
#include "qom/object.h"
#include "hw/isa/isa.h"
#include "hw/pci/pci.h"
@@ -34,7 +35,6 @@ struct soundhw {
const char *name;
const char *descr;
const char *typename;
- int enabled;
int isa;
int (*init_pci) (PCIBus *bus);
};
@@ -64,10 +64,16 @@ void deprecated_register_soundhw(const char *name, const char *descr,
soundhw_count++;
}
+static struct soundhw *selected = NULL;
+
void select_soundhw(const char *optarg)
{
struct soundhw *c;
+ if (selected) {
+ error_setg(&error_fatal, "only one -soundhw option is allowed");
+ }
+
if (is_help_option(optarg)) {
show_valid_cards:
@@ -84,44 +90,15 @@ void select_soundhw(const char *optarg)
exit(!is_help_option(optarg));
}
else {
- size_t l;
- const char *p;
- char *e;
- int bad_card = 0;
-
- if (!strcmp(optarg, "all")) {
- for (c = soundhw; c->name; ++c) {
- c->enabled = 1;
+ for (c = soundhw; c->name; ++c) {
+ if (g_str_equal(c->name, optarg)) {
+ selected = c;
+ break;
}
- return;
}
- p = optarg;
- while (*p) {
- e = strchr(p, ',');
- l = !e ? strlen(p) : (size_t) (e - p);
-
- for (c = soundhw; c->name; ++c) {
- if (!strncmp(c->name, p, l) && !c->name[l]) {
- c->enabled = 1;
- break;
- }
- }
-
- if (!c->name) {
- if (l > 80) {
- error_report("Unknown sound card name (too big to show)");
- }
- else {
- error_report("Unknown sound card name `%.*s'",
- (int) l, p);
- }
- bad_card = 1;
- }
- p += l + (e != NULL);
- }
-
- if (bad_card) {
+ if (!c->name) {
+ error_report("Unknown sound card name `%s'", optarg);
goto show_valid_cards;
}
}
@@ -129,30 +106,29 @@ void select_soundhw(const char *optarg)
void soundhw_init(void)
{
- struct soundhw *c;
+ struct soundhw *c = selected;
ISABus *isa_bus = (ISABus *) object_resolve_path_type("", TYPE_ISA_BUS, NULL);
PCIBus *pci_bus = (PCIBus *) object_resolve_path_type("", TYPE_PCI_BUS, NULL);
- for (c = soundhw; c->name; ++c) {
- if (c->enabled) {
- if (c->typename) {
- warn_report("'-soundhw %s' is deprecated, "
- "please use '-device %s' instead",
- c->name, c->typename);
- if (c->isa) {
- isa_create_simple(isa_bus, c->typename);
- } else {
- pci_create_simple(pci_bus, -1, c->typename);
- }
- } else {
- assert(!c->isa);
- if (!pci_bus) {
- error_report("PCI bus not available for %s", c->name);
- exit(1);
- }
- c->init_pci(pci_bus);
- }
+ if (!c) {
+ return;
+ }
+ if (c->typename) {
+ warn_report("'-soundhw %s' is deprecated, "
+ "please use '-device %s' instead",
+ c->name, c->typename);
+ if (c->isa) {
+ isa_create_simple(isa_bus, c->typename);
+ } else {
+ pci_create_simple(pci_bus, -1, c->typename);
}
+ } else {
+ assert(!c->isa);
+ if (!pci_bus) {
+ error_report("PCI bus not available for %s", c->name);
+ exit(1);
+ }
+ c->init_pci(pci_bus);
}
}
--
2.35.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [RFC PATCH 3/6] soundhw: extract soundhw help to a separate function
2022-04-27 11:32 [RFC PATCH 0/6] replace -soundhw with -audio Paolo Bonzini
2022-04-27 11:32 ` [RFC PATCH 1/6] pc: remove -soundhw pcspk Paolo Bonzini
2022-04-27 11:32 ` [RFC PATCH 2/6] soundhw: remove ability to create multiple soundcards Paolo Bonzini
@ 2022-04-27 11:32 ` Paolo Bonzini
2022-04-27 11:32 ` [RFC PATCH 4/6] soundhw: unify initialization for ISA and PCI soundhw Paolo Bonzini
` (2 subsequent siblings)
5 siblings, 0 replies; 12+ messages in thread
From: Paolo Bonzini @ 2022-04-27 11:32 UTC (permalink / raw)
To: qemu-devel; +Cc: mkletzan, berrange, kraxel
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
hw/audio/soundhw.c | 33 +++++++++++++++++++--------------
include/hw/audio/soundhw.h | 1 +
2 files changed, 20 insertions(+), 14 deletions(-)
diff --git a/hw/audio/soundhw.c b/hw/audio/soundhw.c
index 097501fee1..0fb64bdc8f 100644
--- a/hw/audio/soundhw.c
+++ b/hw/audio/soundhw.c
@@ -64,6 +64,21 @@ void deprecated_register_soundhw(const char *name, const char *descr,
soundhw_count++;
}
+void show_valid_soundhw(void)
+{
+ struct soundhw *c;
+
+ if (soundhw_count) {
+ printf("Valid sound card names (comma separated):\n");
+ for (c = soundhw; c->name; ++c) {
+ printf ("%-11s %s\n", c->name, c->descr);
+ }
+ } else {
+ printf("Machine has no user-selectable audio hardware "
+ "(it may or may not have always-present audio hardware).\n");
+ }
+}
+
static struct soundhw *selected = NULL;
void select_soundhw(const char *optarg)
@@ -75,19 +90,8 @@ void select_soundhw(const char *optarg)
}
if (is_help_option(optarg)) {
- show_valid_cards:
-
- if (soundhw_count) {
- printf("Valid sound card names (comma separated):\n");
- for (c = soundhw; c->name; ++c) {
- printf ("%-11s %s\n", c->name, c->descr);
- }
- printf("\n-soundhw all will enable all of the above\n");
- } else {
- printf("Machine has no user-selectable audio hardware "
- "(it may or may not have always-present audio hardware).\n");
- }
- exit(!is_help_option(optarg));
+ show_valid_soundhw();
+ exit(0);
}
else {
for (c = soundhw; c->name; ++c) {
@@ -99,7 +103,8 @@ void select_soundhw(const char *optarg)
if (!c->name) {
error_report("Unknown sound card name `%s'", optarg);
- goto show_valid_cards;
+ show_valid_soundhw();
+ exit(1);
}
}
}
diff --git a/include/hw/audio/soundhw.h b/include/hw/audio/soundhw.h
index e68685fcda..dec5c0cdca 100644
--- a/include/hw/audio/soundhw.h
+++ b/include/hw/audio/soundhw.h
@@ -7,6 +7,7 @@ void deprecated_register_soundhw(const char *name, const char *descr,
int isa, const char *typename);
void soundhw_init(void);
+void show_valid_soundhw(void);
void select_soundhw(const char *optarg);
#endif
--
2.35.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [RFC PATCH 4/6] soundhw: unify initialization for ISA and PCI soundhw
2022-04-27 11:32 [RFC PATCH 0/6] replace -soundhw with -audio Paolo Bonzini
` (2 preceding siblings ...)
2022-04-27 11:32 ` [RFC PATCH 3/6] soundhw: extract soundhw help to a separate function Paolo Bonzini
@ 2022-04-27 11:32 ` Paolo Bonzini
2022-05-16 14:06 ` Martin Kletzander
2022-04-27 11:32 ` [RFC PATCH 5/6] soundhw: move help handling to vl.c Paolo Bonzini
2022-04-27 11:32 ` [RFC PATCH 6/6] vl: introduce -audio as a replacement for -soundhw Paolo Bonzini
5 siblings, 1 reply; 12+ messages in thread
From: Paolo Bonzini @ 2022-04-27 11:32 UTC (permalink / raw)
To: qemu-devel; +Cc: mkletzan, berrange, kraxel
Use qdev_new instead of distinguishing isa_create_simple/pci_create_simple.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
hw/audio/soundhw.c | 26 ++++++++++++++------------
1 file changed, 14 insertions(+), 12 deletions(-)
diff --git a/hw/audio/soundhw.c b/hw/audio/soundhw.c
index 0fb64bdc8f..a9d8807b18 100644
--- a/hw/audio/soundhw.c
+++ b/hw/audio/soundhw.c
@@ -114,25 +114,27 @@ void soundhw_init(void)
struct soundhw *c = selected;
ISABus *isa_bus = (ISABus *) object_resolve_path_type("", TYPE_ISA_BUS, NULL);
PCIBus *pci_bus = (PCIBus *) object_resolve_path_type("", TYPE_PCI_BUS, NULL);
+ BusState *bus;
- if (!c) {
- return;
- }
- if (c->typename) {
- warn_report("'-soundhw %s' is deprecated, "
- "please use '-device %s' instead",
- c->name, c->typename);
- if (c->isa) {
- isa_create_simple(isa_bus, c->typename);
- } else {
- pci_create_simple(pci_bus, -1, c->typename);
+ if (c->isa) {
+ if (!isa_bus) {
+ error_report("ISA bus not available for %s", c->name);
+ exit(1);
}
+ bus = BUS(isa_bus);
} else {
- assert(!c->isa);
if (!pci_bus) {
error_report("PCI bus not available for %s", c->name);
exit(1);
}
+ bus = BUS(pci_bus);
+ }
+
+ if (c->typename) {
+ DeviceState *dev = qdev_new(c->typename);
+ qdev_realize_and_unref(dev, bus, &error_fatal);
+ } else {
+ assert(!c->isa);
c->init_pci(pci_bus);
}
}
--
2.35.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [RFC PATCH 4/6] soundhw: unify initialization for ISA and PCI soundhw
2022-04-27 11:32 ` [RFC PATCH 4/6] soundhw: unify initialization for ISA and PCI soundhw Paolo Bonzini
@ 2022-05-16 14:06 ` Martin Kletzander
0 siblings, 0 replies; 12+ messages in thread
From: Martin Kletzander @ 2022-05-16 14:06 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel, kraxel, berrange
[-- Attachment #1: Type: text/plain, Size: 2025 bytes --]
On Wed, Apr 27, 2022 at 01:32:23PM +0200, Paolo Bonzini wrote:
>Use qdev_new instead of distinguishing isa_create_simple/pci_create_simple.
>
>Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
By trying to rebase my series on top of this series I noticed this patch
breaks almost everything.
>---
> hw/audio/soundhw.c | 26 ++++++++++++++------------
> 1 file changed, 14 insertions(+), 12 deletions(-)
>
>diff --git a/hw/audio/soundhw.c b/hw/audio/soundhw.c
>index 0fb64bdc8f..a9d8807b18 100644
>--- a/hw/audio/soundhw.c
>+++ b/hw/audio/soundhw.c
>@@ -114,25 +114,27 @@ void soundhw_init(void)
> struct soundhw *c = selected;
> ISABus *isa_bus = (ISABus *) object_resolve_path_type("", TYPE_ISA_BUS, NULL);
> PCIBus *pci_bus = (PCIBus *) object_resolve_path_type("", TYPE_PCI_BUS, NULL);
>+ BusState *bus;
>
>- if (!c) {
>- return;
>- }
This can still happen if there is no -audio. Without this check and
without any -audio parameter qemu obviously crashes.
>- if (c->typename) {
>- warn_report("'-soundhw %s' is deprecated, "
>- "please use '-device %s' instead",
>- c->name, c->typename);
>- if (c->isa) {
>- isa_create_simple(isa_bus, c->typename);
>- } else {
>- pci_create_simple(pci_bus, -1, c->typename);
>+ if (c->isa) {
>+ if (!isa_bus) {
>+ error_report("ISA bus not available for %s", c->name);
>+ exit(1);
> }
>+ bus = BUS(isa_bus);
> } else {
>- assert(!c->isa);
> if (!pci_bus) {
> error_report("PCI bus not available for %s", c->name);
> exit(1);
> }
>+ bus = BUS(pci_bus);
>+ }
>+
>+ if (c->typename) {
>+ DeviceState *dev = qdev_new(c->typename);
>+ qdev_realize_and_unref(dev, bus, &error_fatal);
>+ } else {
>+ assert(!c->isa);
> c->init_pci(pci_bus);
> }
> }
>--
>2.35.1
>
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* [RFC PATCH 5/6] soundhw: move help handling to vl.c
2022-04-27 11:32 [RFC PATCH 0/6] replace -soundhw with -audio Paolo Bonzini
` (3 preceding siblings ...)
2022-04-27 11:32 ` [RFC PATCH 4/6] soundhw: unify initialization for ISA and PCI soundhw Paolo Bonzini
@ 2022-04-27 11:32 ` Paolo Bonzini
2022-04-27 11:32 ` [RFC PATCH 6/6] vl: introduce -audio as a replacement for -soundhw Paolo Bonzini
5 siblings, 0 replies; 12+ messages in thread
From: Paolo Bonzini @ 2022-04-27 11:32 UTC (permalink / raw)
To: qemu-devel; +Cc: mkletzan, berrange, kraxel
This will allow processing "-audio model=help" even if the backend
part of the option is missing.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
hw/audio/soundhw.c | 24 +++++++++---------------
softmmu/vl.c | 4 ++++
2 files changed, 13 insertions(+), 15 deletions(-)
diff --git a/hw/audio/soundhw.c b/hw/audio/soundhw.c
index a9d8807b18..d81ae91136 100644
--- a/hw/audio/soundhw.c
+++ b/hw/audio/soundhw.c
@@ -89,23 +89,17 @@ void select_soundhw(const char *optarg)
error_setg(&error_fatal, "only one -soundhw option is allowed");
}
- if (is_help_option(optarg)) {
- show_valid_soundhw();
- exit(0);
+ for (c = soundhw; c->name; ++c) {
+ if (g_str_equal(c->name, optarg)) {
+ selected = c;
+ break;
+ }
}
- else {
- for (c = soundhw; c->name; ++c) {
- if (g_str_equal(c->name, optarg)) {
- selected = c;
- break;
- }
- }
- if (!c->name) {
- error_report("Unknown sound card name `%s'", optarg);
- show_valid_soundhw();
- exit(1);
- }
+ if (!c->name) {
+ error_report("Unknown sound card name `%s'", optarg);
+ show_valid_soundhw();
+ exit(1);
}
}
diff --git a/softmmu/vl.c b/softmmu/vl.c
index c2919579fd..5bea0eb3eb 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -3019,6 +3019,10 @@ void qemu_init(int argc, char **argv, char **envp)
audio_parse_option(optarg);
break;
case QEMU_OPTION_soundhw:
+ if (is_help_option(optarg)) {
+ show_valid_soundhw();
+ exit(0);
+ }
select_soundhw (optarg);
break;
case QEMU_OPTION_h:
--
2.35.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [RFC PATCH 6/6] vl: introduce -audio as a replacement for -soundhw
2022-04-27 11:32 [RFC PATCH 0/6] replace -soundhw with -audio Paolo Bonzini
` (4 preceding siblings ...)
2022-04-27 11:32 ` [RFC PATCH 5/6] soundhw: move help handling to vl.c Paolo Bonzini
@ 2022-04-27 11:32 ` Paolo Bonzini
2022-04-27 13:41 ` Mark Cave-Ayland
2022-04-29 14:54 ` Martin Kletzander
5 siblings, 2 replies; 12+ messages in thread
From: Paolo Bonzini @ 2022-04-27 11:32 UTC (permalink / raw)
To: qemu-devel; +Cc: mkletzan, berrange, kraxel
-audio is used like "-audio pa,model=sb16". It is almost as simple as
-soundhw, but it reuses the -audiodev parsing machinery and attaches an
audiodev to the newly-created device. The main 'feature' is that
it knows about adding the codec device for model=intel-hda, and adding
the audiodev to the codec device.
In the future, it could be extended to support default models or
builtin devices, just like -nic, or even a default backend. For now,
keep it simple.
JSON parsing is not supported for -audio. This is okay because the
option is targeted at end users, not programs.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
audio/audio.c | 8 +++++-
audio/audio.h | 1 +
docs/about/deprecated.rst | 9 ------
docs/about/removed-features.rst | 7 +++++
hw/audio/intel-hda.c | 5 ++--
hw/audio/soundhw.c | 12 +++++---
include/hw/audio/soundhw.h | 4 +--
qemu-options.hx | 51 ++++++++++++++++-----------------
softmmu/vl.c | 28 ++++++++++++++++--
9 files changed, 76 insertions(+), 49 deletions(-)
diff --git a/audio/audio.c b/audio/audio.c
index 9e91a5a4f2..a02f3ce5c6 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -2099,13 +2099,19 @@ static void audio_validate_opts(Audiodev *dev, Error **errp)
void audio_parse_option(const char *opt)
{
- AudiodevListEntry *e;
Audiodev *dev = NULL;
Visitor *v = qobject_input_visitor_new_str(opt, "driver", &error_fatal);
visit_type_Audiodev(v, NULL, &dev, &error_fatal);
visit_free(v);
+ audio_define(dev);
+}
+
+void audio_define(Audiodev *dev)
+{
+ AudiodevListEntry *e;
+
audio_validate_opts(dev, &error_fatal);
e = g_new0(AudiodevListEntry, 1);
diff --git a/audio/audio.h b/audio/audio.h
index 3d5ecdecd5..b5e17cd218 100644
--- a/audio/audio.h
+++ b/audio/audio.h
@@ -168,6 +168,7 @@ void audio_sample_to_uint64(const void *samples, int pos,
void audio_sample_from_uint64(void *samples, int pos,
uint64_t left, uint64_t right);
+void audio_define(Audiodev *audio);
void audio_parse_option(const char *opt);
void audio_init_audiodevs(void);
void audio_legacy_help(void);
diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index 896e5a97ab..70885d09f3 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -39,15 +39,6 @@ should specify an ``audiodev=`` property. Additionally, when using
vnc, you should specify an ``audiodev=`` property if you plan to
transmit audio through the VNC protocol.
-Creating sound card devices using ``-soundhw`` (since 5.1)
-''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
-
-Sound card devices should be created using ``-device`` instead. The
-names are the same for most devices. The exceptions are ``hda`` which
-needs two devices (``-device intel-hda -device hda-duplex``) and
-``pcspk`` which can be activated using ``-machine
-pcspk-audiodev=<name>``.
-
``-chardev`` backend aliases ``tty`` and ``parport`` (since 6.0)
''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
diff --git a/docs/about/removed-features.rst b/docs/about/removed-features.rst
index 3f324d0536..eabc6c63ac 100644
--- a/docs/about/removed-features.rst
+++ b/docs/about/removed-features.rst
@@ -632,6 +632,13 @@ tripped up the CI testing and was suspected to be quite broken. For that
reason the maintainers strongly suspected no one actually used it.
+Creating sound card devices using ``-soundhw`` (removed in 7.1)
+''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
+
+Sound card devices should be created using ``-device`` or ``-audio``.
+The exception is ``pcspk`` which can be activated using ``-machine
+pcspk-audiodev=<name>``.
+
TCG introspection features
--------------------------
diff --git a/hw/audio/intel-hda.c b/hw/audio/intel-hda.c
index bc77e3d8c9..f38117057b 100644
--- a/hw/audio/intel-hda.c
+++ b/hw/audio/intel-hda.c
@@ -1311,17 +1311,16 @@ static const TypeInfo hda_codec_device_type_info = {
* create intel hda controller with codec attached to it,
* so '-soundhw hda' works.
*/
-static int intel_hda_and_codec_init(PCIBus *bus)
+static int intel_hda_and_codec_init(PCIBus *bus, const char *audiodev)
{
DeviceState *controller;
BusState *hdabus;
DeviceState *codec;
- warn_report("'-soundhw hda' is deprecated, "
- "please use '-device intel-hda -device hda-duplex' instead");
controller = DEVICE(pci_create_simple(bus, -1, "intel-hda"));
hdabus = QLIST_FIRST(&controller->child_bus);
codec = qdev_new("hda-duplex");
+ qdev_prop_set_string(codec, "audiodev", audiodev);
qdev_realize_and_unref(codec, hdabus, &error_fatal);
return 0;
}
diff --git a/hw/audio/soundhw.c b/hw/audio/soundhw.c
index d81ae91136..e979be08ce 100644
--- a/hw/audio/soundhw.c
+++ b/hw/audio/soundhw.c
@@ -27,6 +27,7 @@
#include "qemu/error-report.h"
#include "qapi/error.h"
#include "qom/object.h"
+#include "hw/qdev-properties.h"
#include "hw/isa/isa.h"
#include "hw/pci/pci.h"
#include "hw/audio/soundhw.h"
@@ -36,14 +37,14 @@ struct soundhw {
const char *descr;
const char *typename;
int isa;
- int (*init_pci) (PCIBus *bus);
+ int (*init_pci) (PCIBus *bus, const char *audiodev);
};
static struct soundhw soundhw[9];
static int soundhw_count;
void pci_register_soundhw(const char *name, const char *descr,
- int (*init_pci)(PCIBus *bus))
+ int (*init_pci)(PCIBus *bus, const char *audiodev))
{
assert(soundhw_count < ARRAY_SIZE(soundhw) - 1);
soundhw[soundhw_count].name = name;
@@ -80,8 +81,9 @@ void show_valid_soundhw(void)
}
static struct soundhw *selected = NULL;
+static const char *audiodev_id;
-void select_soundhw(const char *optarg)
+void select_soundhw(const char *optarg, const char *audiodev)
{
struct soundhw *c;
@@ -92,6 +94,7 @@ void select_soundhw(const char *optarg)
for (c = soundhw; c->name; ++c) {
if (g_str_equal(c->name, optarg)) {
selected = c;
+ audiodev_id = audiodev;
break;
}
}
@@ -126,10 +129,11 @@ void soundhw_init(void)
if (c->typename) {
DeviceState *dev = qdev_new(c->typename);
+ qdev_prop_set_string(dev, "audiodev", audiodev_id);
qdev_realize_and_unref(dev, bus, &error_fatal);
} else {
assert(!c->isa);
- c->init_pci(pci_bus);
+ c->init_pci(pci_bus, audiodev_id);
}
}
diff --git a/include/hw/audio/soundhw.h b/include/hw/audio/soundhw.h
index dec5c0cdca..270717a06a 100644
--- a/include/hw/audio/soundhw.h
+++ b/include/hw/audio/soundhw.h
@@ -2,12 +2,12 @@
#define HW_SOUNDHW_H
void pci_register_soundhw(const char *name, const char *descr,
- int (*init_pci)(PCIBus *bus));
+ int (*init_pci)(PCIBus *bus, const char *audiodev));
void deprecated_register_soundhw(const char *name, const char *descr,
int isa, const char *typename);
void soundhw_init(void);
void show_valid_soundhw(void);
-void select_soundhw(const char *optarg);
+void select_soundhw(const char *optarg, const char *audiodev);
#endif
diff --git a/qemu-options.hx b/qemu-options.hx
index bc196808ae..862263d435 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -661,6 +661,30 @@ SRST
(deprecated) environment variables.
ERST
+DEF("audio", HAS_ARG, QEMU_OPTION_audio,
+ "-audio [driver=]driver,model=value[,prop[=value][,...]]\n"
+ " specifies the audio backend and device to use;\n"
+ " apart from 'model', options are the same as for -audiodev.\n"
+ " use '-audio model=help' to show possible devices.\n",
+ QEMU_ARCH_ALL)
+SRST
+``-audio [driver=]driver,model=value[,prop[=value][,...]]``
+ This option is a shortcut for configuring both the guest audio
+ hardware and the host audio backend in one go.
+ The host backend options are the same as with the corresponding
+ ``-audiodev`` options below. The guest hardware model can be set with
+ ``model=modelname``. Use ``model=help`` to list the available device
+ types.
+
+ The following two example do exactly the same, to show how ``-audio``
+ can be used to shorten the command line length:
+
+ .. parsed-literal::
+
+ |qemu_system| -audiodev pa,id=pa -device sb16,audiodev=pa
+ |qemu_system| -audio pa,model=sb16
+ERST
+
DEF("audiodev", HAS_ARG, QEMU_OPTION_audiodev,
"-audiodev [driver=]driver,id=id[,prop[=value][,...]]\n"
" specifies the audio backend to use\n"
@@ -892,33 +916,6 @@ SRST
``qemu.wav``.
ERST
-DEF("soundhw", HAS_ARG, QEMU_OPTION_soundhw,
- "-soundhw c1,... enable audio support\n"
- " and only specified sound cards (comma separated list)\n"
- " use '-soundhw help' to get the list of supported cards\n"
- " use '-soundhw all' to enable all of them\n", QEMU_ARCH_ALL)
-SRST
-``-soundhw card1[,card2,...] or -soundhw all``
- Enable audio and selected sound hardware. Use 'help' to print all
- available sound hardware. For example:
-
- .. parsed-literal::
-
- |qemu_system_x86| -soundhw sb16,adlib disk.img
- |qemu_system_x86| -soundhw es1370 disk.img
- |qemu_system_x86| -soundhw ac97 disk.img
- |qemu_system_x86| -soundhw hda disk.img
- |qemu_system_x86| -soundhw all disk.img
- |qemu_system_x86| -soundhw help
-
- Note that Linux's i810\_audio OSS kernel (for AC97) module might
- require manually specifying clocking.
-
- ::
-
- modprobe i810_audio clocking=48000
-ERST
-
DEF("device", HAS_ARG, QEMU_OPTION_device,
"-device driver[,prop[=value][,...]]\n"
" add device (based on driver)\n"
diff --git a/softmmu/vl.c b/softmmu/vl.c
index 5bea0eb3eb..979bbda5aa 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -116,6 +116,8 @@
#include "crypto/init.h"
#include "sysemu/replay.h"
#include "qapi/qapi-events-run-state.h"
+#include "qapi/qapi-types-audio.h"
+#include "qapi/qapi-visit-audio.h"
#include "qapi/qapi-visit-block-core.h"
#include "qapi/qapi-visit-compat.h"
#include "qapi/qapi-visit-ui.h"
@@ -3018,13 +3020,33 @@ void qemu_init(int argc, char **argv, char **envp)
case QEMU_OPTION_audiodev:
audio_parse_option(optarg);
break;
- case QEMU_OPTION_soundhw:
- if (is_help_option(optarg)) {
+ case QEMU_OPTION_audio: {
+ QDict *dict = keyval_parse(optarg, "driver", NULL, &error_fatal);
+ char *model;
+ Audiodev *dev = NULL;
+ Visitor *v;
+
+ if (!qdict_haskey(dict, "id")) {
+ qdict_put_str(dict, "id", "audiodev0");
+ }
+ if (!qdict_haskey(dict, "model")) {
+ error_setg(&error_fatal, "Parameter 'model' is missing");
+ }
+ model = g_strdup(qdict_get_str(dict, "model"));
+ qdict_del(dict, "model");
+ if (is_help_option(model)) {
show_valid_soundhw();
exit(0);
}
- select_soundhw (optarg);
+ v = qobject_input_visitor_new_keyval(QOBJECT(dict));
+ qobject_unref(dict);
+ visit_type_Audiodev(v, NULL, &dev, &error_fatal);
+ visit_free(v);
+ audio_define(dev);
+ select_soundhw(model, dev->id);
+ g_free(model);
break;
+ }
case QEMU_OPTION_h:
help(0);
break;
--
2.35.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [RFC PATCH 6/6] vl: introduce -audio as a replacement for -soundhw
2022-04-27 11:32 ` [RFC PATCH 6/6] vl: introduce -audio as a replacement for -soundhw Paolo Bonzini
@ 2022-04-27 13:41 ` Mark Cave-Ayland
2022-04-27 14:21 ` Paolo Bonzini
2022-04-29 14:54 ` Martin Kletzander
1 sibling, 1 reply; 12+ messages in thread
From: Mark Cave-Ayland @ 2022-04-27 13:41 UTC (permalink / raw)
To: Paolo Bonzini, qemu-devel; +Cc: mkletzan, berrange, kraxel
On 27/04/2022 12:32, Paolo Bonzini wrote:
> -audio is used like "-audio pa,model=sb16". It is almost as simple as
> -soundhw, but it reuses the -audiodev parsing machinery and attaches an
> audiodev to the newly-created device. The main 'feature' is that
> it knows about adding the codec device for model=intel-hda, and adding
> the audiodev to the codec device.
>
> In the future, it could be extended to support default models or
> builtin devices, just like -nic, or even a default backend. For now,
> keep it simple.
>
> JSON parsing is not supported for -audio. This is okay because the
> option is targeted at end users, not programs.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> audio/audio.c | 8 +++++-
> audio/audio.h | 1 +
> docs/about/deprecated.rst | 9 ------
> docs/about/removed-features.rst | 7 +++++
> hw/audio/intel-hda.c | 5 ++--
> hw/audio/soundhw.c | 12 +++++---
> include/hw/audio/soundhw.h | 4 +--
> qemu-options.hx | 51 ++++++++++++++++-----------------
> softmmu/vl.c | 28 ++++++++++++++++--
> 9 files changed, 76 insertions(+), 49 deletions(-)
>
> diff --git a/audio/audio.c b/audio/audio.c
> index 9e91a5a4f2..a02f3ce5c6 100644
> --- a/audio/audio.c
> +++ b/audio/audio.c
> @@ -2099,13 +2099,19 @@ static void audio_validate_opts(Audiodev *dev, Error **errp)
>
> void audio_parse_option(const char *opt)
> {
> - AudiodevListEntry *e;
> Audiodev *dev = NULL;
>
> Visitor *v = qobject_input_visitor_new_str(opt, "driver", &error_fatal);
> visit_type_Audiodev(v, NULL, &dev, &error_fatal);
> visit_free(v);
>
> + audio_define(dev);
> +}
> +
> +void audio_define(Audiodev *dev)
> +{
> + AudiodevListEntry *e;
> +
> audio_validate_opts(dev, &error_fatal);
>
> e = g_new0(AudiodevListEntry, 1);
> diff --git a/audio/audio.h b/audio/audio.h
> index 3d5ecdecd5..b5e17cd218 100644
> --- a/audio/audio.h
> +++ b/audio/audio.h
> @@ -168,6 +168,7 @@ void audio_sample_to_uint64(const void *samples, int pos,
> void audio_sample_from_uint64(void *samples, int pos,
> uint64_t left, uint64_t right);
>
> +void audio_define(Audiodev *audio);
> void audio_parse_option(const char *opt);
> void audio_init_audiodevs(void);
> void audio_legacy_help(void);
> diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
> index 896e5a97ab..70885d09f3 100644
> --- a/docs/about/deprecated.rst
> +++ b/docs/about/deprecated.rst
> @@ -39,15 +39,6 @@ should specify an ``audiodev=`` property. Additionally, when using
> vnc, you should specify an ``audiodev=`` property if you plan to
> transmit audio through the VNC protocol.
>
> -Creating sound card devices using ``-soundhw`` (since 5.1)
> -''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
> -
> -Sound card devices should be created using ``-device`` instead. The
> -names are the same for most devices. The exceptions are ``hda`` which
> -needs two devices (``-device intel-hda -device hda-duplex``) and
> -``pcspk`` which can be activated using ``-machine
> -pcspk-audiodev=<name>``.
> -
> ``-chardev`` backend aliases ``tty`` and ``parport`` (since 6.0)
> ''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
>
> diff --git a/docs/about/removed-features.rst b/docs/about/removed-features.rst
> index 3f324d0536..eabc6c63ac 100644
> --- a/docs/about/removed-features.rst
> +++ b/docs/about/removed-features.rst
> @@ -632,6 +632,13 @@ tripped up the CI testing and was suspected to be quite broken. For that
> reason the maintainers strongly suspected no one actually used it.
>
>
> +Creating sound card devices using ``-soundhw`` (removed in 7.1)
> +''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
> +
> +Sound card devices should be created using ``-device`` or ``-audio``.
> +The exception is ``pcspk`` which can be activated using ``-machine
> +pcspk-audiodev=<name>``.
> +
> TCG introspection features
> --------------------------
>
> diff --git a/hw/audio/intel-hda.c b/hw/audio/intel-hda.c
> index bc77e3d8c9..f38117057b 100644
> --- a/hw/audio/intel-hda.c
> +++ b/hw/audio/intel-hda.c
> @@ -1311,17 +1311,16 @@ static const TypeInfo hda_codec_device_type_info = {
> * create intel hda controller with codec attached to it,
> * so '-soundhw hda' works.
> */
> -static int intel_hda_and_codec_init(PCIBus *bus)
> +static int intel_hda_and_codec_init(PCIBus *bus, const char *audiodev)
> {
> DeviceState *controller;
> BusState *hdabus;
> DeviceState *codec;
>
> - warn_report("'-soundhw hda' is deprecated, "
> - "please use '-device intel-hda -device hda-duplex' instead");
> controller = DEVICE(pci_create_simple(bus, -1, "intel-hda"));
> hdabus = QLIST_FIRST(&controller->child_bus);
> codec = qdev_new("hda-duplex");
> + qdev_prop_set_string(codec, "audiodev", audiodev);
> qdev_realize_and_unref(codec, hdabus, &error_fatal);
> return 0;
> }
> diff --git a/hw/audio/soundhw.c b/hw/audio/soundhw.c
> index d81ae91136..e979be08ce 100644
> --- a/hw/audio/soundhw.c
> +++ b/hw/audio/soundhw.c
> @@ -27,6 +27,7 @@
> #include "qemu/error-report.h"
> #include "qapi/error.h"
> #include "qom/object.h"
> +#include "hw/qdev-properties.h"
> #include "hw/isa/isa.h"
> #include "hw/pci/pci.h"
> #include "hw/audio/soundhw.h"
> @@ -36,14 +37,14 @@ struct soundhw {
> const char *descr;
> const char *typename;
> int isa;
> - int (*init_pci) (PCIBus *bus);
> + int (*init_pci) (PCIBus *bus, const char *audiodev);
> };
>
> static struct soundhw soundhw[9];
> static int soundhw_count;
>
> void pci_register_soundhw(const char *name, const char *descr,
> - int (*init_pci)(PCIBus *bus))
> + int (*init_pci)(PCIBus *bus, const char *audiodev))
> {
> assert(soundhw_count < ARRAY_SIZE(soundhw) - 1);
> soundhw[soundhw_count].name = name;
> @@ -80,8 +81,9 @@ void show_valid_soundhw(void)
> }
>
> static struct soundhw *selected = NULL;
> +static const char *audiodev_id;
>
> -void select_soundhw(const char *optarg)
> +void select_soundhw(const char *optarg, const char *audiodev)
> {
> struct soundhw *c;
>
> @@ -92,6 +94,7 @@ void select_soundhw(const char *optarg)
> for (c = soundhw; c->name; ++c) {
> if (g_str_equal(c->name, optarg)) {
> selected = c;
> + audiodev_id = audiodev;
> break;
> }
> }
> @@ -126,10 +129,11 @@ void soundhw_init(void)
>
> if (c->typename) {
> DeviceState *dev = qdev_new(c->typename);
> + qdev_prop_set_string(dev, "audiodev", audiodev_id);
> qdev_realize_and_unref(dev, bus, &error_fatal);
> } else {
> assert(!c->isa);
> - c->init_pci(pci_bus);
> + c->init_pci(pci_bus, audiodev_id);
> }
> }
>
> diff --git a/include/hw/audio/soundhw.h b/include/hw/audio/soundhw.h
> index dec5c0cdca..270717a06a 100644
> --- a/include/hw/audio/soundhw.h
> +++ b/include/hw/audio/soundhw.h
> @@ -2,12 +2,12 @@
> #define HW_SOUNDHW_H
>
> void pci_register_soundhw(const char *name, const char *descr,
> - int (*init_pci)(PCIBus *bus));
> + int (*init_pci)(PCIBus *bus, const char *audiodev));
> void deprecated_register_soundhw(const char *name, const char *descr,
> int isa, const char *typename);
>
> void soundhw_init(void);
> void show_valid_soundhw(void);
> -void select_soundhw(const char *optarg);
> +void select_soundhw(const char *optarg, const char *audiodev);
>
> #endif
> diff --git a/qemu-options.hx b/qemu-options.hx
> index bc196808ae..862263d435 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -661,6 +661,30 @@ SRST
> (deprecated) environment variables.
> ERST
>
> +DEF("audio", HAS_ARG, QEMU_OPTION_audio,
> + "-audio [driver=]driver,model=value[,prop[=value][,...]]\n"
> + " specifies the audio backend and device to use;\n"
> + " apart from 'model', options are the same as for -audiodev.\n"
> + " use '-audio model=help' to show possible devices.\n",
> + QEMU_ARCH_ALL)
> +SRST
> +``-audio [driver=]driver,model=value[,prop[=value][,...]]``
> + This option is a shortcut for configuring both the guest audio
> + hardware and the host audio backend in one go.
> + The host backend options are the same as with the corresponding
> + ``-audiodev`` options below. The guest hardware model can be set with
> + ``model=modelname``. Use ``model=help`` to list the available device
> + types.
> +
> + The following two example do exactly the same, to show how ``-audio``
> + can be used to shorten the command line length:
> +
> + .. parsed-literal::
> +
> + |qemu_system| -audiodev pa,id=pa -device sb16,audiodev=pa
> + |qemu_system| -audio pa,model=sb16
> +ERST
> +
> DEF("audiodev", HAS_ARG, QEMU_OPTION_audiodev,
> "-audiodev [driver=]driver,id=id[,prop[=value][,...]]\n"
> " specifies the audio backend to use\n"
> @@ -892,33 +916,6 @@ SRST
> ``qemu.wav``.
> ERST
>
> -DEF("soundhw", HAS_ARG, QEMU_OPTION_soundhw,
> - "-soundhw c1,... enable audio support\n"
> - " and only specified sound cards (comma separated list)\n"
> - " use '-soundhw help' to get the list of supported cards\n"
> - " use '-soundhw all' to enable all of them\n", QEMU_ARCH_ALL)
> -SRST
> -``-soundhw card1[,card2,...] or -soundhw all``
> - Enable audio and selected sound hardware. Use 'help' to print all
> - available sound hardware. For example:
> -
> - .. parsed-literal::
> -
> - |qemu_system_x86| -soundhw sb16,adlib disk.img
> - |qemu_system_x86| -soundhw es1370 disk.img
> - |qemu_system_x86| -soundhw ac97 disk.img
> - |qemu_system_x86| -soundhw hda disk.img
> - |qemu_system_x86| -soundhw all disk.img
> - |qemu_system_x86| -soundhw help
> -
> - Note that Linux's i810\_audio OSS kernel (for AC97) module might
> - require manually specifying clocking.
> -
> - ::
> -
> - modprobe i810_audio clocking=48000
> -ERST
> -
> DEF("device", HAS_ARG, QEMU_OPTION_device,
> "-device driver[,prop[=value][,...]]\n"
> " add device (based on driver)\n"
> diff --git a/softmmu/vl.c b/softmmu/vl.c
> index 5bea0eb3eb..979bbda5aa 100644
> --- a/softmmu/vl.c
> +++ b/softmmu/vl.c
> @@ -116,6 +116,8 @@
> #include "crypto/init.h"
> #include "sysemu/replay.h"
> #include "qapi/qapi-events-run-state.h"
> +#include "qapi/qapi-types-audio.h"
> +#include "qapi/qapi-visit-audio.h"
> #include "qapi/qapi-visit-block-core.h"
> #include "qapi/qapi-visit-compat.h"
> #include "qapi/qapi-visit-ui.h"
> @@ -3018,13 +3020,33 @@ void qemu_init(int argc, char **argv, char **envp)
> case QEMU_OPTION_audiodev:
> audio_parse_option(optarg);
> break;
> - case QEMU_OPTION_soundhw:
> - if (is_help_option(optarg)) {
> + case QEMU_OPTION_audio: {
> + QDict *dict = keyval_parse(optarg, "driver", NULL, &error_fatal);
> + char *model;
> + Audiodev *dev = NULL;
> + Visitor *v;
> +
> + if (!qdict_haskey(dict, "id")) {
> + qdict_put_str(dict, "id", "audiodev0");
> + }
> + if (!qdict_haskey(dict, "model")) {
> + error_setg(&error_fatal, "Parameter 'model' is missing");
> + }
> + model = g_strdup(qdict_get_str(dict, "model"));
> + qdict_del(dict, "model");
> + if (is_help_option(model)) {
> show_valid_soundhw();
> exit(0);
> }
> - select_soundhw (optarg);
> + v = qobject_input_visitor_new_keyval(QOBJECT(dict));
> + qobject_unref(dict);
> + visit_type_Audiodev(v, NULL, &dev, &error_fatal);
> + visit_free(v);
> + audio_define(dev);
> + select_soundhw(model, dev->id);
> + g_free(model);
> break;
> + }
> case QEMU_OPTION_h:
> help(0);
> break;
Is it possible to change select_soundhw() to take an AudioDev pointer rather than a
string, and then add a new qdev_prop_set_audiodev() function similar to
qdev_prop_set_chr() and qdev_prop_set_netdev()?
In reality the underlying QOM property is still a string, but I think having the
stronger typing for AudioDev properties is useful and potentially allows for the
various *dev backend properties to become QOM links in future.
ATB,
Mark.
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [RFC PATCH 6/6] vl: introduce -audio as a replacement for -soundhw
2022-04-27 13:41 ` Mark Cave-Ayland
@ 2022-04-27 14:21 ` Paolo Bonzini
0 siblings, 0 replies; 12+ messages in thread
From: Paolo Bonzini @ 2022-04-27 14:21 UTC (permalink / raw)
To: Mark Cave-Ayland, qemu-devel; +Cc: mkletzan, berrange, kraxel
On 4/27/22 15:41, Mark Cave-Ayland wrote:
>> + select_soundhw(model, dev->id);
>> + g_free(model);
>> break;
>> + }
>> case QEMU_OPTION_h:
>> help(0);
>> break;
>
> Is it possible to change select_soundhw() to take an AudioDev pointer
> rather than a string, and then add a new qdev_prop_set_audiodev()
> function similar to qdev_prop_set_chr() and qdev_prop_set_netdev()?
>
> In reality the underlying QOM property is still a string, but I think
> having the stronger typing for AudioDev properties is useful and
> potentially allows for the various *dev backend properties to become QOM
> links in future.
I didn't consider that because there are just two uses and I don't
expect them to grow much, but yes it's possible.
Paolo
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RFC PATCH 6/6] vl: introduce -audio as a replacement for -soundhw
2022-04-27 11:32 ` [RFC PATCH 6/6] vl: introduce -audio as a replacement for -soundhw Paolo Bonzini
2022-04-27 13:41 ` Mark Cave-Ayland
@ 2022-04-29 14:54 ` Martin Kletzander
1 sibling, 0 replies; 12+ messages in thread
From: Martin Kletzander @ 2022-04-29 14:54 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: berrange, qemu-devel, kraxel
[-- Attachment #1: Type: text/plain, Size: 3100 bytes --]
On Wed, Apr 27, 2022 at 01:32:25PM +0200, Paolo Bonzini wrote:
>-audio is used like "-audio pa,model=sb16". It is almost as simple as
>-soundhw, but it reuses the -audiodev parsing machinery and attaches an
>audiodev to the newly-created device. The main 'feature' is that
>it knows about adding the codec device for model=intel-hda, and adding
>the audiodev to the codec device.
>
>In the future, it could be extended to support default models or
>builtin devices, just like -nic, or even a default backend. For now,
>keep it simple.
>
>JSON parsing is not supported for -audio. This is okay because the
>option is targeted at end users, not programs.
>
>Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>---
> audio/audio.c | 8 +++++-
> audio/audio.h | 1 +
> docs/about/deprecated.rst | 9 ------
> docs/about/removed-features.rst | 7 +++++
> hw/audio/intel-hda.c | 5 ++--
> hw/audio/soundhw.c | 12 +++++---
> include/hw/audio/soundhw.h | 4 +--
> qemu-options.hx | 51 ++++++++++++++++-----------------
> softmmu/vl.c | 28 ++++++++++++++++--
> 9 files changed, 76 insertions(+), 49 deletions(-)
>
[...]
>diff --git a/softmmu/vl.c b/softmmu/vl.c
>index 5bea0eb3eb..979bbda5aa 100644
>--- a/softmmu/vl.c
>+++ b/softmmu/vl.c
>@@ -3018,13 +3020,33 @@ void qemu_init(int argc, char **argv, char **envp)
> case QEMU_OPTION_audiodev:
> audio_parse_option(optarg);
> break;
>- case QEMU_OPTION_soundhw:
>- if (is_help_option(optarg)) {
>+ case QEMU_OPTION_audio: {
>+ QDict *dict = keyval_parse(optarg, "driver", NULL, &error_fatal);
>+ char *model;
>+ Audiodev *dev = NULL;
>+ Visitor *v;
>+
>+ if (!qdict_haskey(dict, "id")) {
>+ qdict_put_str(dict, "id", "audiodev0");
>+ }
>+ if (!qdict_haskey(dict, "model")) {
>+ error_setg(&error_fatal, "Parameter 'model' is missing");
>+ }
>+ model = g_strdup(qdict_get_str(dict, "model"));
>+ qdict_del(dict, "model");
>+ if (is_help_option(model)) {
> show_valid_soundhw();
> exit(0);
> }
>- select_soundhw (optarg);
>+ v = qobject_input_visitor_new_keyval(QOBJECT(dict));
>+ qobject_unref(dict);
>+ visit_type_Audiodev(v, NULL, &dev, &error_fatal);
>+ visit_free(v);
>+ audio_define(dev);
I was looking at this and thought that you might be creating multiple
audiodevs if there are multiple -audio options. And I found out that
even with current master it is possible to create multiple audiodevs not
only with the same driver, but even with the same ID (and different
drivers).
I guess that is not preferable, but I can't say for sure how this is
supposed to be handled.
Other than that it looks fine to me.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread