Re: [RFC PATCH 4/6] soundhw: unify initialization for ISA and PCI soundhw

[Martin Kletzander]

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 


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, _fatal);
+} else {
+assert(!c->isa);
c->init_pci(pci_bus);
}
}
--
2.35.1




signature.asc
Description: PGP signature

Re: [RFC PATCH 6/6] vl: introduce -audio as a replacement for -soundhw

[Martin Kletzander]

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 
---
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, 
_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(_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, , _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.


signature.asc
Description: PGP signature

Re: [PATCH 00/18] RFC: Remove deprecated audio features

[Martin Kletzander]

On Mon, Apr 25, 2022 at 06:05:56PM +0100, Mark Cave-Ayland wrote:

On 25/04/2022 09:21, Martin Kletzander wrote:


I wanted to deal with https://bugzilla.redhat.com/2043498 and I got a
suggesstion that removing deprecated features could actually make it
easier to propagate the error.  In the end (last patch) it turns out the
error is still just reported with error_fatal, so it probably is not
really needed, but I really wanted to dig into QEMU more and learn some
of the internals for quite some time now.  So I used the opportunity.
The one-liner ended up being an 18 patch series which was, for someone
who has just one commit in QEMU codebase, a pretty challenging task.
Although I tried my best to do things properly, I am not sure whether I
handled everything correctly, hence the RFC.

Any comments are very much appreciated.  Thanks and have a nice day ;)

Martin Kletzander (18):
   hw/audio: Remove -soundhw support
   hw/input/tsc210x: Extract common init code into new function
   hw/audio: Simplify hda audio init
   hw/audio/lm4549: Add errp error reporting to init function
   tests/qtest: Specify audiodev= and -audiodev
   ui/vnc: Require audiodev=
   Introduce machine's default-audiodev property
   audio: Add easy dummy audio initialiser
   hw/display/xlnx_dp.c: Add audiodev property
   hw/input/tsc210x.c: Support machine-default audiodev with fallback
   hw/arm: Support machine-default audiodev with fallback
   hw/ppc: Support machine-default audiodev with fallback
   audio: Make AUD_register_card fallible and require audiodev=
   audio: Require AudioState in AUD_add_capture
   audio: Be more strict during audio backend initialisation
   audio: Remove legacy audio environment variables and options
   audio: Remove unused can_be_default
   audio/spiceaudio: Fail initialisation when not using spice



[...]



Thanks for the tidy-up! I'm not too familiar with the audio code, however one 
thing I
noticed is that in patch 11 you are using qdev_prop_set_string() to set the 
audiodev
for the machines.

For CharDevs there already exists a qdev_prop_set_chr() function which is used 
to
assign the chardev backend to the device, so I'm wondering if it makes sense to 
add a
similar qdev_prop_set_audiodev() function in the same way? Then if the method of
referencing the audiodev from the device ever changes from being a string 
containing
the name, it won't require updating all of the callers.



I guess that could be possible, but I'm not familiar with QEMU code much
and to be honest have no idea how I would go about doing that right now.


Finally a quick glance at the tsc210x and lm4549 devices indicates that they are
using global device _init() functions which shouldn't really be used in modern
implementations. If you are interested in QOMify-ing these devices as part of 
this
work, then I can certainly help provide some pointers for getting started.



I thought about it for some devices.  Not all the devices I touched
here, because I feel like that would lead to a rabbit hole.  And 18
patches just because I wanted to error out in one condition was already
too much =)  But I'll be glad for any pointers on where to start and how
should I get to it.  Thanks.



ATB,

Mark.



signature.asc
Description: PGP signature

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

[Martin Kletzander]

On Mon, Apr 25, 2022 at 03:06:14PM +0100, Daniel P. Berrangé wrote:

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


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.



Which is what I wanted to avoid because it creates huge amount of names
which are not easy to find out.  I imagine that the default audio
devices which already exist are not much known and mostly used as-is
without the users needing to figure out what they are.  Especially those
that are enabled even with -nodefaults.  And I expect people who just
want to just emulate an old Palm do not really want to specify the names
of both devices which need an audiodev, especially when one of them is a
touch display or something weird like that.  Moreover you cannot specify
anything for these devices currently, so you cannot use two different
backends now.

I understand that the naming suggests it would be a default audiodev for
anything without one explicitly specified, but I could not think of any
other suitable name.  And I felt like having a default that would be
used for all devices defeats the purpose of removing default audiodev.

If adding a machine option for every such device is the way to go, then
I can try changing that.  Hopefully that would not turn into another
18-patch series =)  So should I prefer that route?




 /* 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 :|



signature.asc
Description: PGP signature

Re: [PATCH 05/18] tests/qtest: Specify audiodev= and -audiodev

[Martin Kletzander]

On Mon, Apr 25, 2022 at 02:42:53PM +0100, Daniel P. Berrangé wrote:

On Mon, Apr 25, 2022 at 10:21:48AM +0200, Martin Kletzander wrote:

This will enable removing deprecated default audiodev support.

I did not figure out how to make the audiodev represented as an
interface node, so this is a workaround.  I am not sure what would be
the proper way.


Not sure I understand what you mean by this 'interface node' reference ?



I meant a qos node, I though that would be the proper way, but since
audiodev is a backend and not a device I wasn't able to plug it in using
qos_node_consumes().  Maybe I was just trying too hard.


The code looks fine though

Reviewed-by: Daniel P. Berrangé 



Signed-off-by: Martin Kletzander 
---
 tests/qtest/ac97-test.c |  3 ++-
 tests/qtest/es1370-test.c   |  3 ++-
 tests/qtest/fuzz/generic_fuzz_configs.h |  6 --
 tests/qtest/intel-hda-test.c| 15 ++-
 4 files changed, 18 insertions(+), 9 deletions(-)

diff --git a/tests/qtest/ac97-test.c b/tests/qtest/ac97-test.c
index e09f2495d24d..9711f1f6d966 100644
--- a/tests/qtest/ac97-test.c
+++ b/tests/qtest/ac97-test.c
@@ -45,7 +45,8 @@ static void *ac97_create(void *pci_bus, QGuestAllocator 
*alloc, void *addr)
 static void ac97_register_nodes(void)
 {
 QOSGraphEdgeOptions opts = {
-.extra_device_opts = "addr=04.0",
+.extra_device_opts = "addr=04.0,audiodev=audio0",
+.before_cmd_line = "-audiodev driver=none,id=audio0",
 };
 add_qpci_address(, &(QPCIAddress) { .devfn = QPCI_DEVFN(4, 0) });

diff --git a/tests/qtest/es1370-test.c b/tests/qtest/es1370-test.c
index 2fd7fd2d3d30..5facda8d0d8d 100644
--- a/tests/qtest/es1370-test.c
+++ b/tests/qtest/es1370-test.c
@@ -46,7 +46,8 @@ static void *es1370_create(void *pci_bus, QGuestAllocator 
*alloc, void *addr)
 static void es1370_register_nodes(void)
 {
 QOSGraphEdgeOptions opts = {
-.extra_device_opts = "addr=04.0",
+.extra_device_opts = "addr=04.0,audiodev=audio0",
+.before_cmd_line = "-audiodev driver=none,id=audio0",
 };
 add_qpci_address(, &(QPCIAddress) { .devfn = QPCI_DEVFN(4, 0) });

diff --git a/tests/qtest/fuzz/generic_fuzz_configs.h 
b/tests/qtest/fuzz/generic_fuzz_configs.h
index 004c701915e1..84a93b3c350b 100644
--- a/tests/qtest/fuzz/generic_fuzz_configs.h
+++ b/tests/qtest/fuzz/generic_fuzz_configs.h
@@ -101,8 +101,10 @@ const generic_fuzz_config predefined_configs[] = {
 },{
 .name = "intel-hda",
 .args = "-machine q35 -nodefaults -device intel-hda,id=hda0 "
-"-device hda-output,bus=hda0.0 -device hda-micro,bus=hda0.0 "
-"-device hda-duplex,bus=hda0.0",
+"-audiodev driver=none,id=audio0",
+"-device hda-output,bus=hda0.0,audiodev=audio0 "
+"-device hda-micro,bus=hda0.0,audiodev=audio0 "
+"-device hda-duplex,bus=hda0.0,audiodev=audio0",
 .objects = "intel-hda",
 },{
 .name = "ide-hd",
diff --git a/tests/qtest/intel-hda-test.c b/tests/qtest/intel-hda-test.c
index a58c98e4d11b..39ced2bc6ac6 100644
--- a/tests/qtest/intel-hda-test.c
+++ b/tests/qtest/intel-hda-test.c
@@ -11,20 +11,24 @@
 #include "libqtest-single.h"

 #define HDA_ID "hda0"
-#define CODEC_DEVICES " -device hda-output,bus=" HDA_ID ".0" \
-  " -device hda-micro,bus=" HDA_ID ".0" \
-  " -device hda-duplex,bus=" HDA_ID ".0"
+#define AUDIODEV " -audiodev driver=none,id=audio0 "
+#define AUDIODEV_REF "audiodev=audio0"
+#define CODEC_DEVICES " -device hda-output,bus=" HDA_ID ".0," AUDIODEV_REF \
+  " -device hda-micro,bus=" HDA_ID ".0," AUDIODEV_REF \
+  " -device hda-duplex,bus=" HDA_ID ".0," AUDIODEV_REF

 /* Tests only initialization so far. TODO: Replace with functional tests */
 static void ich6_test(void)
 {
-qtest_start("-device intel-hda,id=" HDA_ID CODEC_DEVICES);
+qtest_start(AUDIODEV "-device intel-hda,id=" HDA_ID CODEC_DEVICES);
 qtest_end();
 }

 static void ich9_test(void)
 {
-qtest_start("-machine q35 -device ich9-intel-hda,bus=pcie.0,addr=1b.0,id="
+qtest_start("-machine q35"
+AUDIODEV
+"-device ich9-intel-hda,bus=pcie.0,addr=1b.0,id="
 HDA_ID CODEC_DEVICES);
 qtest_end();
 }
@@ -39,6 +43,7 @@ static void test_issue542_ich6(void)
 QTestState *s;

 s = qtest_init("-nographic -nodefaults -M pc-q35-6.2 "
+   AUDIODEV
"-device intel-hda,id=" HDA_ID CODEC_DEVICES);

 qtest_outl(s, 0xcf8, 0x8804);
--
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 :|



signature.asc
Description: PGP signature

[PATCH 18/18] audio/spiceaudio: Fail initialisation when not using spice

[Martin Kletzander]

The caller would already fail, but this way the message can better
express the reason for the failure.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2043498

Signed-off-by: Martin Kletzander 
---
 audio/spiceaudio.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/audio/spiceaudio.c b/audio/spiceaudio.c
index a8d370fe6f31..fdbd7dc285ad 100644
--- a/audio/spiceaudio.c
+++ b/audio/spiceaudio.c
@@ -74,8 +74,9 @@ static const SpiceRecordInterface record_sif = {
 static void *spice_audio_init(Audiodev *dev)
 {
 if (!using_spice) {
-return NULL;
+error_setg(_fatal, "Cannot use spice audio without -spice");
 }
+
 return _audio_init;
 }
 
-- 
2.35.1

[PATCH 05/18] tests/qtest: Specify audiodev= and -audiodev

[Martin Kletzander]

This will enable removing deprecated default audiodev support.

I did not figure out how to make the audiodev represented as an
interface node, so this is a workaround.  I am not sure what would be
the proper way.

Signed-off-by: Martin Kletzander 
---
 tests/qtest/ac97-test.c |  3 ++-
 tests/qtest/es1370-test.c   |  3 ++-
 tests/qtest/fuzz/generic_fuzz_configs.h |  6 --
 tests/qtest/intel-hda-test.c| 15 ++-
 4 files changed, 18 insertions(+), 9 deletions(-)

diff --git a/tests/qtest/ac97-test.c b/tests/qtest/ac97-test.c
index e09f2495d24d..9711f1f6d966 100644
--- a/tests/qtest/ac97-test.c
+++ b/tests/qtest/ac97-test.c
@@ -45,7 +45,8 @@ static void *ac97_create(void *pci_bus, QGuestAllocator 
*alloc, void *addr)
 static void ac97_register_nodes(void)
 {
 QOSGraphEdgeOptions opts = {
-.extra_device_opts = "addr=04.0",
+.extra_device_opts = "addr=04.0,audiodev=audio0",
+.before_cmd_line = "-audiodev driver=none,id=audio0",
 };
 add_qpci_address(, &(QPCIAddress) { .devfn = QPCI_DEVFN(4, 0) });
 
diff --git a/tests/qtest/es1370-test.c b/tests/qtest/es1370-test.c
index 2fd7fd2d3d30..5facda8d0d8d 100644
--- a/tests/qtest/es1370-test.c
+++ b/tests/qtest/es1370-test.c
@@ -46,7 +46,8 @@ static void *es1370_create(void *pci_bus, QGuestAllocator 
*alloc, void *addr)
 static void es1370_register_nodes(void)
 {
 QOSGraphEdgeOptions opts = {
-.extra_device_opts = "addr=04.0",
+.extra_device_opts = "addr=04.0,audiodev=audio0",
+.before_cmd_line = "-audiodev driver=none,id=audio0",
 };
 add_qpci_address(, &(QPCIAddress) { .devfn = QPCI_DEVFN(4, 0) });
 
diff --git a/tests/qtest/fuzz/generic_fuzz_configs.h 
b/tests/qtest/fuzz/generic_fuzz_configs.h
index 004c701915e1..84a93b3c350b 100644
--- a/tests/qtest/fuzz/generic_fuzz_configs.h
+++ b/tests/qtest/fuzz/generic_fuzz_configs.h
@@ -101,8 +101,10 @@ const generic_fuzz_config predefined_configs[] = {
 },{
 .name = "intel-hda",
 .args = "-machine q35 -nodefaults -device intel-hda,id=hda0 "
-"-device hda-output,bus=hda0.0 -device hda-micro,bus=hda0.0 "
-"-device hda-duplex,bus=hda0.0",
+"-audiodev driver=none,id=audio0",
+"-device hda-output,bus=hda0.0,audiodev=audio0 "
+"-device hda-micro,bus=hda0.0,audiodev=audio0 "
+"-device hda-duplex,bus=hda0.0,audiodev=audio0",
 .objects = "intel-hda",
 },{
 .name = "ide-hd",
diff --git a/tests/qtest/intel-hda-test.c b/tests/qtest/intel-hda-test.c
index a58c98e4d11b..39ced2bc6ac6 100644
--- a/tests/qtest/intel-hda-test.c
+++ b/tests/qtest/intel-hda-test.c
@@ -11,20 +11,24 @@
 #include "libqtest-single.h"
 
 #define HDA_ID "hda0"
-#define CODEC_DEVICES " -device hda-output,bus=" HDA_ID ".0" \
-  " -device hda-micro,bus=" HDA_ID ".0" \
-  " -device hda-duplex,bus=" HDA_ID ".0"
+#define AUDIODEV " -audiodev driver=none,id=audio0 "
+#define AUDIODEV_REF "audiodev=audio0"
+#define CODEC_DEVICES " -device hda-output,bus=" HDA_ID ".0," AUDIODEV_REF \
+  " -device hda-micro,bus=" HDA_ID ".0," AUDIODEV_REF \
+  " -device hda-duplex,bus=" HDA_ID ".0," AUDIODEV_REF
 
 /* Tests only initialization so far. TODO: Replace with functional tests */
 static void ich6_test(void)
 {
-qtest_start("-device intel-hda,id=" HDA_ID CODEC_DEVICES);
+qtest_start(AUDIODEV "-device intel-hda,id=" HDA_ID CODEC_DEVICES);
 qtest_end();
 }
 
 static void ich9_test(void)
 {
-qtest_start("-machine q35 -device ich9-intel-hda,bus=pcie.0,addr=1b.0,id="
+qtest_start("-machine q35"
+AUDIODEV
+"-device ich9-intel-hda,bus=pcie.0,addr=1b.0,id="
 HDA_ID CODEC_DEVICES);
 qtest_end();
 }
@@ -39,6 +43,7 @@ static void test_issue542_ich6(void)
 QTestState *s;
 
 s = qtest_init("-nographic -nodefaults -M pc-q35-6.2 "
+   AUDIODEV
"-device intel-hda,id=" HDA_ID CODEC_DEVICES);
 
 qtest_outl(s, 0xcf8, 0x8804);
-- 
2.35.1

[PATCH 12/18] hw/ppc: Support machine-default audiodev with fallback

[Martin Kletzander]

Signed-off-by: Martin Kletzander 
---
 hw/ppc/prep.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c
index bf622aa38fab..52d801afb307 100644
--- a/hw/ppc/prep.c
+++ b/hw/ppc/prep.c
@@ -46,6 +46,7 @@
 #include "elf.h"
 #include "qemu/units.h"
 #include "kvm_ppc.h"
+#include "audio/audio.h"
 
 /* SMP is not enabled, for now */
 #define MAX_CPUS 1
@@ -304,6 +305,9 @@ static void ibm_40p_init(MachineState *machine)
 dev = DEVICE(isa_dev);
 qdev_prop_set_uint32(dev, "iobase", 0x830);
 qdev_prop_set_uint32(dev, "irq", 10);
+
+qdev_prop_set_string(dev, "audiodev",
+ audio_maybe_init_dummy("ppc.defaudio"));
 isa_realize_and_unref(isa_dev, isa_bus, _fatal);
 
 isa_dev = isa_new("pc87312");
-- 
2.35.1

[PATCH 16/18] audio: Remove legacy audio environment variables and options

[Martin Kletzander]

Signed-off-by: Martin Kletzander 
---
 audio/audio.c   |  13 -
 audio/audio.h   |   1 -
 audio/audio_legacy.c| 555 
 audio/meson.build   |   1 -
 docs/about/deprecated.rst   |   7 -
 docs/about/removed-features.rst |   9 +
 qemu-options.hx |  10 -
 softmmu/vl.c|   4 -
 8 files changed, 9 insertions(+), 591 deletions(-)
 delete mode 100644 audio/audio_legacy.c

diff --git a/audio/audio.c b/audio/audio.c
index c944cf817cf9..b3ecc8fa6508 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -49,19 +49,6 @@
 
 #define SW_NAME(sw) (sw)->name ? (sw)->name : "unknown"
 
-
-/* Order of CONFIG_AUDIO_DRIVERS is import.
-   The 1st one is the one used by default, that is the reason
-that we generate the list.
-*/
-const char *audio_prio_list[] = {
-"spice",
-CONFIG_AUDIO_DRIVERS
-"none",
-"wav",
-NULL
-};
-
 static QLIST_HEAD(, audio_driver) audio_drivers;
 static AudiodevListHead audiodevs = QSIMPLEQ_HEAD_INITIALIZER(audiodevs);
 
diff --git a/audio/audio.h b/audio/audio.h
index 9deed8ed6830..ae10a7f0fa81 100644
--- a/audio/audio.h
+++ b/audio/audio.h
@@ -170,7 +170,6 @@ void audio_sample_from_uint64(void *samples, int pos,
 
 void audio_parse_option(const char *opt);
 void audio_init_audiodevs(void);
-void audio_legacy_help(void);
 
 AudioState *audio_state_by_name(const char *name);
 const char *audio_get_id(QEMUSoundCard *card);
diff --git a/audio/audio_legacy.c b/audio/audio_legacy.c
deleted file mode 100644
index 595949f52cd4..
--- a/audio/audio_legacy.c
+++ /dev/null
@@ -1,555 +0,0 @@
-/*
- * QEMU Audio subsystem: legacy configuration handling
- *
- * Copyright (c) 2015-2019 Zoltán Kővágó 
- *
- * Permission is hereby granted, free of charge, to any person obtaining a copy
- * of this software and associated documentation files (the "Software"), to 
deal
- * in the Software without restriction, including without limitation the rights
- * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
- * copies of the Software, and to permit persons to whom the Software is
- * furnished to do so, subject to the following conditions:
- *
- * The above copyright notice and this permission notice shall be included in
- * all copies or substantial portions of the Software.
- *
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
- * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
- * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
- * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
- * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
FROM,
- * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
- * THE SOFTWARE.
- */
-#include "qemu/osdep.h"
-#include "audio.h"
-#include "audio_int.h"
-#include "qemu/cutils.h"
-#include "qemu/timer.h"
-#include "qapi/error.h"
-#include "qapi/qapi-visit-audio.h"
-#include "qapi/visitor-impl.h"
-
-#define AUDIO_CAP "audio-legacy"
-#include "audio_int.h"
-
-static uint32_t toui32(const char *str)
-{
-unsigned long long ret;
-if (parse_uint_full(str, , 10) || ret > UINT32_MAX) {
-dolog("Invalid integer value `%s'\n", str);
-exit(1);
-}
-return ret;
-}
-
-/* helper functions to convert env variables */
-static void get_bool(const char *env, bool *dst, bool *has_dst)
-{
-const char *val = getenv(env);
-if (val) {
-*dst = toui32(val) != 0;
-*has_dst = true;
-}
-}
-
-static void get_int(const char *env, uint32_t *dst, bool *has_dst)
-{
-const char *val = getenv(env);
-if (val) {
-*dst = toui32(val);
-*has_dst = true;
-}
-}
-
-static void get_str(const char *env, char **dst, bool *has_dst)
-{
-const char *val = getenv(env);
-if (val) {
-if (*has_dst) {
-g_free(*dst);
-}
-*dst = g_strdup(val);
-*has_dst = true;
-}
-}
-
-static void get_fmt(const char *env, AudioFormat *dst, bool *has_dst)
-{
-const char *val = getenv(env);
-if (val) {
-size_t i;
-for (i = 0; AudioFormat_lookup.size; ++i) {
-if (strcasecmp(val, AudioFormat_lookup.array[i]) == 0) {
-*dst = i;
-*has_dst = true;
-return;
-}
-}
-
-dolog("Invalid audio format `%s'\n", val);
-exit(1);
-}
-}
-
-
-static void get_millis_to_usecs(const char *env, uint32_t *dst, bool *has_dst)
-{
-const char *val = getenv(env);
-if (val) {
-*dst = toui32(val) * 1000;
-*has_dst = true;
-}
-}
-
-static uint32_t frames_to_usecs(uint32_t frames,
-   

[PATCH 11/18] hw/arm: Support machine-default audiodev with fallback

[Martin Kletzander]

Signed-off-by: Martin Kletzander 
---
 hw/arm/integratorcp.c |  8 +++-
 hw/arm/musicpal.c |  8 +++-
 hw/arm/omap2.c|  8 
 hw/arm/realview.c |  3 +++
 hw/arm/spitz.c| 10 +++---
 hw/arm/versatilepb.c  |  3 +++
 hw/arm/vexpress.c |  3 +++
 hw/arm/xlnx-zcu102.c  |  4 
 hw/arm/z2.c   | 12 +++-
 9 files changed, 53 insertions(+), 6 deletions(-)

diff --git a/hw/arm/integratorcp.c b/hw/arm/integratorcp.c
index b109ece3ae02..0a6d4186dad6 100644
--- a/hw/arm/integratorcp.c
+++ b/hw/arm/integratorcp.c
@@ -27,6 +27,7 @@
 #include "hw/irq.h"
 #include "hw/sd/sd.h"
 #include "qom/object.h"
+#include "audio/audio.h"
 
 #define TYPE_INTEGRATOR_CM "integrator_core"
 OBJECT_DECLARE_SIMPLE_TYPE(IntegratorCMState, INTEGRATOR_CM)
@@ -660,7 +661,12 @@ static void integratorcp_init(MachineState *machine)
_fatal);
 }
 
-sysbus_create_varargs("pl041", 0x1d00, pic[25], NULL);
+dev = qdev_new("pl041");
+qdev_prop_set_string(dev, "audiodev",
+ audio_maybe_init_dummy("integrator.defaudio"));
+sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), _fatal);
+sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, 0x1d00);
+sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, pic[25]);
 
 if (nd_table[0].used)
 smc91c111_init(_table[0], 0xc800, pic[27]);
diff --git a/hw/arm/musicpal.c b/hw/arm/musicpal.c
index 7c840fb4283e..5d53ed3a8709 100644
--- a/hw/arm/musicpal.c
+++ b/hw/arm/musicpal.c
@@ -35,6 +35,7 @@
 #include "qemu/cutils.h"
 #include "qom/object.h"
 #include "hw/net/mv88w8618_eth.h"
+#include "audio/audio.h"
 
 #define MP_MISC_BASE0x80002000
 #define MP_MISC_SIZE0x1000
@@ -1324,7 +1325,12 @@ static void musicpal_init(MachineState *machine)
 qdev_connect_gpio_out(key_dev, i, qdev_get_gpio_in(dev, i + 15));
 }
 
-wm8750_dev = i2c_slave_create_simple(i2c, TYPE_WM8750, MP_WM_ADDR);
+wm8750_dev = i2c_slave_new(TYPE_WM8750, MP_WM_ADDR);
+
+qdev_prop_set_string(DEVICE(wm8750_dev), "audiodev",
+ audio_maybe_init_dummy("musicpal.defaudio"));
+i2c_slave_realize_and_unref(wm8750_dev, i2c, _abort);
+
 dev = qdev_new(TYPE_MV88W8618_AUDIO);
 s = SYS_BUS_DEVICE(dev);
 object_property_set_link(OBJECT(dev), "wm8750", OBJECT(wm8750_dev),
diff --git a/hw/arm/omap2.c b/hw/arm/omap2.c
index 02b1aa8c974b..4ae524a1a1a6 100644
--- a/hw/arm/omap2.c
+++ b/hw/arm/omap2.c
@@ -604,12 +604,20 @@ static struct omap_eac_s *omap_eac_init(struct 
omap_target_agent_s *ta,
 qemu_irq irq, qemu_irq *drq, omap_clk fclk, omap_clk iclk)
 {
 struct omap_eac_s *s = g_new0(struct omap_eac_s, 1);
+const char *audiodev_id = audio_maybe_init_dummy("eac.defaudio");
 
 s->irq = irq;
 s->codec.rxdrq = *drq ++;
 s->codec.txdrq = *drq;
 omap_eac_reset(s);
 
+s->codec.card.name = g_strdup(audiodev_id);
+s->codec.card.state = audio_state_by_name(s->codec.card.name);
+if (!s->codec.card.state) {
+error_setg(_fatal, "Cannot find audiodev with id '%s'",
+   s->codec.card.name);
+}
+
 AUD_register_card("OMAP EAC", >codec.card);
 
 memory_region_init_io(>iomem, NULL, _eac_ops, s, "omap.eac",
diff --git a/hw/arm/realview.c b/hw/arm/realview.c
index d2dc8a895258..4fcdd821789a 100644
--- a/hw/arm/realview.c
+++ b/hw/arm/realview.c
@@ -29,6 +29,7 @@
 #include "hw/irq.h"
 #include "hw/i2c/arm_sbcon_i2c.h"
 #include "hw/sd/sd.h"
+#include "audio/audio.h"
 
 #define SMP_BOOT_ADDR 0xe000
 #define SMP_BOOTREG_ADDR 0x1030
@@ -207,6 +208,8 @@ static void realview_init(MachineState *machine,
 
 pl041 = qdev_new("pl041");
 qdev_prop_set_uint32(pl041, "nc_fifo_depth", 512);
+qdev_prop_set_string(pl041, "audiodev",
+ audio_maybe_init_dummy("realview.defaudio"));
 sysbus_realize_and_unref(SYS_BUS_DEVICE(pl041), _fatal);
 sysbus_mmio_map(SYS_BUS_DEVICE(pl041), 0, 0x10004000);
 sysbus_connect_irq(SYS_BUS_DEVICE(pl041), 0, pic[19]);
diff --git a/hw/arm/spitz.c b/hw/arm/spitz.c
index 5aab0b856574..94fff2f460ca 100644
--- a/hw/arm/spitz.c
+++ b/hw/arm/spitz.c
@@ -35,6 +35,7 @@
 #include "exec/address-spaces.h"
 #include "cpu.h"
 #include "qom/object.h"
+#include "audio/audio.h"
 
 enum spitz_model_e { spitz, akita, borzoi, terrier };
 
@@ -779,10 +780,13 @@ static void spitz_i2c_setup(PXA2xxState *cpu)
 /* Attach the CPU on one end of our I2C bus.  */
 I2CBus *bus = pxa2xx_i2c_bus(cpu->i2c[0]);
 
-DeviceState *wm;
-
 /* Attach a WM8750 to the bus */
-wm =

[PATCH 10/18] hw/input/tsc210x.c: Support machine-default audiodev with fallback

[Martin Kletzander]

Signed-off-by: Martin Kletzander 
---
 hw/input/tsc210x.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/hw/input/tsc210x.c b/hw/input/tsc210x.c
index f16a8090b7c7..f0b02bc72280 100644
--- a/hw/input/tsc210x.c
+++ b/hw/input/tsc210x.c
@@ -1098,6 +1098,14 @@ static void tsc210x_init(TSC210xState *s,
 
 qemu_add_mouse_event_handler(tsc210x_touchscreen_event, s, 1, name);
 
+const char *audiodev_id = audio_maybe_init_dummy("tsc.defaudio");
+s->card.name = g_strdup(audiodev_id);
+s->card.state = audio_state_by_name(s->card.name);
+if (!s->card.state) {
+error_setg(_fatal, "Cannot find audiodev with id '%s'",
+   s->card.name);
+}
+
 AUD_register_card(s->name, >card);
 
 qemu_register_reset((void *) tsc210x_reset, s);
-- 
2.35.1

[PATCH 13/18] audio: Make AUD_register_card fallible and require audiodev=

[Martin Kletzander]

Now that all callers support error reporting with errp and all machine-default
devices use an explicit audiodev, this can be changed.  To make the detection
easier make AUD_register_card() return false on error.

Signed-off-by: Martin Kletzander 
---
 audio/audio.c| 7 +--
 audio/audio.h| 2 +-
 hw/arm/omap2.c   | 3 ++-
 hw/audio/ac97.c  | 6 +-
 hw/audio/adlib.c | 7 +--
 hw/audio/cs4231a.c   | 6 --
 hw/audio/es1370.c| 5 -
 hw/audio/gus.c   | 4 +++-
 hw/audio/hda-codec.c | 5 -
 hw/audio/lm4549.c| 4 +++-
 hw/audio/pcspk.c | 4 +++-
 hw/audio/sb16.c  | 6 --
 hw/audio/wm8750.c| 5 -
 hw/display/xlnx_dp.c | 6 --
 hw/input/tsc210x.c   | 3 ++-
 hw/usb/dev-audio.c   | 5 -
 16 files changed, 57 insertions(+), 21 deletions(-)

diff --git a/audio/audio.c b/audio/audio.c
index 671845c65d18..b95aca444382 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -1822,15 +1822,18 @@ void audio_free_audiodev_list(AudiodevListHead *head)
 }
 }
 
-void AUD_register_card (const char *name, QEMUSoundCard *card)
+bool AUD_register_card (const char *name, QEMUSoundCard *card, Error **errp)
 {
 if (!card->state) {
-card->state = audio_init(NULL, name);
+error_setg(errp, "No audiodev specified for %s", name);
+return false;
 }
 
 card->name = g_strdup (name);
 memset (>entries, 0, sizeof (card->entries));
 QLIST_INSERT_HEAD(>state->card_head, card, entries);
+
+return true;
 }
 
 void AUD_remove_card (QEMUSoundCard *card)
diff --git a/audio/audio.h b/audio/audio.h
index 335704a4ddb1..9deed8ed6830 100644
--- a/audio/audio.h
+++ b/audio/audio.h
@@ -94,7 +94,7 @@ typedef struct QEMUAudioTimeStamp {
 void AUD_vlog (const char *cap, const char *fmt, va_list ap) G_GNUC_PRINTF(2, 
0);
 void AUD_log (const char *cap, const char *fmt, ...) G_GNUC_PRINTF(2, 3);
 
-void AUD_register_card (const char *name, QEMUSoundCard *card);
+bool AUD_register_card (const char *name, QEMUSoundCard *card, Error **errp);
 void AUD_remove_card (QEMUSoundCard *card);
 CaptureVoiceOut *AUD_add_capture(
 AudioState *s,
diff --git a/hw/arm/omap2.c b/hw/arm/omap2.c
index 4ae524a1a1a6..407c24551c84 100644
--- a/hw/arm/omap2.c
+++ b/hw/arm/omap2.c
@@ -618,7 +618,8 @@ static struct omap_eac_s *omap_eac_init(struct 
omap_target_agent_s *ta,
s->codec.card.name);
 }
 
-AUD_register_card("OMAP EAC", >codec.card);
+/* We can quit here because this only gets called from machine class init 
*/
+AUD_register_card("OMAP EAC", >codec.card, _fatal);
 
 memory_region_init_io(>iomem, NULL, _eac_ops, s, "omap.eac",
   omap_l4_region_size(ta, 0));
diff --git a/hw/audio/ac97.c b/hw/audio/ac97.c
index fd0b3b97d5b5..8242ddb0f93d 100644
--- a/hw/audio/ac97.c
+++ b/hw/audio/ac97.c
@@ -1345,6 +1345,10 @@ static void ac97_realize(PCIDevice *dev, Error **errp)
 AC97LinkState *s = AC97(dev);
 uint8_t *c = s->dev.config;
 
+if (!AUD_register_card ("ac97", >card, errp)) {
+return;
+}
+
 /* TODO: no need to override */
 c[PCI_COMMAND] = 0x00;  /* pcicmd pci command rw, ro */
 c[PCI_COMMAND + 1] = 0x00;
@@ -1378,7 +1382,7 @@ static void ac97_realize(PCIDevice *dev, Error **errp)
"ac97-nabm", 256);
 pci_register_bar (>dev, 0, PCI_BASE_ADDRESS_SPACE_IO, >io_nam);
 pci_register_bar (>dev, 1, PCI_BASE_ADDRESS_SPACE_IO, >io_nabm);
-AUD_register_card ("ac97", >card);
+
 ac97_on_reset(DEVICE(s));
 }
 
diff --git a/hw/audio/adlib.c b/hw/audio/adlib.c
index ba1be6c8378d..39932654f7f5 100644
--- a/hw/audio/adlib.c
+++ b/hw/audio/adlib.c
@@ -269,8 +269,6 @@ static void adlib_realizefn (DeviceState *dev, Error **errp)
 as.fmt = AUDIO_FORMAT_S16;
 as.endianness = AUDIO_HOST_ENDIANNESS;
 
-AUD_register_card ("adlib", >card);
-
 s->voice = AUD_open_out (
 >card,
 s->voice,
@@ -285,6 +283,11 @@ static void adlib_realizefn (DeviceState *dev, Error 
**errp)
 return;
 }
 
+if (!AUD_register_card ("adlib", >card, errp)) {
+Adlib_fini (s);
+return;
+}
+
 s->samples = AUD_get_buffer_size_out (s->voice) >> SHIFT;
 s->mixbuf = g_malloc0 (s->samples << SHIFT);
 
diff --git a/hw/audio/cs4231a.c b/hw/audio/cs4231a.c
index f510b862efbe..d9353a51ec66 100644
--- a/hw/audio/cs4231a.c
+++ b/hw/audio/cs4231a.c
@@ -676,13 +676,15 @@ static void cs4231a_realizefn (DeviceState *dev, Error 
**errp)
 return;
 }
 
+if (!AUD_register_card ("cs4231a", >card, errp)) {
+return;
+}
+
 s->pic = isa_get_irq(d, s->irq);
 k = ISADMA_GET_CLASS(s->isa_dma);
 k->register_channel(s->isa_dma, s->dma, cs_dma_read, s);
 
 isa_register

[PATCH 03/18] hw/audio: Simplify hda audio init

[Martin Kletzander]

No return values are used anywhere, so switch the functions to be void
and add support for error reporting using errp for use in next patches.

Signed-off-by: Martin Kletzander 
---
 hw/audio/hda-codec.c | 32 ++--
 hw/audio/intel-hda.c |  4 +---
 hw/audio/intel-hda.h |  2 +-
 3 files changed, 20 insertions(+), 18 deletions(-)

diff --git a/hw/audio/hda-codec.c b/hw/audio/hda-codec.c
index feb8f9e2bb7a..e86a2adf31a0 100644
--- a/hw/audio/hda-codec.c
+++ b/hw/audio/hda-codec.c
@@ -676,7 +676,9 @@ static void hda_audio_stream(HDACodecDevice *hda, uint32_t 
stnr, bool running, b
 }
 }
 
-static int hda_audio_init(HDACodecDevice *hda, const struct desc_codec *desc)
+static void hda_audio_init(HDACodecDevice *hda,
+   const struct desc_codec *desc,
+   Error **errp)
 {
 HDAAudioState *a = HDA_AUDIO(hda);
 HDAAudioStream *st;
@@ -719,7 +721,6 @@ static int hda_audio_init(HDACodecDevice *hda, const struct 
desc_codec *desc)
 break;
 }
 }
