[libvirt] [PATCH 0/6] implement support to configure sound output
Pavel Hrdina (6): tests: add test cases for default sound output qemu: move QEMU_AUDIO_DRIVER out of graphic into sound qemu: explicitly disable audio if there is no sound device conf: introduce <output> element for <sound> devices qemu: implement <output> element for <sound> devices tests: add test cases for specific sound output docs/formatdomain.html.in | 11 ++++ docs/schemas/domaincommon.rng | 14 ++++ src/conf/domain_conf.c | 61 ++++++++++++++++++ src/conf/domain_conf.h | 14 ++++ src/libvirt_private.syms | 2 + src/qemu/qemu_command.c | 75 ++++++++++++++-------- src/qemu/qemu_domain.c | 54 ++++++++++++++++ src/qemu/qemu_process.c | 41 ++++++++++++ .../qemuxml2argv-channel-spicevmc-old.args | 2 +- .../qemuxml2argv-channel-spicevmc.args | 2 +- .../qemuxml2argv-clock-france.args | 2 +- .../qemuxml2argv-graphics-sdl-fullscreen.args | 1 + .../qemuxml2argv-graphics-sdl.args | 1 + ...emuxml2argv-graphics-spice-agent-file-xfer.args | 2 +- .../qemuxml2argv-graphics-spice-agentmouse.args | 2 +- ...emuxml2argv-graphics-spice-auto-socket-cfg.args | 2 +- .../qemuxml2argv-graphics-spice-auto-socket.args | 2 +- .../qemuxml2argv-graphics-spice-compression.args | 2 +- .../qemuxml2argv-graphics-spice-no-args.args | 2 +- .../qemuxml2argv-graphics-spice-qxl-vga.args | 2 +- .../qemuxml2argv-graphics-spice-sasl.args | 2 +- .../qemuxml2argv-graphics-spice-socket.args | 2 +- .../qemuxml2argv-graphics-spice-usb-redir.args | 2 +- .../qemuxml2argv-graphics-spice.args | 2 +- .../qemuxml2argvdata/qemuxml2argv-name-escape.args | 2 +- .../qemuxml2argv-serial-spiceport.args | 2 +- .../qemuxml2argv-sound-default-output-sdl.args | 23 +++++++ .../qemuxml2argv-sound-default-output-sdl.xml | 24 +++++++ ...emuxml2argv-sound-default-output-spice-vnc.args | 25 ++++++++ ...qemuxml2argv-sound-default-output-spice-vnc.xml | 25 ++++++++ .../qemuxml2argv-sound-default-output-spice.args | 24 +++++++ .../qemuxml2argv-sound-default-output-spice.xml | 24 +++++++ ...emuxml2argv-sound-default-output-vnc-spice.args | 25 ++++++++ ...qemuxml2argv-sound-default-output-vnc-spice.xml | 25 ++++++++ .../qemuxml2argv-sound-default-output-vnc.args | 24 +++++++ .../qemuxml2argv-sound-default-output-vnc.xml | 24 +++++++ ...xml2argv-sound-multi-different-output-spice.xml | 29 +++++++++ .../qemuxml2argv-sound-multi-pa-output-spice.args | 26 ++++++++ .../qemuxml2argv-sound-multi-pa-output-spice.xml | 27 ++++++++ .../qemuxml2argv-sound-pa-output-spice.args | 24 +++++++ .../qemuxml2argv-sound-pa-output-spice.xml | 26 ++++++++ .../qemuxml2argv-video-virtio-gpu-spice-gl.args | 2 +- tests/qemuxml2argvtest.c | 34 ++++++++++ 43 files changed, 675 insertions(+), 42 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-sound-default-output-sdl.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-sound-default-output-sdl.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-sound-default-output-spice-vnc.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-sound-default-output-spice-vnc.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-sound-default-output-spice.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-sound-default-output-spice.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-sound-default-output-vnc-spice.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-sound-default-output-vnc-spice.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-sound-default-output-vnc.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-sound-default-output-vnc.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-sound-multi-different-output-spice.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-sound-multi-pa-output-spice.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-sound-multi-pa-output-spice.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-sound-pa-output-spice.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-sound-pa-output-spice.xml -- 2.13.6
These test cases models current situation where there is no way how to specify sound output and that it's based on which graphic device is the last one. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- .../qemuxml2argv-sound-default-output-sdl.args | 23 ++++++++++++++++++++ .../qemuxml2argv-sound-default-output-sdl.xml | 24 +++++++++++++++++++++ ...emuxml2argv-sound-default-output-spice-vnc.args | 25 ++++++++++++++++++++++ ...qemuxml2argv-sound-default-output-spice-vnc.xml | 25 ++++++++++++++++++++++ .../qemuxml2argv-sound-default-output-spice.args | 24 +++++++++++++++++++++ .../qemuxml2argv-sound-default-output-spice.xml | 24 +++++++++++++++++++++ ...emuxml2argv-sound-default-output-vnc-spice.args | 25 ++++++++++++++++++++++ ...qemuxml2argv-sound-default-output-vnc-spice.xml | 25 ++++++++++++++++++++++ .../qemuxml2argv-sound-default-output-vnc.args | 24 +++++++++++++++++++++ .../qemuxml2argv-sound-default-output-vnc.xml | 24 +++++++++++++++++++++ tests/qemuxml2argvtest.c | 22 +++++++++++++++++++ 11 files changed, 265 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-sound-default-output-sdl.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-sound-default-output-sdl.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-sound-default-output-spice-vnc.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-sound-default-output-spice-vnc.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-sound-default-output-spice.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-sound-default-output-spice.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-sound-default-output-vnc-spice.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-sound-default-output-vnc-spice.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-sound-default-output-vnc.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-sound-default-output-vnc.xml diff --git a/tests/qemuxml2argvdata/qemuxml2argv-sound-default-output-sdl.args b/tests/qemuxml2argvdata/qemuxml2argv-sound-default-output-sdl.args new file mode 100644 index 0000000000..3e5982b9af --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-sound-default-output-sdl.args @@ -0,0 +1,23 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +/usr/bin/qemu-system-i686 \ +-name QEMUGuest1 \ +-S \ +-M pc \ +-m 214 \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-nodefaults \ +-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,\ +server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=readline \ +-no-acpi \ +-boot c \ +-usb \ +-sdl \ +-vga cirrus \ +-device intel-hda,id=sound0,bus=pci.0,addr=0x3 \ +-device hda-duplex,id=sound0-codec0,bus=sound0.0,cad=0 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-sound-default-output-sdl.xml b/tests/qemuxml2argvdata/qemuxml2argv-sound-default-output-sdl.xml new file mode 100644 index 0000000000..37750ae924 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-sound-default-output-sdl.xml @@ -0,0 +1,24 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-i686</emulator> + <controller type='ide' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <sound model='ich6'/> + <graphics type='sdl'/> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-sound-default-output-spice-vnc.args b/tests/qemuxml2argvdata/qemuxml2argv-sound-default-output-spice-vnc.args new file mode 100644 index 0000000000..f5460887c4 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-sound-default-output-spice-vnc.args @@ -0,0 +1,25 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu-system-i686 \ +-name QEMUGuest1 \ +-S \ +-M pc \ +-m 214 \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-nodefaults \ +-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,\ +server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=readline \ +-no-acpi \ +-boot c \ +-usb \ +-spice port=0 \ +-vnc 127.0.0.1:0 \ +-vga cirrus \ +-device intel-hda,id=sound0,bus=pci.0,addr=0x3 \ +-device hda-duplex,id=sound0-codec0,bus=sound0.0,cad=0 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-sound-default-output-spice-vnc.xml b/tests/qemuxml2argvdata/qemuxml2argv-sound-default-output-spice-vnc.xml new file mode 100644 index 0000000000..4e953162e1 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-sound-default-output-spice-vnc.xml @@ -0,0 +1,25 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-i686</emulator> + <controller type='ide' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <sound model='ich6'/> + <graphics type='spice'/> + <graphics type='vnc'/> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-sound-default-output-spice.args b/tests/qemuxml2argvdata/qemuxml2argv-sound-default-output-spice.args new file mode 100644 index 0000000000..596d5d9412 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-sound-default-output-spice.args @@ -0,0 +1,24 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=spice \ +/usr/bin/qemu-system-i686 \ +-name QEMUGuest1 \ +-S \ +-M pc \ +-m 214 \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-nodefaults \ +-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,\ +server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=readline \ +-no-acpi \ +-boot c \ +-usb \ +-spice port=0 \ +-vga cirrus \ +-device intel-hda,id=sound0,bus=pci.0,addr=0x3 \ +-device hda-duplex,id=sound0-codec0,bus=sound0.0,cad=0 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-sound-default-output-spice.xml b/tests/qemuxml2argvdata/qemuxml2argv-sound-default-output-spice.xml new file mode 100644 index 0000000000..7f8c15307a --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-sound-default-output-spice.xml @@ -0,0 +1,24 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-i686</emulator> + <controller type='ide' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <sound model='ich6'/> + <graphics type='spice'/> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-sound-default-output-vnc-spice.args b/tests/qemuxml2argvdata/qemuxml2argv-sound-default-output-vnc-spice.args new file mode 100644 index 0000000000..eebdeb081e --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-sound-default-output-vnc-spice.args @@ -0,0 +1,25 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=spice \ +/usr/bin/qemu-system-i686 \ +-name QEMUGuest1 \ +-S \ +-M pc \ +-m 214 \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-nodefaults \ +-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,\ +server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=readline \ +-no-acpi \ +-boot c \ +-usb \ +-vnc 127.0.0.1:0 \ +-spice port=0 \ +-vga cirrus \ +-device intel-hda,id=sound0,bus=pci.0,addr=0x3 \ +-device hda-duplex,id=sound0-codec0,bus=sound0.0,cad=0 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-sound-default-output-vnc-spice.xml b/tests/qemuxml2argvdata/qemuxml2argv-sound-default-output-vnc-spice.xml new file mode 100644 index 0000000000..d6d2aa659e --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-sound-default-output-vnc-spice.xml @@ -0,0 +1,25 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-i686</emulator> + <controller type='ide' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <sound model='ich6'/> + <graphics type='vnc'/> + <graphics type='spice'/> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-sound-default-output-vnc.args b/tests/qemuxml2argvdata/qemuxml2argv-sound-default-output-vnc.args new file mode 100644 index 0000000000..d3007a69c2 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-sound-default-output-vnc.args @@ -0,0 +1,24 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu-system-i686 \ +-name QEMUGuest1 \ +-S \ +-M pc \ +-m 214 \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-nodefaults \ +-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,\ +server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=readline \ +-no-acpi \ +-boot c \ +-usb \ +-vnc 127.0.0.1:0 \ +-vga cirrus \ +-device intel-hda,id=sound0,bus=pci.0,addr=0x3 \ +-device hda-duplex,id=sound0-codec0,bus=sound0.0,cad=0 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-sound-default-output-vnc.xml b/tests/qemuxml2argvdata/qemuxml2argv-sound-default-output-vnc.xml new file mode 100644 index 0000000000..c6733aaf2a --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-sound-default-output-vnc.xml @@ -0,0 +1,24 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-i686</emulator> + <controller type='ide' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <sound model='ich6'/> + <graphics type='vnc'/> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 2e07b85aa6..6c80e0bc77 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1551,6 +1551,28 @@ mymain(void) QEMU_CAPS_HDA_DUPLEX, QEMU_CAPS_HDA_MICRO, QEMU_CAPS_DEVICE_ICH9_INTEL_HDA, QEMU_CAPS_OBJECT_USB_AUDIO); + DO_TEST("sound-default-output-spice", + QEMU_CAPS_HDA_DUPLEX, + QEMU_CAPS_SPICE, + QEMU_CAPS_DEVICE_CIRRUS_VGA); + DO_TEST("sound-default-output-vnc", + QEMU_CAPS_HDA_DUPLEX, + QEMU_CAPS_VNC, + QEMU_CAPS_DEVICE_CIRRUS_VGA); + DO_TEST("sound-default-output-sdl", + QEMU_CAPS_HDA_DUPLEX, + QEMU_CAPS_SDL, + QEMU_CAPS_DEVICE_CIRRUS_VGA); + DO_TEST("sound-default-output-spice-vnc", + QEMU_CAPS_HDA_DUPLEX, + QEMU_CAPS_SPICE, + QEMU_CAPS_VNC, + QEMU_CAPS_DEVICE_CIRRUS_VGA); + DO_TEST("sound-default-output-vnc-spice", + QEMU_CAPS_HDA_DUPLEX, + QEMU_CAPS_SPICE, + QEMU_CAPS_VNC, + QEMU_CAPS_DEVICE_CIRRUS_VGA); DO_TEST("fs9p", QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_FSDEV, QEMU_CAPS_FSDEV_WRITEOUT); -- 2.13.6
On 11/14/2017 08:45 AM, Pavel Hrdina wrote:
These test cases models current situation where there is no way how to specify sound output and that it's based on which graphic device is the last one.
Personally had a hard time parsing what the commit message said, but based on what happens 2 patches from now, I think you're just trying to add vm's w/ a sound device. Not so sure it's the "default" sound device or just "a" sound device. Not sure how much clearer you could make things or if it really matters - I'll leave it up to you to reread and be sure what's happening is what you expect!
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- .../qemuxml2argv-sound-default-output-sdl.args | 23 ++++++++++++++++++++ .../qemuxml2argv-sound-default-output-sdl.xml | 24 +++++++++++++++++++++ ...emuxml2argv-sound-default-output-spice-vnc.args | 25 ++++++++++++++++++++++ ...qemuxml2argv-sound-default-output-spice-vnc.xml | 25 ++++++++++++++++++++++ .../qemuxml2argv-sound-default-output-spice.args | 24 +++++++++++++++++++++ .../qemuxml2argv-sound-default-output-spice.xml | 24 +++++++++++++++++++++ ...emuxml2argv-sound-default-output-vnc-spice.args | 25 ++++++++++++++++++++++ ...qemuxml2argv-sound-default-output-vnc-spice.xml | 25 ++++++++++++++++++++++ .../qemuxml2argv-sound-default-output-vnc.args | 24 +++++++++++++++++++++ .../qemuxml2argv-sound-default-output-vnc.xml | 24 +++++++++++++++++++++ tests/qemuxml2argvtest.c | 22 +++++++++++++++++++ 11 files changed, 265 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-sound-default-output-sdl.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-sound-default-output-sdl.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-sound-default-output-spice-vnc.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-sound-default-output-spice-vnc.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-sound-default-output-spice.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-sound-default-output-spice.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-sound-default-output-vnc-spice.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-sound-default-output-vnc-spice.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-sound-default-output-vnc.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-sound-default-output-vnc.xml
Reviewed-by: John Ferlan <jferlan@redhat.com> John
On Mon, Nov 20, 2017 at 05:22:48PM -0500, John Ferlan wrote:
On 11/14/2017 08:45 AM, Pavel Hrdina wrote:
These test cases models current situation where there is no way how to specify sound output and that it's based on which graphic device is the last one.
Personally had a hard time parsing what the commit message said, but based on what happens 2 patches from now, I think you're just trying to add vm's w/ a sound device. Not so sure it's the "default" sound device or just "a" sound device. Not sure how much clearer you could make things or if it really matters - I'll leave it up to you to reread and be sure what's happening is what you expect!
Well I'm sure that this patch does what I expect from it :). How about this commit message: These test cases models current situation where there is no way how to specify audio output of sound devices. The audio output is currently based on which graphic device is configured as the last one in domain XML. Pavel
On 11/21/2017 03:38 AM, Pavel Hrdina wrote:
On Mon, Nov 20, 2017 at 05:22:48PM -0500, John Ferlan wrote:
On 11/14/2017 08:45 AM, Pavel Hrdina wrote:
These test cases models current situation where there is no way how to specify sound output and that it's based on which graphic device is the last one.
Personally had a hard time parsing what the commit message said, but based on what happens 2 patches from now, I think you're just trying to add vm's w/ a sound device. Not so sure it's the "default" sound device or just "a" sound device. Not sure how much clearer you could make things or if it really matters - I'll leave it up to you to reread and be sure what's happening is what you expect!
Well I'm sure that this patch does what I expect from it :).
How about this commit message:
These test cases models current situation where there is no way how
s/models/model the/ s/situation/environment/ s/way how/mechanism/
to specify audio output of sound devices. The audio output is s/audio/the audio/
currently based on which graphic device is configured as the last one in domain XML.
Pavel
Reviewed-by: John Ferlan <jferlan@redhat.com> John
Setting the default audio output depends on specific graphic device but requires having sound device configured as well and it's the sound device that handles the audio. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_command.c | 84 +++++++++++++++------- .../qemuxml2argv-clock-france.args | 2 +- 2 files changed, 58 insertions(+), 28 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index eb72db33ba..e1ef1b05fa 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4442,10 +4442,62 @@ qemuBuildSoundCodecStr(virDomainSoundDefPtr sound, } +static void +qemuBuildSoundAudioEnv(virCommandPtr cmd, + const virDomainDef *def, + virQEMUDriverConfigPtr cfg) +{ + if (def->ngraphics == 0) { + if (cfg->nogfxAllowHostAudio) + virCommandAddEnvPassBlockSUID(cmd, "QEMU_AUDIO_DRV", NULL); + else + virCommandAddEnvString(cmd, "QEMU_AUDIO_DRV=none"); + } else { + switch (def->graphics[def->ngraphics - 1]->type) { + case VIR_DOMAIN_GRAPHICS_TYPE_SDL: + /* If using SDL for video, then we should just let it + * use QEMU's host audio drivers, possibly SDL too + * User can set these two before starting libvirtd + */ + virCommandAddEnvPassBlockSUID(cmd, "QEMU_AUDIO_DRV", NULL); + virCommandAddEnvPassBlockSUID(cmd, "SDL_AUDIODRIVER", NULL); + + break; + + case VIR_DOMAIN_GRAPHICS_TYPE_VNC: + /* Unless user requested it, set the audio backend to none, to + * prevent it opening the host OS audio devices, since that causes + * security issues and might not work when using VNC. + */ + if (cfg->vncAllowHostAudio) + virCommandAddEnvPassBlockSUID(cmd, "QEMU_AUDIO_DRV", NULL); + else + virCommandAddEnvString(cmd, "QEMU_AUDIO_DRV=none"); + + break; + + case VIR_DOMAIN_GRAPHICS_TYPE_SPICE: + /* SPICE includes native support for tunnelling audio, so we + * set the audio backend to point at SPICE's own driver + */ + virCommandAddEnvString(cmd, "QEMU_AUDIO_DRV=spice"); + + break; + + case VIR_DOMAIN_GRAPHICS_TYPE_RDP: + case VIR_DOMAIN_GRAPHICS_TYPE_DESKTOP: + case VIR_DOMAIN_GRAPHICS_TYPE_LAST: + break; + } + } +} + + static int qemuBuildSoundCommandLine(virCommandPtr cmd, const virDomainDef *def, - virQEMUCapsPtr qemuCaps) + virQEMUCapsPtr qemuCaps, + virQEMUDriverConfigPtr cfg) { size_t i, j; @@ -4498,6 +4550,9 @@ qemuBuildSoundCommandLine(virCommandPtr cmd, } } } + + qemuBuildSoundAudioEnv(cmd, def, cfg); + return 0; } @@ -7951,15 +8006,6 @@ qemuBuildGraphicsVNCCommandLine(virQEMUDriverConfigPtr cfg, if (graphics->data.vnc.keymap) virCommandAddArgList(cmd, "-k", graphics->data.vnc.keymap, NULL); - /* Unless user requested it, set the audio backend to none, to - * prevent it opening the host OS audio devices, since that causes - * security issues and might not work when using VNC. - */ - if (cfg->vncAllowHostAudio) - virCommandAddEnvPassBlockSUID(cmd, "QEMU_AUDIO_DRV", NULL); - else - virCommandAddEnvString(cmd, "QEMU_AUDIO_DRV=none"); - return 0; error: @@ -8201,10 +8247,6 @@ qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg, if (graphics->data.spice.keymap) virCommandAddArgList(cmd, "-k", graphics->data.spice.keymap, NULL); - /* SPICE includes native support for tunnelling audio, so we - * set the audio backend to point at SPICE's own driver - */ - virCommandAddEnvString(cmd, "QEMU_AUDIO_DRV=spice"); return 0; @@ -8235,13 +8277,6 @@ qemuBuildGraphicsCommandLine(virQEMUDriverConfigPtr cfg, if (graphics->data.sdl.fullscreen) virCommandAddArg(cmd, "-full-screen"); - /* If using SDL for video, then we should just let it - * use QEMU's host audio drivers, possibly SDL too - * User can set these two before starting libvirtd - */ - virCommandAddEnvPassBlockSUID(cmd, "QEMU_AUDIO_DRV", NULL); - virCommandAddEnvPassBlockSUID(cmd, "SDL_AUDIODRIVER", NULL); - /* New QEMU has this flag to let us explicitly ask for * SDL graphics. This is better than relying on the * default, since the default changes :-( */ @@ -9995,11 +10030,6 @@ qemuBuildCommandLine(virQEMUDriverPtr driver, } else { virCommandAddArg(cmd, "-nographic"); } - - if (cfg->nogfxAllowHostAudio) - virCommandAddEnvPassBlockSUID(cmd, "QEMU_AUDIO_DRV", NULL); - else - virCommandAddEnvString(cmd, "QEMU_AUDIO_DRV=none"); } /* Disable global config files and default devices */ @@ -10083,7 +10113,7 @@ qemuBuildCommandLine(virQEMUDriverPtr driver, if (qemuBuildVideoCommandLine(cmd, def, qemuCaps) < 0) goto error; - if (qemuBuildSoundCommandLine(cmd, def, qemuCaps) < 0) + if (qemuBuildSoundCommandLine(cmd, def, qemuCaps, cfg) < 0) goto error; if (qemuBuildWatchdogCommandLine(cmd, def, qemuCaps) < 0) diff --git a/tests/qemuxml2argvdata/qemuxml2argv-clock-france.args b/tests/qemuxml2argvdata/qemuxml2argv-clock-france.args index 9bde6d967b..2701179273 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-clock-france.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-clock-france.args @@ -3,8 +3,8 @@ PATH=/bin \ HOME=/home/test \ USER=test \ LOGNAME=test \ -QEMU_AUDIO_DRV=none \ TZ=Europe/Paris \ +QEMU_AUDIO_DRV=none \ /usr/bin/qemu-system-i686 \ -name QEMUGuest1 \ -S \ -- 2.13.6
On 11/14/2017 08:45 AM, Pavel Hrdina wrote:
Setting the default audio output depends on specific graphic device but requires having sound device configured as well and it's the sound device that handles the audio.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_command.c | 84 +++++++++++++++------- .../qemuxml2argv-clock-france.args | 2 +- 2 files changed, 58 insertions(+), 28 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index eb72db33ba..e1ef1b05fa 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4442,10 +4442,62 @@ qemuBuildSoundCodecStr(virDomainSoundDefPtr sound, }
+static void +qemuBuildSoundAudioEnv(virCommandPtr cmd, + const virDomainDef *def, + virQEMUDriverConfigPtr cfg) +{ + if (def->ngraphics == 0) { + if (cfg->nogfxAllowHostAudio) + virCommandAddEnvPassBlockSUID(cmd, "QEMU_AUDIO_DRV", NULL); + else + virCommandAddEnvString(cmd, "QEMU_AUDIO_DRV=none"); + } else { + switch (def->graphics[def->ngraphics - 1]->type) {
So it's the "last" graphics device that then defines "how" this all works? Makes sense for QEMU_AUDIO_DRV since whichever is last would be the winner and get the chicken dinner, but...
+ case VIR_DOMAIN_GRAPHICS_TYPE_SDL: + /* If using SDL for video, then we should just let it + * use QEMU's host audio drivers, possibly SDL too + * User can set these two before starting libvirtd + */ + virCommandAddEnvPassBlockSUID(cmd, "QEMU_AUDIO_DRV", NULL); + virCommandAddEnvPassBlockSUID(cmd, "SDL_AUDIODRIVER", NULL);
... if there was more than one graphics device defined, where one was SDL and it wasn't the last one, then theoretically at least this would not be defined. I think it's easily fixable...
+ + break; + + case VIR_DOMAIN_GRAPHICS_TYPE_VNC: + /* Unless user requested it, set the audio backend to none, to + * prevent it opening the host OS audio devices, since that causes + * security issues and might not work when using VNC. + */ + if (cfg->vncAllowHostAudio) + virCommandAddEnvPassBlockSUID(cmd, "QEMU_AUDIO_DRV", NULL); + else + virCommandAddEnvString(cmd, "QEMU_AUDIO_DRV=none"); + + break; + + case VIR_DOMAIN_GRAPHICS_TYPE_SPICE: + /* SPICE includes native support for tunnelling audio, so we + * set the audio backend to point at SPICE's own driver + */ + virCommandAddEnvString(cmd, "QEMU_AUDIO_DRV=spice"); + + break; + + case VIR_DOMAIN_GRAPHICS_TYPE_RDP: + case VIR_DOMAIN_GRAPHICS_TYPE_DESKTOP: + case VIR_DOMAIN_GRAPHICS_TYPE_LAST: + break; + } + } +} + + static int qemuBuildSoundCommandLine(virCommandPtr cmd, const virDomainDef *def, - virQEMUCapsPtr qemuCaps) + virQEMUCapsPtr qemuCaps, + virQEMUDriverConfigPtr cfg) { size_t i, j;
@@ -4498,6 +4550,9 @@ qemuBuildSoundCommandLine(virCommandPtr cmd, } } } + + qemuBuildSoundAudioEnv(cmd, def, cfg); + return 0; }
@@ -7951,15 +8006,6 @@ qemuBuildGraphicsVNCCommandLine(virQEMUDriverConfigPtr cfg, if (graphics->data.vnc.keymap) virCommandAddArgList(cmd, "-k", graphics->data.vnc.keymap, NULL);
- /* Unless user requested it, set the audio backend to none, to - * prevent it opening the host OS audio devices, since that causes - * security issues and might not work when using VNC. - */ - if (cfg->vncAllowHostAudio) - virCommandAddEnvPassBlockSUID(cmd, "QEMU_AUDIO_DRV", NULL); - else - virCommandAddEnvString(cmd, "QEMU_AUDIO_DRV=none"); - return 0;
error: @@ -8201,10 +8247,6 @@ qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg, if (graphics->data.spice.keymap) virCommandAddArgList(cmd, "-k", graphics->data.spice.keymap, NULL); - /* SPICE includes native support for tunnelling audio, so we - * set the audio backend to point at SPICE's own driver - */ - virCommandAddEnvString(cmd, "QEMU_AUDIO_DRV=spice");
return 0;
@@ -8235,13 +8277,6 @@ qemuBuildGraphicsCommandLine(virQEMUDriverConfigPtr cfg, if (graphics->data.sdl.fullscreen) virCommandAddArg(cmd, "-full-screen");
- /* If using SDL for video, then we should just let it - * use QEMU's host audio drivers, possibly SDL too - * User can set these two before starting libvirtd - */ - virCommandAddEnvPassBlockSUID(cmd, "QEMU_AUDIO_DRV", NULL); - virCommandAddEnvPassBlockSUID(cmd, "SDL_AUDIODRIVER", NULL);
... perhaps the fix to above is to just keep SDL_AUDIODRIVER here, just in case there's more than one graphics device and SDL isn't last. Reviewed-by: John Ferlan <jferlan@redhat.com> John
- /* New QEMU has this flag to let us explicitly ask for * SDL graphics. This is better than relying on the * default, since the default changes :-( */ @@ -9995,11 +10030,6 @@ qemuBuildCommandLine(virQEMUDriverPtr driver, } else { virCommandAddArg(cmd, "-nographic"); } - - if (cfg->nogfxAllowHostAudio) - virCommandAddEnvPassBlockSUID(cmd, "QEMU_AUDIO_DRV", NULL); - else - virCommandAddEnvString(cmd, "QEMU_AUDIO_DRV=none"); }
/* Disable global config files and default devices */ @@ -10083,7 +10113,7 @@ qemuBuildCommandLine(virQEMUDriverPtr driver, if (qemuBuildVideoCommandLine(cmd, def, qemuCaps) < 0) goto error;
- if (qemuBuildSoundCommandLine(cmd, def, qemuCaps) < 0) + if (qemuBuildSoundCommandLine(cmd, def, qemuCaps, cfg) < 0) goto error;
if (qemuBuildWatchdogCommandLine(cmd, def, qemuCaps) < 0) diff --git a/tests/qemuxml2argvdata/qemuxml2argv-clock-france.args b/tests/qemuxml2argvdata/qemuxml2argv-clock-france.args index 9bde6d967b..2701179273 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-clock-france.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-clock-france.args @@ -3,8 +3,8 @@ PATH=/bin \ HOME=/home/test \ USER=test \ LOGNAME=test \ -QEMU_AUDIO_DRV=none \ TZ=Europe/Paris \ +QEMU_AUDIO_DRV=none \ /usr/bin/qemu-system-i686 \ -name QEMUGuest1 \ -S \
On Mon, Nov 20, 2017 at 05:39:38PM -0500, John Ferlan wrote:
On 11/14/2017 08:45 AM, Pavel Hrdina wrote:
Setting the default audio output depends on specific graphic device but requires having sound device configured as well and it's the sound device that handles the audio.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_command.c | 84 +++++++++++++++------- .../qemuxml2argv-clock-france.args | 2 +- 2 files changed, 58 insertions(+), 28 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index eb72db33ba..e1ef1b05fa 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4442,10 +4442,62 @@ qemuBuildSoundCodecStr(virDomainSoundDefPtr sound, }
+static void +qemuBuildSoundAudioEnv(virCommandPtr cmd, + const virDomainDef *def, + virQEMUDriverConfigPtr cfg) +{ + if (def->ngraphics == 0) { + if (cfg->nogfxAllowHostAudio) + virCommandAddEnvPassBlockSUID(cmd, "QEMU_AUDIO_DRV", NULL); + else + virCommandAddEnvString(cmd, "QEMU_AUDIO_DRV=none"); + } else { + switch (def->graphics[def->ngraphics - 1]->type) {
So it's the "last" graphics device that then defines "how" this all works? Makes sense for QEMU_AUDIO_DRV since whichever is last would be the winner and get the chicken dinner, but...
+ case VIR_DOMAIN_GRAPHICS_TYPE_SDL: + /* If using SDL for video, then we should just let it + * use QEMU's host audio drivers, possibly SDL too + * User can set these two before starting libvirtd + */ + virCommandAddEnvPassBlockSUID(cmd, "QEMU_AUDIO_DRV", NULL); + virCommandAddEnvPassBlockSUID(cmd, "SDL_AUDIODRIVER", NULL);
... if there was more than one graphics device defined, where one was SDL and it wasn't the last one, then theoretically at least this would not be defined.
This is intentional, I should have mentioned it in the commit message. The original design was just wrong, nothing else. Setting "SDL_AUDIODRIVER" if the SDL audio output is not used is pointless and we shouldn't do it. I can move this change to separate patch. Pavel
On 11/21/2017 03:59 AM, Pavel Hrdina wrote:
On Mon, Nov 20, 2017 at 05:39:38PM -0500, John Ferlan wrote:
On 11/14/2017 08:45 AM, Pavel Hrdina wrote:
Setting the default audio output depends on specific graphic device but requires having sound device configured as well and it's the sound device that handles the audio.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_command.c | 84 +++++++++++++++------- .../qemuxml2argv-clock-france.args | 2 +- 2 files changed, 58 insertions(+), 28 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index eb72db33ba..e1ef1b05fa 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4442,10 +4442,62 @@ qemuBuildSoundCodecStr(virDomainSoundDefPtr sound, }
+static void +qemuBuildSoundAudioEnv(virCommandPtr cmd, + const virDomainDef *def, + virQEMUDriverConfigPtr cfg) +{ + if (def->ngraphics == 0) { + if (cfg->nogfxAllowHostAudio) + virCommandAddEnvPassBlockSUID(cmd, "QEMU_AUDIO_DRV", NULL); + else + virCommandAddEnvString(cmd, "QEMU_AUDIO_DRV=none"); + } else { + switch (def->graphics[def->ngraphics - 1]->type) {
So it's the "last" graphics device that then defines "how" this all works? Makes sense for QEMU_AUDIO_DRV since whichever is last would be the winner and get the chicken dinner, but...
+ case VIR_DOMAIN_GRAPHICS_TYPE_SDL: + /* If using SDL for video, then we should just let it + * use QEMU's host audio drivers, possibly SDL too + * User can set these two before starting libvirtd + */ + virCommandAddEnvPassBlockSUID(cmd, "QEMU_AUDIO_DRV", NULL); + virCommandAddEnvPassBlockSUID(cmd, "SDL_AUDIODRIVER", NULL);
... if there was more than one graphics device defined, where one was SDL and it wasn't the last one, then theoretically at least this would not be defined.
This is intentional, I should have mentioned it in the commit message. The original design was just wrong, nothing else. Setting "SDL_AUDIODRIVER" if the SDL audio output is not used is pointless and we shouldn't do it. I can move this change to separate patch.
Pavel
I figured you had gone through the history - I certain hadn't ;-)... I would say by this point in the review I was still "missing" the final detail regarding the bug you're working on O:) - that the audio was being 'tied to' that last device only. Guess I was thinking it could somehow be magically applied to "any" device. Anyway, long way of saying - it's fine here. Adding a bit more detail to the commit message will help though. John
If there is no sound device configured for the guest we can disable the audio output because hot-plugging sound devices isn't supported. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_command.c | 5 +++++ tests/qemuxml2argvdata/qemuxml2argv-channel-spicevmc-old.args | 2 +- tests/qemuxml2argvdata/qemuxml2argv-channel-spicevmc.args | 2 +- tests/qemuxml2argvdata/qemuxml2argv-graphics-sdl-fullscreen.args | 1 + tests/qemuxml2argvdata/qemuxml2argv-graphics-sdl.args | 1 + .../qemuxml2argv-graphics-spice-agent-file-xfer.args | 2 +- tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-agentmouse.args | 2 +- .../qemuxml2argv-graphics-spice-auto-socket-cfg.args | 2 +- tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-auto-socket.args | 2 +- tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-compression.args | 2 +- tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-no-args.args | 2 +- tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-qxl-vga.args | 2 +- tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-sasl.args | 2 +- tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-socket.args | 2 +- tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-usb-redir.args | 2 +- tests/qemuxml2argvdata/qemuxml2argv-graphics-spice.args | 2 +- tests/qemuxml2argvdata/qemuxml2argv-name-escape.args | 2 +- tests/qemuxml2argvdata/qemuxml2argv-serial-spiceport.args | 2 +- tests/qemuxml2argvdata/qemuxml2argv-video-virtio-gpu-spice-gl.args | 2 +- 19 files changed, 23 insertions(+), 16 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index e1ef1b05fa..c5c7bd7e54 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4447,6 +4447,11 @@ qemuBuildSoundAudioEnv(virCommandPtr cmd, const virDomainDef *def, virQEMUDriverConfigPtr cfg) { + if (def->nsounds == 0) { + virCommandAddEnvString(cmd, "QEMU_AUDIO_DRV=none"); + return; + } + if (def->ngraphics == 0) { if (cfg->nogfxAllowHostAudio) virCommandAddEnvPassBlockSUID(cmd, "QEMU_AUDIO_DRV", NULL); diff --git a/tests/qemuxml2argvdata/qemuxml2argv-channel-spicevmc-old.args b/tests/qemuxml2argvdata/qemuxml2argv-channel-spicevmc-old.args index 19f7e11d22..dae3636f6b 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-channel-spicevmc-old.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-channel-spicevmc-old.args @@ -3,7 +3,7 @@ PATH=/bin \ HOME=/home/test \ USER=test \ LOGNAME=test \ -QEMU_AUDIO_DRV=spice \ +QEMU_AUDIO_DRV=none \ /usr/bin/qemu-system-i686 \ -name QEMUGuest1 \ -S \ diff --git a/tests/qemuxml2argvdata/qemuxml2argv-channel-spicevmc.args b/tests/qemuxml2argvdata/qemuxml2argv-channel-spicevmc.args index fa9f4c5279..1f49107632 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-channel-spicevmc.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-channel-spicevmc.args @@ -3,7 +3,7 @@ PATH=/bin \ HOME=/home/test \ USER=test \ LOGNAME=test \ -QEMU_AUDIO_DRV=spice \ +QEMU_AUDIO_DRV=none \ /usr/bin/qemu-system-i686 \ -name QEMUGuest1 \ -S \ diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-sdl-fullscreen.args b/tests/qemuxml2argvdata/qemuxml2argv-graphics-sdl-fullscreen.args index cc833970cc..ec858ddcb0 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-graphics-sdl-fullscreen.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-sdl-fullscreen.args @@ -5,6 +5,7 @@ USER=test \ LOGNAME=test \ XAUTHORITY=/root/.Xauthority \ DISPLAY=:0.1 \ +QEMU_AUDIO_DRV=none \ /usr/bin/qemu-system-i686 \ -name QEMUGuest1 \ -S \ diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-sdl.args b/tests/qemuxml2argvdata/qemuxml2argv-graphics-sdl.args index b9492e83f4..3f7631dc07 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-graphics-sdl.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-sdl.args @@ -5,6 +5,7 @@ USER=test \ LOGNAME=test \ XAUTHORITY=/root/.Xauthority \ DISPLAY=:0.1 \ +QEMU_AUDIO_DRV=none \ /usr/bin/qemu-system-i686 \ -name QEMUGuest1 \ -S \ diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-agent-file-xfer.args b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-agent-file-xfer.args index 9492458831..433b5c5b68 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-agent-file-xfer.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-agent-file-xfer.args @@ -3,7 +3,7 @@ PATH=/bin \ HOME=/home/test \ USER=test \ LOGNAME=test \ -QEMU_AUDIO_DRV=spice \ +QEMU_AUDIO_DRV=none \ /usr/bin/qemu-system-i686 \ -name QEMUGuest1 \ -S \ diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-agentmouse.args b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-agentmouse.args index a45ab2205c..7d40c10fcd 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-agentmouse.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-agentmouse.args @@ -3,7 +3,7 @@ PATH=/bin \ HOME=/home/test \ USER=test \ LOGNAME=test \ -QEMU_AUDIO_DRV=spice \ +QEMU_AUDIO_DRV=none \ /usr/bin/qemu-system-i686 \ -name QEMUGuest1 \ -S \ diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-auto-socket-cfg.args b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-auto-socket-cfg.args index b0c16077d6..9dfb3c6843 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-auto-socket-cfg.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-auto-socket-cfg.args @@ -3,7 +3,7 @@ PATH=/bin \ HOME=/home/test \ USER=test \ LOGNAME=test \ -QEMU_AUDIO_DRV=spice \ +QEMU_AUDIO_DRV=none \ /usr/bin/qemu-system-i686 \ -name QEMUGuest1 \ -S \ diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-auto-socket.args b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-auto-socket.args index b0c16077d6..9dfb3c6843 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-auto-socket.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-auto-socket.args @@ -3,7 +3,7 @@ PATH=/bin \ HOME=/home/test \ USER=test \ LOGNAME=test \ -QEMU_AUDIO_DRV=spice \ +QEMU_AUDIO_DRV=none \ /usr/bin/qemu-system-i686 \ -name QEMUGuest1 \ -S \ diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-compression.args b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-compression.args index 07a1d12bda..c7dc9e4b8a 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-compression.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-compression.args @@ -3,7 +3,7 @@ PATH=/bin \ HOME=/home/test \ USER=test \ LOGNAME=test \ -QEMU_AUDIO_DRV=spice \ +QEMU_AUDIO_DRV=none \ /usr/bin/qemu-system-i686 \ -name QEMUGuest1 \ -S \ diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-no-args.args b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-no-args.args index e7b402169f..50ac953368 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-no-args.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-no-args.args @@ -3,7 +3,7 @@ PATH=/bin \ HOME=/home/test \ USER=test \ LOGNAME=test \ -QEMU_AUDIO_DRV=spice \ +QEMU_AUDIO_DRV=none \ /usr/bin/qemu-system-i686 \ -name QEMUGuest1 \ -S \ diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-qxl-vga.args b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-qxl-vga.args index f6c25af18a..0d88091675 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-qxl-vga.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-qxl-vga.args @@ -3,7 +3,7 @@ PATH=/bin \ HOME=/home/test \ USER=test \ LOGNAME=test \ -QEMU_AUDIO_DRV=spice \ +QEMU_AUDIO_DRV=none \ /usr/bin/qemu-system-i686 \ -name QEMUGuest1 \ -S \ diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-sasl.args b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-sasl.args index 6198510aa0..e3483e9a71 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-sasl.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-sasl.args @@ -4,7 +4,7 @@ HOME=/home/test \ USER=test \ LOGNAME=test \ SASL_CONF_PATH=/root/.sasl2 \ -QEMU_AUDIO_DRV=spice \ +QEMU_AUDIO_DRV=none \ /usr/bin/qemu-system-i686 \ -name QEMUGuest1 \ -S \ diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-socket.args b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-socket.args index 34a4dced0a..d3a4774cb9 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-socket.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-socket.args @@ -3,7 +3,7 @@ PATH=/bin \ HOME=/home/test \ USER=test \ LOGNAME=test \ -QEMU_AUDIO_DRV=spice \ +QEMU_AUDIO_DRV=none \ /usr/bin/qemu-system-i686 \ -name QEMUGuest1 \ -S \ diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-usb-redir.args b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-usb-redir.args index 8deaee335f..49cc42b792 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-usb-redir.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-usb-redir.args @@ -3,7 +3,7 @@ PATH=/bin \ HOME=/home/test \ USER=test \ LOGNAME=test \ -QEMU_AUDIO_DRV=spice \ +QEMU_AUDIO_DRV=none \ /usr/bin/qemu-system-i686 \ -name QEMUGuest1 \ -S \ diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice.args b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice.args index d5d1869645..2cb76e929e 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice.args @@ -3,7 +3,7 @@ PATH=/bin \ HOME=/home/test \ USER=test \ LOGNAME=test \ -QEMU_AUDIO_DRV=spice \ +QEMU_AUDIO_DRV=none \ /usr/bin/qemu-system-i686 \ -name QEMUGuest1 \ -S \ diff --git a/tests/qemuxml2argvdata/qemuxml2argv-name-escape.args b/tests/qemuxml2argvdata/qemuxml2argv-name-escape.args index d94ab76312..9e631ee5ec 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-name-escape.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-name-escape.args @@ -3,7 +3,7 @@ PATH=/bin \ HOME=/home/test \ USER=test \ LOGNAME=test \ -QEMU_AUDIO_DRV=spice \ +QEMU_AUDIO_DRV=none \ /usr/bin/qemu-system-i686 \ -name guest=foo=1,,bar=2,debug-threads=on \ -S \ diff --git a/tests/qemuxml2argvdata/qemuxml2argv-serial-spiceport.args b/tests/qemuxml2argvdata/qemuxml2argv-serial-spiceport.args index a3981499a2..93d758864e 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-serial-spiceport.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-serial-spiceport.args @@ -3,7 +3,7 @@ PATH=/bin \ HOME=/home/test \ USER=test \ LOGNAME=test \ -QEMU_AUDIO_DRV=spice \ +QEMU_AUDIO_DRV=none \ /usr/bin/qemu-system-i686 \ -name QEMUGuest1 \ -S \ diff --git a/tests/qemuxml2argvdata/qemuxml2argv-video-virtio-gpu-spice-gl.args b/tests/qemuxml2argvdata/qemuxml2argv-video-virtio-gpu-spice-gl.args index 9c6ba79578..04327951f6 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-video-virtio-gpu-spice-gl.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-video-virtio-gpu-spice-gl.args @@ -3,7 +3,7 @@ PATH=/bin \ HOME=/home/test \ USER=test \ LOGNAME=test \ -QEMU_AUDIO_DRV=spice \ +QEMU_AUDIO_DRV=none \ /usr/bin/qemu-system-i686 \ -name QEMUGuest1 \ -S \ -- 2.13.6
On 11/14/2017 08:45 AM, Pavel Hrdina wrote:
If there is no sound device configured for the guest we can disable the audio output because hot-plugging sound devices isn't supported.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_command.c | 5 +++++ tests/qemuxml2argvdata/qemuxml2argv-channel-spicevmc-old.args | 2 +- tests/qemuxml2argvdata/qemuxml2argv-channel-spicevmc.args | 2 +- tests/qemuxml2argvdata/qemuxml2argv-graphics-sdl-fullscreen.args | 1 + tests/qemuxml2argvdata/qemuxml2argv-graphics-sdl.args | 1 + .../qemuxml2argv-graphics-spice-agent-file-xfer.args | 2 +- tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-agentmouse.args | 2 +- .../qemuxml2argv-graphics-spice-auto-socket-cfg.args | 2 +- tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-auto-socket.args | 2 +- tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-compression.args | 2 +- tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-no-args.args | 2 +- tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-qxl-vga.args | 2 +- tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-sasl.args | 2 +- tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-socket.args | 2 +- tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-usb-redir.args | 2 +- tests/qemuxml2argvdata/qemuxml2argv-graphics-spice.args | 2 +- tests/qemuxml2argvdata/qemuxml2argv-name-escape.args | 2 +- tests/qemuxml2argvdata/qemuxml2argv-serial-spiceport.args | 2 +- tests/qemuxml2argvdata/qemuxml2argv-video-virtio-gpu-spice-gl.args | 2 +- 19 files changed, 23 insertions(+), 16 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index e1ef1b05fa..c5c7bd7e54 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4447,6 +4447,11 @@ qemuBuildSoundAudioEnv(virCommandPtr cmd, const virDomainDef *def, virQEMUDriverConfigPtr cfg) { + if (def->nsounds == 0) { + virCommandAddEnvString(cmd, "QEMU_AUDIO_DRV=none"); + return; + } +
But based on the changes to the .args file for Spice - wouldn't the default be whatever Spice had? Now we're requiring someone to configure the sound for Spice? What about a migration... On hostA with 3.9.0 - we have sound... We migrate to hostB with these patches and the sound goes away?
if (def->ngraphics == 0) { if (cfg->nogfxAllowHostAudio)
Also if there was no graphics and no sound device previously, the domain would be started with whatever QEMU_AUDIO_DRV was set to (outside libvirt context), with this path, right? So in this case, we then also would "lose" the sound - I think that'd be the text console case. Maybe I just need to be convince more on this one. Always "of concern" to remove some default just in case "someone" has assumed that [and I haven't looked ahead yet, so my opinion could change again ;-)] John
virCommandAddEnvPassBlockSUID(cmd, "QEMU_AUDIO_DRV", NULL); diff --git a/tests/qemuxml2argvdata/qemuxml2argv-channel-spicevmc-old.args b/tests/qemuxml2argvdata/qemuxml2argv-channel-spicevmc-old.args index 19f7e11d22..dae3636f6b 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-channel-spicevmc-old.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-channel-spicevmc-old.args @@ -3,7 +3,7 @@ PATH=/bin \ HOME=/home/test \ USER=test \ LOGNAME=test \ -QEMU_AUDIO_DRV=spice \ +QEMU_AUDIO_DRV=none \ /usr/bin/qemu-system-i686 \ -name QEMUGuest1 \ -S \ diff --git a/tests/qemuxml2argvdata/qemuxml2argv-channel-spicevmc.args b/tests/qemuxml2argvdata/qemuxml2argv-channel-spicevmc.args index fa9f4c5279..1f49107632 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-channel-spicevmc.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-channel-spicevmc.args @@ -3,7 +3,7 @@ PATH=/bin \ HOME=/home/test \ USER=test \ LOGNAME=test \ -QEMU_AUDIO_DRV=spice \ +QEMU_AUDIO_DRV=none \ /usr/bin/qemu-system-i686 \ -name QEMUGuest1 \ -S \ diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-sdl-fullscreen.args b/tests/qemuxml2argvdata/qemuxml2argv-graphics-sdl-fullscreen.args index cc833970cc..ec858ddcb0 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-graphics-sdl-fullscreen.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-sdl-fullscreen.args @@ -5,6 +5,7 @@ USER=test \ LOGNAME=test \ XAUTHORITY=/root/.Xauthority \ DISPLAY=:0.1 \ +QEMU_AUDIO_DRV=none \ /usr/bin/qemu-system-i686 \ -name QEMUGuest1 \ -S \ diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-sdl.args b/tests/qemuxml2argvdata/qemuxml2argv-graphics-sdl.args index b9492e83f4..3f7631dc07 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-graphics-sdl.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-sdl.args @@ -5,6 +5,7 @@ USER=test \ LOGNAME=test \ XAUTHORITY=/root/.Xauthority \ DISPLAY=:0.1 \ +QEMU_AUDIO_DRV=none \ /usr/bin/qemu-system-i686 \ -name QEMUGuest1 \ -S \ diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-agent-file-xfer.args b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-agent-file-xfer.args index 9492458831..433b5c5b68 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-agent-file-xfer.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-agent-file-xfer.args @@ -3,7 +3,7 @@ PATH=/bin \ HOME=/home/test \ USER=test \ LOGNAME=test \ -QEMU_AUDIO_DRV=spice \ +QEMU_AUDIO_DRV=none \ /usr/bin/qemu-system-i686 \ -name QEMUGuest1 \ -S \ diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-agentmouse.args b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-agentmouse.args index a45ab2205c..7d40c10fcd 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-agentmouse.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-agentmouse.args @@ -3,7 +3,7 @@ PATH=/bin \ HOME=/home/test \ USER=test \ LOGNAME=test \ -QEMU_AUDIO_DRV=spice \ +QEMU_AUDIO_DRV=none \ /usr/bin/qemu-system-i686 \ -name QEMUGuest1 \ -S \ diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-auto-socket-cfg.args b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-auto-socket-cfg.args index b0c16077d6..9dfb3c6843 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-auto-socket-cfg.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-auto-socket-cfg.args @@ -3,7 +3,7 @@ PATH=/bin \ HOME=/home/test \ USER=test \ LOGNAME=test \ -QEMU_AUDIO_DRV=spice \ +QEMU_AUDIO_DRV=none \ /usr/bin/qemu-system-i686 \ -name QEMUGuest1 \ -S \ diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-auto-socket.args b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-auto-socket.args index b0c16077d6..9dfb3c6843 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-auto-socket.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-auto-socket.args @@ -3,7 +3,7 @@ PATH=/bin \ HOME=/home/test \ USER=test \ LOGNAME=test \ -QEMU_AUDIO_DRV=spice \ +QEMU_AUDIO_DRV=none \ /usr/bin/qemu-system-i686 \ -name QEMUGuest1 \ -S \ diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-compression.args b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-compression.args index 07a1d12bda..c7dc9e4b8a 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-compression.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-compression.args @@ -3,7 +3,7 @@ PATH=/bin \ HOME=/home/test \ USER=test \ LOGNAME=test \ -QEMU_AUDIO_DRV=spice \ +QEMU_AUDIO_DRV=none \ /usr/bin/qemu-system-i686 \ -name QEMUGuest1 \ -S \ diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-no-args.args b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-no-args.args index e7b402169f..50ac953368 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-no-args.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-no-args.args @@ -3,7 +3,7 @@ PATH=/bin \ HOME=/home/test \ USER=test \ LOGNAME=test \ -QEMU_AUDIO_DRV=spice \ +QEMU_AUDIO_DRV=none \ /usr/bin/qemu-system-i686 \ -name QEMUGuest1 \ -S \ diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-qxl-vga.args b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-qxl-vga.args index f6c25af18a..0d88091675 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-qxl-vga.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-qxl-vga.args @@ -3,7 +3,7 @@ PATH=/bin \ HOME=/home/test \ USER=test \ LOGNAME=test \ -QEMU_AUDIO_DRV=spice \ +QEMU_AUDIO_DRV=none \ /usr/bin/qemu-system-i686 \ -name QEMUGuest1 \ -S \ diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-sasl.args b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-sasl.args index 6198510aa0..e3483e9a71 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-sasl.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-sasl.args @@ -4,7 +4,7 @@ HOME=/home/test \ USER=test \ LOGNAME=test \ SASL_CONF_PATH=/root/.sasl2 \ -QEMU_AUDIO_DRV=spice \ +QEMU_AUDIO_DRV=none \ /usr/bin/qemu-system-i686 \ -name QEMUGuest1 \ -S \ diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-socket.args b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-socket.args index 34a4dced0a..d3a4774cb9 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-socket.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-socket.args @@ -3,7 +3,7 @@ PATH=/bin \ HOME=/home/test \ USER=test \ LOGNAME=test \ -QEMU_AUDIO_DRV=spice \ +QEMU_AUDIO_DRV=none \ /usr/bin/qemu-system-i686 \ -name QEMUGuest1 \ -S \ diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-usb-redir.args b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-usb-redir.args index 8deaee335f..49cc42b792 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-usb-redir.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-usb-redir.args @@ -3,7 +3,7 @@ PATH=/bin \ HOME=/home/test \ USER=test \ LOGNAME=test \ -QEMU_AUDIO_DRV=spice \ +QEMU_AUDIO_DRV=none \ /usr/bin/qemu-system-i686 \ -name QEMUGuest1 \ -S \ diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice.args b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice.args index d5d1869645..2cb76e929e 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice.args @@ -3,7 +3,7 @@ PATH=/bin \ HOME=/home/test \ USER=test \ LOGNAME=test \ -QEMU_AUDIO_DRV=spice \ +QEMU_AUDIO_DRV=none \ /usr/bin/qemu-system-i686 \ -name QEMUGuest1 \ -S \ diff --git a/tests/qemuxml2argvdata/qemuxml2argv-name-escape.args b/tests/qemuxml2argvdata/qemuxml2argv-name-escape.args index d94ab76312..9e631ee5ec 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-name-escape.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-name-escape.args @@ -3,7 +3,7 @@ PATH=/bin \ HOME=/home/test \ USER=test \ LOGNAME=test \ -QEMU_AUDIO_DRV=spice \ +QEMU_AUDIO_DRV=none \ /usr/bin/qemu-system-i686 \ -name guest=foo=1,,bar=2,debug-threads=on \ -S \ diff --git a/tests/qemuxml2argvdata/qemuxml2argv-serial-spiceport.args b/tests/qemuxml2argvdata/qemuxml2argv-serial-spiceport.args index a3981499a2..93d758864e 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-serial-spiceport.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-serial-spiceport.args @@ -3,7 +3,7 @@ PATH=/bin \ HOME=/home/test \ USER=test \ LOGNAME=test \ -QEMU_AUDIO_DRV=spice \ +QEMU_AUDIO_DRV=none \ /usr/bin/qemu-system-i686 \ -name QEMUGuest1 \ -S \ diff --git a/tests/qemuxml2argvdata/qemuxml2argv-video-virtio-gpu-spice-gl.args b/tests/qemuxml2argvdata/qemuxml2argv-video-virtio-gpu-spice-gl.args index 9c6ba79578..04327951f6 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-video-virtio-gpu-spice-gl.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-video-virtio-gpu-spice-gl.args @@ -3,7 +3,7 @@ PATH=/bin \ HOME=/home/test \ USER=test \ LOGNAME=test \ -QEMU_AUDIO_DRV=spice \ +QEMU_AUDIO_DRV=none \ /usr/bin/qemu-system-i686 \ -name QEMUGuest1 \ -S \
On Mon, Nov 20, 2017 at 05:55:10PM -0500, John Ferlan wrote:
On 11/14/2017 08:45 AM, Pavel Hrdina wrote:
If there is no sound device configured for the guest we can disable the audio output because hot-plugging sound devices isn't supported.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_command.c | 5 +++++ tests/qemuxml2argvdata/qemuxml2argv-channel-spicevmc-old.args | 2 +- tests/qemuxml2argvdata/qemuxml2argv-channel-spicevmc.args | 2 +- tests/qemuxml2argvdata/qemuxml2argv-graphics-sdl-fullscreen.args | 1 + tests/qemuxml2argvdata/qemuxml2argv-graphics-sdl.args | 1 + .../qemuxml2argv-graphics-spice-agent-file-xfer.args | 2 +- tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-agentmouse.args | 2 +- .../qemuxml2argv-graphics-spice-auto-socket-cfg.args | 2 +- tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-auto-socket.args | 2 +- tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-compression.args | 2 +- tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-no-args.args | 2 +- tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-qxl-vga.args | 2 +- tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-sasl.args | 2 +- tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-socket.args | 2 +- tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-usb-redir.args | 2 +- tests/qemuxml2argvdata/qemuxml2argv-graphics-spice.args | 2 +- tests/qemuxml2argvdata/qemuxml2argv-name-escape.args | 2 +- tests/qemuxml2argvdata/qemuxml2argv-serial-spiceport.args | 2 +- tests/qemuxml2argvdata/qemuxml2argv-video-virtio-gpu-spice-gl.args | 2 +- 19 files changed, 23 insertions(+), 16 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index e1ef1b05fa..c5c7bd7e54 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4447,6 +4447,11 @@ qemuBuildSoundAudioEnv(virCommandPtr cmd, const virDomainDef *def, virQEMUDriverConfigPtr cfg) { + if (def->nsounds == 0) { + virCommandAddEnvString(cmd, "QEMU_AUDIO_DRV=none"); + return; + } +
But based on the changes to the .args file for Spice - wouldn't the default be whatever Spice had? Now we're requiring someone to configure the sound for Spice?
What about a migration... On hostA with 3.9.0 - we have sound... We migrate to hostB with these patches and the sound goes away?
if (def->ngraphics == 0) { if (cfg->nogfxAllowHostAudio)
Also if there was no graphics and no sound device previously, the domain would be started with whatever QEMU_AUDIO_DRV was set to (outside libvirt context), with this path, right? So in this case, we then also would "lose" the sound - I think that'd be the text console case.
Maybe I just need to be convince more on this one. Always "of concern" to remove some default just in case "someone" has assumed that [and I haven't looked ahead yet, so my opinion could change again ;-)]
If there is no sound device you have no audio even if you have graphic device configured. It's the same as in real world, if you don't have sound card you don't have audio output. So there is no issue with migration because there was no audio output on hostA. This is what is wrong with the current implementation, the audio output is based on graphic device but there is no connection to sound device. The only connection is that you can configure SPICE audio output and the audio will be send to client via SPICE protocol. So no matter what the QEMU_AUDIO_DRV is set to, if there is no sound device there is no audio. Pavel
On Tue, Nov 14, 2017 at 02:45:08PM +0100, Pavel Hrdina wrote:
If there is no sound device configured for the guest we can disable the audio output because hot-plugging sound devices isn't supported.
Are you sure about that. While libvirt may not have wired up ability to hotplug sound devices, I'm pretty sure that QEMU is able to hotplug them. Ff libvirt forceably disables the audio backend, now, and then future libvirt enables the pre-existing QEMU support for hotplug, existing VMs will be doomed. IOW, I don't think this patch is desirable. 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 :|
On Tue, Nov 21, 2017 at 11:24:06 +0000, Daniel Berrange wrote:
On Tue, Nov 14, 2017 at 02:45:08PM +0100, Pavel Hrdina wrote:
If there is no sound device configured for the guest we can disable the audio output because hot-plugging sound devices isn't supported.
Are you sure about that. While libvirt may not have wired up ability to hotplug sound devices, I'm pretty sure that QEMU is able to hotplug them.
At least the USB sound card should allow hotplug in qemu, so I agree with this... On the other hand the output should be a property which can be configured individually for every sound card. I think it's desirable to have a soundcard dedicated to one output method and a second one for a different output method and let the guest OS decide on which cards the sound will play.
Ff libvirt forceably disables the audio backend, now, and then future libvirt enables the pre-existing QEMU support for hotplug, existing VMs will be doomed.
IOW, I don't think this patch is desirable.
We could allow hotplug only if qemu will allow to specify the sound output per-soundcard which would avoid this problem.
On Tue, Nov 21, 2017 at 12:44:44PM +0100, Peter Krempa wrote:
On Tue, Nov 21, 2017 at 11:24:06 +0000, Daniel Berrange wrote:
On Tue, Nov 14, 2017 at 02:45:08PM +0100, Pavel Hrdina wrote:
If there is no sound device configured for the guest we can disable the audio output because hot-plugging sound devices isn't supported.
Are you sure about that. While libvirt may not have wired up ability to hotplug sound devices, I'm pretty sure that QEMU is able to hotplug them.
At least the USB sound card should allow hotplug in qemu, so I agree with this...
On the other hand the output should be a property which can be configured individually for every sound card. I think it's desirable to have a soundcard dedicated to one output method and a second one for a different output method and let the guest OS decide on which cards the sound will play.
Ff libvirt forceably disables the audio backend, now, and then future libvirt enables the pre-existing QEMU support for hotplug, existing VMs will be doomed.
IOW, I don't think this patch is desirable.
We could allow hotplug only if qemu will allow to specify the sound output per-soundcard which would avoid this problem.
I agree that having ability to configure distinct outputs per sound card might be useful, I don't think it is a blocking feature for hotplugging sound cards. ie, there's no reason why a user should not be able to unplug their current sound card, and plug in a new sound card for a running guests - they would only ever have 1 sound card present at a time in that scenario, so distinct outputs is not a requirement for that usecase. 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 :|
On Tue, Nov 21, 2017 at 11:48:20 +0000, Daniel Berrange wrote:
On Tue, Nov 21, 2017 at 12:44:44PM +0100, Peter Krempa wrote:
On Tue, Nov 21, 2017 at 11:24:06 +0000, Daniel Berrange wrote:
IOW, I don't think this patch is desirable.
We could allow hotplug only if qemu will allow to specify the sound output per-soundcard which would avoid this problem.
I agree that having ability to configure distinct outputs per sound card might be useful, I don't think it is a blocking feature for hotplugging sound cards. ie, there's no reason why a user should not be able to unplug their current sound card, and plug in a new sound card for a running guests - they would only ever have 1 sound card present at a time in that scenario, so distinct outputs is not a requirement for that usecase.
While not a requirement, mandating that new features are available only with new software sometimes saves a lot of headaches. I would not mind if sound device hotplug will be supported only when specific outputs can be selected when adding the sound card and would not work without that. Unfortunately qemu does not yet support this and I don't think that they will any soon. Also code for outputting to pulseaudio does not get much love. I had to fix it so that the virtual sound mixer does not mess with physical michrophone volume setting in the host. And it was broken for years. I agree that this patch could bite us though, given that I don't see qemu fixing the sound backends soon as they did not do it until now.
On Tue, Nov 21, 2017 at 01:02:05PM +0100, Peter Krempa wrote:
On Tue, Nov 21, 2017 at 11:48:20 +0000, Daniel Berrange wrote:
On Tue, Nov 21, 2017 at 12:44:44PM +0100, Peter Krempa wrote:
On Tue, Nov 21, 2017 at 11:24:06 +0000, Daniel Berrange wrote:
IOW, I don't think this patch is desirable.
We could allow hotplug only if qemu will allow to specify the sound output per-soundcard which would avoid this problem.
I agree that having ability to configure distinct outputs per sound card might be useful, I don't think it is a blocking feature for hotplugging sound cards. ie, there's no reason why a user should not be able to unplug their current sound card, and plug in a new sound card for a running guests - they would only ever have 1 sound card present at a time in that scenario, so distinct outputs is not a requirement for that usecase.
While not a requirement, mandating that new features are available only with new software sometimes saves a lot of headaches. I would not mind if sound device hotplug will be supported only when specific outputs can be selected when adding the sound card and would not work without that.
Unfortunately qemu does not yet support this and I don't think that they will any soon.
Agreed, and the most interesting thing for multiple audio cards is probably multi-seat support, which wouldn't need choosing between different backends, instead choosing different spice/vnc instances. Basically, I think this patch should be dropped. 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 :|
On Tue, Nov 21, 2017 at 12:09:54PM +0000, Daniel P. Berrange wrote:
On Tue, Nov 21, 2017 at 01:02:05PM +0100, Peter Krempa wrote:
On Tue, Nov 21, 2017 at 11:48:20 +0000, Daniel Berrange wrote:
On Tue, Nov 21, 2017 at 12:44:44PM +0100, Peter Krempa wrote:
On Tue, Nov 21, 2017 at 11:24:06 +0000, Daniel Berrange wrote:
IOW, I don't think this patch is desirable.
We could allow hotplug only if qemu will allow to specify the sound output per-soundcard which would avoid this problem.
I agree that having ability to configure distinct outputs per sound card might be useful, I don't think it is a blocking feature for hotplugging sound cards. ie, there's no reason why a user should not be able to unplug their current sound card, and plug in a new sound card for a running guests - they would only ever have 1 sound card present at a time in that scenario, so distinct outputs is not a requirement for that usecase.
While not a requirement, mandating that new features are available only with new software sometimes saves a lot of headaches. I would not mind if sound device hotplug will be supported only when specific outputs can be selected when adding the sound card and would not work without that.
Unfortunately qemu does not yet support this and I don't think that they will any soon.
Agreed, and the most interesting thing for multiple audio cards is probably multi-seat support, which wouldn't need choosing between different backends, instead choosing different spice/vnc instances.
Basically, I think this patch should be dropped.
Ok, I don't mind dropping this patch. At least it pointed out that QEMU is design is broken. This patch only affects use case where the domain is started without any sound device and I personally don't care if the audio output will be basically random unless you have spice graphic configured. However, if we enabled hot-plugging sound device we need to make sure that we don't allow configuring sound output until we have a decent way how to configure it. Pavel
On Tue, Nov 21, 2017 at 01:21:26PM +0100, Pavel Hrdina wrote:
On Tue, Nov 21, 2017 at 12:09:54PM +0000, Daniel P. Berrange wrote:
On Tue, Nov 21, 2017 at 01:02:05PM +0100, Peter Krempa wrote:
On Tue, Nov 21, 2017 at 11:48:20 +0000, Daniel Berrange wrote:
On Tue, Nov 21, 2017 at 12:44:44PM +0100, Peter Krempa wrote:
On Tue, Nov 21, 2017 at 11:24:06 +0000, Daniel Berrange wrote:
IOW, I don't think this patch is desirable.
We could allow hotplug only if qemu will allow to specify the sound output per-soundcard which would avoid this problem.
I agree that having ability to configure distinct outputs per sound card might be useful, I don't think it is a blocking feature for hotplugging sound cards. ie, there's no reason why a user should not be able to unplug their current sound card, and plug in a new sound card for a running guests - they would only ever have 1 sound card present at a time in that scenario, so distinct outputs is not a requirement for that usecase.
While not a requirement, mandating that new features are available only with new software sometimes saves a lot of headaches. I would not mind if sound device hotplug will be supported only when specific outputs can be selected when adding the sound card and would not work without that.
Unfortunately qemu does not yet support this and I don't think that they will any soon.
Agreed, and the most interesting thing for multiple audio cards is probably multi-seat support, which wouldn't need choosing between different backends, instead choosing different spice/vnc instances.
Basically, I think this patch should be dropped.
Ok, I don't mind dropping this patch. At least it pointed out that QEMU is design is broken. This patch only affects use case where the domain is started without any sound device and I personally don't care if the audio output will be basically random unless you have spice graphic configured.
However, if we enabled hot-plugging sound device we need to make sure that we don't allow configuring sound output until we have a decent way how to configure it.
Agreed - that's pretty much why i never exposed sound backend in the XML and instead hardcoded it with optional override in qemu.conf when first doing this 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 :|
On Tue, Nov 21, 2017 at 11:24:06AM +0000, Daniel P. Berrange wrote:
On Tue, Nov 14, 2017 at 02:45:08PM +0100, Pavel Hrdina wrote:
If there is no sound device configured for the guest we can disable the audio output because hot-plugging sound devices isn't supported.
Are you sure about that. While libvirt may not have wired up ability to hotplug sound devices, I'm pretty sure that QEMU is able to hotplug them.
Ff libvirt forceably disables the audio backend, now, and then future libvirt enables the pre-existing QEMU support for hotplug, existing VMs will be doomed.
IOW, I don't think this patch is desirable.
Right, I meant from libvirt POV. Anyway, if we allow to hot-plug sound device, you have no control where the audio output will be connected. Configuration of audio output is really stupid in QEMU. You need to use environment variable, you have only one so you cannot configure different sound devices to have different audio output and from documentation it is not clear what is the default if you don't set the QEMU_AUDIO_DRV. Checking the code gives you headache :). The default audio output depends on what audio drivers are enabled and compiled in QEMU and on the order of the audio drivers, these can be used as default: alsa, dsound, core, none, oss, pa, sdl, spice (if there is spice graphics). So based on all of the findings, if we allow hot-plugging sound device and the QEMU_AUDIO_DRV is not configured in advance there is no way how you can configure the audio output and no way how we can tell which one will be the default. I would rather have this limitation that you should start the domain with sound device configured instead of allowing hot-plug since the audio output design in QEMU is foobar. Pavel
On Tue, Nov 21, 2017 at 12:57:36PM +0100, Pavel Hrdina wrote:
On Tue, Nov 21, 2017 at 11:24:06AM +0000, Daniel P. Berrange wrote:
On Tue, Nov 14, 2017 at 02:45:08PM +0100, Pavel Hrdina wrote:
If there is no sound device configured for the guest we can disable the audio output because hot-plugging sound devices isn't supported.
Are you sure about that. While libvirt may not have wired up ability to hotplug sound devices, I'm pretty sure that QEMU is able to hotplug them.
Ff libvirt forceably disables the audio backend, now, and then future libvirt enables the pre-existing QEMU support for hotplug, existing VMs will be doomed.
IOW, I don't think this patch is desirable.
Right, I meant from libvirt POV. Anyway, if we allow to hot-plug sound device, you have no control where the audio output will be connected. Configuration of audio output is really stupid in QEMU. You need to use environment variable, you have only one so you cannot configure different sound devices to have different audio output and from documentation it is not clear what is the default if you don't set the QEMU_AUDIO_DRV. Checking the code gives you headache :).
The default audio output depends on what audio drivers are enabled and compiled in QEMU and on the order of the audio drivers, these can be used as default: alsa, dsound, core, none, oss, pa, sdl, spice (if there is spice graphics).
So based on all of the findings, if we allow hot-plugging sound device and the QEMU_AUDIO_DRV is not configured in advance there is no way how you can configure the audio output and no way how we can tell which one will be the default.
I would rather have this limitation that you should start the domain with sound device configured instead of allowing hot-plug since the audio output design in QEMU is foobar.
As mentioned in my reply to Peter, I don't think ability to configure the sound output is a blocker for hotplug, as there's valid reasons for hotplug whih don't involve multiple cards being present at once. Also 95+% of users of libvirt will have SPICE enabled, so all audio will get routed via SPICE regardless. So what would be most interesting there is not the ability to pick output backend, but the ability to pick which spice server instance is used. This is a more general task of wiring up multi-seat support that QEMU has had for a while, but libvirt doesn't yet support. 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 :|
So far it was not possible to specify how the audio output from guest should be presented to host/users. Now it will be possible to do so via <output> element for <sound> device where you specify the output "type". Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- docs/formatdomain.html.in | 9 +++++++ docs/schemas/domaincommon.rng | 14 ++++++++++ src/conf/domain_conf.c | 61 +++++++++++++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 14 ++++++++++ src/libvirt_private.syms | 2 ++ 5 files changed, 100 insertions(+) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 47c43d0666..3b7c367fc7 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -7058,6 +7058,15 @@ qemu-kvm -net nic,model=? /dev/null slot, <a href="#elementsAddress">documented above</a>. </p> + <p> + <span class="since">Since 3.10.0</span> sound device can have + an optional <code>output</code> element which configures where + the audio output is connected within host. There is mandatory + <code>type</code> attribute where valid values are 'none' to + disable the audio output, 'spice', 'pa', 'sdl', 'alsa', 'oss'. + This might not be supported by all hypervisors. + </p> + <h4><a id="elementsWatchdog">Watchdog device</a></h4> <p> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 9cec1a0637..c499229c43 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -3803,6 +3803,20 @@ <zeroOrMore> <ref name="codec"/> </zeroOrMore> + <optional> + <element name="output"> + <attribute name="type"> + <choice> + <value>none</value> + <value>spice</value> + <value>pa</value> + <value>sdl</value> + <value>alsa</value> + <value>oss</value> + </choice> + </attribute> + </element> + </optional> </interleave> </element> </define> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index fffcc8e9da..33e59c7667 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -517,6 +517,15 @@ VIR_ENUM_IMPL(virDomainSoundModel, VIR_DOMAIN_SOUND_MODEL_LAST, "ich9", "usb") +VIR_ENUM_IMPL(virDomainSoundOutput, VIR_DOMAIN_SOUND_OUTPUT_TYPE_LAST, + "default", + "none", + "spice", + "pa", + "sdl", + "alsa", + "oss") + VIR_ENUM_IMPL(virDomainKeyWrapCipherName, VIR_DOMAIN_KEY_WRAP_CIPHER_NAME_LAST, "aes", @@ -13687,6 +13696,50 @@ virDomainSoundCodecDefParseXML(xmlNodePtr node) } +static int +virDomainSoundOutputParseXML(xmlXPathContextPtr ctxt, + virDomainSoundDefPtr sound) +{ + int ret = -1; + char *type = NULL; + int typeVal; + xmlNodePtr *outputNodes = NULL; + int noutputs; + + noutputs = virXPathNodeSet("./output", ctxt, &outputNodes); + if (noutputs < 0) + return -1; + + if (noutputs > 1) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("sound device can have only one output configured")); + goto cleanup; + } + + if (noutputs > 0) { + if (!(type = virXMLPropString(outputNodes[0], "type"))) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("sound output type must be specified")); + goto cleanup; + } + + if ((typeVal = virDomainSoundOutputTypeFromString(type)) < 0) { + virReportError(VIR_ERR_INVALID_ARG, + _("invalid sound output type '%s'"), type); + goto cleanup; + } + + sound->output = typeVal; + } + + ret = 0; + cleanup: + VIR_FREE(outputNodes); + VIR_FREE(type); + return ret; +} + + static virDomainSoundDefPtr virDomainSoundDefParseXML(virDomainXMLOptionPtr xmlopt, xmlNodePtr node, @@ -13741,6 +13794,9 @@ virDomainSoundDefParseXML(virDomainXMLOptionPtr xmlopt, } } + if (virDomainSoundOutputParseXML(ctxt, def) < 0) + goto error; + if (virDomainDeviceInfoParseXML(xmlopt, node, NULL, &def->info, flags) < 0) goto error; @@ -24111,6 +24167,11 @@ virDomainSoundDefFormat(virBufferPtr buf, virDomainDeviceInfoFormat(&childBuf, &def->info, flags); + if (def->output != VIR_DOMAIN_SOUND_OUTPUT_TYPE_DEFAULT) { + virBufferAsprintf(&childBuf, "<output type='%s'/>\n", + virDomainSoundOutputTypeToString(def->output)); + } + if (virBufferCheckError(&childBuf) < 0) return -1; diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index e3f060b122..55a984c781 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1332,12 +1332,25 @@ struct _virDomainSoundCodecDef { int cad; }; +typedef enum { + VIR_DOMAIN_SOUND_OUTPUT_TYPE_DEFAULT, + VIR_DOMAIN_SOUND_OUTPUT_TYPE_NONE, + VIR_DOMAIN_SOUND_OUTPUT_TYPE_SPICE, + VIR_DOMAIN_SOUND_OUTPUT_TYPE_PA, + VIR_DOMAIN_SOUND_OUTPUT_TYPE_SDL, + VIR_DOMAIN_SOUND_OUTPUT_TYPE_ALSA, + VIR_DOMAIN_SOUND_OUTPUT_TYPE_OSS, + + VIR_DOMAIN_SOUND_OUTPUT_TYPE_LAST +} virDomainSoundOutputType; + struct _virDomainSoundDef { int model; virDomainDeviceInfo info; size_t ncodecs; virDomainSoundCodecDefPtr *codecs; + virDomainSoundOutputType output; }; typedef enum { @@ -3246,6 +3259,7 @@ VIR_ENUM_DECL(virDomainChrTcpProtocol) VIR_ENUM_DECL(virDomainChrSpicevmc) VIR_ENUM_DECL(virDomainSoundCodec) VIR_ENUM_DECL(virDomainSoundModel) +VIR_ENUM_DECL(virDomainSoundOutput) VIR_ENUM_DECL(virDomainKeyWrapCipherName) VIR_ENUM_DECL(virDomainMemballoonModel) VIR_ENUM_DECL(virDomainSmbiosMode) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 5a4d50471d..0ef7e896d8 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -516,6 +516,8 @@ virDomainSmbiosModeTypeToString; virDomainSoundDefFree; virDomainSoundModelTypeFromString; virDomainSoundModelTypeToString; +virDomainSoundOutputTypeFromString; +virDomainSoundOutputTypeToString; virDomainStartupPolicyTypeFromString; virDomainStartupPolicyTypeToString; virDomainStateReasonFromString; -- 2.13.6
On 11/14/2017 08:45 AM, Pavel Hrdina wrote:
So far it was not possible to specify how the audio output from guest
s/So far it was/It is/
should be presented to host/users. Now it will be possible to do so via <output> element for <sound> device where you specify the output "type".
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- docs/formatdomain.html.in | 9 +++++++ docs/schemas/domaincommon.rng | 14 ++++++++++ src/conf/domain_conf.c | 61 +++++++++++++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 14 ++++++++++ src/libvirt_private.syms | 2 ++ 5 files changed, 100 insertions(+)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 47c43d0666..3b7c367fc7 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -7058,6 +7058,15 @@ qemu-kvm -net nic,model=? /dev/null slot, <a href="#elementsAddress">documented above</a>. </p>
+ <p> + <span class="since">Since 3.10.0</span> sound device can have
s/sound/, a sound/
+ an optional <code>output</code> element which configures where + the audio output is connected within host. There is mandatory + <code>type</code> attribute where valid values are 'none' to + disable the audio output, 'spice', 'pa', 'sdl', 'alsa', 'oss'.
I've become preferential to seeing these in a list. I have no idea based on the text here what 'spice', ... 'oss' really means. IOW: Why would I want to set to something else - what does it do/provide?
+ This might not be supported by all hypervisors.
True, but perhaps also true of many other things, too.
+ </p> + <h4><a id="elementsWatchdog">Watchdog device</a></h4>
<p>
Reviewed-by: John Ferlan <jferlan@redhat.com> John [...]
On Mon, Nov 20, 2017 at 06:04:27PM -0500, John Ferlan wrote:
On 11/14/2017 08:45 AM, Pavel Hrdina wrote:
So far it was not possible to specify how the audio output from guest
s/So far it was/It is/
should be presented to host/users. Now it will be possible to do so via <output> element for <sound> device where you specify the output "type".
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- docs/formatdomain.html.in | 9 +++++++ docs/schemas/domaincommon.rng | 14 ++++++++++ src/conf/domain_conf.c | 61 +++++++++++++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 14 ++++++++++ src/libvirt_private.syms | 2 ++ 5 files changed, 100 insertions(+)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 47c43d0666..3b7c367fc7 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -7058,6 +7058,15 @@ qemu-kvm -net nic,model=? /dev/null slot, <a href="#elementsAddress">documented above</a>. </p>
+ <p> + <span class="since">Since 3.10.0</span> sound device can have
s/sound/, a sound/
+ an optional <code>output</code> element which configures where + the audio output is connected within host. There is mandatory + <code>type</code> attribute where valid values are 'none' to + disable the audio output, 'spice', 'pa', 'sdl', 'alsa', 'oss'.
I've become preferential to seeing these in a list. I have no idea based on the text here what 'spice', ... 'oss' really means. IOW: Why would I want to set to something else - what does it do/provide?
I'm not sure how to describe it more than providing links to all the projects. SPICE is the only unique output since the audio stream is sent over SPICE protocol, but the remaining outputs are IMHO self-explanatory because the are standard linux/unix audio layers/libraries. Describing all the differences is out of scope of our documentation.
+ This might not be supported by all hypervisors.
True, but perhaps also true of many other things, too.
I can change it to "Currently supported only by QEMU driver."
+ </p> + <h4><a id="elementsWatchdog">Watchdog device</a></h4>
<p>
Reviewed-by: John Ferlan <jferlan@redhat.com>
John
[...]
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 11/21/2017 08:12 AM, Pavel Hrdina wrote:
On Mon, Nov 20, 2017 at 06:04:27PM -0500, John Ferlan wrote:
On 11/14/2017 08:45 AM, Pavel Hrdina wrote:
So far it was not possible to specify how the audio output from guest
s/So far it was/It is/
should be presented to host/users. Now it will be possible to do so via <output> element for <sound> device where you specify the output "type".
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- docs/formatdomain.html.in | 9 +++++++ docs/schemas/domaincommon.rng | 14 ++++++++++ src/conf/domain_conf.c | 61 +++++++++++++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 14 ++++++++++ src/libvirt_private.syms | 2 ++ 5 files changed, 100 insertions(+)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 47c43d0666..3b7c367fc7 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -7058,6 +7058,15 @@ qemu-kvm -net nic,model=? /dev/null slot, <a href="#elementsAddress">documented above</a>. </p>
+ <p> + <span class="since">Since 3.10.0</span> sound device can have
s/sound/, a sound/
+ an optional <code>output</code> element which configures where + the audio output is connected within host. There is mandatory + <code>type</code> attribute where valid values are 'none' to + disable the audio output, 'spice', 'pa', 'sdl', 'alsa', 'oss'.
I've become preferential to seeing these in a list. I have no idea based on the text here what 'spice', ... 'oss' really means. IOW: Why would I want to set to something else - what does it do/provide?
I'm not sure how to describe it more than providing links to all the projects. SPICE is the only unique output since the audio stream is sent over SPICE protocol, but the remaining outputs are IMHO self-explanatory because the are standard linux/unix audio layers/libraries.
I was reading literally vis-a-vis a list of supported values and there was something missing. I was not thinking in terms that the list is/was from the standard linux/unix audio layers/libraries. As I've learned since originally looking at this - the overall design of graphics/sound is a bit, well, odd/unusual... Anwyay restated: There is mandatory <code>type</code> attribute where valid values to enable the audio output are 'spice', 'pa', 'sdl', 'alsa', or 'oss'. Use 'none' in order to disable the audio output. With a bit more thought on this now - why would someone add a sound card, but select an output of 'none'? Why even add the sound card then? You have a sound card that doesn't emit sound anywhere? Why have it?! And in summary, it seems to me now that what the 'output' is designed to do is supply the driver for the QEMU_AUDIO_DRV environment variable, true? To override what may be globally set?
Describing all the differences is out of scope of our documentation.
Hmmm.. maybe I wasn't clear enough - it wasn't the differences that I cared about it was the what are those shorthand acronyms. Someone reading the list literally may not know what they are. I certainly don't think open sound system (I had to look it up)... So pa is perhaps a known synonym for pulseaudio...
+ This might not be supported by all hypervisors.
True, but perhaps also true of many other things, too.
I can change it to "Currently supported only by QEMU driver."
OK, so this makes sense in light of what appears to be the goal - to provide something for the env variable. BTW: Probably should add the xml2xml parsing tests to this patch - although they are present later - it is something important to ask of any XML changes. John
+ </p> + <h4><a id="elementsWatchdog">Watchdog device</a></h4>
<p>
Reviewed-by: John Ferlan <jferlan@redhat.com>
John
[...]
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Tue, Nov 14, 2017 at 02:45:09PM +0100, Pavel Hrdina wrote:
So far it was not possible to specify how the audio output from guest should be presented to host/users. Now it will be possible to do so via <output> element for <sound> device where you specify the output "type".
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- docs/formatdomain.html.in | 9 +++++++ docs/schemas/domaincommon.rng | 14 ++++++++++ src/conf/domain_conf.c | 61 +++++++++++++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 14 ++++++++++ src/libvirt_private.syms | 2 ++ 5 files changed, 100 insertions(+)
+static int +virDomainSoundOutputParseXML(xmlXPathContextPtr ctxt, + virDomainSoundDefPtr sound) +{ + int ret = -1; + char *type = NULL; + int typeVal; + xmlNodePtr *outputNodes = NULL; + int noutputs; + + noutputs = virXPathNodeSet("./output", ctxt, &outputNodes); + if (noutputs < 0) + return -1; + + if (noutputs > 1) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("sound device can have only one output configured")); + goto cleanup; + }
The implication of this is that if you have multiple display backends enabled only one of them will ever be selectable as the audio backend. It is not unreasonable to consider that audio should go to all backends, or depending on which is in use. eg if QEMU has SDL + SPICE enabled, audio could conceptually go to SDL by default, and get dynamically switched to SPICE whenver there is a SPICE client actively connected. This is a risk of trying to design a configuration approach for this, without us asking QEMU to consider their corresponding design possibilities 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 :|
On Tue, Nov 21, 2017 at 01:25:33PM +0000, Daniel P. Berrange wrote:
On Tue, Nov 14, 2017 at 02:45:09PM +0100, Pavel Hrdina wrote:
So far it was not possible to specify how the audio output from guest should be presented to host/users. Now it will be possible to do so via <output> element for <sound> device where you specify the output "type".
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- docs/formatdomain.html.in | 9 +++++++ docs/schemas/domaincommon.rng | 14 ++++++++++ src/conf/domain_conf.c | 61 +++++++++++++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 14 ++++++++++ src/libvirt_private.syms | 2 ++ 5 files changed, 100 insertions(+)
+static int +virDomainSoundOutputParseXML(xmlXPathContextPtr ctxt, + virDomainSoundDefPtr sound) +{ + int ret = -1; + char *type = NULL; + int typeVal; + xmlNodePtr *outputNodes = NULL; + int noutputs; + + noutputs = virXPathNodeSet("./output", ctxt, &outputNodes); + if (noutputs < 0) + return -1; + + if (noutputs > 1) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("sound device can have only one output configured")); + goto cleanup; + }
The implication of this is that if you have multiple display backends enabled only one of them will ever be selectable as the audio backend. It is not unreasonable to consider that audio should go to all backends, or depending on which is in use. eg if QEMU has SDL + SPICE enabled, audio could conceptually go to SDL by default, and get dynamically switched to SPICE whenver there is a SPICE client actively connected. This is a risk of trying to design a configuration approach for this, without us asking QEMU to consider their corresponding design possibilities
Having this limitation now doesn't mean that we cannot remove it in the future if QEMU implements a way how to configure multiple audio backends. I can move it into qemuValidateCallback to make it QEMU specific and if we need to remove it we can do so. Pavel
So far we were configuring the sound output based on what graphic device was configured in domain XML. This had a several disadvantages, it's not transparent, in case of multiple graphic devices it was overwritten by the last one and there was no simple way how to configure this per domain. The new <output> element for <sound> devices allows you to configure which output will be used for each domain, however QEMU has a limitation that all sound devices will always use the same output because it is configured by environment variable QEMU_AUDIO_DRV per domain. For backward compatibility we need to preserve the defaults if no output is specified: - for vnc graphic it's by default NONE unless "vnc_allow_host_audio" was enabled, in that case we use DEFAULT which means it will pass the environment variable visible by libvirtd - for sdl graphic by default we pass the environment variable - for spice graphic we configure the SPICE output - if no graphic is configured we use by default NONE unless "nogfx_allow_host_audio" was enabled, in that case we pass the environment variable Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1375433 Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- docs/formatdomain.html.in | 4 ++- src/qemu/qemu_command.c | 84 +++++++++++++++++++++-------------------------- src/qemu/qemu_domain.c | 54 ++++++++++++++++++++++++++++++ src/qemu/qemu_process.c | 41 +++++++++++++++++++++++ 4 files changed, 135 insertions(+), 48 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 3b7c367fc7..ae0d8b86be 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -7064,7 +7064,9 @@ qemu-kvm -net nic,model=? /dev/null the audio output is connected within host. There is mandatory <code>type</code> attribute where valid values are 'none' to disable the audio output, 'spice', 'pa', 'sdl', 'alsa', 'oss'. - This might not be supported by all hypervisors. + This might not be supported by all hypervisors. QEMU driver + has a limitation that all sound devices have to use the same + output. </p> <h4><a id="elementsWatchdog">Watchdog device</a></h4> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index c5c7bd7e54..5cbd1d0d46 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4442,67 +4442,57 @@ qemuBuildSoundCodecStr(virDomainSoundDefPtr sound, } -static void +static int qemuBuildSoundAudioEnv(virCommandPtr cmd, - const virDomainDef *def, - virQEMUDriverConfigPtr cfg) + const virDomainDef *def) { + char *envStr = NULL; + if (def->nsounds == 0) { virCommandAddEnvString(cmd, "QEMU_AUDIO_DRV=none"); - return; + return 0; } - if (def->ngraphics == 0) { - if (cfg->nogfxAllowHostAudio) - virCommandAddEnvPassBlockSUID(cmd, "QEMU_AUDIO_DRV", NULL); - else - virCommandAddEnvString(cmd, "QEMU_AUDIO_DRV=none"); - } else { - switch (def->graphics[def->ngraphics - 1]->type) { - case VIR_DOMAIN_GRAPHICS_TYPE_SDL: - /* If using SDL for video, then we should just let it - * use QEMU's host audio drivers, possibly SDL too - * User can set these two before starting libvirtd - */ - virCommandAddEnvPassBlockSUID(cmd, "QEMU_AUDIO_DRV", NULL); + /* QEMU doesn't allow setting different audio output per sound device + * so it will always be the same for all devices. */ + switch (def->sounds[0]->output) { + case VIR_DOMAIN_SOUND_OUTPUT_TYPE_DEFAULT: + /* The default output is used only as backward compatible way to + * pass-through environment variables configured before starting + * libvirtd. */ + virCommandAddEnvPassBlockSUID(cmd, "QEMU_AUDIO_DRV", NULL); + if (def->ngraphics > 0 && + def->graphics[def->ngraphics - 1]->type == VIR_DOMAIN_GRAPHICS_TYPE_SDL) { virCommandAddEnvPassBlockSUID(cmd, "SDL_AUDIODRIVER", NULL); + } + break; - break; - - case VIR_DOMAIN_GRAPHICS_TYPE_VNC: - /* Unless user requested it, set the audio backend to none, to - * prevent it opening the host OS audio devices, since that causes - * security issues and might not work when using VNC. - */ - if (cfg->vncAllowHostAudio) - virCommandAddEnvPassBlockSUID(cmd, "QEMU_AUDIO_DRV", NULL); - else - virCommandAddEnvString(cmd, "QEMU_AUDIO_DRV=none"); - - break; - - case VIR_DOMAIN_GRAPHICS_TYPE_SPICE: - /* SPICE includes native support for tunnelling audio, so we - * set the audio backend to point at SPICE's own driver - */ - virCommandAddEnvString(cmd, "QEMU_AUDIO_DRV=spice"); - - break; - - case VIR_DOMAIN_GRAPHICS_TYPE_RDP: - case VIR_DOMAIN_GRAPHICS_TYPE_DESKTOP: - case VIR_DOMAIN_GRAPHICS_TYPE_LAST: - break; + case VIR_DOMAIN_SOUND_OUTPUT_TYPE_NONE: + case VIR_DOMAIN_SOUND_OUTPUT_TYPE_SPICE: + case VIR_DOMAIN_SOUND_OUTPUT_TYPE_PA: + case VIR_DOMAIN_SOUND_OUTPUT_TYPE_SDL: + case VIR_DOMAIN_SOUND_OUTPUT_TYPE_ALSA: + case VIR_DOMAIN_SOUND_OUTPUT_TYPE_OSS: + if (virAsprintf(&envStr, "QEMU_AUDIO_DRV=%s", + virDomainSoundOutputTypeToString(def->sounds[0]->output)) < 0) { + return -1; } + virCommandAddEnvString(cmd, envStr); + VIR_FREE(envStr); + break; + + case VIR_DOMAIN_SOUND_OUTPUT_TYPE_LAST: + break; } + + return 0; } static int qemuBuildSoundCommandLine(virCommandPtr cmd, const virDomainDef *def, - virQEMUCapsPtr qemuCaps, - virQEMUDriverConfigPtr cfg) + virQEMUCapsPtr qemuCaps) { size_t i, j; @@ -4556,7 +4546,7 @@ qemuBuildSoundCommandLine(virCommandPtr cmd, } } - qemuBuildSoundAudioEnv(cmd, def, cfg); + qemuBuildSoundAudioEnv(cmd, def); return 0; } @@ -10118,7 +10108,7 @@ qemuBuildCommandLine(virQEMUDriverPtr driver, if (qemuBuildVideoCommandLine(cmd, def, qemuCaps) < 0) goto error; - if (qemuBuildSoundCommandLine(cmd, def, qemuCaps, cfg) < 0) + if (qemuBuildSoundCommandLine(cmd, def, qemuCaps) < 0) goto error; if (qemuBuildWatchdogCommandLine(cmd, def, qemuCaps) < 0) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index a36e157529..3b8fa2d79c 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -3148,6 +3148,31 @@ qemuDomainDefVerifyFeatures(const virDomainDef *def) } +static void +qemuDomainDefSoundPostParse(virDomainDefPtr def) +{ + size_t i; + virDomainSoundOutputType output = VIR_DOMAIN_SOUND_OUTPUT_TYPE_DEFAULT; + + for (i = 0; i < def->nsounds; i++) { + if (output != def->sounds[i]->output) { + output = def->sounds[i]->output; + break; + } + } + + /* For convenience we will copy the first configured sound output to all + * sound devices that doesn't have any output configured because QEMU + * will use only one output for all sound devices. */ + if (output != VIR_DOMAIN_SOUND_OUTPUT_TYPE_DEFAULT) { + for (i = 0; i < def->nsounds; i++) { + if (def->sounds[i]->output == VIR_DOMAIN_SOUND_OUTPUT_TYPE_DEFAULT) + def->sounds[i]->output = output; + } + } +} + + static int qemuDomainDefPostParseBasic(virDomainDefPtr def, virCapsPtr caps, @@ -3221,6 +3246,8 @@ qemuDomainDefPostParse(virDomainDefPtr def, if (qemuDomainDefCPUPostParse(def) < 0) goto cleanup; + qemuDomainDefSoundPostParse(def); + ret = 0; cleanup: virObjectUnref(cfg); @@ -3301,6 +3328,30 @@ qemuDomainDefValidateVideo(const virDomainDef *def) } +static int +qemuDomainDefValidateSound(const virDomainDef *def) +{ + size_t i; + virDomainSoundOutputType output = VIR_DOMAIN_SOUND_OUTPUT_TYPE_DEFAULT; + + for (i = 0; i < def->nsounds; i++) { + if (output == VIR_DOMAIN_SOUND_OUTPUT_TYPE_DEFAULT) { + output = def->sounds[i]->output; + continue; + } + + if (output != def->sounds[i]->output) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("all sound devices must be configured to use " + "the same output")); + return -1; + } + } + + return 0; +} + + #define QEMU_MAX_VCPUS_WITHOUT_EIM 255 @@ -3403,6 +3454,9 @@ qemuDomainDefValidate(const virDomainDef *def, if (qemuDomainDefValidateVideo(def) < 0) goto cleanup; + if (qemuDomainDefValidateSound(def) < 0) + goto cleanup; + ret = 0; cleanup: diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 6d242b1b51..2957c4a074 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -5393,6 +5393,45 @@ qemuProcessPrepareAllowReboot(virDomainObjPtr vm) } +static void +qemuProcessPrepareSound(virDomainDefPtr def, + virQEMUDriverConfigPtr cfg) +{ + virDomainSoundOutputType output = VIR_DOMAIN_SOUND_OUTPUT_TYPE_DEFAULT; + size_t i; + + if (def->nsounds == 0) + return; + + if (def->ngraphics == 0) { + if (!cfg->nogfxAllowHostAudio) + output = VIR_DOMAIN_SOUND_OUTPUT_TYPE_NONE; + } else { + switch (def->graphics[def->ngraphics - 1]->type) { + case VIR_DOMAIN_GRAPHICS_TYPE_SPICE: + output = VIR_DOMAIN_SOUND_OUTPUT_TYPE_SPICE; + break; + + case VIR_DOMAIN_GRAPHICS_TYPE_VNC: + if (!cfg->vncAllowHostAudio) + output = VIR_DOMAIN_SOUND_OUTPUT_TYPE_NONE; + break; + + case VIR_DOMAIN_GRAPHICS_TYPE_SDL: + case VIR_DOMAIN_GRAPHICS_TYPE_RDP: + case VIR_DOMAIN_GRAPHICS_TYPE_DESKTOP: + case VIR_DOMAIN_GRAPHICS_TYPE_LAST: + break; + } + } + + for (i = 0; i < def->nsounds; i++) { + if (def->sounds[i]->output == VIR_DOMAIN_SOUND_OUTPUT_TYPE_DEFAULT) + def->sounds[i]->output = output; + } +} + + /** * qemuProcessPrepareDomain: * @conn: connection object (for looking up storage volumes) @@ -5513,6 +5552,8 @@ qemuProcessPrepareDomain(virConnectPtr conn, goto cleanup; } + qemuProcessPrepareSound(vm->def, cfg); + ret = 0; cleanup: virObjectUnref(caps); -- 2.13.6
On 11/14/2017 08:45 AM, Pavel Hrdina wrote:
So far we were configuring the sound output based on what graphic device was configured in domain XML. This had a several disadvantages, it's not transparent, in case of multiple graphic devices it was overwritten by the last one and there was no simple way how to configure this per domain.
The new <output> element for <sound> devices allows you to configure which output will be used for each domain, however QEMU has a limitation that all sound devices will always use the same output because it is configured by environment variable QEMU_AUDIO_DRV per domain.
For backward compatibility we need to preserve the defaults if no output is specified:
- for vnc graphic it's by default NONE unless "vnc_allow_host_audio" was enabled, in that case we use DEFAULT which means it will pass the environment variable visible by libvirtd
- for sdl graphic by default we pass the environment variable
- for spice graphic we configure the SPICE output
- if no graphic is configured we use by default NONE unless "nogfx_allow_host_audio" was enabled, in that case we pass the environment variable
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1375433
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- docs/formatdomain.html.in | 4 ++- src/qemu/qemu_command.c | 84 +++++++++++++++++++++-------------------------- src/qemu/qemu_domain.c | 54 ++++++++++++++++++++++++++++++ src/qemu/qemu_process.c | 41 +++++++++++++++++++++++ 4 files changed, 135 insertions(+), 48 deletions(-)
Is there any way to make less things happen in one patch? Maybe even the PostParse, Prepare, and Validate together? I need to look at this one with fresh eyes in the morning - it's confusing at best especially since I don't find the functions self documenting. I'm having trouble with the settings between PostParse and Prepare as well as setting a default output which essentially changes an optional parameter into a required one, doesn't it? I'm sure there's a harder and even more confusing way to do things ;-). John
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 3b7c367fc7..ae0d8b86be 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -7064,7 +7064,9 @@ qemu-kvm -net nic,model=? /dev/null the audio output is connected within host. There is mandatory <code>type</code> attribute where valid values are 'none' to disable the audio output, 'spice', 'pa', 'sdl', 'alsa', 'oss'. - This might not be supported by all hypervisors. + This might not be supported by all hypervisors. QEMU driver + has a limitation that all sound devices have to use the same + output. </p>
<h4><a id="elementsWatchdog">Watchdog device</a></h4> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index c5c7bd7e54..5cbd1d0d46 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4442,67 +4442,57 @@ qemuBuildSoundCodecStr(virDomainSoundDefPtr sound, }
-static void +static int qemuBuildSoundAudioEnv(virCommandPtr cmd, - const virDomainDef *def, - virQEMUDriverConfigPtr cfg) + const virDomainDef *def) { + char *envStr = NULL; + if (def->nsounds == 0) { virCommandAddEnvString(cmd, "QEMU_AUDIO_DRV=none"); - return; + return 0; }
- if (def->ngraphics == 0) { - if (cfg->nogfxAllowHostAudio) - virCommandAddEnvPassBlockSUID(cmd, "QEMU_AUDIO_DRV", NULL); - else - virCommandAddEnvString(cmd, "QEMU_AUDIO_DRV=none"); - } else { - switch (def->graphics[def->ngraphics - 1]->type) { - case VIR_DOMAIN_GRAPHICS_TYPE_SDL: - /* If using SDL for video, then we should just let it - * use QEMU's host audio drivers, possibly SDL too - * User can set these two before starting libvirtd - */ - virCommandAddEnvPassBlockSUID(cmd, "QEMU_AUDIO_DRV", NULL); + /* QEMU doesn't allow setting different audio output per sound device + * so it will always be the same for all devices. */
This is a case where the SoundPostParse should either be in its own patch or in the previous patch... Of course reading this "out of order" made me wonder how the the [0]th element was filled in...
+ switch (def->sounds[0]->output) { + case VIR_DOMAIN_SOUND_OUTPUT_TYPE_DEFAULT: + /* The default output is used only as backward compatible way to + * pass-through environment variables configured before starting + * libvirtd. */ + virCommandAddEnvPassBlockSUID(cmd, "QEMU_AUDIO_DRV", NULL); + if (def->ngraphics > 0 && + def->graphics[def->ngraphics - 1]->type == VIR_DOMAIN_GRAPHICS_TYPE_SDL) { virCommandAddEnvPassBlockSUID(cmd, "SDL_AUDIODRIVER", NULL); + } + break;
- break; - - case VIR_DOMAIN_GRAPHICS_TYPE_VNC: - /* Unless user requested it, set the audio backend to none, to - * prevent it opening the host OS audio devices, since that causes - * security issues and might not work when using VNC. - */ - if (cfg->vncAllowHostAudio) - virCommandAddEnvPassBlockSUID(cmd, "QEMU_AUDIO_DRV", NULL); - else - virCommandAddEnvString(cmd, "QEMU_AUDIO_DRV=none"); - - break; - - case VIR_DOMAIN_GRAPHICS_TYPE_SPICE: - /* SPICE includes native support for tunnelling audio, so we - * set the audio backend to point at SPICE's own driver - */ - virCommandAddEnvString(cmd, "QEMU_AUDIO_DRV=spice"); - - break; - - case VIR_DOMAIN_GRAPHICS_TYPE_RDP: - case VIR_DOMAIN_GRAPHICS_TYPE_DESKTOP: - case VIR_DOMAIN_GRAPHICS_TYPE_LAST: - break; + case VIR_DOMAIN_SOUND_OUTPUT_TYPE_NONE: + case VIR_DOMAIN_SOUND_OUTPUT_TYPE_SPICE: + case VIR_DOMAIN_SOUND_OUTPUT_TYPE_PA: + case VIR_DOMAIN_SOUND_OUTPUT_TYPE_SDL: + case VIR_DOMAIN_SOUND_OUTPUT_TYPE_ALSA: + case VIR_DOMAIN_SOUND_OUTPUT_TYPE_OSS: + if (virAsprintf(&envStr, "QEMU_AUDIO_DRV=%s", + virDomainSoundOutputTypeToString(def->sounds[0]->output)) < 0) { + return -1; } + virCommandAddEnvString(cmd, envStr); + VIR_FREE(envStr); + break; + + case VIR_DOMAIN_SOUND_OUTPUT_TYPE_LAST: + break; } + + return 0; }
static int qemuBuildSoundCommandLine(virCommandPtr cmd, const virDomainDef *def, - virQEMUCapsPtr qemuCaps, - virQEMUDriverConfigPtr cfg) + virQEMUCapsPtr qemuCaps) { size_t i, j;
@@ -4556,7 +4546,7 @@ qemuBuildSoundCommandLine(virCommandPtr cmd, } }
- qemuBuildSoundAudioEnv(cmd, def, cfg); + qemuBuildSoundAudioEnv(cmd, def);
return 0; } @@ -10118,7 +10108,7 @@ qemuBuildCommandLine(virQEMUDriverPtr driver, if (qemuBuildVideoCommandLine(cmd, def, qemuCaps) < 0) goto error;
- if (qemuBuildSoundCommandLine(cmd, def, qemuCaps, cfg) < 0) + if (qemuBuildSoundCommandLine(cmd, def, qemuCaps) < 0) goto error;
if (qemuBuildWatchdogCommandLine(cmd, def, qemuCaps) < 0) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index a36e157529..3b8fa2d79c 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -3148,6 +3148,31 @@ qemuDomainDefVerifyFeatures(const virDomainDef *def) }
+static void +qemuDomainDefSoundPostParse(virDomainDefPtr def) +{ + size_t i; + virDomainSoundOutputType output = VIR_DOMAIN_SOUND_OUTPUT_TYPE_DEFAULT; + + for (i = 0; i < def->nsounds; i++) { + if (output != def->sounds[i]->output) { + output = def->sounds[i]->output; + break; + } + } + + /* For convenience we will copy the first configured sound output to all + * sound devices that doesn't have any output configured because QEMU
s/doesn't/don't/
+ * will use only one output for all sound devices. */ + if (output != VIR_DOMAIN_SOUND_OUTPUT_TYPE_DEFAULT) { + for (i = 0; i < def->nsounds; i++) { + if (def->sounds[i]->output == VIR_DOMAIN_SOUND_OUTPUT_TYPE_DEFAULT) + def->sounds[i]->output = output; + } + } +} + + static int qemuDomainDefPostParseBasic(virDomainDefPtr def, virCapsPtr caps, @@ -3221,6 +3246,8 @@ qemuDomainDefPostParse(virDomainDefPtr def, if (qemuDomainDefCPUPostParse(def) < 0) goto cleanup;
+ qemuDomainDefSoundPostParse(def); + ret = 0; cleanup: virObjectUnref(cfg); @@ -3301,6 +3328,30 @@ qemuDomainDefValidateVideo(const virDomainDef *def) }
+static int +qemuDomainDefValidateSound(const virDomainDef *def) +{ + size_t i; + virDomainSoundOutputType output = VIR_DOMAIN_SOUND_OUTPUT_TYPE_DEFAULT; + + for (i = 0; i < def->nsounds; i++) { + if (output == VIR_DOMAIN_SOUND_OUTPUT_TYPE_DEFAULT) { + output = def->sounds[i]->output; + continue; + } + + if (output != def->sounds[i]->output) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("all sound devices must be configured to use " + "the same output")); + return -1; + } + } + + return 0; +} + + #define QEMU_MAX_VCPUS_WITHOUT_EIM 255
@@ -3403,6 +3454,9 @@ qemuDomainDefValidate(const virDomainDef *def, if (qemuDomainDefValidateVideo(def) < 0) goto cleanup;
+ if (qemuDomainDefValidateSound(def) < 0) + goto cleanup; + ret = 0;
cleanup: diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 6d242b1b51..2957c4a074 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -5393,6 +5393,45 @@ qemuProcessPrepareAllowReboot(virDomainObjPtr vm) }
+static void +qemuProcessPrepareSound(virDomainDefPtr def, + virQEMUDriverConfigPtr cfg) +{ + virDomainSoundOutputType output = VIR_DOMAIN_SOUND_OUTPUT_TYPE_DEFAULT; + size_t i; + + if (def->nsounds == 0) + return; + + if (def->ngraphics == 0) { + if (!cfg->nogfxAllowHostAudio) + output = VIR_DOMAIN_SOUND_OUTPUT_TYPE_NONE; + } else { + switch (def->graphics[def->ngraphics - 1]->type) { + case VIR_DOMAIN_GRAPHICS_TYPE_SPICE: + output = VIR_DOMAIN_SOUND_OUTPUT_TYPE_SPICE; + break; + + case VIR_DOMAIN_GRAPHICS_TYPE_VNC: + if (!cfg->vncAllowHostAudio) + output = VIR_DOMAIN_SOUND_OUTPUT_TYPE_NONE; + break; + + case VIR_DOMAIN_GRAPHICS_TYPE_SDL: + case VIR_DOMAIN_GRAPHICS_TYPE_RDP: + case VIR_DOMAIN_GRAPHICS_TYPE_DESKTOP: + case VIR_DOMAIN_GRAPHICS_TYPE_LAST: + break; + } + } + + for (i = 0; i < def->nsounds; i++) { + if (def->sounds[i]->output == VIR_DOMAIN_SOUND_OUTPUT_TYPE_DEFAULT) + def->sounds[i]->output = output; + } +} + + /** * qemuProcessPrepareDomain: * @conn: connection object (for looking up storage volumes) @@ -5513,6 +5552,8 @@ qemuProcessPrepareDomain(virConnectPtr conn, goto cleanup; }
+ qemuProcessPrepareSound(vm->def, cfg); + ret = 0; cleanup: virObjectUnref(caps);
On Mon, Nov 20, 2017 at 07:44:25PM -0500, John Ferlan wrote:
On 11/14/2017 08:45 AM, Pavel Hrdina wrote:
So far we were configuring the sound output based on what graphic device was configured in domain XML. This had a several disadvantages, it's not transparent, in case of multiple graphic devices it was overwritten by the last one and there was no simple way how to configure this per domain.
The new <output> element for <sound> devices allows you to configure which output will be used for each domain, however QEMU has a limitation that all sound devices will always use the same output because it is configured by environment variable QEMU_AUDIO_DRV per domain.
For backward compatibility we need to preserve the defaults if no output is specified:
- for vnc graphic it's by default NONE unless "vnc_allow_host_audio" was enabled, in that case we use DEFAULT which means it will pass the environment variable visible by libvirtd
- for sdl graphic by default we pass the environment variable
- for spice graphic we configure the SPICE output
- if no graphic is configured we use by default NONE unless "nogfx_allow_host_audio" was enabled, in that case we pass the environment variable
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1375433
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- docs/formatdomain.html.in | 4 ++- src/qemu/qemu_command.c | 84 +++++++++++++++++++++-------------------------- src/qemu/qemu_domain.c | 54 ++++++++++++++++++++++++++++++ src/qemu/qemu_process.c | 41 +++++++++++++++++++++++ 4 files changed, 135 insertions(+), 48 deletions(-)
Is there any way to make less things happen in one patch? Maybe even the PostParse, Prepare, and Validate together?
It would be probably possible to split this patch into two separate changes: 1. move the code out of command line generation into qemuProcessPrepareSound() 2. the rest of this patch
I need to look at this one with fresh eyes in the morning - it's confusing at best especially since I don't find the functions self documenting.
I'm having trouble with the settings between PostParse and Prepare as well as setting a default output which essentially changes an optional parameter into a required one, doesn't it? I'm sure there's a harder and even more confusing way to do things ;-).
The PostParse function tries to find the first sound device with output configured (the first for loop) and sets this output to all other sound devices without any output configured (the second for loop). This is executed while parsing domain XML and implements the new feature. The Prepare function is executed only while starting domain and basically supplements the code removed from building command line. It prepares only live definition that is about to be started so the qemu command line code can only take the definition and convert it into command line without any logic or without modifying the live definition.
John
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 3b7c367fc7..ae0d8b86be 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -7064,7 +7064,9 @@ qemu-kvm -net nic,model=? /dev/null the audio output is connected within host. There is mandatory <code>type</code> attribute where valid values are 'none' to disable the audio output, 'spice', 'pa', 'sdl', 'alsa', 'oss'. - This might not be supported by all hypervisors. + This might not be supported by all hypervisors. QEMU driver + has a limitation that all sound devices have to use the same + output. </p>
<h4><a id="elementsWatchdog">Watchdog device</a></h4> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index c5c7bd7e54..5cbd1d0d46 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4442,67 +4442,57 @@ qemuBuildSoundCodecStr(virDomainSoundDefPtr sound, }
-static void +static int qemuBuildSoundAudioEnv(virCommandPtr cmd, - const virDomainDef *def, - virQEMUDriverConfigPtr cfg) + const virDomainDef *def) { + char *envStr = NULL; + if (def->nsounds == 0) { virCommandAddEnvString(cmd, "QEMU_AUDIO_DRV=none"); - return; + return 0; }
- if (def->ngraphics == 0) { - if (cfg->nogfxAllowHostAudio) - virCommandAddEnvPassBlockSUID(cmd, "QEMU_AUDIO_DRV", NULL); - else - virCommandAddEnvString(cmd, "QEMU_AUDIO_DRV=none"); - } else { - switch (def->graphics[def->ngraphics - 1]->type) { - case VIR_DOMAIN_GRAPHICS_TYPE_SDL: - /* If using SDL for video, then we should just let it - * use QEMU's host audio drivers, possibly SDL too - * User can set these two before starting libvirtd - */ - virCommandAddEnvPassBlockSUID(cmd, "QEMU_AUDIO_DRV", NULL); + /* QEMU doesn't allow setting different audio output per sound device + * so it will always be the same for all devices. */
This is a case where the SoundPostParse should either be in its own patch or in the previous patch... Of course reading this "out of order" made me wonder how the the [0]th element was filled in...
Actually no, this is tied together, the PostParse and Validate callbacks make sure that the definition is correct so that qemu build command line doesn't have to check anything and simply takes the definition as it is and converts it into command line. Pavel
On 11/21/2017 08:42 AM, Pavel Hrdina wrote:
On Mon, Nov 20, 2017 at 07:44:25PM -0500, John Ferlan wrote:
On 11/14/2017 08:45 AM, Pavel Hrdina wrote:
So far we were configuring the sound output based on what graphic device was configured in domain XML. This had a several disadvantages, it's not transparent, in case of multiple graphic devices it was overwritten by the last one and there was no simple way how to configure this per domain.
The new <output> element for <sound> devices allows you to configure which output will be used for each domain, however QEMU has a limitation that all sound devices will always use the same output because it is configured by environment variable QEMU_AUDIO_DRV per domain.
For backward compatibility we need to preserve the defaults if no output is specified:
- for vnc graphic it's by default NONE unless "vnc_allow_host_audio" was enabled, in that case we use DEFAULT which means it will pass the environment variable visible by libvirtd
- for sdl graphic by default we pass the environment variable
- for spice graphic we configure the SPICE output
- if no graphic is configured we use by default NONE unless "nogfx_allow_host_audio" was enabled, in that case we pass the environment variable
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1375433
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- docs/formatdomain.html.in | 4 ++- src/qemu/qemu_command.c | 84 +++++++++++++++++++++-------------------------- src/qemu/qemu_domain.c | 54 ++++++++++++++++++++++++++++++ src/qemu/qemu_process.c | 41 +++++++++++++++++++++++ 4 files changed, 135 insertions(+), 48 deletions(-)
Is there any way to make less things happen in one patch? Maybe even the PostParse, Prepare, and Validate together?
It would be probably possible to split this patch into two separate changes:
1. move the code out of command line generation into qemuProcessPrepareSound()
2. the rest of this patch
I need to look at this one with fresh eyes in the morning - it's confusing at best especially since I don't find the functions self documenting.
I'm having trouble with the settings between PostParse and Prepare as well as setting a default output which essentially changes an optional parameter into a required one, doesn't it? I'm sure there's a harder and even more confusing way to do things ;-).
The PostParse function tries to find the first sound device with output configured (the first for loop) and sets this output to all other sound devices without any output configured (the second for loop). This is executed while parsing domain XML and implements the new feature.
Understood, but it also changes each configured sound device that didn't have <output> defined to have the <output> value from the one that did have it. That would then be saved - something that would have been shown in the qemuxml2xmltest output, e.g the multi output from patch 6 would be: <sound model='ich6'> <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> <output type='pa'/> </sound> <sound model='ich6'> <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> <output type='pa'/> </sound> I also note that <output> comes after <address>... not that it matters, but my brain recollects that <address> is generally last for most elements, although not required to be last - it just is generally. In any case, I'm not sure it's "right" to change/add that output. It should be simple enough to ignore those that don't define some output. That was my point about making an optional element required. Being able to provide/determine some default when no sound device has an output defined would thus become the "job" of the Prepare API I think.
The Prepare function is executed only while starting domain and basically supplements the code removed from building command line. It prepares only live definition that is about to be started so the qemu command line code can only take the definition and convert it into command line without any logic or without modifying the live definition.
Right - these are the live entries not the config/saved defs - so to that degree altering things does make some sense. However, your choice to modify each live entry, but really only use the [0]th one in the command line building makes me want to consider whether it's better to create some sort of qemuDomainObjPrivate field instead. Since there can only be "one" (at this time) it would seem to be mechanism used elsewhere to keep track of QEMU private and specific shtuff.
John
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 3b7c367fc7..ae0d8b86be 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -7064,7 +7064,9 @@ qemu-kvm -net nic,model=? /dev/null the audio output is connected within host. There is mandatory <code>type</code> attribute where valid values are 'none' to disable the audio output, 'spice', 'pa', 'sdl', 'alsa', 'oss'. - This might not be supported by all hypervisors. + This might not be supported by all hypervisors. QEMU driver + has a limitation that all sound devices have to use the same + output. </p>
<h4><a id="elementsWatchdog">Watchdog device</a></h4> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index c5c7bd7e54..5cbd1d0d46 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4442,67 +4442,57 @@ qemuBuildSoundCodecStr(virDomainSoundDefPtr sound, }
-static void +static int qemuBuildSoundAudioEnv(virCommandPtr cmd, - const virDomainDef *def, - virQEMUDriverConfigPtr cfg) + const virDomainDef *def) { + char *envStr = NULL; + if (def->nsounds == 0) { virCommandAddEnvString(cmd, "QEMU_AUDIO_DRV=none"); - return; + return 0; }
- if (def->ngraphics == 0) { - if (cfg->nogfxAllowHostAudio) - virCommandAddEnvPassBlockSUID(cmd, "QEMU_AUDIO_DRV", NULL); - else - virCommandAddEnvString(cmd, "QEMU_AUDIO_DRV=none"); - } else { - switch (def->graphics[def->ngraphics - 1]->type) { - case VIR_DOMAIN_GRAPHICS_TYPE_SDL: - /* If using SDL for video, then we should just let it - * use QEMU's host audio drivers, possibly SDL too - * User can set these two before starting libvirtd - */ - virCommandAddEnvPassBlockSUID(cmd, "QEMU_AUDIO_DRV", NULL); + /* QEMU doesn't allow setting different audio output per sound device + * so it will always be the same for all devices. */
This is a case where the SoundPostParse should either be in its own patch or in the previous patch... Of course reading this "out of order" made me wonder how the the [0]th element was filled in...
Actually no, this is tied together, the PostParse and Validate callbacks make sure that the definition is correct so that qemu build command line doesn't have to check anything and simply takes the definition as it is and converts it into command line.
Pavel
I always have to go back through to context switch in the ordering of processing and the "rules" between PostParse and Validate... The reason I remarked here was purely based on order. That is, it's not clear until later that the [0]th entry could/would be filled in automagically... John
On Tue, Nov 21, 2017 at 10:52:48AM -0500, John Ferlan wrote:
On 11/21/2017 08:42 AM, Pavel Hrdina wrote:
On Mon, Nov 20, 2017 at 07:44:25PM -0500, John Ferlan wrote:
On 11/14/2017 08:45 AM, Pavel Hrdina wrote:
So far we were configuring the sound output based on what graphic device was configured in domain XML. This had a several disadvantages, it's not transparent, in case of multiple graphic devices it was overwritten by the last one and there was no simple way how to configure this per domain.
The new <output> element for <sound> devices allows you to configure which output will be used for each domain, however QEMU has a limitation that all sound devices will always use the same output because it is configured by environment variable QEMU_AUDIO_DRV per domain.
For backward compatibility we need to preserve the defaults if no output is specified:
- for vnc graphic it's by default NONE unless "vnc_allow_host_audio" was enabled, in that case we use DEFAULT which means it will pass the environment variable visible by libvirtd
- for sdl graphic by default we pass the environment variable
- for spice graphic we configure the SPICE output
- if no graphic is configured we use by default NONE unless "nogfx_allow_host_audio" was enabled, in that case we pass the environment variable
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1375433
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- docs/formatdomain.html.in | 4 ++- src/qemu/qemu_command.c | 84 +++++++++++++++++++++-------------------------- src/qemu/qemu_domain.c | 54 ++++++++++++++++++++++++++++++ src/qemu/qemu_process.c | 41 +++++++++++++++++++++++ 4 files changed, 135 insertions(+), 48 deletions(-)
Is there any way to make less things happen in one patch? Maybe even the PostParse, Prepare, and Validate together?
It would be probably possible to split this patch into two separate changes:
1. move the code out of command line generation into qemuProcessPrepareSound()
2. the rest of this patch
I need to look at this one with fresh eyes in the morning - it's confusing at best especially since I don't find the functions self documenting.
I'm having trouble with the settings between PostParse and Prepare as well as setting a default output which essentially changes an optional parameter into a required one, doesn't it? I'm sure there's a harder and even more confusing way to do things ;-).
The PostParse function tries to find the first sound device with output configured (the first for loop) and sets this output to all other sound devices without any output configured (the second for loop). This is executed while parsing domain XML and implements the new feature.
Understood, but it also changes each configured sound device that didn't have <output> defined to have the <output> value from the one that did have it.
That would then be saved - something that would have been shown in the qemuxml2xmltest output, e.g the multi output from patch 6 would be:
<sound model='ich6'> <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> <output type='pa'/> </sound> <sound model='ich6'> <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> <output type='pa'/> </sound>
I also note that <output> comes after <address>... not that it matters, but my brain recollects that <address> is generally last for most elements, although not required to be last - it just is generally.
Yeah, semantically its fine, but it'd be better to stuck with our convention that address & alias are last. 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 :|
On Tue, Nov 21, 2017 at 10:52:48AM -0500, John Ferlan wrote:
On 11/21/2017 08:42 AM, Pavel Hrdina wrote:
On Mon, Nov 20, 2017 at 07:44:25PM -0500, John Ferlan wrote:
On 11/14/2017 08:45 AM, Pavel Hrdina wrote:
So far we were configuring the sound output based on what graphic device was configured in domain XML. This had a several disadvantages, it's not transparent, in case of multiple graphic devices it was overwritten by the last one and there was no simple way how to configure this per domain.
The new <output> element for <sound> devices allows you to configure which output will be used for each domain, however QEMU has a limitation that all sound devices will always use the same output because it is configured by environment variable QEMU_AUDIO_DRV per domain.
For backward compatibility we need to preserve the defaults if no output is specified:
- for vnc graphic it's by default NONE unless "vnc_allow_host_audio" was enabled, in that case we use DEFAULT which means it will pass the environment variable visible by libvirtd
- for sdl graphic by default we pass the environment variable
- for spice graphic we configure the SPICE output
- if no graphic is configured we use by default NONE unless "nogfx_allow_host_audio" was enabled, in that case we pass the environment variable
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1375433
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- docs/formatdomain.html.in | 4 ++- src/qemu/qemu_command.c | 84 +++++++++++++++++++++-------------------------- src/qemu/qemu_domain.c | 54 ++++++++++++++++++++++++++++++ src/qemu/qemu_process.c | 41 +++++++++++++++++++++++ 4 files changed, 135 insertions(+), 48 deletions(-)
Is there any way to make less things happen in one patch? Maybe even the PostParse, Prepare, and Validate together?
It would be probably possible to split this patch into two separate changes:
1. move the code out of command line generation into qemuProcessPrepareSound()
2. the rest of this patch
I need to look at this one with fresh eyes in the morning - it's confusing at best especially since I don't find the functions self documenting.
I'm having trouble with the settings between PostParse and Prepare as well as setting a default output which essentially changes an optional parameter into a required one, doesn't it? I'm sure there's a harder and even more confusing way to do things ;-).
The PostParse function tries to find the first sound device with output configured (the first for loop) and sets this output to all other sound devices without any output configured (the second for loop). This is executed while parsing domain XML and implements the new feature.
Understood, but it also changes each configured sound device that didn't have <output> defined to have the <output> value from the one that did have it.
That would then be saved - something that would have been shown in the qemuxml2xmltest output, e.g the multi output from patch 6 would be:
<sound model='ich6'> <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> <output type='pa'/> </sound> <sound model='ich6'> <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> <output type='pa'/> </sound>
Which part of the PostParse code does that? If you configure the domain XML like this: <sound model='ich6'/> <sound model='ich6'/> The resulting config XML saved to disk would be: <sound model='ich6'> <<address type='pci' domain='0x0000' bus='0x00' slot='0x03' unction='0x0'/> </sound> <sound model='ich6'> <<address type='pci' domain='0x0000' bus='0x00' slot='0x04' unction='0x0'/> </sound> One of the reasons why I didn't add qemuxml2xml tests is that only the offline part is somehow working, but the active and status part of that test is broken and doesn't reflect how libvirt changes the active and status XML.
I also note that <output> comes after <address>... not that it matters, but my brain recollects that <address> is generally last for most elements, although not required to be last - it just is generally.
Good point, I'll fix that.
In any case, I'm not sure it's "right" to change/add that output. It should be simple enough to ignore those that don't define some output. That was my point about making an optional element required.
Being able to provide/determine some default when no sound device has an output defined would thus become the "job" of the Prepare API I think.
Well, that's how it works with this patches.
The Prepare function is executed only while starting domain and basically supplements the code removed from building command line. It prepares only live definition that is about to be started so the qemu command line code can only take the definition and convert it into command line without any logic or without modifying the live definition.
Right - these are the live entries not the config/saved defs - so to that degree altering things does make some sense. However, your choice to modify each live entry, but really only use the [0]th one in the command line building makes me want to consider whether it's better to create some sort of qemuDomainObjPrivate field instead. Since there can only be "one" (at this time) it would seem to be mechanism used elsewhere to keep track of QEMU private and specific shtuff.
If there wouldn't be the output attribute introduced by this patch series I would agree with you to using qemuDomainObjPrivate field, but since this patch series introduces the output attribute and required field in structure for sound devices we should use that one and not introduce yet another place where to store this information. Pavel
On 11/21/2017 12:12 PM, Pavel Hrdina wrote:
On Tue, Nov 21, 2017 at 10:52:48AM -0500, John Ferlan wrote:
On 11/21/2017 08:42 AM, Pavel Hrdina wrote:
On Mon, Nov 20, 2017 at 07:44:25PM -0500, John Ferlan wrote:
On 11/14/2017 08:45 AM, Pavel Hrdina wrote:
So far we were configuring the sound output based on what graphic device was configured in domain XML. This had a several disadvantages, it's not transparent, in case of multiple graphic devices it was overwritten by the last one and there was no simple way how to configure this per domain.
The new <output> element for <sound> devices allows you to configure which output will be used for each domain, however QEMU has a limitation that all sound devices will always use the same output because it is configured by environment variable QEMU_AUDIO_DRV per domain.
For backward compatibility we need to preserve the defaults if no output is specified:
- for vnc graphic it's by default NONE unless "vnc_allow_host_audio" was enabled, in that case we use DEFAULT which means it will pass the environment variable visible by libvirtd
- for sdl graphic by default we pass the environment variable
- for spice graphic we configure the SPICE output
- if no graphic is configured we use by default NONE unless "nogfx_allow_host_audio" was enabled, in that case we pass the environment variable
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1375433
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- docs/formatdomain.html.in | 4 ++- src/qemu/qemu_command.c | 84 +++++++++++++++++++++-------------------------- src/qemu/qemu_domain.c | 54 ++++++++++++++++++++++++++++++ src/qemu/qemu_process.c | 41 +++++++++++++++++++++++ 4 files changed, 135 insertions(+), 48 deletions(-)
Is there any way to make less things happen in one patch? Maybe even the PostParse, Prepare, and Validate together?
It would be probably possible to split this patch into two separate changes:
1. move the code out of command line generation into qemuProcessPrepareSound()
2. the rest of this patch
I need to look at this one with fresh eyes in the morning - it's confusing at best especially since I don't find the functions self documenting.
I'm having trouble with the settings between PostParse and Prepare as well as setting a default output which essentially changes an optional parameter into a required one, doesn't it? I'm sure there's a harder and even more confusing way to do things ;-).
The PostParse function tries to find the first sound device with output configured (the first for loop) and sets this output to all other sound devices without any output configured (the second for loop). This is executed while parsing domain XML and implements the new feature.
Understood, but it also changes each configured sound device that didn't have <output> defined to have the <output> value from the one that did have it.
That would then be saved - something that would have been shown in the qemuxml2xmltest output, e.g the multi output from patch 6 would be:
<sound model='ich6'> <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> <output type='pa'/> </sound> <sound model='ich6'> <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> <output type='pa'/> </sound>
Which part of the PostParse code does that? If you configure the domain XML like this:
<sound model='ich6'/> <sound model='ich6'/>
The resulting config XML saved to disk would be:
<sound model='ich6'> <<address type='pci' domain='0x0000' bus='0x00' slot='0x03' unction='0x0'/> </sound> <sound model='ich6'> <<address type='pci' domain='0x0000' bus='0x00' slot='0x04' unction='0x0'/> </sound>
From patch 6 I used:
tests/qemuxml2argvdata/qemuxml2argv-sound-multi-pa-output-spice.xml which has <sound model='ich6'> <output type='pa'/> </sound> <sound model='ich6'/>
One of the reasons why I didn't add qemuxml2xml tests is that only the offline part is somehow working, but the active and status part of that test is broken and doesn't reflect how libvirt changes the active and status XML.
I didn't pay close attention to which was which, just that some output xml was generated by the regenerate. John
I also note that <output> comes after <address>... not that it matters, but my brain recollects that <address> is generally last for most elements, although not required to be last - it just is generally.
Good point, I'll fix that.
In any case, I'm not sure it's "right" to change/add that output. It should be simple enough to ignore those that don't define some output. That was my point about making an optional element required.
Being able to provide/determine some default when no sound device has an output defined would thus become the "job" of the Prepare API I think.
Well, that's how it works with this patches.
The Prepare function is executed only while starting domain and basically supplements the code removed from building command line. It prepares only live definition that is about to be started so the qemu command line code can only take the definition and convert it into command line without any logic or without modifying the live definition.
Right - these are the live entries not the config/saved defs - so to that degree altering things does make some sense. However, your choice to modify each live entry, but really only use the [0]th one in the command line building makes me want to consider whether it's better to create some sort of qemuDomainObjPrivate field instead. Since there can only be "one" (at this time) it would seem to be mechanism used elsewhere to keep track of QEMU private and specific shtuff.
If there wouldn't be the output attribute introduced by this patch series I would agree with you to using qemuDomainObjPrivate field, but since this patch series introduces the output attribute and required field in structure for sound devices we should use that one and not introduce yet another place where to store this information.
Pavel
On Tue, Nov 21, 2017 at 12:44:04PM -0500, John Ferlan wrote:
On 11/21/2017 12:12 PM, Pavel Hrdina wrote:
On Tue, Nov 21, 2017 at 10:52:48AM -0500, John Ferlan wrote:
On 11/21/2017 08:42 AM, Pavel Hrdina wrote:
On Mon, Nov 20, 2017 at 07:44:25PM -0500, John Ferlan wrote:
On 11/14/2017 08:45 AM, Pavel Hrdina wrote:
So far we were configuring the sound output based on what graphic device was configured in domain XML. This had a several disadvantages, it's not transparent, in case of multiple graphic devices it was overwritten by the last one and there was no simple way how to configure this per domain.
The new <output> element for <sound> devices allows you to configure which output will be used for each domain, however QEMU has a limitation that all sound devices will always use the same output because it is configured by environment variable QEMU_AUDIO_DRV per domain.
For backward compatibility we need to preserve the defaults if no output is specified:
- for vnc graphic it's by default NONE unless "vnc_allow_host_audio" was enabled, in that case we use DEFAULT which means it will pass the environment variable visible by libvirtd
- for sdl graphic by default we pass the environment variable
- for spice graphic we configure the SPICE output
- if no graphic is configured we use by default NONE unless "nogfx_allow_host_audio" was enabled, in that case we pass the environment variable
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1375433
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- docs/formatdomain.html.in | 4 ++- src/qemu/qemu_command.c | 84 +++++++++++++++++++++-------------------------- src/qemu/qemu_domain.c | 54 ++++++++++++++++++++++++++++++ src/qemu/qemu_process.c | 41 +++++++++++++++++++++++ 4 files changed, 135 insertions(+), 48 deletions(-)
Is there any way to make less things happen in one patch? Maybe even the PostParse, Prepare, and Validate together?
It would be probably possible to split this patch into two separate changes:
1. move the code out of command line generation into qemuProcessPrepareSound()
2. the rest of this patch
I need to look at this one with fresh eyes in the morning - it's confusing at best especially since I don't find the functions self documenting.
I'm having trouble with the settings between PostParse and Prepare as well as setting a default output which essentially changes an optional parameter into a required one, doesn't it? I'm sure there's a harder and even more confusing way to do things ;-).
The PostParse function tries to find the first sound device with output configured (the first for loop) and sets this output to all other sound devices without any output configured (the second for loop). This is executed while parsing domain XML and implements the new feature.
Understood, but it also changes each configured sound device that didn't have <output> defined to have the <output> value from the one that did have it.
That would then be saved - something that would have been shown in the qemuxml2xmltest output, e.g the multi output from patch 6 would be:
<sound model='ich6'> <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> <output type='pa'/> </sound> <sound model='ich6'> <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> <output type='pa'/> </sound>
Which part of the PostParse code does that? If you configure the domain XML like this:
<sound model='ich6'/> <sound model='ich6'/>
The resulting config XML saved to disk would be:
<sound model='ich6'> <<address type='pci' domain='0x0000' bus='0x00' slot='0x03' unction='0x0'/> </sound> <sound model='ich6'> <<address type='pci' domain='0x0000' bus='0x00' slot='0x04' unction='0x0'/> </sound>
From patch 6 I used:
tests/qemuxml2argvdata/qemuxml2argv-sound-multi-pa-output-spice.xml
which has
<sound model='ich6'> <output type='pa'/> </sound> <sound model='ich6'/>
Right, it was not clear what you meant, next time please include this input XML right away, it will save both of us some time to understand each other :). I see your point that this update could have been done in Prepare function. I chose this implementation to make it clear that there is the limitation with QEMU, but it would also work to put it into Prepare function. That way if QEMU introduces support to configure different output for each sound device users would benefit from it automatically for existing domains once we add it also into libvirt.
One of the reasons why I didn't add qemuxml2xml tests is that only the offline part is somehow working, but the active and status part of that test is broken and doesn't reflect how libvirt changes the active and status XML.
I didn't pay close attention to which was which, just that some output xml was generated by the regenerate.
I'll send a v2 to make sure that all the points and notes were addressed correctly. Pavel
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- ...xml2argv-sound-multi-different-output-spice.xml | 29 ++++++++++++++++++++++ .../qemuxml2argv-sound-multi-pa-output-spice.args | 26 +++++++++++++++++++ .../qemuxml2argv-sound-multi-pa-output-spice.xml | 27 ++++++++++++++++++++ .../qemuxml2argv-sound-pa-output-spice.args | 24 ++++++++++++++++++ .../qemuxml2argv-sound-pa-output-spice.xml | 26 +++++++++++++++++++ tests/qemuxml2argvtest.c | 12 +++++++++ 6 files changed, 144 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-sound-multi-different-output-spice.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-sound-multi-pa-output-spice.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-sound-multi-pa-output-spice.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-sound-pa-output-spice.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-sound-pa-output-spice.xml diff --git a/tests/qemuxml2argvdata/qemuxml2argv-sound-multi-different-output-spice.xml b/tests/qemuxml2argvdata/qemuxml2argv-sound-multi-different-output-spice.xml new file mode 100644 index 0000000000..80751b7e02 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-sound-multi-different-output-spice.xml @@ -0,0 +1,29 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-i686</emulator> + <controller type='ide' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <sound model='ich6'> + <output type='pa'/> + </sound> + <sound model='ich6'> + <output type='none'/> + </sound> + <graphics type='spice'/> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-sound-multi-pa-output-spice.args b/tests/qemuxml2argvdata/qemuxml2argv-sound-multi-pa-output-spice.args new file mode 100644 index 0000000000..4a54d50e9f --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-sound-multi-pa-output-spice.args @@ -0,0 +1,26 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=pa \ +/usr/bin/qemu-system-i686 \ +-name QEMUGuest1 \ +-S \ +-M pc \ +-m 214 \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-nodefaults \ +-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,\ +server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=readline \ +-no-acpi \ +-boot c \ +-usb \ +-spice port=0 \ +-vga cirrus \ +-device intel-hda,id=sound0,bus=pci.0,addr=0x3 \ +-device hda-duplex,id=sound0-codec0,bus=sound0.0,cad=0 \ +-device intel-hda,id=sound1,bus=pci.0,addr=0x4 \ +-device hda-duplex,id=sound1-codec0,bus=sound1.0,cad=0 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-sound-multi-pa-output-spice.xml b/tests/qemuxml2argvdata/qemuxml2argv-sound-multi-pa-output-spice.xml new file mode 100644 index 0000000000..4e76c2c30d --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-sound-multi-pa-output-spice.xml @@ -0,0 +1,27 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-i686</emulator> + <controller type='ide' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <sound model='ich6'> + <output type='pa'/> + </sound> + <sound model='ich6'/> + <graphics type='spice'/> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-sound-pa-output-spice.args b/tests/qemuxml2argvdata/qemuxml2argv-sound-pa-output-spice.args new file mode 100644 index 0000000000..8d5bb9158e --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-sound-pa-output-spice.args @@ -0,0 +1,24 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=pa \ +/usr/bin/qemu-system-i686 \ +-name QEMUGuest1 \ +-S \ +-M pc \ +-m 214 \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-nodefaults \ +-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,\ +server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=readline \ +-no-acpi \ +-boot c \ +-usb \ +-spice port=0 \ +-vga cirrus \ +-device intel-hda,id=sound0,bus=pci.0,addr=0x3 \ +-device hda-duplex,id=sound0-codec0,bus=sound0.0,cad=0 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-sound-pa-output-spice.xml b/tests/qemuxml2argvdata/qemuxml2argv-sound-pa-output-spice.xml new file mode 100644 index 0000000000..2c4de0fe57 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-sound-pa-output-spice.xml @@ -0,0 +1,26 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-i686</emulator> + <controller type='ide' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <sound model='ich6'> + <output type='pa'/> + </sound> + <graphics type='spice'/> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 6c80e0bc77..ed01979fc8 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1573,6 +1573,18 @@ mymain(void) QEMU_CAPS_SPICE, QEMU_CAPS_VNC, QEMU_CAPS_DEVICE_CIRRUS_VGA); + DO_TEST("sound-pa-output-spice", + QEMU_CAPS_HDA_DUPLEX, + QEMU_CAPS_SPICE, + QEMU_CAPS_DEVICE_CIRRUS_VGA); + DO_TEST("sound-multi-pa-output-spice", + QEMU_CAPS_HDA_DUPLEX, + QEMU_CAPS_SPICE, + QEMU_CAPS_DEVICE_CIRRUS_VGA); + DO_TEST_PARSE_ERROR("sound-multi-different-output-spice", + QEMU_CAPS_HDA_DUPLEX, + QEMU_CAPS_SPICE, + QEMU_CAPS_DEVICE_CIRRUS_VGA); DO_TEST("fs9p", QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_FSDEV, QEMU_CAPS_FSDEV_WRITEOUT); -- 2.13.6
On 11/14/2017 08:45 AM, Pavel Hrdina wrote:
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- ...xml2argv-sound-multi-different-output-spice.xml | 29 ++++++++++++++++++++++ .../qemuxml2argv-sound-multi-pa-output-spice.args | 26 +++++++++++++++++++ .../qemuxml2argv-sound-multi-pa-output-spice.xml | 27 ++++++++++++++++++++ .../qemuxml2argv-sound-pa-output-spice.args | 24 ++++++++++++++++++ .../qemuxml2argv-sound-pa-output-spice.xml | 26 +++++++++++++++++++ tests/qemuxml2argvtest.c | 12 +++++++++ 6 files changed, 144 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-sound-multi-different-output-spice.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-sound-multi-pa-output-spice.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-sound-multi-pa-output-spice.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-sound-pa-output-spice.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-sound-pa-output-spice.xml
As pointed out elsewhere along the way - there's no qemuxml2xmltest being done. That's something we generally require when the domain conf and rng is adjusted. John
participants (4)
-
Daniel P. Berrange -
John Ferlan -
Pavel Hrdina -
Peter Krempa