On Mon, Jun 26, 2023 at 2:13 PM Michal Privoznik <mpriv...@redhat.com> wrote:
> Historically, the way to set PC speaker for a guest was to pass: > > -soundhw pcspk > > but as of QEMU commit v5.1.0-rc0~28^2~3 this is deprecated and we > should use: > > -machine pcspk-audiodev=$id > > instead. The old way was then removed in commit v7.1.0-rc0~99^2~3. > > Now, ideally we would have a capability selecting whether we talk > to a QEMU that understands the new way or not. But it's not that > simple - the machine attribute is just an alias to the .audiodev= > attribute of 'isa-pcspk' object and both are created in > pc_machine_initfn() function, i.e. not then the PC_MACHINE() class > is initialized, but when it's instantiated. IOW, it's not possible > for us to query whether we're dealing with older or newer QEMU. > > But given that the newer version is supported since v5.1.0 and the > minimal version we require is v4.2.0 (i.e. there are two releases > which don't understand the newer cmd line) and how frequently this > feature is (un-)used (the issue was reported after ~1 year since it > stopped working), I believe we can live without any capability and > just use the newer cmd line unconditionally. > > Resolves: https://gitlab.com/libvirt/libvirt/-/issues/490 > Signed-off-by: Michal Privoznik <mpriv...@redhat.com> > --- > src/qemu/qemu_command.c | 57 ++++++++++++------- > .../sound-device.x86_64-4.2.0.args | 3 +- > .../sound-device.x86_64-latest.args | 3 +- > 3 files changed, 38 insertions(+), 25 deletions(-) > > diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c > index a19902988c..3b0b162b8b 100644 > --- a/src/qemu/qemu_command.c > +++ b/src/qemu/qemu_command.c > @@ -4477,31 +4477,30 @@ qemuBuildSoundCommandLine(virCommand *cmd, > for (i = 0; i < def->nsounds; i++) { > virDomainSoundDef *sound = def->sounds[i]; > > - /* Sadly pcspk device doesn't use -device syntax. Fortunately > - * we don't need to set any PCI address on it, so we don't > - * mind too much */ > - if (sound->model == VIR_DOMAIN_SOUND_MODEL_PCSPK) { > - virCommandAddArgList(cmd, "-soundhw", "pcspk", NULL); > - } else { > - if (qemuCommandAddExtDevice(cmd, &sound->info, def, qemuCaps) > < 0) > - return -1; > + /* Sadly pcspk device doesn't use -device syntax. And it > + * was handled already in qemuBuildMachineCommandLine(). > + */ > + if (sound->model == VIR_DOMAIN_SOUND_MODEL_PCSPK) > + continue; > > - if (qemuBuildSoundDevCmd(cmd, def, sound, qemuCaps) < 0) > - return -1; > + if (qemuCommandAddExtDevice(cmd, &sound->info, def, qemuCaps) < 0) > + return -1; > > - if (virDomainSoundModelSupportsCodecs(sound)) { > - for (j = 0; j < sound->ncodecs; j++) { > - if (qemuBuildSoundCodecCmd(cmd, def, sound, > sound->codecs[j], > - qemuCaps) < 0) > - return -1; > - } > + if (qemuBuildSoundDevCmd(cmd, def, sound, qemuCaps) < 0) > + return -1; > > - if (j == 0) { > - virDomainSoundCodecDef codec = { > VIR_DOMAIN_SOUND_CODEC_TYPE_DUPLEX, 0 }; > + if (virDomainSoundModelSupportsCodecs(sound)) { > + for (j = 0; j < sound->ncodecs; j++) { > + if (qemuBuildSoundCodecCmd(cmd, def, sound, > sound->codecs[j], > + qemuCaps) < 0) > + return -1; > + } > > - if (qemuBuildSoundCodecCmd(cmd, def, sound, &codec, > qemuCaps) < 0) > - return -1; > - } > + if (j == 0) { > + virDomainSoundCodecDef codec = { > VIR_DOMAIN_SOUND_CODEC_TYPE_DUPLEX, 0 }; > + > + if (qemuBuildSoundCodecCmd(cmd, def, sound, &codec, > qemuCaps) < 0) > + return -1; > } > } > } > @@ -7029,6 +7028,22 @@ qemuBuildMachineCommandLine(virCommand *cmd, > } > } > > + /* PC speaker is a bit different than the rest of sound cards > + * which is handled in qemuBuildSoundCommandLine(). */ > maybe *which are handled... ? > + for (i = 0; i < def->nsounds; i++) { > + const virDomainSoundDef *sound = def->sounds[i]; > + g_autofree char *audioid = NULL; > + > + if (sound->model != VIR_DOMAIN_SOUND_MODEL_PCSPK) > + continue; > + > + if (!(audioid = qemuGetAudioIDString(def, sound->audioId))) > + return -1; > + > + virBufferAsprintf(&buf, ",pcspk-audiodev=%s", audioid); > + break; > + } > + > qemuBuildMachineACPI(&buf, def, qemuCaps); > > virCommandAddArgBuffer(cmd, &buf); > diff --git a/tests/qemuxml2argvdata/sound-device.x86_64-4.2.0.args > b/tests/qemuxml2argvdata/sound-device.x86_64-4.2.0.args > index b2a5afd8c8..1e3cc9eaa2 100644 > --- a/tests/qemuxml2argvdata/sound-device.x86_64-4.2.0.args > +++ b/tests/qemuxml2argvdata/sound-device.x86_64-4.2.0.args > @@ -10,7 +10,7 @@ > XDG_CONFIG_HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1/.config \ > -name guest=QEMUGuest1,debug-threads=on \ > -S \ > -object > secret,id=masterKey0,format=raw,file=/var/lib/libvirt/qemu/domain--1-QEMUGuest1/master-key.aes > \ > --machine pc-i440fx-4.2,usb=off,dump-guest-core=off \ > +-machine pc-i440fx-4.2,usb=off,dump-guest-core=off,pcspk-audiodev=audio1 \ > -accel tcg \ > -cpu qemu64 \ > -m 214 \ > @@ -28,7 +28,6 @@ > XDG_CONFIG_HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1/.config \ > -boot strict=on \ > -device piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 \ > -audiodev '{"id":"audio1","driver":"none"}' \ > --soundhw pcspk \ > -device ES1370,id=sound1,audiodev=audio1,bus=pci.0,addr=0x2 \ > -device sb16,id=sound2,audiodev=audio1 \ > -device AC97,id=sound3,audiodev=audio1,bus=pci.0,addr=0x3 \ > diff --git a/tests/qemuxml2argvdata/sound-device.x86_64-latest.args > b/tests/qemuxml2argvdata/sound-device.x86_64-latest.args > index e0a2f21b31..7c90c1271c 100644 > --- a/tests/qemuxml2argvdata/sound-device.x86_64-latest.args > +++ b/tests/qemuxml2argvdata/sound-device.x86_64-latest.args > @@ -10,7 +10,7 @@ > XDG_CONFIG_HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1/.config \ > -name guest=QEMUGuest1,debug-threads=on \ > -S \ > -object > '{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/var/lib/libvirt/qemu/domain--1-QEMUGuest1/master-key.aes"}' > \ > --machine pc,usb=off,dump-guest-core=off,memory-backend=pc.ram,acpi=off \ > +-machine > pc,usb=off,dump-guest-core=off,memory-backend=pc.ram,pcspk-audiodev=audio1,acpi=off > \ > -accel tcg \ > -cpu qemu64 \ > -m 214 \ > @@ -28,7 +28,6 @@ > XDG_CONFIG_HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1/.config \ > -boot strict=on \ > -device > '{"driver":"piix3-usb-uhci","id":"usb","bus":"pci.0","addr":"0x1.0x2"}' \ > -audiodev '{"id":"audio1","driver":"none"}' \ > --soundhw pcspk \ > -device > '{"driver":"ES1370","id":"sound1","audiodev":"audio1","bus":"pci.0","addr":"0x2"}' > \ > -device '{"driver":"sb16","id":"sound2","audiodev":"audio1"}' \ > -device > '{"driver":"AC97","id":"sound3","audiodev":"audio1","bus":"pci.0","addr":"0x3"}' > \ > -- > 2.39.3 > > Reviewed-by: Kristina Hanicova <khani...@redhat.com> Kristina