-return 0;
 }
 
 static void hda_audio_exit(HDACodecDevice *hda)
@@ -849,37 +850,40 @@ static Property hda_audio_properties[] = {
 DEFINE_PROP_END_OF_LIST(),
 };
 
-static int hda_audio_init_output(HDACodecDevice *hda)
+static void hda_audio_init_output(HDACodecDevice *hda, Error **errp)
 {
 HDAAudioState *a = HDA_AUDIO(hda);
+const struct desc_codec *desc = _nomixemu;
 
 if (!a->mixer) {
-return hda_audio_init(hda, _nomixemu);
-} else {
-return hda_audio_init(hda, _mixemu);
+desc = _mixemu;
 }
+
+hda_audio_init(hda, desc, errp);
 }
 
-static int hda_audio_init_duplex(HDACodecDevice *hda)
+static void hda_audio_init_duplex(HDACodecDevice *hda, Error **errp)
 {
 HDAAudioState *a = HDA_AUDIO(hda);
+const struct desc_codec *desc = _nomixemu;
 
 if (!a->mixer) {
-return hda_audio_init(hda, _nomixemu);
-} else {
-return hda_audio_init(hda, _mixemu);
+desc = _mixemu;
 }
+
+hda_audio_init(hda, desc, errp);
 }
 
-static int hda_audio_init_micro(HDACodecDevice *hda)
+static void hda_audio_init_micro(HDACodecDevice *hda, Error **errp)
 {
 HDAAudioState *a = HDA_AUDIO(hda);
+const struct desc_codec *desc = _nomixemu;
 
 if (!a->mixer) {
-return hda_audio_init(hda, _nomixemu);
-} else {
-return hda_audio_init(hda, _mixemu);
+desc = _mixemu;
 }
+
+hda_audio_init(hda, desc, errp);
 }
 
 static void hda_audio_base_class_init(ObjectClass *klass, void *data)
diff --git a/hw/audio/intel-hda.c b/hw/audio/intel-hda.c
index e77552363a4c..a17002812240 100644
--- a/hw/audio/intel-hda.c
+++ b/hw/audio/intel-hda.c
@@ -70,9 +70,7 @@ static void hda_codec_dev_realize(DeviceState *qdev, Error 
**errp)
 return;
 }
 bus->next_cad = dev->cad + 1;
-if (cdc->init(dev) != 0) {
-error_setg(errp, "HDA audio init failed");
-}
+cdc->init(dev, errp);
 }
 
 static void hda_codec_dev_unrealize(DeviceState *qdev)
diff --git a/hw/audio/intel-hda.h b/hw/audio/intel-hda.h
index f78c1833e341..8d710eee5d66 100644
--- a/hw/audio/intel-hda.h
+++ b/hw/audio/intel-hda.h
@@ -31,7 +31,7 @@ struct HDACodecBus {
 struct HDACodecDeviceClass {
 DeviceClass parent_class;
 
-int (*init)(HDACodecDevice *dev);
+void (*init)(HDACodecDevice *dev, Error **errp);
 void (*exit)(HDACodecDevice *dev);
 void (*command)(HDACodecDevice *dev, uint32_t nid, uint32_t data);
 void (*stream)(HDACodecDevice *dev, uint32_t stnr, bool running, bool 
output);
-- 
2.35.1

[PATCH 08/18] audio: Add easy dummy audio initialiser

[Martin Kletzander]

In case of sound devices which are created by default (some of them even with
-nodefaults) it would not be nice to users if qemu required explicit:

  -audiodev driver=none,id=audio0 -machine default-audiodev=audio0

when they just want to run a VM and not care about any audio output.  Instead
this little helper makes it easy to create a dummy audiodev (driver=none) in
case there is no default-audiodev specified for the machine.  To make sure users
are not surprised by no sound output a helping message is also printed out.

Signed-off-by: Martin Kletzander 
---
 audio/audio.c | 34 ++
 audio/audio.h |  2 ++
 2 files changed, 36 insertions(+)

diff --git a/audio/audio.c b/audio/audio.c
index 9e91a5a4f2b8..671845c65d18 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -37,6 +37,7 @@
 #include "sysemu/runstate.h"
 #include "ui/qemu-spice.h"
 #include "trace.h"
+#include "hw/boards.h"
 
 #define AUDIO_CAP "audio"
 #include "audio_int.h"
@@ -2122,6 +2123,39 @@ void audio_init_audiodevs(void)
 }
 }
 
+static void audio_init_dummy(const char *id)
+{
+Audiodev *dev = g_new0(Audiodev, 1);
+AudiodevListEntry *e = g_new0(AudiodevListEntry, 1);
+
+dev->driver = AUDIODEV_DRIVER_NONE;
+dev->id = g_strdup(id);
+
+audio_validate_opts(dev, _abort);
+audio_init(dev, NULL);
+
+e->dev = dev;
+QSIMPLEQ_INSERT_TAIL(, e, next);
+}
+
+const char *audio_maybe_init_dummy(const char *default_id)
+{
+MachineState *ms = MACHINE(qdev_get_machine());
+
+if (ms->default_audiodev) {
+return ms->default_audiodev;
+}
+
+dolog("warning: No audiodev specified for implicit machine devices, "
+  "no audio output will be available for these. "
+  "For setting a backend audiodev for such devices try using "
+  "the default-audiodev machine option.\n");
+
+audio_init_dummy(default_id);
+
+return default_id;
+}
+
 audsettings audiodev_to_audsettings(AudiodevPerDirectionOptions *pdo)
 {
 return (audsettings) {
diff --git a/audio/audio.h b/audio/audio.h
index 3d5ecdecd5c1..335704a4ddb1 100644
--- a/audio/audio.h
+++ b/audio/audio.h
@@ -175,6 +175,8 @@ void audio_legacy_help(void);
 AudioState *audio_state_by_name(const char *name);
 const char *audio_get_id(QEMUSoundCard *card);
 
+const char *audio_maybe_init_dummy(const char *default_id);
+
 #define DEFINE_AUDIO_PROPERTIES(_s, _f) \
 DEFINE_PROP_AUDIODEV("audiodev", _s, _f)
 
-- 
2.35.1

[PATCH 15/18] audio: Be more strict during audio backend initialisation

[Martin Kletzander]

Now that audiodev= is required and audio_init() will not be called
without and AudioDev we can remove the fallback functionality and error
out in case audio drivers fail initialisation or when the driver does
not exist.

Signed-off-by: Martin Kletzander 
---
 audio/audio.c   | 146 ++--
 docs/about/deprecated.rst   |   8 --
 docs/about/removed-features.rst |   8 ++
 3 files changed, 34 insertions(+), 128 deletions(-)

diff --git a/audio/audio.c b/audio/audio.c
index 97eb645764c1..c944cf817cf9 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -87,6 +87,8 @@ audio_driver *audio_driver_lookup(const char *name)
 }
 }
 
+error_setg(_fatal, "Unknown audio driver `%s'", name);
+/* Avoid compiler complaining that we do not return in non-void function */
 return NULL;
 }
 
@@ -104,8 +106,6 @@ const struct mixeng_volume nominal_volume = {
 #endif
 };
 
-static bool legacy_config = true;
-
 int audio_bug (const char *funcname, int cond)
 {
 if (cond) {
@@ -1532,31 +1532,27 @@ size_t audio_generic_read(HWVoiceIn *hw, void *buf, 
size_t size)
 return total;
 }
 
-static int audio_driver_init(AudioState *s, struct audio_driver *drv,
- bool msg, Audiodev *dev)
+static void audio_driver_init(AudioState *s, struct audio_driver *drv,
+  Audiodev *dev)
 {
 s->drv_opaque = drv->init(dev);
 
-if (s->drv_opaque) {
-if (!drv->pcm_ops->get_buffer_in) {
-drv->pcm_ops->get_buffer_in = audio_generic_get_buffer_in;
-drv->pcm_ops->put_buffer_in = audio_generic_put_buffer_in;
-}
-if (!drv->pcm_ops->get_buffer_out) {
-drv->pcm_ops->get_buffer_out = audio_generic_get_buffer_out;
-drv->pcm_ops->put_buffer_out = audio_generic_put_buffer_out;
-}
+if (!s->drv_opaque) {
+error_setg(_fatal, "Could not init `%s' audio driver", 
drv->name);
+}
 
-audio_init_nb_voices_out(s, drv);
-audio_init_nb_voices_in(s, drv);
-s->drv = drv;
-return 0;
-} else {
-if (msg) {
-dolog("Could not init `%s' audio driver\n", drv->name);
-}
-return -1;
+if (!drv->pcm_ops->get_buffer_in) {
+drv->pcm_ops->get_buffer_in = audio_generic_get_buffer_in;
+drv->pcm_ops->put_buffer_in = audio_generic_put_buffer_in;
 }
+if (!drv->pcm_ops->get_buffer_out) {
+drv->pcm_ops->get_buffer_out = audio_generic_get_buffer_out;
+drv->pcm_ops->put_buffer_out = audio_generic_put_buffer_out;
+}
+
+audio_init_nb_voices_out(s, drv);
+audio_init_nb_voices_in(s, drv);
+s->drv = drv;
 }
 
 static void audio_vm_change_state_handler (void *opaque, bool running,
@@ -1661,79 +1657,19 @@ static const VMStateDescription vmstate_audio = {
 
 static void audio_validate_opts(Audiodev *dev, Error **errp);
 
-static AudiodevListEntry *audiodev_find(
-AudiodevListHead *head, const char *drvname)
-{
-AudiodevListEntry *e;
-QSIMPLEQ_FOREACH(e, head, next) {
-if (strcmp(AudiodevDriver_str(e->dev->driver), drvname) == 0) {
-return e;
-}
-}
-
-return NULL;
-}
-
-/*
- * if we have dev, this function was called because of an -audiodev argument =>
- *   initialize a new state with it
- * if dev == NULL => legacy implicit initialization, return the already created
- *   state or create a new one
- */
-static AudioState *audio_init(Audiodev *dev, const char *name)
+static AudioState *audio_init(Audiodev *dev)
 {
 static bool atexit_registered;
-size_t i;
-int done = 0;
 const char *drvname = NULL;
 VMChangeStateEntry *e;
 AudioState *s;
-struct audio_driver *driver;
-/* silence gcc warning about uninitialized variable */
-AudiodevListHead head = QSIMPLEQ_HEAD_INITIALIZER(head);
-
-if (using_spice) {
-/*
- * When using spice allow the spice audio driver being picked
- * as default.
- *
- * Temporary hack.  Using audio devices without explicit
- * audiodev= property is already deprecated.  Same goes for
- * the -soundhw switch.  Once this support gets finally
- * removed we can also drop the concept of a default audio
- * backend and this can go away.
- */
-driver = audio_driver_lookup("spice");
-if (driver) {
-driver->can_be_default = 1;
-}
-}
 
-if (dev) {
-/* -audiodev option */
-legacy_config = false;
-drvname = AudiodevDriver_str(dev->driver);
-} else if (!QTAILQ_EMPTY(_states)) {
-if (!legacy_config) {
-dolog("Device %s: audiodev default parameter is deprecated, please 
"
-  "specify audiodev=%s\n", name,
-

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

[Martin Kletzander]

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 
---
 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");
+
 /* 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

[PATCH 09/18] hw/display/xlnx_dp.c: Add audiodev property

[Martin Kletzander]

There was no way to set this and we need that for it to be able to properly
initialise.

Signed-off-by: Martin Kletzander 
---
 hw/display/xlnx_dp.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/hw/display/xlnx_dp.c b/hw/display/xlnx_dp.c
index 9bb781e31254..b16d6be2b5cc 100644
--- a/hw/display/xlnx_dp.c
+++ b/hw/display/xlnx_dp.c
@@ -1357,6 +1357,11 @@ static void xlnx_dp_reset(DeviceState *dev)
 xlnx_dp_update_irq(s);
 }
 
+static Property xlnx_dp_device_properties[] = {
+DEFINE_AUDIO_PROPERTIES(XlnxDPState, aud_card),
+DEFINE_PROP_END_OF_LIST(),
+};
+
 static void xlnx_dp_class_init(ObjectClass *oc, void *data)
 {
 DeviceClass *dc = DEVICE_CLASS(oc);
@@ -1364,6 +1369,7 @@ static void xlnx_dp_class_init(ObjectClass *oc, void 
*data)
 dc->realize = xlnx_dp_realize;
 dc->vmsd = _dp;
 dc->reset = xlnx_dp_reset;
+device_class_set_props(dc, xlnx_dp_device_properties);
 }
 
 static const TypeInfo xlnx_dp_info = {
-- 
2.35.1

[PATCH 00/18] RFC: Remove deprecated audio features

[Martin Kletzander]

I wanted to deal with https://bugzilla.redhat.com/2043498 and I got a
suggesstion that removing deprecated features could actually make it
easier to propagate the error.  In the end (last patch) it turns out the
error is still just reported with error_fatal, so it probably is not
really needed, but I really wanted to dig into QEMU more and learn some
of the internals for quite some time now.  So I used the opportunity.
The one-liner ended up being an 18 patch series which was, for someone
who has just one commit in QEMU codebase, a pretty challenging task.
Although I tried my best to do things properly, I am not sure whether I
handled everything correctly, hence the RFC.

Any comments are very much appreciated.  Thanks and have a nice day ;)

Martin Kletzander (18):
  hw/audio: Remove -soundhw support
  hw/input/tsc210x: Extract common init code into new function
  hw/audio: Simplify hda audio init
  hw/audio/lm4549: Add errp error reporting to init function
  tests/qtest: Specify audiodev= and -audiodev
  ui/vnc: Require audiodev=
  Introduce machine's default-audiodev property
  audio: Add easy dummy audio initialiser
  hw/display/xlnx_dp.c: Add audiodev property
  hw/input/tsc210x.c: Support machine-default audiodev with fallback
  hw/arm: Support machine-default audiodev with fallback
  hw/ppc: Support machine-default audiodev with fallback
  audio: Make AUD_register_card fallible and require audiodev=
  audio: Require AudioState in AUD_add_capture
  audio: Be more strict during audio backend initialisation
  audio: Remove legacy audio environment variables and options
  audio: Remove unused can_be_default
  audio/spiceaudio: Fail initialisation when not using spice

 audio/alsaaudio.c |   1 -
 audio/audio.c | 204 +++
 audio/audio.h |   5 +-
 audio/audio_int.h |   1 -
 audio/audio_legacy.c  | 555 --
 audio/coreaudio.m |   1 -
 audio/dbusaudio.c |   1 -
 audio/dsoundaudio.c   |   1 -
 audio/jackaudio.c |   1 -
 audio/meson.build |   1 -
 audio/noaudio.c   |   1 -
 audio/ossaudio.c  |   1 -
 audio/paaudio.c   |   1 -
 audio/sdlaudio.c  |   1 -
 audio/spiceaudio.c|   3 +-
 audio/wavaudio.c  |   1 -
 docs/about/deprecated.rst |  24 -
 docs/about/removed-features.rst   |  27 +
 docs/qdev-device-use.txt  |  21 +-
 docs/replay.txt   |   2 +-
 hw/arm/integratorcp.c |   8 +-
 hw/arm/musicpal.c |   8 +-
 hw/arm/omap2.c|  11 +-
 hw/arm/realview.c |   3 +
 hw/arm/spitz.c|  10 +-
 hw/arm/versatilepb.c  |   3 +
 hw/arm/vexpress.c |   3 +
 hw/arm/xlnx-zcu102.c  |   4 +
 hw/arm/z2.c   |  12 +-
 hw/audio/ac97.c   |   9 +-
 hw/audio/adlib.c  |   9 +-
 hw/audio/cs4231a.c|   8 +-
 hw/audio/es1370.c |   8 +-
 hw/audio/gus.c|   6 +-
 hw/audio/hda-codec.c  |  37 +-
 hw/audio/intel-hda.c  |  25 +-
 hw/audio/intel-hda.h  |   2 +-
 hw/audio/lm4549.c |   7 +-
 hw/audio/lm4549.h |   3 +-
 hw/audio/meson.build  |   1 -
 hw/audio/pcspk.c  |  15 +-
 hw/audio/pl041.c  |   2 +-
 hw/audio/sb16.c   |   9 +-
 hw/audio/soundhw.c| 177 --
 hw/audio/wm8750.c |   5 +-
 hw/core/machine.c |  23 +
 hw/display/xlnx_dp.c  |  12 +-
 hw/input/tsc210x.c|  79 ++-
 hw/ppc/prep.c |   4 +
 hw/usb/dev-audio.c|   5 +-
 include/hw/audio/soundhw.h|  15 -
 include/hw/boards.h   |   1 +
 qemu-options.hx   |  37 --
 .../codeconverter/test_regexps.py |   1 -
 softmmu/qdev-monitor.c|   2 -
 softmmu/vl.c  |  10 -
 tests/qtest/ac97-test.c   |   3 +-
 tests/qtest/es1370-test.c |   3 +-
 tests/qtest/fuzz/generic_fuzz_configs.h

[PATCH 02/18] hw/input/tsc210x: Extract common init code into new function

[Martin Kletzander]

This deduplicates several lines and will make future changes more
concise.

Signed-off-by: Martin Kletzander 
---
 hw/input/tsc210x.c | 68 --
 1 file changed, 24 insertions(+), 44 deletions(-)

diff --git a/hw/input/tsc210x.c b/hw/input/tsc210x.c
index df7313db5d7f..f16a8090b7c7 100644
--- a/hw/input/tsc210x.c
+++ b/hw/input/tsc210x.c
@@ -31,6 +31,7 @@
 #include "hw/input/tsc2xxx.h"
 #include "hw/irq.h"
 #include "migration/vmstate.h"
+#include "qapi/error.h"
 
 #define TSC_DATA_REGISTERS_PAGE0x0
 #define TSC_CONTROL_REGISTERS_PAGE 0x1
@@ -1070,20 +1071,10 @@ static const VMStateDescription vmstate_tsc2301 = {
 .fields = vmstatefields_tsc210x,
 };
 
-uWireSlave *tsc2102_init(qemu_irq pint)
+static void tsc210x_init(TSC210xState *s,
+ const char *name,
+ const VMStateDescription *vmsd)
 {
-TSC210xState *s;
-
-s = g_new0(TSC210xState, 1);
-s->x = 160;
-s->y = 160;
-s->pressure = 0;
-s->precision = s->nextprecision = 0;
-s->timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, tsc210x_timer_tick, s);
-s->pint = pint;
-s->model = 0x2102;
-s->name = "tsc2102";
-
 s->tr[0] = 0;
 s->tr[1] = 1;
 s->tr[2] = 1;
@@ -1105,13 +1096,29 @@ uWireSlave *tsc2102_init(qemu_irq pint)
 
 tsc210x_reset(s);
 
-qemu_add_mouse_event_handler(tsc210x_touchscreen_event, s, 1,
-"QEMU TSC2102-driven Touchscreen");
+qemu_add_mouse_event_handler(tsc210x_touchscreen_event, s, 1, name);
 
 AUD_register_card(s->name, >card);
 
 qemu_register_reset((void *) tsc210x_reset, s);
-vmstate_register(NULL, 0, _tsc2102, s);
+vmstate_register(NULL, 0, vmsd, s);
+}
+
+uWireSlave *tsc2102_init(qemu_irq pint)
+{
+TSC210xState *s;
+
+s = g_new0(TSC210xState, 1);
+s->x = 160;
+s->y = 160;
+s->pressure = 0;
+s->precision = s->nextprecision = 0;
+s->timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, tsc210x_timer_tick, s);
+s->pint = pint;
+s->model = 0x2102;
+s->name = "tsc2102";
+
+tsc210x_init(s, "QEMU TSC2102-driven Touchscreen", _tsc2102);
 
 return >chip;
 }
@@ -1132,34 +1139,7 @@ uWireSlave *tsc2301_init(qemu_irq penirq, qemu_irq 
kbirq, qemu_irq dav)
 s->model = 0x2301;
 s->name = "tsc2301";
 
-s->tr[0] = 0;
-s->tr[1] = 1;
-s->tr[2] = 1;
-s->tr[3] = 0;
-s->tr[4] = 1;
-s->tr[5] = 0;
-s->tr[6] = 1;
-s->tr[7] = 0;
-
-s->chip.opaque = s;
-s->chip.send = (void *) tsc210x_write;
-s->chip.receive = (void *) tsc210x_read;
-
-s->codec.opaque = s;
-s->codec.tx_swallow = (void *) tsc210x_i2s_swallow;
-s->codec.set_rate = (void *) tsc210x_i2s_set_rate;
-s->codec.in.fifo = s->in_fifo;
-s->codec.out.fifo = s->out_fifo;
-
-tsc210x_reset(s);
-
-qemu_add_mouse_event_handler(tsc210x_touchscreen_event, s, 1,
-"QEMU TSC2301-driven Touchscreen");
-
-AUD_register_card(s->name, >card);
-
-qemu_register_reset((void *) tsc210x_reset, s);
-vmstate_register(NULL, 0, _tsc2301, s);
+tsc210x_init(s, "QEMU TSC2301-driven Touchscreen", _tsc2301);
 
 return >chip;
 }
-- 
2.35.1

[PATCH 06/18] ui/vnc: Require audiodev=

[Martin Kletzander]

Signed-off-by: Martin Kletzander 
---
 ui/vnc.c | 15 +--
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/ui/vnc.c b/ui/vnc.c
index badf1d7664fe..2e7af139b030 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -4188,12 +4188,15 @@ void vnc_display_open(const char *id, Error **errp)
 vd->ledstate = 0;
 
 audiodev = qemu_opt_get(opts, "audiodev");
-if (audiodev) {
-vd->audio_state = audio_state_by_name(audiodev);
-if (!vd->audio_state) {
-error_setg(errp, "Audiodev '%s' not found", audiodev);
-goto fail;
-}
+if (!audiodev) {
+error_setg(errp, "Audiodev parameter for vnc required");
+goto fail;
+}
+
+vd->audio_state = audio_state_by_name(audiodev);
+if (!vd->audio_state) {
+error_setg(errp, "Audiodev '%s' not found", audiodev);
+goto fail;
 }
 
 device_id = qemu_opt_get(opts, "display");
-- 
2.35.1

[PATCH 14/18] audio: Require AudioState in AUD_add_capture

[Martin Kletzander]

Since all callers require a valid audiodev this function can now safely
abort in case of missing AudioState.

Signed-off-by: Martin Kletzander 
---
 audio/audio.c | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/audio/audio.c b/audio/audio.c
index b95aca444382..97eb645764c1 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -1855,10 +1855,8 @@ CaptureVoiceOut *AUD_add_capture(
 struct capture_callback *cb;
 
 if (!s) {
-if (!legacy_config) {
-dolog("Capturing without setting an audiodev is deprecated\n");
-}
-s = audio_init(NULL, NULL);
+error_setg(_abort,
+   "Capturing without setting an audiodev is not supported");
 }
 
 if (!audio_get_pdo_out(s->dev)->mixing_engine) {
-- 
2.35.1

[PATCH 01/18] hw/audio: Remove -soundhw support

[Martin Kletzander]

One thing I am not sure about is whether to keep the aliases of ac97 and
es1370 in the qdev_alias_table.

Signed-off-by: Martin Kletzander 
---
 docs/about/deprecated.rst |   9 -
 docs/about/removed-features.rst   |  10 +
 docs/qdev-device-use.txt  |  21 +--
 docs/replay.txt   |   2 +-
 hw/audio/ac97.c   |   3 -
 hw/audio/adlib.c  |   2 -
 hw/audio/cs4231a.c|   2 -
 hw/audio/es1370.c |   3 -
 hw/audio/gus.c|   2 -
 hw/audio/intel-hda.c  |  21 ---
 hw/audio/meson.build  |   1 -
 hw/audio/pcspk.c  |  11 --
 hw/audio/sb16.c   |   3 -
 hw/audio/soundhw.c| 177 --
 include/hw/audio/soundhw.h|  15 --
 qemu-options.hx   |  27 ---
 .../codeconverter/test_regexps.py |   1 -
 softmmu/qdev-monitor.c|   2 -
 softmmu/vl.c  |   6 -
 19 files changed, 19 insertions(+), 299 deletions(-)
 delete mode 100644 hw/audio/soundhw.c
 delete mode 100644 include/hw/audio/soundhw.h

diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index cf02ef6821e4..7ba71ebd3435 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=``.
-
 ``-chardev`` backend aliases ``tty`` and ``parport`` (since 6.0)
 
 
diff --git a/docs/about/removed-features.rst b/docs/about/removed-features.rst
index 4b831ea29176..086ba3edb042 100644
--- a/docs/about/removed-features.rst
+++ b/docs/about/removed-features.rst
@@ -336,6 +336,16 @@ for the RISC-V ``virt`` machine and ``sifive_u`` machine.
 The ``-no-quit`` was a synonym for ``-display ...,window-close=off`` which
 should be used instead.
 
+Creating sound card devices using ``-soundhw`` (removed in 7.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=``.  And ``AC97`` and ``ES1370`` now have to be
+specified in uppercase.
+
 
 QEMU Machine Protocol (QMP) commands
 
diff --git a/docs/qdev-device-use.txt b/docs/qdev-device-use.txt
index 240888933482..30e7eaa3e66d 100644
--- a/docs/qdev-device-use.txt
+++ b/docs/qdev-device-use.txt
@@ -311,21 +311,16 @@ constraints.
 
 Host and guest part of audio devices have always been separate.
 
-The old way to define guest audio devices is -soundhw C1,...
+Host side (backend) is defined using -audiodev with a specific driver:
 
-The new way is to define each guest audio device separately with
--device.
+spice
+pa
+none
 
-Map from -soundhw sound card name to -device:
-
-ac97-device AC97
-cs4231a -device cs4231a,iobase=IOADDR,irq=IRQ,dma=DMA
-es1370  -device ES1370
-gus -device gus,iobase=IOADDR,irq=IRQ,dma=DMA,freq=F
-hda -device intel-hda,msi=MSI -device hda-duplex
-sb16-device 
sb16,iobase=IOADDR,irq=IRQ,dma=DMA,dma16=DMA16,version=V
-adlib   not yet available with -device
-pcspk   not yet available with -device
+And each guest audio device is then defined with -device with
+audiodev=AUDIODEV_ID that refers to the audio backend above.  Exceptions are
+pcspk and adlib which are note yet available with -device and are part of a
+machine type.
 
 For PCI devices, you can add bus=PCI-BUS,addr=DEVFN to control the PCI
 device address, as usual.
diff --git a/docs/replay.txt b/docs/replay.txt
index 5b008ca4911f..c329767c148a 100644
--- a/docs/replay.txt
+++ b/docs/replay.txt
@@ -294,7 +294,7 @@ Audio devices
 
 Audio data is recorded and replay automatically. The command line for recording
 and replaying must contain identical specifications of audio hardware, e.g.:
- -soundhw ac97
+ -audiodev driver=pa -device ac97,audiodev=audio0
 
 Serial ports
 
diff --git a/hw/audio/ac97.c b/hw/audio/ac97.c

[PATCH 17/18] audio: Remove unused can_be_default

[Martin Kletzander]

Since there is no fallback mechanism and default-guessing this is now
not used and can be safely removed.

Signed-off-by: Martin Kletzander 
---
 audio/alsaaudio.c   | 1 -
 audio/audio_int.h   | 1 -
 audio/coreaudio.m   | 1 -
 audio/dbusaudio.c   | 1 -
 audio/dsoundaudio.c | 1 -
 audio/jackaudio.c   | 1 -
 audio/noaudio.c | 1 -
 audio/ossaudio.c| 1 -
 audio/paaudio.c | 1 -
 audio/sdlaudio.c| 1 -
 audio/wavaudio.c| 1 -
 11 files changed, 11 deletions(-)

diff --git a/audio/alsaaudio.c b/audio/alsaaudio.c
index 4a61378cd757..edbee63f97c5 100644
--- a/audio/alsaaudio.c
+++ b/audio/alsaaudio.c
@@ -933,7 +933,6 @@ static struct audio_driver alsa_audio_driver = {
 .init   = alsa_audio_init,
 .fini   = alsa_audio_fini,
 .pcm_ops= _pcm_ops,
-.can_be_default = 1,
 .max_voices_out = INT_MAX,
 .max_voices_in  = INT_MAX,
 .voice_size_out = sizeof (ALSAVoiceOut),
diff --git a/audio/audio_int.h b/audio/audio_int.h
index 2a6914d2aa65..082b13247657 100644
--- a/audio/audio_int.h
+++ b/audio/audio_int.h
@@ -148,7 +148,6 @@ struct audio_driver {
 void (*set_dbus_server) (AudioState *s, GDBusObjectManagerServer *manager);
 #endif
 struct audio_pcm_ops *pcm_ops;
-int can_be_default;
 int max_voices_out;
 int max_voices_in;
 int voice_size_out;
diff --git a/audio/coreaudio.m b/audio/coreaudio.m
index 4695291621a3..e5612138a74b 100644
--- a/audio/coreaudio.m
+++ b/audio/coreaudio.m
@@ -673,7 +673,6 @@ static void coreaudio_audio_fini (void *opaque)
 .init   = coreaudio_audio_init,
 .fini   = coreaudio_audio_fini,
 .pcm_ops= _pcm_ops,
-.can_be_default = 1,
 .max_voices_out = 1,
 .max_voices_in  = 0,
 .voice_size_out = sizeof (coreaudioVoiceOut),
diff --git a/audio/dbusaudio.c b/audio/dbusaudio.c
index a3d656d3b017..bbcad7051413 100644
--- a/audio/dbusaudio.c
+++ b/audio/dbusaudio.c
@@ -638,7 +638,6 @@ static struct audio_driver dbus_audio_driver = {
 .fini= dbus_audio_fini,
 .set_dbus_server = dbus_audio_set_server,
 .pcm_ops = _pcm_ops,
-.can_be_default  = 1,
 .max_voices_out  = INT_MAX,
 .max_voices_in   = INT_MAX,
 .voice_size_out  = sizeof(DBusVoiceOut),
diff --git a/audio/dsoundaudio.c b/audio/dsoundaudio.c
index 3fb67ec3eed4..311e34218465 100644
--- a/audio/dsoundaudio.c
+++ b/audio/dsoundaudio.c
@@ -721,7 +721,6 @@ static struct audio_driver dsound_audio_driver = {
 .init   = dsound_audio_init,
 .fini   = dsound_audio_fini,
 .pcm_ops= _pcm_ops,
-.can_be_default = 1,
 .max_voices_out = INT_MAX,
 .max_voices_in  = 1,
 .voice_size_out = sizeof (DSoundVoiceOut),
diff --git a/audio/jackaudio.c b/audio/jackaudio.c
index 5bdf3d7a78d6..fd2d2fd5acb7 100644
--- a/audio/jackaudio.c
+++ b/audio/jackaudio.c
@@ -669,7 +669,6 @@ static struct audio_driver jack_driver = {
 .init   = qjack_init,
 .fini   = qjack_fini,
 .pcm_ops= _pcm_ops,
-.can_be_default = 1,
 .max_voices_out = INT_MAX,
 .max_voices_in  = INT_MAX,
 .voice_size_out = sizeof(QJackOut),
diff --git a/audio/noaudio.c b/audio/noaudio.c
index 84a6bfbb1c87..111aef4a24ce 100644
--- a/audio/noaudio.c
+++ b/audio/noaudio.c
@@ -135,7 +135,6 @@ static struct audio_driver no_audio_driver = {
 .init   = no_audio_init,
 .fini   = no_audio_fini,
 .pcm_ops= _pcm_ops,
-.can_be_default = 1,
 .max_voices_out = INT_MAX,
 .max_voices_in  = INT_MAX,
 .voice_size_out = sizeof (NoVoiceOut),
diff --git a/audio/ossaudio.c b/audio/ossaudio.c
index 8e075edb70d6..31b582e8e4b5 100644
--- a/audio/ossaudio.c
+++ b/audio/ossaudio.c
@@ -781,7 +781,6 @@ static struct audio_driver oss_audio_driver = {
 .init   = oss_audio_init,
 .fini   = oss_audio_fini,
 .pcm_ops= _pcm_ops,
-.can_be_default = 1,
 .max_voices_out = INT_MAX,
 .max_voices_in  = INT_MAX,
 .voice_size_out = sizeof (OSSVoiceOut),
diff --git a/audio/paaudio.c b/audio/paaudio.c
index e91116f2396c..38f09017eb96 100644
--- a/audio/paaudio.c
+++ b/audio/paaudio.c
@@ -928,7 +928,6 @@ static struct audio_driver pa_audio_driver = {
 .init   = qpa_audio_init,
 .fini   = qpa_audio_fini,
 .pcm_ops= _pcm_ops,
-.can_be_default = 1,
 .max_voices_out = INT_MAX,
 .max_voices_in  = INT_MAX,
 .voice_size_out = sizeof (PAVoiceOut),
diff --git a/audio/sdlaudio.c b/audio/sdlaudio.c
index 68a237b76b45..5177e31d9b4a 100644
--- a/audio/sdlaudio.c
+++ b/audio/sdlaudio.c
@@ -493,7 +493,6 @@ static struct audio_driver sdl_audio_driver = {
 .init   = sdl_audio_init,
 .fini   = sdl_audio_fini,
 .pcm_ops= _pcm_ops,
-.can_be_default = 1,
 .max_voices_out = INT_MAX,
 .max_voices_in  = INT_MAX,
 .voice_size_out = sizeof(SDLVoiceOut),
diff --git a/audio/wavaudio.c b/audio

[PATCH 04/18] hw/audio/lm4549: Add errp error reporting to init function

[Martin Kletzander]

This will be used in future commit.

Signed-off-by: Martin Kletzander 
---
 hw/audio/lm4549.c | 3 ++-
 hw/audio/lm4549.h | 3 ++-
 hw/audio/pl041.c  | 2 +-
 3 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/hw/audio/lm4549.c b/hw/audio/lm4549.c
index 32b1481b5614..418041bc9c6c 100644
--- a/hw/audio/lm4549.c
+++ b/hw/audio/lm4549.c
@@ -276,7 +276,8 @@ static int lm4549_post_load(void *opaque, int version_id)
 return 0;
 }
 
-void lm4549_init(lm4549_state *s, lm4549_callback data_req_cb, void* opaque)
+void lm4549_init(lm4549_state *s, lm4549_callback data_req_cb, void* opaque,
+ Error **errp)
 {
 struct audsettings as;
 
diff --git a/hw/audio/lm4549.h b/hw/audio/lm4549.h
index aba9bb5b077c..61c3ab12dd33 100644
--- a/hw/audio/lm4549.h
+++ b/hw/audio/lm4549.h
@@ -36,7 +36,8 @@ typedef struct {
 extern const VMStateDescription vmstate_lm4549_state;
 
 
-void lm4549_init(lm4549_state *s, lm4549_callback data_req, void *opaque);
+void lm4549_init(lm4549_state *s, lm4549_callback data_req, void *opaque,
+ Error **errp);
 uint32_t lm4549_read(lm4549_state *s, hwaddr offset);
 void lm4549_write(lm4549_state *s, hwaddr offset, uint32_t value);
 uint32_t lm4549_write_samples(lm4549_state *s, uint32_t left, uint32_t right);
diff --git a/hw/audio/pl041.c b/hw/audio/pl041.c
index 03acd4fe344b..868dffbfd321 100644
--- a/hw/audio/pl041.c
+++ b/hw/audio/pl041.c
@@ -564,7 +564,7 @@ static void pl041_realize(DeviceState *dev, Error **errp)
 }
 
 /* Init the codec */
-lm4549_init(>codec, _request_data, (void *)s);
+lm4549_init(>codec, _request_data, (void *)s, errp);
 }
 
 static const VMStateDescription vmstate_pl041_regfile = {
-- 
2.35.1

Re: [Qemu-devel] Possibly incorrect data sparsification by qemu-img

[Martin Kletzander]

On Mon, Apr 29, 2019 at 08:58:37AM +, Vladimir Sementsov-Ogievskiy wrote:

29.04.2019 10:27, Martin Kletzander wrote:

On Wed, Apr 24, 2019 at 09:19:17AM +0200, Kevin Wolf wrote:

Am 24.04.2019 um 08:40 hat Vladimir Sementsov-Ogievskiy geschrieben:

23.04.2019 18:08, Kevin Wolf wrote:
> Am 23.04.2019 um 16:26 hat Martin Kletzander geschrieben:
>> On Tue, Apr 23, 2019 at 02:12:18PM +0200, Kevin Wolf wrote:
>>> Am 23.04.2019 um 13:30 hat Martin Kletzander geschrieben:
>>>> Hi,
>>>>
>>>> I am using qemu-img with nbdkit to transfer a disk image and the update it 
with
>>>> extra data from newer snapshots.  The end image cannot be transferred 
because
>>>> the snapshots will be created later than the first transfer and we want to 
save
>>>> some time up front.  You might think of it as a continuous 
synchronisation.  It
>>>> looks something like this:
>>>>
>>>> I first transfer the whole image:
>>>>
>>>>   qemu-img convert -p $nbd disk.raw
>>>>
>>>> Where `$nbd` is something along the lines of 
`nbd+unix:///?socket=nbdkit.sock`
>>>>
>>>> Then, after the next snapshot is created, I can update it thanks to the 
`-n`
>>>> parameter (the $nbd now points to the newer snapshot with unchanged data 
looking
>>>> like holes in the file):
>>>>
>>>>   qemu-img convert -p -n $nbd disk.raw
>>>>
>>>> This is fast and efficient as it uses block status nbd extension, so it 
only
>>>> transfers new data.
>>>
>>> This is an implementation detail. Don't rely on it. What you're doing is
>>> abusing 'qemu-img convert', so problems like what you describe are to be
>>> expected.
>>>
>>>> This can be done over and over again to keep the local
>>>> `disk.raw` image up to date with the latest remote snapshot.
>>>>
>>>> However, when the guest OS zeroes some of the data and it gets written 
into the
>>>> snapshot, qemu-img scans for those zeros and does not write them to the
>>>> destination image.  Checking the output of `qemu-img map --output=json 
$nbd`
>>>> shows that the zeroed data is properly marked as `data: true`.
>>>>
>>>> Using `-S 0` would write zeros even where the holes are, effectively 
overwriting
>>>> the data from the last snapshot even though they should not be changed.
>>>>
>>>> Having gone through some workarounds I would like there to be another way. 
 I
>>>> know this is far from the typical usage of qemu-img, but is this really the
>>>> expected behaviour or is this just something nobody really needed before?  
If it
>>>> is the former, would it be possible to have a parameter that would control 
this
>>>> behaviour?  If the latter is the case, can that behaviour be changed so 
that it
>>>> properly replicates the data when `-n` parameter is used?
>>>>
>>>> Basically the only thing we need is to either:
>>>>
>>>> 1) write zeros where they actually are or
>>>>
>>>> 2) turn off explicit sparsification without requesting dense image 
(basically
>>>> sparsify only the par that is reported as hole on the source) or
>>>>
>>>> 3) ideally, just FALLOC_FL_PUNCH_HOLE in places where source did report 
data,
>>>> but qemu-img found they are all zeros (or source reported HOLE+ZERO 
which, I
>>>> believe, is effectively the same)
>>>>
>>>> If you want to try this out, I found the easiest reproducible way is using
>>>> nbdkit's data plugin, which can simulate whatever source image you like.
>>>
>>> I think what you _really_ want is a commit block job. The problem is
>>> just that you don't have a proper backing file chain, but just a bunch
>>> of NBD connections.
>>>
>>> Can't you get an NBD connection that already provides the condensed form
>>> of the whole snapshot chain directly at the source? If the NBD server
>>> was QEMU, this would actually be easier than providing each snapshot
>>> individually.
>>>
>>> If this isn't possible, I think you need to replicate the backing chain
>>> on the destination instead of converting into the same image again and
>>> again so that qemu-img knows that it must take existing data of the
>>> backing file into consideration:
>>>
>>> qemu-img convert -O qcow2 nbd://... base.qcow2
>>> qemu-img convert -O qcow2 -F qcow2 -B base.q

Re: [Qemu-devel] Possibly incorrect data sparsification by qemu-img

[Martin Kletzander]

On Wed, Apr 24, 2019 at 09:19:17AM +0200, Kevin Wolf wrote:

Am 24.04.2019 um 08:40 hat Vladimir Sementsov-Ogievskiy geschrieben:

23.04.2019 18:08, Kevin Wolf wrote:
> Am 23.04.2019 um 16:26 hat Martin Kletzander geschrieben:
>> On Tue, Apr 23, 2019 at 02:12:18PM +0200, Kevin Wolf wrote:
>>> Am 23.04.2019 um 13:30 hat Martin Kletzander geschrieben:
>>>> Hi,
>>>>
>>>> I am using qemu-img with nbdkit to transfer a disk image and the update it 
with
>>>> extra data from newer snapshots.  The end image cannot be transferred 
because
>>>> the snapshots will be created later than the first transfer and we want to 
save
>>>> some time up front.  You might think of it as a continuous 
synchronisation.  It
>>>> looks something like this:
>>>>
>>>> I first transfer the whole image:
>>>>
>>>>   qemu-img convert -p $nbd disk.raw
>>>>
>>>> Where `$nbd` is something along the lines of 
`nbd+unix:///?socket=nbdkit.sock`
>>>>
>>>> Then, after the next snapshot is created, I can update it thanks to the 
`-n`
>>>> parameter (the $nbd now points to the newer snapshot with unchanged data 
looking
>>>> like holes in the file):
>>>>
>>>>   qemu-img convert -p -n $nbd disk.raw
>>>>
>>>> This is fast and efficient as it uses block status nbd extension, so it 
only
>>>> transfers new data.
>>>
>>> This is an implementation detail. Don't rely on it. What you're doing is
>>> abusing 'qemu-img convert', so problems like what you describe are to be
>>> expected.
>>>
>>>> This can be done over and over again to keep the local
>>>> `disk.raw` image up to date with the latest remote snapshot.
>>>>
>>>> However, when the guest OS zeroes some of the data and it gets written 
into the
>>>> snapshot, qemu-img scans for those zeros and does not write them to the
>>>> destination image.  Checking the output of `qemu-img map --output=json 
$nbd`
>>>> shows that the zeroed data is properly marked as `data: true`.
>>>>
>>>> Using `-S 0` would write zeros even where the holes are, effectively 
overwriting
>>>> the data from the last snapshot even though they should not be changed.
>>>>
>>>> Having gone through some workarounds I would like there to be another way. 
 I
>>>> know this is far from the typical usage of qemu-img, but is this really the
>>>> expected behaviour or is this just something nobody really needed before?  
If it
>>>> is the former, would it be possible to have a parameter that would control 
this
>>>> behaviour?  If the latter is the case, can that behaviour be changed so 
that it
>>>> properly replicates the data when `-n` parameter is used?
>>>>
>>>> Basically the only thing we need is to either:
>>>>
>>>> 1) write zeros where they actually are or
>>>>
>>>> 2) turn off explicit sparsification without requesting dense image 
(basically
>>>> sparsify only the par that is reported as hole on the source) or
>>>>
>>>> 3) ideally, just FALLOC_FL_PUNCH_HOLE in places where source did report 
data,
>>>> but qemu-img found they are all zeros (or source reported HOLE+ZERO 
which, I
>>>> believe, is effectively the same)
>>>>
>>>> If you want to try this out, I found the easiest reproducible way is using
>>>> nbdkit's data plugin, which can simulate whatever source image you like.
>>>
>>> I think what you _really_ want is a commit block job. The problem is
>>> just that you don't have a proper backing file chain, but just a bunch
>>> of NBD connections.
>>>
>>> Can't you get an NBD connection that already provides the condensed form
>>> of the whole snapshot chain directly at the source? If the NBD server
>>> was QEMU, this would actually be easier than providing each snapshot
>>> individually.
>>>
>>> If this isn't possible, I think you need to replicate the backing chain
>>> on the destination instead of converting into the same image again and
>>> again so that qemu-img knows that it must take existing data of the
>>> backing file into consideration:
>>>
>>> qemu-img convert -O qcow2 nbd://... base.qcow2
>>> qemu-img convert -O qcow2 -F qcow2 -B base.qcow2 nbd://... 
overlay1.qcow2
>>> qemu-img convert -O qcow2 -F qcow2 -B overlay1.qcow2 nbd://... 
overlay2.qco

Re: [Qemu-devel] Possibly incorrect data sparsification by qemu-img

[Martin Kletzander]

On Wed, Apr 24, 2019 at 09:19:17AM +0200, Kevin Wolf wrote:

Am 24.04.2019 um 08:40 hat Vladimir Sementsov-Ogievskiy geschrieben:

23.04.2019 18:08, Kevin Wolf wrote:
> Am 23.04.2019 um 16:26 hat Martin Kletzander geschrieben:
>> On Tue, Apr 23, 2019 at 02:12:18PM +0200, Kevin Wolf wrote:
>>> Am 23.04.2019 um 13:30 hat Martin Kletzander geschrieben:
>>>> Hi,
>>>>
>>>> I am using qemu-img with nbdkit to transfer a disk image and the update it 
with
>>>> extra data from newer snapshots.  The end image cannot be transferred 
because
>>>> the snapshots will be created later than the first transfer and we want to 
save
>>>> some time up front.  You might think of it as a continuous 
synchronisation.  It
>>>> looks something like this:
>>>>
>>>> I first transfer the whole image:
>>>>
>>>>   qemu-img convert -p $nbd disk.raw
>>>>
>>>> Where `$nbd` is something along the lines of 
`nbd+unix:///?socket=nbdkit.sock`
>>>>
>>>> Then, after the next snapshot is created, I can update it thanks to the 
`-n`
>>>> parameter (the $nbd now points to the newer snapshot with unchanged data 
looking
>>>> like holes in the file):
>>>>
>>>>   qemu-img convert -p -n $nbd disk.raw
>>>>
>>>> This is fast and efficient as it uses block status nbd extension, so it 
only
>>>> transfers new data.
>>>
>>> This is an implementation detail. Don't rely on it. What you're doing is
>>> abusing 'qemu-img convert', so problems like what you describe are to be
>>> expected.
>>>
>>>> This can be done over and over again to keep the local
>>>> `disk.raw` image up to date with the latest remote snapshot.
>>>>
>>>> However, when the guest OS zeroes some of the data and it gets written 
into the
>>>> snapshot, qemu-img scans for those zeros and does not write them to the
>>>> destination image.  Checking the output of `qemu-img map --output=json 
$nbd`
>>>> shows that the zeroed data is properly marked as `data: true`.
>>>>
>>>> Using `-S 0` would write zeros even where the holes are, effectively 
overwriting
>>>> the data from the last snapshot even though they should not be changed.
>>>>
>>>> Having gone through some workarounds I would like there to be another way. 
 I
>>>> know this is far from the typical usage of qemu-img, but is this really the
>>>> expected behaviour or is this just something nobody really needed before?  
If it
>>>> is the former, would it be possible to have a parameter that would control 
this
>>>> behaviour?  If the latter is the case, can that behaviour be changed so 
that it
>>>> properly replicates the data when `-n` parameter is used?
>>>>
>>>> Basically the only thing we need is to either:
>>>>
>>>> 1) write zeros where they actually are or
>>>>
>>>> 2) turn off explicit sparsification without requesting dense image 
(basically
>>>> sparsify only the par that is reported as hole on the source) or
>>>>
>>>> 3) ideally, just FALLOC_FL_PUNCH_HOLE in places where source did report 
data,
>>>> but qemu-img found they are all zeros (or source reported HOLE+ZERO 
which, I
>>>> believe, is effectively the same)
>>>>
>>>> If you want to try this out, I found the easiest reproducible way is using
>>>> nbdkit's data plugin, which can simulate whatever source image you like.
>>>
>>> I think what you _really_ want is a commit block job. The problem is
>>> just that you don't have a proper backing file chain, but just a bunch
>>> of NBD connections.
>>>
>>> Can't you get an NBD connection that already provides the condensed form
>>> of the whole snapshot chain directly at the source? If the NBD server
>>> was QEMU, this would actually be easier than providing each snapshot
>>> individually.
>>>
>>> If this isn't possible, I think you need to replicate the backing chain
>>> on the destination instead of converting into the same image again and
>>> again so that qemu-img knows that it must take existing data of the
>>> backing file into consideration:
>>>
>>> qemu-img convert -O qcow2 nbd://... base.qcow2
>>> qemu-img convert -O qcow2 -F qcow2 -B base.qcow2 nbd://... 
overlay1.qcow2
>>> qemu-img convert -O qcow2 -F qcow2 -B overlay1.qcow2 nbd://... 
overlay2.qc

Re: [Qemu-devel] Possibly incorrect data sparsification by qemu-img

[Martin Kletzander]

On Tue, Apr 23, 2019 at 02:12:18PM +0200, Kevin Wolf wrote:

Am 23.04.2019 um 13:30 hat Martin Kletzander geschrieben:

Hi,

I am using qemu-img with nbdkit to transfer a disk image and the update it with
extra data from newer snapshots.  The end image cannot be transferred because
the snapshots will be created later than the first transfer and we want to save
some time up front.  You might think of it as a continuous synchronisation.  It
looks something like this:

I first transfer the whole image:

 qemu-img convert -p $nbd disk.raw

Where `$nbd` is something along the lines of `nbd+unix:///?socket=nbdkit.sock`

Then, after the next snapshot is created, I can update it thanks to the `-n`
parameter (the $nbd now points to the newer snapshot with unchanged data looking
like holes in the file):

 qemu-img convert -p -n $nbd disk.raw

This is fast and efficient as it uses block status nbd extension, so it only
transfers new data.


This is an implementation detail. Don't rely on it. What you're doing is
abusing 'qemu-img convert', so problems like what you describe are to be
expected.


This can be done over and over again to keep the local
`disk.raw` image up to date with the latest remote snapshot.

However, when the guest OS zeroes some of the data and it gets written into the
snapshot, qemu-img scans for those zeros and does not write them to the
destination image.  Checking the output of `qemu-img map --output=json $nbd`
shows that the zeroed data is properly marked as `data: true`.

Using `-S 0` would write zeros even where the holes are, effectively overwriting
the data from the last snapshot even though they should not be changed.

Having gone through some workarounds I would like there to be another way.  I
know this is far from the typical usage of qemu-img, but is this really the
expected behaviour or is this just something nobody really needed before?  If it
is the former, would it be possible to have a parameter that would control this
behaviour?  If the latter is the case, can that behaviour be changed so that it
properly replicates the data when `-n` parameter is used?

Basically the only thing we need is to either:

1) write zeros where they actually are or

2) turn off explicit sparsification without requesting dense image (basically
   sparsify only the par that is reported as hole on the source) or

3) ideally, just FALLOC_FL_PUNCH_HOLE in places where source did report data,
   but qemu-img found they are all zeros (or source reported HOLE+ZERO which, I
   believe, is effectively the same)

If you want to try this out, I found the easiest reproducible way is using
nbdkit's data plugin, which can simulate whatever source image you like.


I think what you _really_ want is a commit block job. The problem is
just that you don't have a proper backing file chain, but just a bunch
of NBD connections.

Can't you get an NBD connection that already provides the condensed form
of the whole snapshot chain directly at the source? If the NBD server
was QEMU, this would actually be easier than providing each snapshot
individually.

If this isn't possible, I think you need to replicate the backing chain
on the destination instead of converting into the same image again and
again so that qemu-img knows that it must take existing data of the
backing file into consideration:

   qemu-img convert -O qcow2 nbd://... base.qcow2
   qemu-img convert -O qcow2 -F qcow2 -B base.qcow2 nbd://... overlay1.qcow2
   qemu-img convert -O qcow2 -F qcow2 -B overlay1.qcow2 nbd://... overlay2.qcow2
   ...



I thought of this, but (to be honest) I did not know that `-B` would work for
nbd.  Does it assume that data are to be taken from the base image if and only
if the source (be it nbd server or just a plain file) says there is a hole?  If
yes, then it could nicely solve the issue.


And at the end you can merge the snapshot chain (using a commit or
stream bĺock job, or qemu-img commit/rebase).

Kevin





signature.asc
Description: PGP signature

[Qemu-devel] Possibly incorrect data sparsification by qemu-img

[Martin Kletzander]

Hi,

I am using qemu-img with nbdkit to transfer a disk image and the update it with
extra data from newer snapshots.  The end image cannot be transferred because
the snapshots will be created later than the first transfer and we want to save
some time up front.  You might think of it as a continuous synchronisation.  It
looks something like this:

I first transfer the whole image:

 qemu-img convert -p $nbd disk.raw

Where `$nbd` is something along the lines of `nbd+unix:///?socket=nbdkit.sock`

Then, after the next snapshot is created, I can update it thanks to the `-n`
parameter (the $nbd now points to the newer snapshot with unchanged data looking
like holes in the file):

 qemu-img convert -p -n $nbd disk.raw

This is fast and efficient as it uses block status nbd extension, so it only
transfers new data.  This can be done over and over again to keep the local
`disk.raw` image up to date with the latest remote snapshot.

However, when the guest OS zeroes some of the data and it gets written into the
snapshot, qemu-img scans for those zeros and does not write them to the
destination image.  Checking the output of `qemu-img map --output=json $nbd`
shows that the zeroed data is properly marked as `data: true`.

Using `-S 0` would write zeros even where the holes are, effectively overwriting
the data from the last snapshot even though they should not be changed.

Having gone through some workarounds I would like there to be another way.  I
know this is far from the typical usage of qemu-img, but is this really the
expected behaviour or is this just something nobody really needed before?  If it
is the former, would it be possible to have a parameter that would control this
behaviour?  If the latter is the case, can that behaviour be changed so that it
properly replicates the data when `-n` parameter is used?

Basically the only thing we need is to either:

1) write zeros where they actually are or

2) turn off explicit sparsification without requesting dense image (basically
   sparsify only the par that is reported as hole on the source) or

3) ideally, just FALLOC_FL_PUNCH_HOLE in places where source did report data,
   but qemu-img found they are all zeros (or source reported HOLE+ZERO which, I
   believe, is effectively the same)

If you want to try this out, I found the easiest reproducible way is using
nbdkit's data plugin, which can simulate whatever source image you like.

The first iteration, which transfers the whole image can be simulated like this:

 nbdkit --run 'qemu-img convert -p $nbd output.raw' data data="1" size=2M

That command will expose an artificial disk with the size of 2MB which has first
byte '1' and the rest is zeros/holes and runs the specified qemu-img command on
that ($nbd is supplied by nbdkit, so the string needs to be enclosed in single
parentheses).

You can see how that data is exposed by running:

 nbdkit --run 'qemu-img map --output=json $nbd' data data="1" size=2M

For completeness I get this output:

 [{ "start": 0, "length": 32768, "depth": 0, "zero": false, "data": true},
  { "start": 32768, "length": 2064384, "depth": 0, "zero": true, "data": false}]

Consequent update from a snapshot (with the first block explicitly zeroed) could
be simulated by running:

 nbdkit --run 'qemu-img convert -n -p $nbd output.raw' data data="0" size=2M

Again, the mapping exposed by nbdkit can be seen by running:

 nbdkit --run 'qemu-img map --output=json $nbd' data data="0" size=2M

For completeness I get this output:

 [{ "start": 0, "length": 32768, "depth": 0, "zero": true, "data": true},
  { "start": 32768, "length": 2064384, "depth": 0, "zero": true, "data": false}]

The resulting image still has `1` as its first byte (following is the output of
`hexdump -C output.raw`):

   01 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  ||
 0010  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  ||
 *
 0020

Have a nice day,
Martin


signature.asc
Description: PGP signature

Re: [Qemu-devel] AMD SEV's /dev/sev permissions and probing QEMU for capabilities

[Martin Kletzander]

On Fri, Jan 18, 2019 at 10:16:38AM +, Daniel P. Berrangé wrote:

On Fri, Jan 18, 2019 at 10:39:35AM +0100, Erik Skultety wrote:

Hi,
this is a summary of a private discussion I've had with guys CC'd on this email
about finding a solution to [1] - basically, the default permissions on
/dev/sev (below) make it impossible to query for SEV platform capabilities,
since by default we run QEMU as qemu:qemu when probing for capabilities. It's
worth noting is that this is only relevant to probing, since for a proper QEMU
VM we create a mount namespace for the process and chown all the nodes (needs a
SEV fix though).

# ll /dev/sev
crw---. 1 root root

I suggested either force running QEMU as root for probing (despite the obvious
security implications) or using namespaces for probing too. Dan argued that
this would have a significant perf impact and suggested we ask systemd to add a
global udev rule.




If the creation of namespaces is poses a performance impact, then why don't we
special-case the probing in a sense that we create one namespace for probing,
once, and probe all QEMU binaries in that one namespace?


I've just realized there is a potential 3rd solution. Remember there is
actually nothing inherantly special about the 'root' user as an account
ID. 'root' gains its powers from the fact that it has many capabilities
by default.  'qemu' can't access /dev/sev because it is owned by a
different user (happens to be root) and 'qemu' does not have capabilities.

So we can make probing work by using our capabilities code to grant
CAP_DAC_OVERRIDE to the qemu process we spawn. So probing still runs
as 'qemu', but can none the less access /dev/sev while it is owned
by root.  We were not using 'qemu' for sake of security, as the probing
process is not executing any untrusthworthy code, so we don't  loose any
security protection by granting CAP_DAC_OVERRIDE.



IMHO CAP_DAC_OVERRIDE is a lot, especially on systems without SELinux.


I proceeded with cloning [1] to systemd and creating an udev rule that I planned
on submitting to systemd upstream - the initial idea was to mimic /dev/kvm and
make it world accessible to which Brijesh from AMD expressed a concern that
regular users might deplete the resources (limit on the number of guests
allowed by the platform). But since the limit is claimed to be around 4, Dan
discouraged me to continue with restricting the udev rule to only the 'kvm'
group which Laszlo suggested earlier as the limit is so small that a malicious
QEMU could easily deplete this during probing. This fact also ruled out any
kind of ACL we could create dynamically. Instead, he suggested that we filter
out the kvm-capable QEMU and put only that one in the namespace without a
significant perf impact.


Yes, my suggestion to mimic /dev/kvm was based on the mistaken mis-understanding
that there was not a finite resource limit. Given that there are one or more
finite resource limits, we need access control on which unprivileged users, and
/or which individual QEMU instances are permitted access. This means /dev/sev
must remain with restrictive user/group/permissions that prevent any unprivilegd
account from having access. This means either root:root 0770/0700, or possibly
having an 'sev' group and using root:sev 0770, so that users can be granted
access via 'sev' group membership which (might?) allow unprivileged libvirtd to
use 'sev' if the user was added.


- my take on this is that there could potentially be more than a single
  kvm-enabled QEMU and therefore we'd need to create more than just a
  single namespace.


True, I guess qemu-system-x86_64 and qemu-system-i386 both get KVM
on an x86_64 host, and likewise for many other 64-bit archs supporting.
32-bit apps.


- I also argued that I can image that the same kind of DOS attack might be
  possible from within the namespace, even if we created the /dev/sev node
  only in SEV-enabled guests (which we currently don't). All of us have
  agreed that allowing /dev/sev in the namespace for only SEV-enabled
  guests is worth doing nonetheless.


There's never any perfect level of protection. We're just striving to
minimize the attack surface by only exposing it where there's a genuine
need to use it.


In the meantime, Christophe went through the kernel code to verify how the SEV
resources are managed and what protection is currently in place to mitigate the
chance of a process easily depleting the limit on SEV guests. He found that
ASID, which determines the encryption key, is allocated from a single ASID
bitmap and essentially guarded by a single 'sev->active' flag.

So, in conclusion, we absolutely need input from Brijesh (AMD) whether there
was something more than the low limit on number of guests behind the default
permissions. Also, we'd like to get some details on how the limit is managed,
helping to assess the approaches mentioned above.


Regardless of this problem, I think it is important to have some docs

Re: [Qemu-devel] AMD SEV's /dev/sev permissions and probing QEMU for capabilities

[Martin Kletzander]

On Fri, Jan 18, 2019 at 11:17:11AM +, Daniel P. Berrangé wrote:

On Fri, Jan 18, 2019 at 12:11:50PM +0100, Martin Kletzander wrote:

On Fri, Jan 18, 2019 at 10:16:38AM +, Daniel P. Berrangé wrote:
> I've just realized there is a potential 3rd solution. Remember there is
> actually nothing inherantly special about the 'root' user as an account
> ID. 'root' gains its powers from the fact that it has many capabilities
> by default.  'qemu' can't access /dev/sev because it is owned by a
> different user (happens to be root) and 'qemu' does not have capabilities.
>
> So we can make probing work by using our capabilities code to grant
> CAP_DAC_OVERRIDE to the qemu process we spawn. So probing still runs
> as 'qemu', but can none the less access /dev/sev while it is owned
> by root.  We were not using 'qemu' for sake of security, as the probing
> process is not executing any untrusthworthy code, so we don't  loose any
> security protection by granting CAP_DAC_OVERRIDE.
>

IMHO CAP_DAC_OVERRIDE is a lot, especially on systems without SELinux.


Probing of QEMU capabilities is not a security critical task. QEMU is
run with "-machine none" so does not even create the virtual machine
hardware, nor have any guest image that it would run. All it is running
is the QEMU class initialization code. The only way for that to act in
a malicious way is for a backdoor to have been inserted when QEMU was
built by the OS vendor, or fo the QEMU binary on disk to have been
replaced by something malcious (which would require root privileges
itself).



So you are trying to protect from buggy qemu with malicious guest, not really a
malicious qemu.  I got confused as SEV is trying to protect against
untrustworthy host including binaries like qemu.  OK



We must of coure *NEVER* give CAP_DAC_OVERRIDE to a QEMU that is running
the real, untrustworty, end user VM.

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


signature.asc
Description: PGP signature

Re: [Qemu-devel] [libvirt] [PATCH 2/5] qemu: Move checks for SMM from command-line creation into validation phase

[Martin Kletzander]

On Thu, May 31, 2018 at 09:33:54AM +0200, Laszlo Ersek wrote:

adding qemu-devel, Paolo and Gerd; comments below:

On 05/30/18 23:08, Martin Kletzander wrote:

On Wed, May 30, 2018 at 11:02:59AM -0400, John Ferlan wrote:



On 05/21/2018 11:00 AM, Martin Kletzander wrote:

We are still hoping all of such checks will be moved there and this
is one small
step in that direction.

One of the things that this is improving is the error message you get
when
starting a domain with SMM and i440fx, for example.  Instead of
saying that the
QEMU binary doesn't support that option, we correctly say that it is
only
supported with q35 machine type.

Signed-off-by: Martin Kletzander 
---
 src/qemu/qemu_capabilities.c | 21 +++--
 src/qemu/qemu_capabilities.h |  4 ++--
 src/qemu/qemu_command.c  | 12 ++--
 src/qemu/qemu_domain.c   | 12 +---
 4 files changed, 28 insertions(+), 21 deletions(-)



I know it's outside the bounds of what you're doing; however,
qemuDomainDefValidateFeatures could check the capabilities for other
bits too...



Probably, but I mostly wanted to do that because SMM is not only about the
capability, but also about the machine.  Good idea for the future, though.


[...]


diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index d3beee5d8760..881d0ea46a75 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -3430,7 +3430,8 @@ qemuDomainDefGetVcpuHotplugGranularity(const
virDomainDef *def)


 static int
-qemuDomainDefValidateFeatures(const virDomainDef *def)
+qemuDomainDefValidateFeatures(const virDomainDef *def,
+  virQEMUCapsPtr qemuCaps)
 {
 size_t i;

@@ -3477,6 +3478,12 @@ qemuDomainDefValidateFeatures(const
virDomainDef *def)
 }
 break;

+    case VIR_DOMAIN_FEATURE_SMM:
+    if (def->features[i] == VIR_TRISTATE_SWITCH_ON &&


Probably should change to != _ABSENT, since qemu_command will supply
smm={on|off}



That makes sense, kind of.  For 'off' we only need to check if we can
specify
the smm= option.  The thing is that you can even specify smm=on with
non-q35
machine type, but it is unclear what that's going to mean since it doesn't
really make sense.

@Laszlo: What would you say?  Should we allow users to specify smm=on
for users?
Or even better, does it makes sense to allow specifying smm=anything for
non-q35
machine types?  If not, we'll leave it like this, that is smm=anything is
forbidden for non-q35 machine types.


Technically the "smm" machine type property is defined for both i440fx
and q35:

$ qemu-system-x86_64 -machine pc,\? 2>&1 | grep smm
pc-i440fx-2.11.smm=OnOffAuto (Enable SMM (pc & q35))

$ qemu-system-x86_64 -machine q35,\? 2>&1 | grep smm
pc-q35-2.11.smm=OnOffAuto (Enable SMM (pc & q35))

The "smm" property controls whether system management mode is emulated,
to my knowledge. That's an x86 processor operating mode, which isn't
specific to the Q35 board. What's specific to Q35 is the TSEG.

The primary reason why OVMF (built with the edk2 SMM driver stack)
requires Q35 is not that i440fx does not emulate SMM, it's that i440fx
doesn't have a large enough SMRAM area. (The legacy SMRAM areas are too
small for OVMF.)

For example, SeaBIOS too includes some SMM code (optionally, if it's
built like that), and that runs on i440fx just fine, because the legacy
SMRAM areas are large enough for SeaBIOS's purposes, AIUI.

(Now, there's at least one other reason why OVMF/SMM needs Q35: because
the SMI broadcast feature too is only implemented on Q35. But that's
rather a consequence of the above "primary reason", and not a
stand-alone reason itself -- we restricted the SMI broadcast feature to
Q35 *because* OVMF could only run on Q35. Anyhow, you need not care
about SMI broadcast because it's automatically negotiated between guest
fw and QEMU.)

Summary:

(1) For making Secure Boot actually secure in OVMF, OVMF needs to be
built with -D SMM_REQUIRE, so that it include the SMM driver stack.

(2) Booting such a firmware binary requires Q35, because only Q35 offers
TSEG, and the legacy -- non-TSEG -- SMRAM ranges are too small even for
a "basic boot" of OVMF.

(3) QEMU has to be configured with "-machine smm=on"; this is already
covered by libvirt via .

(4) QEMU has to be configured to restrict pflash writes to code that
executes in SMM, with "-global
driver=cfi.pflash01,property=secure,value=on". Libvirt covers this
already with the @secure=yes attribute in .

(5) There are two *quality of service* features related to SMM; with
both of them being Q35-only. The first feature is SMI broadcast; libvirt
need not care because it's auto-negotiated.

(6) The second QoS feature is *extended* TSEG (a paravirt feature on top
of the standard TSEG), which lets the edk2 SMM driver stack scale to
large RAM sizes and high VCPU counts. Libvirt should let the user
configure the extend

Re: [Qemu-devel] [libvirt-users] Inter Shared Memory ( Ivshmem ) : Cannot Bind

[Martin Kletzander]

On Mon, Dec 11, 2017 at 03:03:34PM +0800, Rogue S.T wrote:

Hello, friends.
I encounter a problem when i use ivshmem with my guest, my ivshmem server is 
not start, and output a error : Example code, do not use in production ,cannot 
bind.


Detail distribution:
Today, I know ivshmem from a topic "QEMU version 2.10.93 User Documentation",
Ivshmem: it can create a shared memory that guest device use, so we can use 
this memory to do something ourselves.
First, I create host memory backend:
-object 
memory-backend-file,size=1M,share,mem-path=/dev/shm/ivshmem,id=hostmem
Twice, Bind a shared memory device to my guest:
   -device ivshmem-plain,memdev=hostmem


You are using plain, but


Then I boot ivshmem servi
   ivshmem-server -p /var/run/ivshmem-server.pid  -S /tmp/ivshmem_socket  -M 
ivshmem -m /dev/shm -l 1M -n 2


You are running a server.  This is not how it works.

Looks like you are playing with something that you don't really need, otherwise
you would do things differetly.  It you are just trying it out, that's fine.
Otherwise make sure it's what you need.  More information on how this hot mess
works is here:

https://git.qemu.org/?p=qemu.git;a=blob;f=docs/specs/ivshmem-spec.txt


Next time i get this error:
   *** Example code, do not use in production ***
   cannot bind
And i search the problem on google, but nothing to get, hope you can help me 
solve it.
following message is my host environment:
   host:  ubuntu 1604
   qemu:  version 2.10.90 (v2.11.0-rc0-dirty)
   libvirt:  libvirtd (libvirt) 3.9.0
   guest:window 7 professor


Refer:
   https://qemu.weilnetz.de/doc/qemu-doc.html#Inter_002dVM-Shared-Memory-device 
(topic qemu documention)
   https://helpmanual.io/help/ivshmem-server/  ( how to use ivshmem-server )



___
libvirt-users mailing list
libvirt-us...@redhat.com
https://www.redhat.com/mailman/listinfo/libvirt-users

Re: [Qemu-devel] [PATCH] block: Add bdrv_runtime_opts to query-command-line-options

[Martin Kletzander]

On Thu, Oct 06, 2016 at 11:40:36AM +0200, Kevin Wolf wrote:

Recently we moved a few options from QemuOptsLists in blockdev.c to
bdrv_runtime_opts in block.c in order to make them accissble using
blockdev-add. However, this has the side effect that these options are
missing from query-command-line-options now, and libvirt consequently
disables the corresponding feature.

This problem was reported as a regression for the 'discard' option,
introduced in commit 818584a4. However, it is more general than that.

Fix it by adding bdrv_runtime_opts to the list of QemuOptsLists that are
returned in query-command-line-options. For the future, libvirt is
advised to use QMP schema introspection for block device options.

Reported-by: Michal Privoznik 
Signed-off-by: Kevin Wolf 
---
block.c | 2 +-
include/sysemu/sysemu.h | 1 +
util/qemu-config.c  | 2 +-
vl.c| 1 +
4 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/block.c b/block.c
index bb1f1ec..40eb570 100644
--- a/block.c
+++ b/block.c
@@ -926,7 +926,7 @@ out:
g_free(gen_node_name);
}

-static QemuOptsList bdrv_runtime_opts = {
+QemuOptsList bdrv_runtime_opts = {
.name = "bdrv_common",
.head = QTAILQ_HEAD_INITIALIZER(bdrv_runtime_opts.head),
.desc = {
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index ef2c50b..b668833 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -235,6 +235,7 @@ bool defaults_enabled(void);
extern QemuOptsList qemu_legacy_drive_opts;
extern QemuOptsList qemu_common_drive_opts;
extern QemuOptsList qemu_drive_opts;
+extern QemuOptsList bdrv_runtime_opts;
extern QemuOptsList qemu_chardev_opts;
extern QemuOptsList qemu_device_opts;
extern QemuOptsList qemu_netdev_opts;
diff --git a/util/qemu-config.c b/util/qemu-config.c
index fb97307..5527100 100644
--- a/util/qemu-config.c
+++ b/util/qemu-config.c
@@ -6,7 +6,7 @@
#include "qmp-commands.h"

static QemuOptsList *vm_config_groups[48];
-static QemuOptsList *drive_config_groups[4];
+static QemuOptsList *drive_config_groups[5];

static QemuOptsList *find_list(QemuOptsList **lists, const char *group,
   Error **errp)
diff --git a/vl.c b/vl.c
index f3abd99..9a2e7d5 100644
--- a/vl.c
+++ b/vl.c
@@ -3035,6 +3035,7 @@ int main(int argc, char **argv, char **envp)
qemu_add_drive_opts(_legacy_drive_opts);
qemu_add_drive_opts(_common_drive_opts);


I'm guessing this doesn't pose a problem with e.g.g "detect-zeroes"
which is in this (_common_drive_opts) as well as


qemu_add_drive_opts(_drive_opts);
+qemu_add_drive_opts(_runtime_opts);


here ^^, right?  Just wanted to make sure ;)  Thanks for the fix.


qemu_add_opts(_chardev_opts);
qemu_add_opts(_device_opts);
qemu_add_opts(_netdev_opts);
--
1.8.3.1



signature.asc
Description: Digital signature

Re: [Qemu-devel] [libvirt-users] Sluggish performance with virtio and Win10

[Martin Kletzander]

On Fri, Feb 19, 2016 at 07:16:12AM +0100, John Obaterspok wrote:

2016-02-18 15:15 GMT+01:00 Martin Kletzander <mklet...@redhat.com>:


On Thu, Feb 18, 2016 at 12:59:52PM +0100, John Obaterspok wrote:


2016-02-18 11:25 GMT+01:00 Martin Kletzander <mklet...@redhat.com>:

On Thu, Feb 18, 2016 at 10:41:42AM +0100, John Obaterspok wrote:


2016-02-18 10:13 GMT+01:00 Martin Kletzander <mklet...@redhat.com>:


On Thu, Feb 18, 2016 at 08:49:38AM +0100, John Obaterspok wrote:



Hello,



I'm using virt-manager on my F23 box to run a Windows 10 image but the
performance is so bad it's killing me.

I have "vmx" flag in /proc/cpuinfo

# lsmod |grep kvm
kvm_intel 167936  6
kvm   503808  1 kvm_intel

virtio-win-0.1.112-1.noarch

But no virtio modules loaded. Should they be loaded nowadays?


Not on the host AFAIK.


The disk format used is vmdk with no caching and native mode.

The io is 100% in windows task manager performing less than 1MB/s


Any clues?


What are the figures from the host?  What is qemu doing and what are
the


other processes and devices doing?


What is the best way to find this out?




{,a,h}top should do for the initial runs, just to see if the block layer

is busy or the CPU is busy or something else is blocking it


atop seems to indicate that sdd is busy?


DSK |  sdd |  busy 96% |  |  read1455 | write
1319 |  KiB/r  5 | KiB/w  9 |   | MBr/s   0.74 | MBw/s
 1.26  | avq 1.01 |   | avio 3.43 ms |

# mount | grep sdd
/dev/sdd2 on /vm type ext4 (rw,relatime,seclabel,data=ordered)



And it doesn't do that in any other process on the host?  It looks like
it's not related to virtualisation...



Hi,

I changed from vmdk to raw and the Write performance went from 1.6 MB/s to
~100 MB/s


Now that's interesting!  Of course raw will be faster but by 2 orders of
magnitude?  That seems unreasonably much more than it should be.


Is vmdk write performance so bad?



I can't say, I wouldn't even expect that -- that's why I didn't even
suggest it.


Result:
http://postimg.org/image/gcqe5affn/



It would be worth asking on qemu-devel, so I'm adding them Cc.  Question
is, was that something wrong with the initial setup that hindered the
performance of the machine or is it just that vmdk performace is bad?
What could we check so that we can hint users in a better direction
performance-wise?

Martin

For a full reference, here is the original thread:

 https://www.redhat.com/archives/libvirt-users/2016-February/msg00048.html


signature.asc
Description: Digital signature

Re: [Qemu-devel] [libvirt] [RFC PATCH 0/2] ARM: add QMP command to query GIC version

[Martin Kletzander]

On Sun, Feb 14, 2016 at 01:41:41PM +0800, Peter Xu wrote:

For ARM platform, we still do not have any interface to query
whether current QEMU/host support specific GIC version. This
patchset is trying to add one QMP interface for that. By querying
the GIC capability using the new interface, one should know exactly
what GIC version(s) the platform will support. The capability bits
will be decided by both QEMU and host kernel.

The current patchset only provides interface for review. Its handler
is a fake one which returns empty always.

The command interface I am planning to add is something like this:

-> { "execute": "query-gic-capability" }
<- { "return": [ "gicv2", "gicv2-kvm", "gicv3-kvm" ] }

Currently, all the possible supported GIC versions are:

- gicv2:  GIC version 2 without kernel IRQ chip
- gicv2-kvm:  GIC version 2 with kernel IRQ chip
- gicv3:  GIC version 3 without kernel IRQ chip (not supported)
- gicv3-kvm:  GIC version 3 with kernel IRQ chip

Since "gicv3" is still not supported (to use GICv3, kernel irqchip
support is required for now, which corresponds to "gicv3-kvm"),
currently the maximum superset of the result should be:

["gicv2", "gicv2-kvm", "gicv3-kvm"]

Please help review whether the interface suits our need, also please
point out any error I have made.



This looks nice.  I have some questions, but I'm not an expert in this
area, so excuse me if they are stupid.

So hardware itself supports some GIC version, let's say 3 for our case.
Does that mean it can be triggered to do v2 as well?  I mean is it
possible that HW supports multiple versions?  If yes, then I suspect
there is (will be) HW that does *not* do it and that's where QEMU (or
KVM) must emulate that version.  If all I'm having in mind is true, then
you are trying to reply with two orthogonal types of information. A)
versions kernel/HW supports and B) qemu/kvm can emulate.  That is fine
if libvirt can choose all the versions specified (e.g. gicv2-kvm,
gicv2), but if we can only select a version, then it might be worth just
returning those two types of information separately, e.g.:

["v2": {"emulated": true, "kvm":true}, "v3": {"emulated": false, "kvm": true}]

But as I said, I don't know the pre-requisites for this and mainly I'm
not dealing with arm support now in libvirt, I'm just curious about it.


One question: how should I make this command "ARM only"? I see that
in qmp-commands.hx, I can use something like "#if defined
TARGET_ARM" to block out ARM specified commands, however how should
I do the similiar thing in qapi-schema.json?



As mentioned in the other thread, making it available everywhere and
just returning a proper error message (so we can parse the class and not
the error message itself) is the best choice, IMHO.


Thanks!
Peter

Peter Xu (2):
 arm: gic: add GICType
 arm: gic: add "query-gic-capability" interface

qapi-schema.json | 28 
qmp-commands.hx  | 25 +
qmp.c|  5 +
scripts/qapi.py  |  1 +
4 files changed, 59 insertions(+)

--
2.4.3

--
libvir-list mailing list
libvir-l...@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


signature.asc
Description: Digital signature

Re: [Qemu-devel] [libvirt] [PATCH] qxl: Fix new function name for spice-server library

[Martin Kletzander]

On Mon, Jul 20, 2015 at 09:43:23AM +0100, Frediano Ziglio wrote:

The new spice-server function to limit the number of monitors (0.12.6)
changed while development from spice_qxl_set_monitors_config_limit to
spice_qxl_max_monitors (accepted upstream).
By mistake I post patch with former name.
This patch fix the function name.

Signed-off-by: Frediano Ziglio fzig...@redhat.com

---
hw/display/qxl.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)

I tested again doing a clean build, unfortunately I did some mistake
and my tests worked.

diff --git a/hw/display/qxl.c b/hw/display/qxl.c
index 4e5ff69..2288238 100644
--- a/hw/display/qxl.c
+++ b/hw/display/qxl.c
@@ -273,8 +273,7 @@ static void qxl_spice_monitors_config_async(PCIQXLDevice 
*qxl, int replay)
} else {
#if SPICE_SERVER_VERSION = 0x000c06 /* release 0.12.6 */
if (qxl-max_outputs) {
-spice_qxl_set_monitors_config_limit(qxl-ssd.qxl,
-qxl-max_outputs);
+spice_qxl_set_max_monitors(qxl-ssd.qxl, qxl-max_outputs);
}
#endif
qxl-guest_monitors_config = qxl-ram-monitors_config;
--
2.1.0



Same as the fix I did in order for this to work with upstream spice.

ACK.  Weak, though, as I'm not a privileged one.

Martin


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH v5] vhost-user: add multi queue support

[Martin Kletzander]

On Thu, Jul 09, 2015 at 02:24:51PM +0200, Maxime Leroy wrote:

Hi Martin, Michael,

On Thu, Jul 9, 2015 at 2:00 PM, Martin Kletzander mklet...@redhat.com wrote:

On Thu, Jul 09, 2015 at 10:06:35AM +0300, Michael S. Tsirkin wrote:


On Thu, Jul 09, 2015 at 12:00:59AM +0200, Maxime Leroy wrote:


Hi Michael,

On Wed, Jul 8, 2015 at 4:29 PM, Michael S. Tsirkin m...@redhat.com
wrote:
 On Thu, May 28, 2015 at 09:23:06AM +0800, Ouyang Changchun wrote:
 Based on patch by Nikolay Nikolaev:
 Vhost-user will implement the multi queue support in a similar way
 to what vhost already has - a separate thread for each queue.
 To enable the multi queue functionality - a new command line parameter
 queues is introduced for the vhost-user netdev.

 Signed-off-by: Nikolay Nikolaev n.nikol...@virtualopensystems.com
 Signed-off-by: Changchun Ouyang changchun.ouy...@intel.com

 So testing turned up a significant issue with the protocol extension in
 this
 one.  Specifically, remote has no idea how many queues guest actually
 wants to use (it's dynamic, guest changes this at any time).
 We need support for enabling and disabling queues dynamically.

 Given we are past hard freeze, and given no one uses this yet
 (dpdk upstream did not merge supporting protocol),
 I think the best thing to do is to disable this functionality for 2.4.
 I will send a patch to do this shortly.

You are making a wrong statement, we already use multiqueue for
vhost-user and we expected to have this support officially integrated
in qemu 2.4.

Libvirt 1.2.17 has been released with multiqueue support for
vhost-user.
(http://libvirt.org/git/?p=libvirt.git;a=commit;h=366c22f2bcf1ddb8253c123f93fd18d1ba9eacd6)
It checks against the version of qemu (i.e. 2.4)  to know if
multiqueue is supported or not by qemu.

(http://libvirt.org/git/?p=libvirt.git;a=commitdiff;h=7971723b985b9adc27122a3503e7ab38ced2b57f;hp=e7f5510ef2d28ca0ae0ed5751b1fd3018130d6c1)



Ouch. Just another example showing how version checks are evil.
We don't want to break libvirt, I agree.



Yes, exactly.  Unfortunately if QEMU doesn't expose it in any way we
don't have many more options.


I think what we can do is accept the command line created
by libvirt, just ignore it and use a single queue only.



Anyway, I think it would be pretty OK to disable it *if and only if*
you error out with a sensible error message (e.g. multiple queues are
not supported yet).


I consider that accepting the queue parameter for vhost-user but only
creates a single queue is a bug.
Unfortunately I don't think we have many solution here for libvirt 1.2.17.0

Agree with Martin, at least, we should display an error message.



And from 2.5 and next libvirt release we can fix this properly
(QEMU - exposing the capability and libvirt - checking for it).


is it possible to backport this fix in the branch 1.2.17 of libvirt ?



Yes, sure, that's what maintenance branches are for ;)


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH v5] vhost-user: add multi queue support

[Martin Kletzander]

On Thu, Jul 09, 2015 at 10:06:35AM +0300, Michael S. Tsirkin wrote:

On Thu, Jul 09, 2015 at 12:00:59AM +0200, Maxime Leroy wrote:

Hi Michael,

On Wed, Jul 8, 2015 at 4:29 PM, Michael S. Tsirkin m...@redhat.com wrote:
 On Thu, May 28, 2015 at 09:23:06AM +0800, Ouyang Changchun wrote:
 Based on patch by Nikolay Nikolaev:
 Vhost-user will implement the multi queue support in a similar way
 to what vhost already has - a separate thread for each queue.
 To enable the multi queue functionality - a new command line parameter
 queues is introduced for the vhost-user netdev.

 Signed-off-by: Nikolay Nikolaev n.nikol...@virtualopensystems.com
 Signed-off-by: Changchun Ouyang changchun.ouy...@intel.com

 So testing turned up a significant issue with the protocol extension in this
 one.  Specifically, remote has no idea how many queues guest actually
 wants to use (it's dynamic, guest changes this at any time).
 We need support for enabling and disabling queues dynamically.

 Given we are past hard freeze, and given no one uses this yet
 (dpdk upstream did not merge supporting protocol),
 I think the best thing to do is to disable this functionality for 2.4.
 I will send a patch to do this shortly.

You are making a wrong statement, we already use multiqueue for
vhost-user and we expected to have this support officially integrated
in qemu 2.4.

Libvirt 1.2.17 has been released with multiqueue support for
vhost-user. 
(http://libvirt.org/git/?p=libvirt.git;a=commit;h=366c22f2bcf1ddb8253c123f93fd18d1ba9eacd6)
It checks against the version of qemu (i.e. 2.4)  to know if
multiqueue is supported or not by qemu.
(http://libvirt.org/git/?p=libvirt.git;a=commitdiff;h=7971723b985b9adc27122a3503e7ab38ced2b57f;hp=e7f5510ef2d28ca0ae0ed5751b1fd3018130d6c1)


Ouch. Just another example showing how version checks are evil.
We don't want to break libvirt, I agree.



Yes, exactly.  Unfortunately if QEMU doesn't expose it in any way we
don't have many more options.


I think what we can do is accept the command line created
by libvirt, just ignore it and use a single queue only.



Anyway, I think it would be pretty OK to disable it *if and only if*
you error out with a sensible error message (e.g. multiple queues are
not supported yet).

And from 2.5 and next libvirt release we can fix this properly
(QEMU - exposing the capability and libvirt - checking for it).


Dynamically enabling/disabling queue between host/guest is a nice
feature to have.
But it's not mandatory.


It's mandated by the virtio spec.


You can still configure manually guest and
host to use the same number of queues.

I think this feature can be implemented later in qemu 2.5.

Regards,

Maxime


Sorry, that's not how virtio is supposed to work. It must downgrade
gracefully, not just stop working or crash guest.

--
MST


signature.asc
Description: PGP signature

Re: [Qemu-devel] [libvirt] Bug: vnc + websocket = websocket autoport not working right at live migration

[Martin Kletzander]

On Tue, Jun 23, 2015 at 02:13:21PM +0200, Piotr Rybicki wrote:

Hello.

Problem description:

When i start qemu via libvirt with vnc websocket defined, it is not
possible to live migrate to host where other qemu process is running
with the same display id.

migration error is:

error: internal error: early end of file from monitor: possible problem:
   [1] = 2015-06-23T11:54:25.590506Z qemu-system-x86_64: -vnc 
0.0.0.0:1,websocket=5700,password: Failed to start VNC server on `(null)': Failed 
to bind socket: Address already in use

(please note vnc display id=1 and websocket=5700 - where it should be 5701)

in libvirt's xml i have:
(...)
   graphics type='vnc' port='-1' autoport='yes' websocket='-1'
listen='0.0.0.0' passwd='xxx'
 listen type='address' address='0.0.0.0'/
   /graphics
(...)

for first and only qemu process on host, this creates qemu commandline:
(...) -vnc 0.0.0.0:0,websocket=5700,password (...)

for second qemu process on the same host:
(...)  -vnc 0.0.0.0:1,websocket=5701,password (...)

There is no problem with migration, when there is no websocket
configuration.

Solution:

I believe, to solve this problem, libvirt has to omit websocket port
definition in commandline string ('websocket=5700' = 'websocket')
when autoport is defined in domain xml definition.



Well, either that or increasing the websocket number as well.  And
that port should be auto-allocated so in case there is something
listening on port 5701 it can select 5702.  That would be even more
error-proof and libvirt would retain full control of qemu (we do that
so that in case the 'websocket = display + 5700' default gets changed,
we still know all the details we set up).


from man qemu:
-vnc display[,option[,option[,...]]]
(...)
 websocket
  Opens an additional TCP listening port dedicated to VNC
Websocket connections.  By definition the Websocket port is
  5700+display. If host is specified connections will
only be allowed from this host.  As an alternative the Websocket port
  could be specified by using websocket=port.  TLS
encryption for the Websocket connection is supported if the required
  certificates are specified with the VNC option x509.

So if I understand it right, not specifying websocket port means
5700+display id, and display id is given via commandline and
increments just fine.

Can anyone confirm this? Or perhaps there is some misconfiguration in
my xml domain definition?



I believe this is a bug in libvirt.  Would you mind creating a bug in
bugzilla for this issue so we can properly track the issue?

Thanks,
Martin


Best regards
Piotr Rybicki

--
libvir-list mailing list
libvir-l...@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


signature.asc
Description: PGP signature

[Qemu-devel] [PATCH] arm: Fix invalid assert logic in op_helper.c

[Martin Kletzander]

Boolean value '!arm_current_el(env)' can never be equal to 3.  Fix it
according to the comment above as currently the build fails with
gcc-5.1.0.

Signed-off-by: Martin Kletzander mklet...@redhat.com
---
 target-arm/op_helper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
index 3f5b9ab5963b..7583ae71217b 100644
--- a/target-arm/op_helper.c
+++ b/target-arm/op_helper.c
@@ -421,7 +421,7 @@ void HELPER(access_check_cp_reg)(CPUARMState *env, void 
*rip, uint32_t syndrome)
 /* Requesting a trap to EL2 when we're in EL3 or S-EL0/1 is
  * a bug in the access function.
  */
-assert(!arm_is_secure(env)  !arm_current_el(env) == 3);
+assert(!arm_is_secure(env)  arm_current_el(env) != 3);
 target_el = 2;
 break;
 case CP_ACCESS_TRAP_EL3:
-- 
2.4.2

Re: [Qemu-devel] [PATCH] arm: Fix invalid assert logic in op_helper.c

[Martin Kletzander]

On Tue, Jun 02, 2015 at 09:10:23AM +0100, Peter Maydell wrote:

On 2 June 2015 at 09:05, Martin Kletzander mklet...@redhat.com wrote:

Boolean value '!arm_current_el(env)' can never be equal to 3.  Fix it
according to the comment above as currently the build fails with
gcc-5.1.0.

Signed-off-by: Martin Kletzander mklet...@redhat.com
---
 target-arm/op_helper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
index 3f5b9ab5963b..7583ae71217b 100644
--- a/target-arm/op_helper.c
+++ b/target-arm/op_helper.c
@@ -421,7 +421,7 @@ void HELPER(access_check_cp_reg)(CPUARMState *env, void 
*rip, uint32_t syndrome)
 /* Requesting a trap to EL2 when we're in EL3 or S-EL0/1 is
  * a bug in the access function.
  */
-assert(!arm_is_secure(env)  !arm_current_el(env) == 3);
+assert(!arm_is_secure(env)  arm_current_el(env) != 3);
 target_el = 2;
 break;
 case CP_ACCESS_TRAP_EL3:


Thanks, but there are already two versions of this patch on the list.



Oh, I'm very sorry for that, I must've missed them.  I'm still missing
the skill to not miss relevant information from the list.

Have a nice day,
Martin


signature.asc
Description: PGP signature

Re: [Qemu-devel] [libvirt] [PATCH 3/5] qemu: add QEMU_CAPS_MACHINE_VMPORT_OPT

[Martin Kletzander]

On Tue, Apr 14, 2015 at 10:07:00AM -0600, Eric Blake wrote:

[adding qemu]

On 04/14/2015 09:58 AM, Marc-André Lureau wrote:

Hi

On Tue, Apr 14, 2015 at 4:25 PM, Martin Kletzander mklet...@redhat.com
wrote:


Is this not exposed in any way in QEMU?  Do we really need to use this
(what we're trying to avoid)?



That works with the following change:

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 768cef1..1b20a7f 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -2510,6 +2510,7 @@ struct virQEMUCapsCommandLineProps {

 static struct virQEMUCapsCommandLineProps virQEMUCapsCommandLine[] = {
 { machine, mem-merge, QEMU_CAPS_MEM_MERGE },
+{ machine, vmport, QEMU_CAPS_MACHINE_VMPORT_OPT },


Ouch.  qemu commit 0a7cf21 fixes what would have been a regression in
2.3 at exposing mem-merge through query-command-line-options, but it
does NOT expose vmport, which is a per-architecture option rather than
a generic -machine option.  Which means that even though qemu 2.2
(perhaps wrongly) advertised vmport for all machines (even when it was
not supported), 2.3 will not advertise it, and we are hoping for a
better solution in 2.4 for properly advertising vmport on an
as-appropriate basis.

Yes, we WANT to use QMP probing,...


 { drive, discard, QEMU_CAPS_DRIVE_DISCARD },
 { realtime, mlock, QEMU_CAPS_MLOCK },
 { boot-opts, strict, QEMU_CAPS_BOOT_STRICT },
@@ -3243,10 +3244,6 @@ virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps,
 if (qemuCaps-version = 1003000)
 virQEMUCapsSet(qemuCaps, QEMU_CAPS_MACHINE_USB_OPT);

-/* vmport option is supported v2.2.0 onwards */
-if (qemuCaps-version = 2002000)
-virQEMUCapsSet(qemuCaps, QEMU_CAPS_MACHINE_VMPORT_OPT);


...and not version comparison, but we'll need something better in QMP
for 2.3 (which is rather late, since we missed 2.3-rc3) if you can't
come up with anything better for learning whether vmport is supported.



Ouch, I missed that.  But that's something we need for more than just
vmport attribute, but also all other machine-specific ones :(

I still think this might go in, though.


signature.asc
Description: PGP signature

Re: [Qemu-devel] [libvirt] Mentors wanted for Outreach Program for Women October 2014

[Martin Kletzander]

On Tue, Aug 26, 2014 at 10:33:27AM +0100, Stefan Hajnoczi wrote:

On Mon, Aug 25, 2014 at 5:40 PM, Marina Zhurakhinskaya
mari...@redhat.com wrote:

- Original Message -

From: Stefan Hajnoczi stefa...@gmail.com
To: Martin Kletzander mklet...@redhat.com
Cc: qemu-devel qemu-devel@nongnu.org, libvir-l...@redhat.com, kvm 
k...@vger.kernel.org, Marina
Zhurakhinskaya mari...@redhat.com
Sent: Monday, August 25, 2014 12:29:27 PM
Subject: Re: [libvirt] Mentors wanted for Outreach Program for Women October 
2014

On Mon, Aug 25, 2014 at 11:52 AM, Martin Kletzander mklet...@redhat.com
wrote:
 On Thu, Aug 21, 2014 at 09:06:39PM +0100, Stefan Hajnoczi wrote:
 Regular code contributors to QEMU, KVM, and libvirt are eligible to
 participate as mentors.

 We also need project ideas that are achievable in 12 weeks by someone
 skilled in programming but not necessarily familiar with open source
 or our codebase.  Ideas welcome!


 It's just a matter of ideas.  Maybe we could revisit some of those we
 had for GSoC.  If I'm reading the deadline for project ideas is
 October 22nd, so I think we'll definitely come up with something.


Thank you for your interest in helping revisit GSoC ideas and come up with new 
ones! October 22 is an application deadline for prospective interns. QEMU would 
need to have some project ideas listed by September 8, though you can add more 
ideas through September. The timeline for the program is at 
https://wiki.gnome.org/OutreachProgramForWomen/2014/DecemberMarch You don't 
need very many ideas, as you are likely to only have at most 2-3 participants.

We don't yet have any funding confirmed for QEMU, but Stefan and I will be 
working on this. If your organization might be able to sponsor QEMU internships 
in OPW, please contact me and Stefan off-list. You can learn more at 
https://wiki.gnome.org/OutreachProgramForWomen/Admin/InfoForOrgs


Thanks for posting the information Marina.

There is now a wiki page for project ideas and OPW information:
http://qemu-project.org/Outreach_Program_for_Women_2014_DecemberMarch



I guess you meant the following link instead:
http://wiki.qemu.org/Outreach_Program_for_Women_2014_DecemberMarch


Mentors: Please post your project ideas on the wiki page.



Will do as soon as I find some free time and energy ;)

Martin


signature.asc
Description: Digital signature

Re: [Qemu-devel] [libvirt] Mentors wanted for Outreach Program for Women October 2014

[Martin Kletzander]

On Thu, Aug 21, 2014 at 09:06:39PM +0100, Stefan Hajnoczi wrote:

Dear mentors and core contributors,
Outreach Program for Women is starting the next round in October 2014.
OPW funds women to work on open source software for 12 weeks with the
help of mentors:
https://wiki.gnome.org/OutreachProgramForWomen/

We just completed a successful round of OPW and Google Summer of Code.
Other organizations have also been participating successfully in OPW
like the Linux kernel with Greg KH and other mentors.

Would you like to mentor in OPW October 2014?



I could use some of my time to help others participate in the
community.


Regular code contributors to QEMU, KVM, and libvirt are eligible to
participate as mentors.

We also need project ideas that are achievable in 12 weeks by someone
skilled in programming but not necessarily familiar with open source
or our codebase.  Ideas welcome!



It's just a matter of ideas.  Maybe we could revisit some of those we
had for GSoC.  If I'm reading the deadline for project ideas is
October 22nd, so I think we'll definitely come up with something.

In first pitch this might be a rewriting of storage driver to handle
jobs (our failed GSoC project from this year), and if admin API gets
added, there will be many APIs and ideas how to expand it.

Martin


Stefan



signature.asc
Description: Digital signature

Re: [Qemu-devel] [PATCH bugfix v1 1/1] char/serial: Fix emptyness check

[Martin Kletzander]

On Mon, Feb 10, 2014 at 10:49:35PM -0800, Peter Crosthwaite wrote:
 This was guarding against a full fifo rather than an empty fifo when
 popping. Fix.

 Signed-off-by: Peter Crosthwaite peter.crosthwa...@xilinx.com
 ---

Reviewed-by: Martin Kletzander mklet...@redhat.com

With this patch qemu doesn't crash in my use-case.


  hw/char/serial.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

 diff --git a/hw/char/serial.c b/hw/char/serial.c
 index 27dab7d..6d3b5af 100644
 --- a/hw/char/serial.c
 +++ b/hw/char/serial.c
 @@ -225,7 +225,7 @@ static gboolean serial_xmit(GIOChannel *chan, 
 GIOCondition cond, void *opaque)

  if (s-tsr_retry = 0) {
  if (s-fcr  UART_FCR_FE) {
 -s-tsr = fifo8_is_full(s-xmit_fifo) ?
 +s-tsr = fifo8_is_empty(s-xmit_fifo) ?
  0 : fifo8_pop(s-xmit_fifo);
  if (!s-xmit_fifo.num) {
  s-lsr |= UART_LSR_THRE;
 --
 1.8.5.4



signature.asc
Description: Digital signature

Re: [Qemu-devel] qemu segfauls with spiceport chardev and isa-serial

[Martin Kletzander]

On Tue, Feb 04, 2014 at 07:05:24AM +0100, Martin Kletzander wrote:
 On Tue, Feb 04, 2014 at 11:40:41AM +1000, Peter Crosthwaite wrote:
  On Tue, Feb 4, 2014 at 4:45 AM, Dr. David Alan Gilbert
  dgilb...@redhat.com wrote:
   (cc'ing in Peter Crosthwaite and Michael Tokarev due to a serial fifo 
   change
   - see below!)
  
   * Martin Kletzander (mklet...@redhat.com) wrote:
   Hello,
  
   Hi Martin,
  I don't know about your spice warnings that triggered this but looking
   down the backtrace I can see something odd:
  
   current HEAD (2f61120c10da9128357510debc8e66880cd2bfdc) segfaults when
   I'm trying to do the following:
  
   I add this to qemu's command-line:
  
-chardev spiceport,id=charserial0,name=org.qemu.console.serial.0 \
-device isa-serial,chardev=charserial0,id=serial0
  
   and then use spicy to connect to that machine.  That spits out the
   following error:
  
GSpice-Message: main channel: opened
port 0x7f74182366e0 org.qemu.console.serial.0: opened
  
(spicy:32386): GSpice-WARNING **: incomplete link header (-104/16)
  
(spicy:32386): GSpice-WARNING **: incomplete link header (-104/16)
GSpice-Message: main channel: closed
  
   I can see that the console works when the window flashes, so there was
   some communication done (Im running the kernel inside with
   console=tty0 console=ttyS0,115200n8 as suggested here:
  
   http://lists.freedesktop.org/archives/spice-devel/2014-January/015919.html
  
   The full command-line with backtrace of all the threads (with
   abort()-ing thread being thread #1 follows.  Let me know if I can help
   anyhow.
  
   Thanks,
   Martin
  
   Command-line:
  
   qemu-system-x86_64 -name rhel7 -S -machine \
   pc-i440fx-1.7,accel=kvm,usb=off,dump-guest-core=off -cpu SandyBridge \
   -m 4101 -realtime mlock=off -smp 1,sockets=1,cores=1,threads=1 -uuid \
   f49fa544-f21d-4267-8958-d82570644f39 -no-user-config -nodefaults \
   -chardev \
   socket,id=charmonitor,path=/var/lib/libvirt/qemu/rhel7.monitor,server,nowait
\
   -mon chardev=charmonitor,id=monitor,mode=control -rtc base=utc \
   -no-shutdown -boot strict=on -device \
   piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 -device \
   virtio-serial-pci,id=virtio-serial0,bus=pci.0,addr=0x6 -drive \
   if=none,id=drive-ide0-0-0,readonly=on,format=raw -device \
   ide-cd,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 -drive \
   file=/home/nert/.config/libvirt/images/rhel7.img,if=none,id=drive-virtio-disk0,format=qcow2
\
   -device \
   virtio-blk-pci,scsi=off,bus=pci.0,addr=0x4,drive=drive-virtio-disk0,id=virtio-disk0,bootindex=1
\
   -netdev tap,fd=20,id=hostnet0,vhost=on,vhostfd=21 -device \
   virtio-net-pci,netdev=hostnet0,id=net0,mac=52:54:00:42:be:45,bus=pci.0,addr=0x3
\
   -chardev spiceport,id=charserial0,name=org.qemu.console.serial.0 \
   -device isa-serial,chardev=charserial0,id=serial0 -device \
   usb-tablet,id=input0 -vnc 127.0.0.1:0 -spice \
   port=5901,tls-port=5902,addr=127.0.0.1,disable-ticketing,x509-dir=/etc/pki/libvirt-spice,seamless-migration=on
\
   -device \
   qxl-vga,id=video0,ram_size=67108864,vram_size=67108864,bus=pci.0,addr=0x2
\
   -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x5
  
   Backtrace:
  
  
   snipped boring threads in poll
  
   Thread 1 (Thread 0x7fee3da66980 (LWP 32022)):
   #0  0x7fee344f1f4e in __GI_raise (sig=sig@entry=6) at 
   ../nptl/sysdeps/unix/sysv/linux/raise.c:56
   #1  0x7fee344f369f in __GI_abort () at abort.c:89
   #2  0x7fee3de72baa in fifo8_pop (fifo=fifo@entry=0x7fee3fc28700) at 
   util/fifo8.c:45
  
   fifo8_pop is aborting because the fifo is empty:
   if (fifo-num == 0) {
   abort();
   }
  
   which seems fair enough
  
   #3  0x7fee3dc0c110 in serial_xmit (chan=optimized out, 
   cond=optimized out, opaque=0x7fee3fc286a0)
   at hw/char/serial.c:228
  
   s-tsr = fifo8_is_full(s-xmit_fifo) ?
   0 : fifo8_pop(s-xmit_fifo);
  
   Hmm, now I don't know anything about the tsr stuff; but that calls
   fifo8_pop whenever the fifo isn't *full* - i.e. it still gets called if 
   empty.
  
   I think the change here comes from Peter's 8e8638fa87ff04 'char/serial: 
   Use generic Fifo8'
   changeset from June which did:
  
   -s-tsr = fifo_get(s,XMIT_FIFO);
   -if (!s-xmit_fifo.count) {
   +s-tsr = fifo8_is_full(s-xmit_fifo) ?
   +0 : fifo8_pop(s-xmit_fifo);
   +if (!s-xmit_fifo.num) {
  
   which makes me think (without having looked at the old data structure
   properly) if that should be   fifo8_is_empty ?
   (The old serial fifo_get routine returned 0 if empty rather than 
   aborting).
  
 
  Hi Dave,
 
  Yes that does looks suss. My bad. Can you confirm your theory by
  making the proposed change? does it fix the bug?
 
  --- a/hw/char/serial.c
  +++ b/hw/char/serial.c
  @@ -225,7 +225,7 @@ static gboolean serial_xmit(GIOChannel *chan

[Qemu-devel] qemu segfauls with spiceport chardev and isa-serial

[Martin Kletzander]

Hello,

current HEAD (2f61120c10da9128357510debc8e66880cd2bfdc) segfaults when
I'm trying to do the following:

I add this to qemu's command-line:

 -chardev spiceport,id=charserial0,name=org.qemu.console.serial.0 \
 -device isa-serial,chardev=charserial0,id=serial0

and then use spicy to connect to that machine.  That spits out the
following error:

 GSpice-Message: main channel: opened
 port 0x7f74182366e0 org.qemu.console.serial.0: opened
 
 (spicy:32386): GSpice-WARNING **: incomplete link header (-104/16)
 
 (spicy:32386): GSpice-WARNING **: incomplete link header (-104/16)
 GSpice-Message: main channel: closed

I can see that the console works when the window flashes, so there was
some communication done (Im running the kernel inside with
console=tty0 console=ttyS0,115200n8 as suggested here:

http://lists.freedesktop.org/archives/spice-devel/2014-January/015919.html

The full command-line with backtrace of all the threads (with
abort()-ing thread being thread #1 follows.  Let me know if I can help
anyhow.

Thanks,
Martin

Command-line:

qemu-system-x86_64 -name rhel7 -S -machine \
pc-i440fx-1.7,accel=kvm,usb=off,dump-guest-core=off -cpu SandyBridge \
-m 4101 -realtime mlock=off -smp 1,sockets=1,cores=1,threads=1 -uuid \
f49fa544-f21d-4267-8958-d82570644f39 -no-user-config -nodefaults \
-chardev \
socket,id=charmonitor,path=/var/lib/libvirt/qemu/rhel7.monitor,server,nowait \
-mon chardev=charmonitor,id=monitor,mode=control -rtc base=utc \
-no-shutdown -boot strict=on -device \
piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 -device \
virtio-serial-pci,id=virtio-serial0,bus=pci.0,addr=0x6 -drive \
if=none,id=drive-ide0-0-0,readonly=on,format=raw -device \
ide-cd,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 -drive \
file=/home/nert/.config/libvirt/images/rhel7.img,if=none,id=drive-virtio-disk0,format=qcow2
 \
-device \
virtio-blk-pci,scsi=off,bus=pci.0,addr=0x4,drive=drive-virtio-disk0,id=virtio-disk0,bootindex=1
 \
-netdev tap,fd=20,id=hostnet0,vhost=on,vhostfd=21 -device \
virtio-net-pci,netdev=hostnet0,id=net0,mac=52:54:00:42:be:45,bus=pci.0,addr=0x3 
\
-chardev spiceport,id=charserial0,name=org.qemu.console.serial.0 \
-device isa-serial,chardev=charserial0,id=serial0 -device \
usb-tablet,id=input0 -vnc 127.0.0.1:0 -spice \
port=5901,tls-port=5902,addr=127.0.0.1,disable-ticketing,x509-dir=/etc/pki/libvirt-spice,seamless-migration=on
 \
-device \
qxl-vga,id=video0,ram_size=67108864,vram_size=67108864,bus=pci.0,addr=0x2 \
-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x5

Backtrace:

Thread 6 (Thread 0x7fed0e1fc700 (LWP 32347)):
#0  sem_timedwait () at 
../nptl/sysdeps/unix/sysv/linux/x86_64/sem_timedwait.S:101
#1  0x7fee3de7096f in qemu_sem_timedwait (sem=sem@entry=0x7fee3faa4e68, 
ms=ms@entry=1)
at util/qemu-thread-posix.c:243
#2  0x7fee3dd2b38c in worker_thread (opaque=0x7fee3faa4dd0) at 
thread-pool.c:97
#3  0x7fee3886a3a5 in start_thread (arg=0x7fed0e1fc700) at 
pthread_create.c:309
#4  0x7fee345b2a3d in clone () at 
../sysdeps/unix/sysv/linux/x86_64/clone.S:111

Thread 5 (Thread 0x7fed0f9ff700 (LWP 32028)):
#0  pthread_cond_wait () at 
../nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_wait.S:185
#1  0x7fee3de7075b in qemu_cond_wait (cond=cond@entry=0x7fee3fd12370, 
mutex=mutex@entry=0x7fee3fd123a0)
at util/qemu-thread-posix.c:121
#2  0x7fee3dd4d1d3 in vnc_worker_thread_loop 
(queue=queue@entry=0x7fee3fd12370) at ui/vnc-jobs.c:222
#3  0x7fee3dd4d680 in vnc_worker_thread (arg=0x7fee3fd12370) at 
ui/vnc-jobs.c:318
#4  0x7fee3886a3a5 in start_thread (arg=0x7fed0f9ff700) at 
pthread_create.c:309
#5  0x7fee345b2a3d in clone () at 
../sysdeps/unix/sysv/linux/x86_64/clone.S:111

Thread 4 (Thread 0x7fecd77fe700 (LWP 32346)):
#0  sem_timedwait () at 
../nptl/sysdeps/unix/sysv/linux/x86_64/sem_timedwait.S:101
#1  0x7fee3de7096f in qemu_sem_timedwait (sem=sem@entry=0x7fee3faa4e68, 
ms=ms@entry=1)
at util/qemu-thread-posix.c:243
#2  0x7fee3dd2b38c in worker_thread (opaque=0x7fee3faa4dd0) at 
thread-pool.c:97
#3  0x7fee3886a3a5 in start_thread (arg=0x7fecd77fe700) at 
pthread_create.c:309
#4  0x7fee345b2a3d in clone () at 
../sysdeps/unix/sysv/linux/x86_64/clone.S:111

Thread 3 (Thread 0x7fee271a7700 (LWP 32025)):
#0  0x7fee345a9917 in ioctl () at ../sysdeps/unix/syscall-template.S:81
#1  0x7fee3ddbda11 in kvm_vcpu_ioctl (cpu=cpu@entry=0x7fee3fc086f0, 
type=type@entry=44672)
at /home/nert/dev/work/qemu/upstream/kvm-all.c:1774
#2  0x7fee3ddbdb07 in kvm_cpu_exec (cpu=cpu@entry=0x7fee3fc086f0)
at /home/nert/dev/work/qemu/upstream/kvm-all.c:1659
#3  0x7fee3dd60562 in qemu_kvm_cpu_thread_fn (arg=0x7fee3fc086f0) at 
/home/nert/dev/work/qemu/upstream/cpus.c:874
#4  0x7fee3886a3a5 in start_thread (arg=0x7fee271a7700) at 
pthread_create.c:309
#5  0x7fee345b2a3d in clone () at 
../sysdeps/unix/sysv/linux/x86_64/clone.S:111

Thread 2 (Thread 0x7fee24fff700 (LWP 32027)):
#0  0x7fee345a7ead in poll () 

Re: [Qemu-devel] qemu segfauls with spiceport chardev and isa-serial

[Martin Kletzander]

On Tue, Feb 04, 2014 at 11:40:41AM +1000, Peter Crosthwaite wrote:
 On Tue, Feb 4, 2014 at 4:45 AM, Dr. David Alan Gilbert
 dgilb...@redhat.com wrote:
  (cc'ing in Peter Crosthwaite and Michael Tokarev due to a serial fifo change
  - see below!)
 
  * Martin Kletzander (mklet...@redhat.com) wrote:
  Hello,
 
  Hi Martin,
 I don't know about your spice warnings that triggered this but looking
  down the backtrace I can see something odd:
 
  current HEAD (2f61120c10da9128357510debc8e66880cd2bfdc) segfaults when
  I'm trying to do the following:
 
  I add this to qemu's command-line:
 
   -chardev spiceport,id=charserial0,name=org.qemu.console.serial.0 \
   -device isa-serial,chardev=charserial0,id=serial0
 
  and then use spicy to connect to that machine.  That spits out the
  following error:
 
   GSpice-Message: main channel: opened
   port 0x7f74182366e0 org.qemu.console.serial.0: opened
 
   (spicy:32386): GSpice-WARNING **: incomplete link header (-104/16)
 
   (spicy:32386): GSpice-WARNING **: incomplete link header (-104/16)
   GSpice-Message: main channel: closed
 
  I can see that the console works when the window flashes, so there was
  some communication done (Im running the kernel inside with
  console=tty0 console=ttyS0,115200n8 as suggested here:
 
  http://lists.freedesktop.org/archives/spice-devel/2014-January/015919.html
 
  The full command-line with backtrace of all the threads (with
  abort()-ing thread being thread #1 follows.  Let me know if I can help
  anyhow.
 
  Thanks,
  Martin
 
  Command-line:
 
  qemu-system-x86_64 -name rhel7 -S -machine \
  pc-i440fx-1.7,accel=kvm,usb=off,dump-guest-core=off -cpu SandyBridge \
  -m 4101 -realtime mlock=off -smp 1,sockets=1,cores=1,threads=1 -uuid \
  f49fa544-f21d-4267-8958-d82570644f39 -no-user-config -nodefaults \
  -chardev \
  socket,id=charmonitor,path=/var/lib/libvirt/qemu/rhel7.monitor,server,nowait
   \
  -mon chardev=charmonitor,id=monitor,mode=control -rtc base=utc \
  -no-shutdown -boot strict=on -device \
  piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 -device \
  virtio-serial-pci,id=virtio-serial0,bus=pci.0,addr=0x6 -drive \
  if=none,id=drive-ide0-0-0,readonly=on,format=raw -device \
  ide-cd,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 -drive \
  file=/home/nert/.config/libvirt/images/rhel7.img,if=none,id=drive-virtio-disk0,format=qcow2
   \
  -device \
  virtio-blk-pci,scsi=off,bus=pci.0,addr=0x4,drive=drive-virtio-disk0,id=virtio-disk0,bootindex=1
   \
  -netdev tap,fd=20,id=hostnet0,vhost=on,vhostfd=21 -device \
  virtio-net-pci,netdev=hostnet0,id=net0,mac=52:54:00:42:be:45,bus=pci.0,addr=0x3
   \
  -chardev spiceport,id=charserial0,name=org.qemu.console.serial.0 \
  -device isa-serial,chardev=charserial0,id=serial0 -device \
  usb-tablet,id=input0 -vnc 127.0.0.1:0 -spice \
  port=5901,tls-port=5902,addr=127.0.0.1,disable-ticketing,x509-dir=/etc/pki/libvirt-spice,seamless-migration=on
   \
  -device \
  qxl-vga,id=video0,ram_size=67108864,vram_size=67108864,bus=pci.0,addr=0x2 \
  -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x5
 
  Backtrace:
 
 
  snipped boring threads in poll
 
  Thread 1 (Thread 0x7fee3da66980 (LWP 32022)):
  #0  0x7fee344f1f4e in __GI_raise (sig=sig@entry=6) at 
  ../nptl/sysdeps/unix/sysv/linux/raise.c:56
  #1  0x7fee344f369f in __GI_abort () at abort.c:89
  #2  0x7fee3de72baa in fifo8_pop (fifo=fifo@entry=0x7fee3fc28700) at 
  util/fifo8.c:45
 
  fifo8_pop is aborting because the fifo is empty:
  if (fifo-num == 0) {
  abort();
  }
 
  which seems fair enough
 
  #3  0x7fee3dc0c110 in serial_xmit (chan=optimized out, 
  cond=optimized out, opaque=0x7fee3fc286a0)
  at hw/char/serial.c:228
 
  s-tsr = fifo8_is_full(s-xmit_fifo) ?
  0 : fifo8_pop(s-xmit_fifo);
 
  Hmm, now I don't know anything about the tsr stuff; but that calls
  fifo8_pop whenever the fifo isn't *full* - i.e. it still gets called if 
  empty.
 
  I think the change here comes from Peter's 8e8638fa87ff04 'char/serial: Use 
  generic Fifo8'
  changeset from June which did:
 
  -s-tsr = fifo_get(s,XMIT_FIFO);
  -if (!s-xmit_fifo.count) {
  +s-tsr = fifo8_is_full(s-xmit_fifo) ?
  +0 : fifo8_pop(s-xmit_fifo);
  +if (!s-xmit_fifo.num) {
 
  which makes me think (without having looked at the old data structure
  properly) if that should be   fifo8_is_empty ?
  (The old serial fifo_get routine returned 0 if empty rather than aborting).
 

 Hi Dave,

 Yes that does looks suss. My bad. Can you confirm your theory by
 making the proposed change? does it fix the bug?

 --- a/hw/char/serial.c
 +++ b/hw/char/serial.c
 @@ -225,7 +225,7 @@ static gboolean serial_xmit(GIOChannel *chan,
 GIOCondition cond, void

  if (s-tsr_retry = 0) {
  if (s-fcr  UART_FCR_FE) {
 -s-tsr = fifo8_is_full(s-xmit_fifo) ?
 +s-tsr = fifo8_is_empty(s-xmit_fifo

Re: [Qemu-devel] [PATCH v2] qmp: expose list of supported character device backends

[Martin Kletzander]

On Fri, Jan 31, 2014 at 10:20:42AM -0700, Eric Blake wrote:
 On 01/31/2014 09:49 AM, Martin Kletzander wrote:
  Introduce 'query-chardev-backends' QMP command which lists all
  supported character device backends.
 
  Signed-off-by: Martin Kletzander mklet...@redhat.com
  ---
  v2:
   - Version changed from 1.8.0 to 2.0
 
   qapi-schema.json | 22 ++
   qemu-char.c  | 19 +++
   qmp-commands.hx  | 41 +
   3 files changed, 82 insertions(+)

  +
  +- { execute: query-chardev-backends }
  +- {
  +  return:[
  + {
  +name:udp,
  + },

 Sorry for not noticing earlier, but this is not valid JSON.  Lose the
 trailing comma after each name:value, since the last element in a
 JSON struct is not permitted to have a comma.


At first, I had it as a list of strings, maybe that's why I left it
there by mistake, hopefully v3 will be OK.

Thanks for the review,

Martin

  + {
  +name:tcp,
  + },
  + {
  +name:unix,
  + },
  + {
  +name:spiceport,
  + }

 With that change to all four spots,

 Reviewed-by: Eric Blake ebl...@redhat.com

 --
 Eric Blake   eblake redhat com+1-919-301-3266
 Libvirt virtualization library http://libvirt.org



signature.asc
Description: Digital signature

[Qemu-devel] [PATCH v3] qmp: expose list of supported character device backends

[Martin Kletzander]

Introduce 'query-chardev-backends' QMP command which lists all
supported character device backends.

Signed-off-by: Martin Kletzander mklet...@redhat.com
---
v3:
 - Omit commas at the end of list in JSON
v2:
 - Version changed from 1.8.0 to 2.0

 qapi-schema.json | 22 ++
 qemu-char.c  | 19 +++
 qmp-commands.hx  | 41 +
 3 files changed, 82 insertions(+)

diff --git a/qapi-schema.json b/qapi-schema.json
index 05ced9d..ebd278a 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -437,6 +437,28 @@
 { 'command': 'query-chardev', 'returns': ['ChardevInfo'] }

 ##
+# @ChardevBackendInfo:
+#
+# Information about a character device backend
+#
+# @name: The backend name
+#
+# Since: 2.0
+##
+{ 'type': 'ChardevBackendInfo', 'data': {'name': 'str'} }
+
+##
+# @query-chardev-backends:
+#
+# Returns information about character device backends.
+#
+# Returns: a list of @ChardevBackendInfo
+#
+# Since: 2.0
+##
+{ 'command': 'query-chardev-backends', 'returns': ['ChardevBackendInfo'] }
+
+##
 # @DataFormat:
 #
 # An enumeration of data format.
diff --git a/qemu-char.c b/qemu-char.c
index 30c5a6a..c88f1c4 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -3432,6 +3432,25 @@ ChardevInfoList *qmp_query_chardev(Error **errp)
 return chr_list;
 }

+ChardevBackendInfoList *qmp_query_chardev_backends(Error **errp)
+{
+ChardevBackendInfoList *backend_list = NULL;
+CharDriver *c = NULL;
+GSList *i = NULL;
+
+for (i = backends; i; i = i-next) {
+ChardevBackendInfoList *info = g_malloc0(sizeof(*info));
+c = i-data;
+info-value = g_malloc0(sizeof(*info-value));
+info-value-name = g_strdup(c-name);
+
+info-next = backend_list;
+backend_list = info;
+}
+
+return backend_list;
+}
+
 CharDriverState *qemu_chr_find(const char *name)
 {
 CharDriverState *chr;
diff --git a/qmp-commands.hx b/qmp-commands.hx
index cce6b81..8a0e832 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -1924,6 +1924,47 @@ EQMP
 },

 SQMP
+query-chardev-backends
+-
+
+List available character device backends.
+
+Each backend is represented by a json-object, the returned value is a 
json-array
+of all backends.
+
+Each json-object contains:
+
+- name: backend name (json-string)
+
+Example:
+
+- { execute: query-chardev-backends }
+- {
+  return:[
+ {
+name:udp
+ },
+ {
+name:tcp
+ },
+ {
+name:unix
+ },
+ {
+name:spiceport
+ }
+  ]
+   }
+
+EQMP
+
+{
+.name   = query-chardev-backends,
+.args_type  = ,
+.mhandler.cmd_new = qmp_marshal_input_query_chardev_backends,
+},
+
+SQMP
 query-block
 ---

--
1.8.5.3

[Qemu-devel] [PATCH] qmp: expose list of supported character device backends

[Martin Kletzander]

Introduce 'query-chardev-backends' QMP command which lists all
supported character device backends.

Signed-off-by: Martin Kletzander mklet...@redhat.com
---
 qapi-schema.json | 22 ++
 qemu-char.c  | 19 +++
 qmp-commands.hx  | 41 +
 3 files changed, 82 insertions(+)

diff --git a/qapi-schema.json b/qapi-schema.json
index 05ced9d..ac1061f 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -437,6 +437,28 @@
 { 'command': 'query-chardev', 'returns': ['ChardevInfo'] }

 ##
+# @ChardevBackendInfo:
+#
+# Information about a character device backend
+#
+# @name: The backend name
+#
+# Since: 1.8.0
+##
+{ 'type': 'ChardevBackendInfo', 'data': {'name': 'str'} }
+
+##
+# @query-chardev-backends:
+#
+# Returns information about character device backends.
+#
+# Returns: a list of @ChardevBackendInfo
+#
+# Since: 1.8.0
+##
+{ 'command': 'query-chardev-backends', 'returns': ['ChardevBackendInfo'] }
+
+##
 # @DataFormat:
 #
 # An enumeration of data format.
diff --git a/qemu-char.c b/qemu-char.c
index 30c5a6a..c88f1c4 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -3432,6 +3432,25 @@ ChardevInfoList *qmp_query_chardev(Error **errp)
 return chr_list;
 }

+ChardevBackendInfoList *qmp_query_chardev_backends(Error **errp)
+{
+ChardevBackendInfoList *backend_list = NULL;
+CharDriver *c = NULL;
+GSList *i = NULL;
+
+for (i = backends; i; i = i-next) {
+ChardevBackendInfoList *info = g_malloc0(sizeof(*info));
+c = i-data;
+info-value = g_malloc0(sizeof(*info-value));
+info-value-name = g_strdup(c-name);
+
+info-next = backend_list;
+backend_list = info;
+}
+
+return backend_list;
+}
+
 CharDriverState *qemu_chr_find(const char *name)
 {
 CharDriverState *chr;
diff --git a/qmp-commands.hx b/qmp-commands.hx
index cce6b81..c38964d 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -1924,6 +1924,47 @@ EQMP
 },

 SQMP
+query-chardev-backends
+-
+
+List available character device backends.
+
+Each backend is represented by a json-object, the returned value is a 
json-array
+of all backends.
+
+Each json-object contains:
+
+- name: backend name (json-string)
+
+Example:
+
+- { execute: query-chardev-backends }
+- {
+  return:[
+ {
+name:udp,
+ },
+ {
+name:tcp,
+ },
+ {
+name:unix,
+ },
+ {
+name:spiceport,
+ }
+  ]
+   }
+
+EQMP
+
+{
+.name   = query-chardev-backends,
+.args_type  = ,
+.mhandler.cmd_new = qmp_marshal_input_query_chardev_backends,
+},
+
+SQMP
 query-block
 ---

-- 
1.8.5.3

[Qemu-devel] [PATCH v2] qmp: expose list of supported character device backends

[Martin Kletzander]

Introduce 'query-chardev-backends' QMP command which lists all
supported character device backends.

Signed-off-by: Martin Kletzander mklet...@redhat.com
---
v2:
 - Version changed from 1.8.0 to 2.0

 qapi-schema.json | 22 ++
 qemu-char.c  | 19 +++
 qmp-commands.hx  | 41 +
 3 files changed, 82 insertions(+)

diff --git a/qapi-schema.json b/qapi-schema.json
index 05ced9d..ebd278a 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -437,6 +437,28 @@
 { 'command': 'query-chardev', 'returns': ['ChardevInfo'] }

 ##
+# @ChardevBackendInfo:
+#
+# Information about a character device backend
+#
+# @name: The backend name
+#
+# Since: 2.0
+##
+{ 'type': 'ChardevBackendInfo', 'data': {'name': 'str'} }
+
+##
+# @query-chardev-backends:
+#
+# Returns information about character device backends.
+#
+# Returns: a list of @ChardevBackendInfo
+#
+# Since: 2.0
+##
+{ 'command': 'query-chardev-backends', 'returns': ['ChardevBackendInfo'] }
+
+##
 # @DataFormat:
 #
 # An enumeration of data format.
diff --git a/qemu-char.c b/qemu-char.c
index 30c5a6a..c88f1c4 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -3432,6 +3432,25 @@ ChardevInfoList *qmp_query_chardev(Error **errp)
 return chr_list;
 }

+ChardevBackendInfoList *qmp_query_chardev_backends(Error **errp)
+{
+ChardevBackendInfoList *backend_list = NULL;
+CharDriver *c = NULL;
+GSList *i = NULL;
+
+for (i = backends; i; i = i-next) {
+ChardevBackendInfoList *info = g_malloc0(sizeof(*info));
+c = i-data;
+info-value = g_malloc0(sizeof(*info-value));
+info-value-name = g_strdup(c-name);
+
+info-next = backend_list;
+backend_list = info;
+}
+
+return backend_list;
+}
+
 CharDriverState *qemu_chr_find(const char *name)
 {
 CharDriverState *chr;
diff --git a/qmp-commands.hx b/qmp-commands.hx
index cce6b81..c38964d 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -1924,6 +1924,47 @@ EQMP
 },

 SQMP
+query-chardev-backends
+-
+
+List available character device backends.
+
+Each backend is represented by a json-object, the returned value is a 
json-array
+of all backends.
+
+Each json-object contains:
+
+- name: backend name (json-string)
+
+Example:
+
+- { execute: query-chardev-backends }
+- {
+  return:[
+ {
+name:udp,
+ },
+ {
+name:tcp,
+ },
+ {
+name:unix,
+ },
+ {
+name:spiceport,
+ }
+  ]
+   }
+
+EQMP
+
+{
+.name   = query-chardev-backends,
+.args_type  = ,
+.mhandler.cmd_new = qmp_marshal_input_query_chardev_backends,
+},
+
+SQMP
 query-block
 ---

--
1.8.5.3

Re: [Qemu-devel] [PATCH v2 0/8] Guest memory allocation fixes cleanup

[Martin Kletzander]

On 06/20/2013 09:54 AM, Markus Armbruster wrote:
 All I wanted to do is exit(1) instead of abort() on guest memory
 allocation failure [07/08].  But that lead me into a minor #ifdef bog,
 and here's what I brought back.  Enjoy!
 
 Testing:
 * Christian Borntraeger reports v1 works fine under LPAR (new S390
   KVM, i.e. generic allocation) and as second guest under z/VM (old
   S390 KVM, i.e. legacy S390 allocation).  Thanks for testing, and for
   catching a stupid mistake.  v2 differs from v1 only in code that
   isn't reachable on S390.
 
 Changes since v1:
 * 5/8: Fix assertion in qemu_ram_remap() (Paolo)
 * All other patches unchanged except for Acked-by in commit messages
 Changes since RFC:
 * 1-3+8/8 unchanged except for commit message tweaks
 * 4+6/8 rewritten to address Paolo's review
 * 5/8 rewritten: don't fix dead code, just assert it's dead
 * 7/8 fix mistakes caught by Richard Henderson and Peter Maydell
 
 Markus Armbruster (8):
   exec: Fix Xen RAM allocation with unusual options
   exec: Clean up fall back when -mem-path allocation fails
   exec: Reduce ifdeffery around -mem-path
   exec: Simplify the guest physical memory allocation hook
   exec: Drop incorrect  dead S390 code in qemu_ram_remap()
   exec: Clean up unnecessary S390 ifdeffery
   exec: Don't abort when we can't allocate guest memory
   pc_sysfw: Fix ISA BIOS init for ridiculously big flash
 
  exec.c  | 121 
 ++--
  hw/block/pc_sysfw.c |   5 +-
  include/exec/cpu-all.h  |   2 -
  include/exec/exec-all.h |   2 +
  include/sysemu/kvm.h|   5 --
  kvm-all.c   |  13 --
  target-s390x/kvm.c  |  23 +++--
  util/oslib-posix.c  |   4 +-
  util/oslib-win32.c  |   5 +-
  9 files changed, 78 insertions(+), 102 deletions(-)
 

I appreciate this.  Works great on x86 and arm architectures.  From
libvirt's POV there's only better error reporting to fix, but that's a
rare race with only a small impact, nothing to do on qemu's side, I guess.

This is what we desired to have, thanks,
Martin

Re: [Qemu-devel] [libvirt] Upstream : virt-install and virt-manager fails to install the guest with the error qemu: could not load PC BIOS 'bios.bin'

[Martin Kletzander]

On 06/12/2013 04:29 PM, chandrashekar shastri wrote:
 
 Hi All,
 
 Upstream : virt-install and virt-manager fails to install the guest with
 the error qemu: could not load PC BIOS 'bios.bin'
 

You are missing bios package for that.  This depends on the distro
you're using, but probably seabios-bin (vgabios, etc., I don't know the
names) could help.  The problem is, however, not caused by you, qemu
itself should have it as a dependency.

 Kernel, Qemu, Libvirt, Virt-Manager is built from the source (git).
 
 kernel version : 3.9.0+
 qemu version : QEMU emulator version 1.5.0
 libvirt version : 1.0.5
 virt-install : 0.600.3
 

From what I know, this should've been fixed [1] in QEMU 1.4.0, but that
might be distribution specific as well.  I guess specific machine types
might require specific BIOSes to be installed.

Hope that helps,
Martin

[1] https://bugzilla.redhat.com/show_bug.cgi?id=901542

  
 Guest installation fails for Standard PC machine type.
 
 virt-install --name vm_name --cdrom iso path --disk disk path  --ram
 2000 --machine pc
 
 Starting install...
 ERRORinternal error process exited while connecting to monitor: char
 device redirected to /dev/pts/1
 qemu: could not load PC BIOS 'bios.bin'
 
 Domain installation does not appear to have been successful.
 If it was, you can restart your domain by running:
   virsh --connect qemu:///system start win7
 otherwise, please restart your installation.
 
 Where as the qemu installation succeeds with the same machine type
 (standard pc)
 
 qemu-system-x86_64 -cdrom iso_path -hda disk_path -boot d -m 1000
 -machine pc
 
 Below is the list of supported machine type:
 
 qemu-system-x86_64 -M ?
 Supported machines are:
 none empty machine
 pc   Standard PC (i440FX + PIIX, 1996) (alias of
 pc-i440fx-1.5)
 pc-i440fx-1.5Standard PC (i440FX + PIIX, 1996) (default)
 pc-i440fx-1.4Standard PC (i440FX + PIIX, 1996)
 pc-1.3   Standard PC
 pc-1.2   Standard PC
 pc-1.1   Standard PC
 pc-1.0   Standard PC
 pc-0.15  Standard PC
 pc-0.14  Standard PC
 pc-0.13  Standard PC
 pc-0.12  Standard PC
 pc-0.11  Standard PC, qemu 0.11
 pc-0.10  Standard PC, qemu 0.10
 isapcISA-only PC
 q35  Standard PC (Q35 + ICH9, 2009) (alias of pc-q35-1.5)
 pc-q35-1.5   Standard PC (Q35 + ICH9, 2009)
 pc-q35-1.4   Standard PC (Q35 + ICH9, 2009)
 
 Installation fails (virt-install) for all machine types expect
 pc-i440fx-1.5 and pc-i440fx-1.4.
 
 Thanks,
 Chandrashekar
 
 
 -- 
 libvir-list mailing list
 libvir-l...@redhat.com
 https://www.redhat.com/mailman/listinfo/libvir-list

[Qemu-devel] Bug in option parsing

[Martin Kletzander]

Hi everybody,

while coding the support for Jason's dump-guest-core option I realized
there is (probably) a problem with the way QEMU parses additional
machine options ('dump-guest-core', 'kvm_shadow_mem' etc.). Running QEMU
with option to -machine works ok, but using '-M' (as libvirt does) works
only w/o additional options, otherwise it ends in an error (the whole
string is probably parsed as a machine name).

Is '-M' so obsolete it shouldn't be used at all or is it just an bug? We
still use '-M' everywhere I know and '-machine' isn't compatible with
older versions and different builds of QEMU.

Should I file a bug or do we have to drop '-M' for this situations?

Have a nice day,
Martin

Re: [Qemu-devel] Bug in option parsing

[Martin Kletzander]

On 08/22/2012 05:15 PM, Jan Kiszka wrote:
 On 2012-08-22 17:05, Martin Kletzander wrote:
 Hi everybody,

 while coding the support for Jason's dump-guest-core option I realized
 there is (probably) a problem with the way QEMU parses additional
 machine options ('dump-guest-core', 'kvm_shadow_mem' etc.). Running QEMU
 with option to -machine works ok, but using '-M' (as libvirt does) works
 only w/o additional options, otherwise it ends in an error (the whole
 string is probably parsed as a machine name).

 Is '-M' so obsolete it shouldn't be used at all or is it just an bug? We
 still use '-M' everywhere I know and '-machine' isn't compatible with
 older versions and different builds of QEMU.

 Should I file a bug or do we have to drop '-M' for this situations?
 
 If libvirt uses -M just like -machine, i.e. with more than a machine
 name, that would be a libvirt bug (but that would have been noticed much
 earlier - did you patch something?). QEMU only keeps -M machine-name
 around to please existing users that didn't switch yet. It is NOT an
 alias for -machine.
 

There is no option *yet* that libvirt would like to use as an additional
option for machine type, but I'm working on the first one and I've hit this.

So your suggestion would be to use -machine wherever it's possible and
fallback to -M only when -machine is not supported?

Martin