[PATCH 0/8] qemu: Unify generators for commandline and monitor chardev backends
Apart from having just one place to fix when changing chardev backends this also adds validation against the schema so we can spot deprecations early. Peter Krempa (8): qemu: capabilities: Explain that QEMU_CAPS_CHARDEV_JSON will be used in tests only qemuxmlconftest: Add 'chardev-backends' test case qemu: Introduce unified chardev backend config generator qemuxmlconftest: Add support for validating schema for 'chardev-add' qemuxmlconftest: Add test case for QMP schema validation of -chardev backends qemu: Move check for chardev backends which can't be hotplugged out of the monitor qemu: Use the new chardev backend JSON props generator also in the monitor qemu: monitor: Remove the old chardev backend generator src/qemu/meson.build | 1 + src/qemu/qemu_block.c | 9 +- src/qemu/qemu_capabilities.h | 2 +- src/qemu/qemu_chardev.c | 488 ++++++++++++++++++ src/qemu/qemu_chardev.h | 22 + src/qemu/qemu_command.c | 202 +------- src/qemu/qemu_hotplug.c | 51 +- src/qemu/qemu_monitor.c | 8 +- src/qemu/qemu_monitor.h | 4 +- src/qemu/qemu_monitor_json.c | 273 +--------- src/qemu/qemu_monitor_json.h | 4 +- tests/qemumonitorjsontest.c | 23 +- .../chardev-backends-json.x86_64-latest.args | 79 +++ .../chardev-backends-json.x86_64-latest.xml | 1 + .../qemuxmlconfdata/chardev-backends-json.xml | 1 + .../chardev-backends.x86_64-latest.args | 79 +++ .../chardev-backends.x86_64-latest.xml | 149 ++++++ tests/qemuxmlconfdata/chardev-backends.xml | 111 ++++ tests/qemuxmlconftest.c | 7 + 19 files changed, 1026 insertions(+), 488 deletions(-) create mode 100644 src/qemu/qemu_chardev.c create mode 100644 src/qemu/qemu_chardev.h create mode 100644 tests/qemuxmlconfdata/chardev-backends-json.x86_64-latest.args create mode 120000 tests/qemuxmlconfdata/chardev-backends-json.x86_64-latest.xml create mode 120000 tests/qemuxmlconfdata/chardev-backends-json.xml create mode 100644 tests/qemuxmlconfdata/chardev-backends.x86_64-latest.args create mode 100644 tests/qemuxmlconfdata/chardev-backends.x86_64-latest.xml create mode 100644 tests/qemuxmlconfdata/chardev-backends.xml -- 2.46.0
I've added that capability a long time ago when I was converting various stuff to use JSON but the support in '-chardev' didn't yet materialize. Fix the comment to make that clear and also that it'll be used in tests for the upcoming refactor of the chardev code (so that we can validate generator against the schema even if that doesn't yet work). Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_capabilities.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 5036d49aab..736d34179e 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -623,7 +623,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for syntax-check */ QEMU_CAPS_MEMORY_BACKEND_RESERVE, /* -object memory-backend-*.reserve= */ QEMU_CAPS_PIIX4_ACPI_ROOT_PCI_HOTPLUG, /* -M pc PIIX4_PM.acpi-root-pci-hotplug */ QEMU_CAPS_NETDEV_JSON, /* -netdev accepts JSON */ - QEMU_CAPS_CHARDEV_JSON, /* -chardev accepts JSON */ + QEMU_CAPS_CHARDEV_JSON, /* Reserved for '-chardev' JSON support. For now used only in tests. */ /* 415 */ X_QEMU_CAPS_DEVICE_JSON_BROKEN_HOTPLUG, /* -device accepts JSON (must not be used - users are filtering the capbility) */ -- 2.46.0
On a Wednesday in 2024, Peter Krempa wrote:
I've added that capability a long time ago when I was converting various stuff to use JSON but the support in '-chardev' didn't yet materialize.
Fix the comment to make that clear and also that it'll be used in tests for the upcoming refactor of the chardev code (so that we can validate generator against the schema even if that doesn't yet work).
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_capabilities.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
The test case attempts to test as many of the chardev backends as possible by adding channels with various configs. The idea is to have a representative sample which will later be used also for QMP schema testing. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- .../chardev-backends.x86_64-latest.args | 79 ++++++++++ .../chardev-backends.x86_64-latest.xml | 149 ++++++++++++++++++ tests/qemuxmlconfdata/chardev-backends.xml | 111 +++++++++++++ tests/qemuxmlconftest.c | 2 + 4 files changed, 341 insertions(+) create mode 100644 tests/qemuxmlconfdata/chardev-backends.x86_64-latest.args create mode 100644 tests/qemuxmlconfdata/chardev-backends.x86_64-latest.xml create mode 100644 tests/qemuxmlconfdata/chardev-backends.xml diff --git a/tests/qemuxmlconfdata/chardev-backends.x86_64-latest.args b/tests/qemuxmlconfdata/chardev-backends.x86_64-latest.args new file mode 100644 index 0000000000..4e6e17587e --- /dev/null +++ b/tests/qemuxmlconfdata/chardev-backends.x86_64-latest.args @@ -0,0 +1,79 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1 \ +USER=test \ +LOGNAME=test \ +XDG_DATA_HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1/.local/share \ +XDG_CACHE_HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1/.cache \ +XDG_CONFIG_HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1/.config \ +/usr/bin/qemu-system-x86_64 \ +-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 \ +-accel tcg \ +-cpu qemu64 \ +-m size=219136k \ +-object '{"qom-type":"memory-backend-ram","id":"pc.ram","size":224395264}' \ +-overcommit mem-lock=off \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-no-user-config \ +-nodefaults \ +-chardev socket,id=charmonitor,fd=1729,server=on,wait=off \ +-mon chardev=charmonitor,id=monitor,mode=control \ +-rtc base=utc \ +-no-shutdown \ +-boot strict=on \ +-device '{"driver":"piix3-usb-uhci","id":"usb","bus":"pci.0","addr":"0x1.0x2"}' \ +-device '{"driver":"virtio-serial-pci","id":"virtio-serial0","bus":"pci.0","addr":"0x3"}' \ +-chardev parallel,id=charparallel0,path=/dev/parport0 \ +-device '{"driver":"isa-parallel","chardev":"charparallel0","id":"parallel0"}' \ +-add-fd set=0,fd=1751,opaque=channel0-log \ +-chardev null,id=charchannel0,logfile=/dev/fdset/0,logappend=on \ +-device '{"driver":"virtserialport","bus":"virtio-serial0.0","nr":1,"chardev":"charchannel0","id":"channel0","name":"chardev-null"}' \ +-add-fd set=1,fd=1751,opaque=channel1-log \ +-chardev vc,id=charchannel1,logfile=/dev/fdset/1,logappend=on \ +-device '{"driver":"virtserialport","bus":"virtio-serial0.0","nr":2,"chardev":"charchannel1","id":"channel1","name":"chardev-vc"}' \ +-chardev pty,id=charchannel2 \ +-device '{"driver":"virtserialport","bus":"virtio-serial0.0","nr":3,"chardev":"charchannel2","id":"channel2","name":"chardev-pty"}' \ +-chardev stdio,id=charchannel3 \ +-device '{"driver":"virtserialport","bus":"virtio-serial0.0","nr":4,"chardev":"charchannel3","id":"channel3","name":"chardev-stdio"}' \ +-add-fd set=2,fd=1750,opaque=channel4-source \ +-chardev file,id=charchannel4,path=/dev/fdset/2,append=on \ +-device '{"driver":"virtserialport","bus":"virtio-serial0.0","nr":5,"chardev":"charchannel4","id":"channel4","name":"chardev-file"}' \ +-chardev pipe,id=charchannel5,path=/path/to/pipe \ +-device '{"driver":"virtserialport","bus":"virtio-serial0.0","nr":6,"chardev":"charchannel5","id":"channel5","name":"chardev-pipe"}' \ +-chardev serial,id=charchannel6,path=/path/to/device \ +-device '{"driver":"virtserialport","bus":"virtio-serial0.0","nr":7,"chardev":"charchannel6","id":"channel6","name":"chardev-dev"}' \ +-chardev socket,id=charchannel7,fd=1729,server=on,wait=off \ +-device '{"driver":"virtserialport","bus":"virtio-serial0.0","nr":8,"chardev":"charchannel7","id":"channel7","name":"chardev-unix-listen"}' \ +-chardev socket,id=charchannel8,path=/path/to/unix-listen,reconnect=2 \ +-device '{"driver":"virtserialport","bus":"virtio-serial0.0","nr":9,"chardev":"charchannel8","id":"channel8","name":"chardev-unix-connect"}' \ +-chardev socket,id=charchannel9,host=1.2.3.4,port=5678,server=on,wait=off \ +-device '{"driver":"virtserialport","bus":"virtio-serial0.0","nr":10,"chardev":"charchannel9","id":"channel9","name":"chardev-tcp-listen-raw"}' \ +-chardev socket,id=charchannel10,host=1.2.3.4,port=5679,telnet=on,server=on,wait=off \ +-device '{"driver":"virtserialport","bus":"virtio-serial0.0","nr":11,"chardev":"charchannel10","id":"channel10","name":"chardev-tcp-listen-telnet"}' \ +-object '{"qom-type":"tls-creds-x509","id":"objcharchannel11_tls0","dir":"/etc/pki/libvirt-chardev","endpoint":"client","verify-peer":true}' \ +-chardev socket,id=charchannel11,host=1.2.3.4,port=5678,reconnect=2,tls-creds=objcharchannel11_tls0 \ +-device '{"driver":"virtserialport","bus":"virtio-serial0.0","nr":12,"chardev":"charchannel11","id":"channel11","name":"chardev-tcp-connect-raw"}' \ +-object '{"qom-type":"tls-creds-x509","id":"objcharchannel12_tls0","dir":"/etc/pki/libvirt-chardev","endpoint":"client","verify-peer":true}' \ +-chardev socket,id=charchannel12,host=hostname.global.,port=5679,telnet=on,reconnect=2,tls-creds=objcharchannel12_tls0 \ +-device '{"driver":"virtserialport","bus":"virtio-serial0.0","nr":13,"chardev":"charchannel12","id":"channel12","name":"chardev-tcp-connect-telnet"}' \ +-chardev udp,id=charchannel13,host=127.0.0.1,port=2222,localaddr=,localport=0 \ +-device '{"driver":"virtserialport","bus":"virtio-serial0.0","nr":14,"chardev":"charchannel13","id":"channel13","name":"chardev-udp-nobind"}' \ +-chardev udp,id=charchannel14,host=127.0.0.1,port=2222,localaddr=127.0.0.1,localport=1111 \ +-device '{"driver":"virtserialport","bus":"virtio-serial0.0","nr":15,"chardev":"charchannel14","id":"channel14","name":"chardev-udp-bind"}' \ +-chardev spicevmc,id=charchannel15,name=vdagent \ +-device '{"driver":"virtserialport","bus":"virtio-serial0.0","nr":16,"chardev":"charchannel15","id":"channel15","name":"com.redhat.spice.0"}' \ +-chardev qemu-vdagent,id=charchannel16,name=vdagent,clipboard=off,mouse=off \ +-device '{"driver":"virtserialport","bus":"virtio-serial0.0","nr":17,"chardev":"charchannel16","id":"channel16","name":"chardev-vdagent"}' \ +-chardev dbus,id=charchannel17,name=test.channel.0 \ +-device '{"driver":"virtserialport","bus":"virtio-serial0.0","nr":18,"chardev":"charchannel17","id":"channel17","name":"chardev-dbus"}' \ +-chardev spiceport,id=charchannel18,name=test.channel.0 \ +-device '{"driver":"virtserialport","bus":"virtio-serial0.0","nr":19,"chardev":"charchannel18","id":"channel18","name":"chardev-spiceport"}' \ +-audiodev '{"id":"audio1","driver":"spice"}' \ +-spice port=5901,addr=0.0.0.0,seamless-migration=on \ +-device '{"driver":"cirrus-vga","id":"video0","bus":"pci.0","addr":"0x2"}' \ +-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \ +-msg timestamp=on diff --git a/tests/qemuxmlconfdata/chardev-backends.x86_64-latest.xml b/tests/qemuxmlconfdata/chardev-backends.x86_64-latest.xml new file mode 100644 index 0000000000..57a16b5b7c --- /dev/null +++ b/tests/qemuxmlconfdata/chardev-backends.x86_64-latest.xml @@ -0,0 +1,149 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219100</memory> + <currentMemory unit='KiB'>219100</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='x86_64' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <cpu mode='custom' match='exact' check='none'> + <model fallback='forbid'>qemu64</model> + </cpu> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + <controller type='usb' index='0' model='piix3-uhci'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> + </controller> + <controller type='pci' index='0' model='pci-root'/> + <controller type='virtio-serial' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> + </controller> + <parallel type='dev'> + <source path='/dev/parport0'/> + <target port='0'/> + </parallel> + <channel type='null'> + <log file='/path/to/null.log' append='off'/> + <target type='virtio' name='chardev-null'/> + <address type='virtio-serial' controller='0' bus='0' port='1'/> + </channel> + <channel type='vc'> + <log file='/path/to/vc.log' append='on'/> + <target type='virtio' name='chardev-vc'/> + <address type='virtio-serial' controller='0' bus='0' port='2'/> + </channel> + <channel type='pty'> + <target type='virtio' name='chardev-pty'/> + <address type='virtio-serial' controller='0' bus='0' port='3'/> + </channel> + <channel type='stdio'> + <target type='virtio' name='chardev-stdio'/> + <address type='virtio-serial' controller='0' bus='0' port='4'/> + </channel> + <channel type='file'> + <source path='/path/to/fileout'/> + <target type='virtio' name='chardev-file'/> + <address type='virtio-serial' controller='0' bus='0' port='5'/> + </channel> + <channel type='pipe'> + <source path='/path/to/pipe'/> + <target type='virtio' name='chardev-pipe'/> + <address type='virtio-serial' controller='0' bus='0' port='6'/> + </channel> + <channel type='dev'> + <source path='/path/to/device'/> + <target type='virtio' name='chardev-dev'/> + <address type='virtio-serial' controller='0' bus='0' port='7'/> + </channel> + <channel type='unix'> + <source mode='bind' path='/path/to/unix-listen'/> + <target type='virtio' name='chardev-unix-listen'/> + <address type='virtio-serial' controller='0' bus='0' port='8'/> + </channel> + <channel type='unix'> + <source mode='connect' path='/path/to/unix-listen'> + <reconnect enabled='yes' timeout='2'/> + </source> + <target type='virtio' name='chardev-unix-connect'/> + <address type='virtio-serial' controller='0' bus='0' port='9'/> + </channel> + <channel type='tcp'> + <source mode='bind' host='1.2.3.4' service='5678'/> + <protocol type='raw'/> + <target type='virtio' name='chardev-tcp-listen-raw'/> + <address type='virtio-serial' controller='0' bus='0' port='10'/> + </channel> + <channel type='tcp'> + <source mode='bind' host='1.2.3.4' service='5679'/> + <protocol type='telnet'/> + <target type='virtio' name='chardev-tcp-listen-telnet'/> + <address type='virtio-serial' controller='0' bus='0' port='11'/> + </channel> + <channel type='tcp'> + <source mode='connect' host='1.2.3.4' service='5678' tls='yes'> + <reconnect enabled='yes' timeout='2'/> + </source> + <protocol type='raw'/> + <target type='virtio' name='chardev-tcp-connect-raw'/> + <address type='virtio-serial' controller='0' bus='0' port='12'/> + </channel> + <channel type='tcp'> + <source mode='connect' host='hostname.global.' service='5679' tls='yes'> + <reconnect enabled='yes' timeout='2'/> + </source> + <protocol type='telnet'/> + <target type='virtio' name='chardev-tcp-connect-telnet'/> + <address type='virtio-serial' controller='0' bus='0' port='13'/> + </channel> + <channel type='udp'> + <source mode='connect' host='127.0.0.1' service='2222'/> + <target type='virtio' name='chardev-udp-nobind'/> + <address type='virtio-serial' controller='0' bus='0' port='14'/> + </channel> + <channel type='udp'> + <source mode='bind' host='127.0.0.1' service='1111'/> + <source mode='connect' host='127.0.0.1' service='2222'/> + <target type='virtio' name='chardev-udp-bind'/> + <address type='virtio-serial' controller='0' bus='0' port='15'/> + </channel> + <channel type='spicevmc'> + <target type='virtio' name='com.redhat.spice.0'/> + <address type='virtio-serial' controller='0' bus='0' port='16'/> + </channel> + <channel type='qemu-vdagent'> + <source> + <clipboard copypaste='no'/> + <mouse mode='server'/> + </source> + <target type='virtio' name='chardev-vdagent'/> + <address type='virtio-serial' controller='0' bus='0' port='17'/> + </channel> + <channel type='dbus'> + <source channel='test.channel.0'/> + <target type='virtio' name='chardev-dbus'/> + <address type='virtio-serial' controller='0' bus='0' port='18'/> + </channel> + <channel type='spiceport'> + <source channel='test.channel.0'/> + <target type='virtio' name='chardev-spiceport'/> + <address type='virtio-serial' controller='0' bus='0' port='19'/> + </channel> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <graphics type='spice' autoport='yes' listen='0.0.0.0'> + <listen type='address' address='0.0.0.0'/> + </graphics> + <audio id='1' type='spice'/> + <video> + <model type='cirrus' vram='16384' heads='1' primary='yes'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> + </video> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxmlconfdata/chardev-backends.xml b/tests/qemuxmlconfdata/chardev-backends.xml new file mode 100644 index 0000000000..c92489ce4c --- /dev/null +++ b/tests/qemuxmlconfdata/chardev-backends.xml @@ -0,0 +1,111 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219100</memory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='x86_64' machine='pc'>hvm</type> + </os> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + <channel type='null'> + <target type='virtio' name='chardev-null'/> + <log file='/path/to/null.log' append='off'/> + </channel> + <channel type='vc'> + <target type='virtio' name='chardev-vc'/> + <log file='/path/to/vc.log' append='on'/> + </channel> + <channel type='pty'> + <target type='virtio' name='chardev-pty'/> + </channel> + <channel type='stdio'> + <target type='virtio' name='chardev-stdio'/> + </channel> + <channel type='file'> + <source path='/path/to/fileout'/> + <target type='virtio' name='chardev-file'/> + </channel> + <channel type='pipe'> + <source path='/path/to/pipe'/> + <target type='virtio' name='chardev-pipe'/> + </channel> + <channel type='dev'> + <source path='/path/to/device'/> + <target type='virtio' name='chardev-dev'/> + </channel> + <channel type='unix'> + <source mode='bind' path='/path/to/unix-listen'/> + <target type='virtio' name='chardev-unix-listen'/> + </channel> + <channel type='unix'> + <source mode='connect' path='/path/to/unix-listen'> + <reconnect enabled='yes' timeout='2'/> + </source> + <target type='virtio' name='chardev-unix-connect'/> + </channel> + <channel type='tcp'> + <source mode='bind' host='1.2.3.4' service='5678'/> + <protocol type='raw'/> + <target type='virtio' name='chardev-tcp-listen-raw'/> + </channel> + <channel type='tcp'> + <source mode='bind' host='1.2.3.4' service='5679'/> + <protocol type='telnet'/> + <target type='virtio' name='chardev-tcp-listen-telnet'/> + </channel> + <channel type='tcp'> + <source mode='connect' host='1.2.3.4' service='5678' tls='yes'> + <reconnect enabled='yes' timeout='2'/> + </source> + <protocol type='raw'/> + <target type='virtio' name='chardev-tcp-connect-raw'/> + </channel> + <channel type='tcp'> + <source mode='connect' host='hostname.global.' service='5679' tls='yes'> + <reconnect enabled='yes' timeout='2'/> + </source> + <protocol type='telnet'/> + <target type='virtio' name='chardev-tcp-connect-telnet'/> + </channel> + <channel type='udp'> + <source mode='connect' host='127.0.0.1' service='2222'/> + <target type='virtio' name='chardev-udp-nobind'/> + </channel> + <channel type='udp'> + <source mode='bind' host='127.0.0.1' service='1111'/> + <source mode='connect' host='127.0.0.1' service='2222'/> + <target type='virtio' name='chardev-udp-bind'/> + </channel> + <!-- The commandline formatter requires that the target is strictly 'com.redhat.spice.0', + and spice graphics is present. --> + <channel type='spicevmc'> + <target type='virtio' name='com.redhat.spice.0'/> + </channel> + <channel type='qemu-vdagent'> + <target type='virtio' name='chardev-vdagent'/> + <source> + <clipboard copypaste='no'/> + <mouse mode='server'/> + </source> + </channel> + <channel type='dbus'> + <source channel='test.channel.0'/> + <target type='virtio' name='chardev-dbus'/> + </channel> + <channel type='spiceport'> + <source channel='test.channel.0'/> + <target type='virtio' name='chardev-spiceport'/> + </channel> + <!-- parallel port with 'dev' type is special as it has a specific backend in qemu --> + <parallel type='dev'> + <source path='/dev/parport0'/> + <target port='0'/> + </parallel> + <!-- spice graphics required by some of the spice-transported backends --> + <graphics type='spice' autoport='yes' listen='0.0.0.0'> + <listen type='address' address='0.0.0.0'/> + </graphics> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxmlconftest.c b/tests/qemuxmlconftest.c index 323fd9d721..6fc8d5eabd 100644 --- a/tests/qemuxmlconftest.c +++ b/tests/qemuxmlconftest.c @@ -1830,6 +1830,8 @@ mymain(void) DO_TEST_CAPS_LATEST("console-compat-auto"); DO_TEST_CAPS_LATEST("console-compat-crash"); + DO_TEST_CAPS_LATEST("chardev-backends"); + DO_TEST_CAPS_LATEST("serial-vc-chardev"); DO_TEST_CAPS_LATEST("serial-pty-chardev"); DO_TEST_CAPS_LATEST("serial-dev-chardev"); -- 2.46.0
On a Wednesday in 2024, Peter Krempa wrote:
The test case attempts to test as many of the chardev backends as possible by adding channels with various configs. The idea is to have a representative sample which will later be used also for QMP schema testing.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- .../chardev-backends.x86_64-latest.args | 79 ++++++++++ .../chardev-backends.x86_64-latest.xml | 149 ++++++++++++++++++ tests/qemuxmlconfdata/chardev-backends.xml | 111 +++++++++++++ tests/qemuxmlconftest.c | 2 + 4 files changed, 341 insertions(+) create mode 100644 tests/qemuxmlconfdata/chardev-backends.x86_64-latest.args create mode 100644 tests/qemuxmlconfdata/chardev-backends.x86_64-latest.xml create mode 100644 tests/qemuxmlconfdata/chardev-backends.xml
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Similarly to how we approach the generators for -device/-object/-blockdev/-netdev rewrite the generator of -chardev to be unified with the generator for the monitor. Unfortunately with -chardev it will be a bit more quirky when compared to the others as the generator itself will need to know whether it generates command line output or not as few field names change and data is nested differently. This first step adds the generator and uses it only for command line generation. This was possible to achieve without changing any of the output in tests. In further patches the same generator will then be used also in the monitor code replacing the both. As basis for the generator I took the monitor code but modified it to have the same field order as the commandline code and extended it further to support all backend types, even those which are not hotpluggable. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/meson.build | 1 + src/qemu/qemu_chardev.c | 488 ++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_chardev.h | 22 ++ src/qemu/qemu_command.c | 202 +---------------- 4 files changed, 513 insertions(+), 200 deletions(-) create mode 100644 src/qemu/qemu_chardev.c create mode 100644 src/qemu/qemu_chardev.h diff --git a/src/qemu/meson.build b/src/qemu/meson.build index 57356451e4..1d904bbc68 100644 --- a/src/qemu/meson.build +++ b/src/qemu/meson.build @@ -6,6 +6,7 @@ qemu_driver_sources = [ 'qemu_blockjob.c', 'qemu_capabilities.c', 'qemu_cgroup.c', + 'qemu_chardev.c', 'qemu_checkpoint.c', 'qemu_command.c', 'qemu_conf.c', diff --git a/src/qemu/qemu_chardev.c b/src/qemu/qemu_chardev.c new file mode 100644 index 0000000000..cc6ba33f51 --- /dev/null +++ b/src/qemu/qemu_chardev.c @@ -0,0 +1,488 @@ +/* + * SPDX-License-Identifier: LGPL-2.1-or-later + */ + +#include <config.h> + +#include "qemu_capabilities.h" +#include "qemu_domain.h" +#include "qemu_fd.h" + +#include "vircommand.h" +#include "virlog.h" +#include "virqemu.h" + +#include "domain_conf.h" + +#include "qemu_chardev.h" + +#define VIR_FROM_THIS VIR_FROM_QEMU + +VIR_LOG_INIT("qemu.qemu_command"); + +static int +qemuChardevBackendAddSocketAddressInet(virJSONValue **backendData, + const char *backendFieldName, + bool commandline, + const char *commandlinePrefix, + const char *host, + const char *port) +{ + if (commandline) { + g_autofree char *hostField = NULL; + g_autofree char *portField = NULL; + + if (!commandlinePrefix) { + hostField = g_strdup("s:host"); + portField = g_strdup("s:port"); + } else { + hostField = g_strdup_printf("s:%saddr", commandlinePrefix); + portField = g_strdup_printf("s:%sport", commandlinePrefix); + } + + if (virJSONValueObjectAdd(backendData, + hostField, host, + portField, port, + NULL) < 0) + return -1; + } else { + g_autoptr(virJSONValue) addr = NULL; + g_autoptr(virJSONValue) data = NULL; + g_autofree char *datafiled = g_strdup_printf("a:%s", backendFieldName); + + if (virJSONValueObjectAdd(&data, + "s:host", host, + "s:port", port, + NULL) < 0) + return -1; + + if (virJSONValueObjectAdd(&addr, + "s:type", "inet", + "a:data", &data, + NULL) < 0) + return -1; + + if (virJSONValueObjectAdd(backendData, + datafiled, &addr, + NULL) < 0) + return -1; + } + + return 0; +} + + +static int +qemuChardevBackendAddSocketAddressFD(virJSONValue **backendData, + const char *backendFieldName, + bool commandline, + const char *fdname) +{ + if (commandline) { + if (virJSONValueObjectAdd(backendData, + "s:fd", fdname, + NULL) < 0) + return -1; + } else { + g_autoptr(virJSONValue) addr = NULL; + g_autoptr(virJSONValue) data = NULL; + g_autofree char *datafiled = g_strdup_printf("a:%s", backendFieldName); + + if (virJSONValueObjectAdd(&data, "s:str", fdname, NULL) < 0) + return -1; + + if (virJSONValueObjectAdd(&addr, + "s:type", "fd", + "a:data", &data, NULL) < 0) + return -1; + + if (virJSONValueObjectAdd(backendData, + datafiled, &addr, + NULL) < 0) + return -1; + } + + return 0; +} + + +static int +qemuChardevBackendAddSocketAddressUNIX(virJSONValue **backendData, + const char *backendFieldName, + bool commandline, + const char *path) +{ + if (commandline) { + if (virJSONValueObjectAdd(backendData, + "s:path", path, + NULL) < 0) + return -1; + } else { + g_autoptr(virJSONValue) addr = NULL; + g_autoptr(virJSONValue) data = NULL; + g_autofree char *datafiled = g_strdup_printf("a:%s", backendFieldName); + + if (virJSONValueObjectAdd(&data, "s:path", path, NULL) < 0) + return -1; + + if (virJSONValueObjectAdd(&addr, + "s:type", "unix", + "a:data", &data, NULL) < 0) + return -1; + + if (virJSONValueObjectAdd(backendData, + datafiled, &addr, + NULL) < 0) + return -1; + } + + return 0; +} + + +int +qemuChardevGetBackendProps(const virDomainChrSourceDef *chr, + bool commandline, + const char *alias, + const char **backendType, + virJSONValue **props) +{ + qemuDomainChrSourcePrivate *chrSourcePriv = QEMU_DOMAIN_CHR_SOURCE_PRIVATE(chr); + const char *dummy = NULL; + + if (!backendType) + backendType = &dummy; + + *props = NULL; + + switch ((virDomainChrType)chr->type) { + case VIR_DOMAIN_CHR_TYPE_NULL: + case VIR_DOMAIN_CHR_TYPE_VC: + case VIR_DOMAIN_CHR_TYPE_PTY: + case VIR_DOMAIN_CHR_TYPE_STDIO: + *backendType = virDomainChrTypeToString(chr->type); + break; + + case VIR_DOMAIN_CHR_TYPE_FILE: { + const char *path = chr->data.file.path; + virTristateSwitch append = chr->data.file.append; + const char *pathfield = "s:out"; + + if (commandline) + pathfield = "s:path"; + + *backendType = "file"; + + if (chrSourcePriv->sourcefd) { + path = qemuFDPassGetPath(chrSourcePriv->sourcefd); + append = VIR_TRISTATE_SWITCH_ON; + } + + if (virJSONValueObjectAdd(props, + pathfield, path, + "T:append", append, + NULL) < 0) + return -1; + } + break; + + case VIR_DOMAIN_CHR_TYPE_PIPE: + case VIR_DOMAIN_CHR_TYPE_DEV: { + const char *pathField = "s:device"; + + if (commandline) + pathField = "s:path"; + + if (chr->type == VIR_DOMAIN_CHR_TYPE_PIPE) { + *backendType = "pipe"; + } else { + if (STRPREFIX(alias, "charparallel")) + *backendType = "parallel"; + else + *backendType = "serial"; + } + + if (virJSONValueObjectAdd(props, + pathField, chr->data.file.path, + NULL) < 0) + return -1; + } + break; + + case VIR_DOMAIN_CHR_TYPE_UNIX: { + virTristateBool waitval = VIR_TRISTATE_BOOL_ABSENT; + virTristateBool server = VIR_TRISTATE_BOOL_ABSENT; + int reconnect = -1; + + *backendType = "socket"; + + if (!commandline) + server = VIR_TRISTATE_BOOL_NO; + + if (chr->data.nix.listen) { + server = VIR_TRISTATE_BOOL_YES; + + if (!chrSourcePriv->wait) + waitval = VIR_TRISTATE_BOOL_NO; + } + + if (chrSourcePriv->directfd) { + if (qemuChardevBackendAddSocketAddressFD(props, "addr", + commandline, + qemuFDPassDirectGetPath(chrSourcePriv->directfd)) < 0) + return -1; + } else { + if (qemuChardevBackendAddSocketAddressUNIX(props, "addr", + commandline, + chr->data.nix.path) < 0) + return -1; + + if (chr->data.nix.reconnect.enabled == VIR_TRISTATE_BOOL_YES) + reconnect = chr->data.nix.reconnect.timeout; + else if (chr->data.nix.reconnect.enabled == VIR_TRISTATE_BOOL_NO) + reconnect = 0; + } + + if (virJSONValueObjectAdd(props, + "T:server", server, + "T:wait", waitval, + "k:reconnect", reconnect, + NULL) < 0) + return -1; + } + break; + + case VIR_DOMAIN_CHR_TYPE_TCP: { + virTristateBool waitval = VIR_TRISTATE_BOOL_ABSENT; + virTristateBool telnet = VIR_TRISTATE_BOOL_ABSENT; + virTristateBool server = VIR_TRISTATE_BOOL_ABSENT; + int reconnect = -1; + + *backendType = "socket"; + + if (!commandline) { + server = VIR_TRISTATE_BOOL_NO; + telnet = VIR_TRISTATE_BOOL_NO; + } + + if (chr->data.tcp.listen) { + server = VIR_TRISTATE_BOOL_YES; + + if (!chrSourcePriv->wait) + waitval = VIR_TRISTATE_BOOL_NO; + } + + if (chr->data.tcp.protocol == VIR_DOMAIN_CHR_TCP_PROTOCOL_TELNET) + telnet = VIR_TRISTATE_BOOL_YES; + + if (chr->data.tcp.reconnect.enabled == VIR_TRISTATE_BOOL_YES) + reconnect = chr->data.tcp.reconnect.timeout; + else if (chr->data.tcp.reconnect.enabled == VIR_TRISTATE_BOOL_NO) + reconnect = 0; + + if (qemuChardevBackendAddSocketAddressInet(props, "addr", + commandline, NULL, + chr->data.tcp.host, + chr->data.tcp.service) < 0) + return -1; + + if (virJSONValueObjectAdd(props, + "T:telnet", telnet, + "T:server", server, + "T:wait", waitval, + "k:reconnect", reconnect, + "S:tls-creds", chrSourcePriv->tlsCredsAlias, + NULL) < 0) + return -1; + } + break; + + case VIR_DOMAIN_CHR_TYPE_UDP: + *backendType = "udp"; + + if (qemuChardevBackendAddSocketAddressInet(props, "remote", + commandline, NULL, + NULLSTR_EMPTY(chr->data.udp.connectHost), + chr->data.udp.connectService) < 0) + return -1; + + if (commandline || chr->data.udp.bindHost || chr->data.udp.bindService) { + const char *bindHost = NULLSTR_EMPTY(chr->data.udp.bindHost); + const char *bindService = chr->data.udp.bindService; + + if (!bindService) + bindService = "0"; + + if (qemuChardevBackendAddSocketAddressInet(props, "local", + commandline, "local", + bindHost, bindService) < 0) + return -1; + } + + break; + + case VIR_DOMAIN_CHR_TYPE_SPICEVMC: { + const char *typeField = "s:type"; + + *backendType = "spicevmc"; + + if (commandline) + typeField = "s:name"; + + if (virJSONValueObjectAdd(props, + typeField, virDomainChrSpicevmcTypeToString(chr->data.spicevmc), + NULL) < 0) + return -1; + } + break; + + case VIR_DOMAIN_CHR_TYPE_QEMU_VDAGENT: { + virTristateBool mouse = VIR_TRISTATE_BOOL_ABSENT; + + *backendType = "qemu-vdagent"; + + switch (chr->data.qemuVdagent.mouse) { + case VIR_DOMAIN_MOUSE_MODE_CLIENT: + mouse = VIR_TRISTATE_BOOL_YES; + break; + case VIR_DOMAIN_MOUSE_MODE_SERVER: + mouse = VIR_TRISTATE_BOOL_NO; + break; + case VIR_DOMAIN_MOUSE_MODE_DEFAULT: + break; + case VIR_DOMAIN_MOUSE_MODE_LAST: + default: + virReportEnumRangeError(virDomainMouseMode, + chr->data.qemuVdagent.mouse); + return -1; + } + + if (commandline) { + if (virJSONValueObjectAdd(props, + "s:name", "vdagent", + NULL) < 0) + return -1; + } + + if (virJSONValueObjectAdd(props, + "T:clipboard", chr->data.qemuVdagent.clipboard, + "T:mouse", mouse, + NULL) < 0) + return -1; + break; + } + + case VIR_DOMAIN_CHR_TYPE_DBUS: + *backendType = "dbus"; + + if (virJSONValueObjectAdd(props, + "s:name", chr->data.dbus.channel, + NULL) < 0) + return -1; + + break; + + case VIR_DOMAIN_CHR_TYPE_SPICEPORT: { + const char *channelField = "s:fqdn"; + + *backendType = "spiceport"; + + if (commandline) + channelField = "s:name"; + + if (virJSONValueObjectAdd(props, + channelField, chr->data.spiceport.channel, + NULL) < 0) + return -1; + } + break; + + + case VIR_DOMAIN_CHR_TYPE_NMDM: + case VIR_DOMAIN_CHR_TYPE_LAST: + default: + virReportEnumRangeError(virDomainChrType, chr->type); + return -1; + } + + if (chr->logfile) { + const char *path = chr->logfile; + virTristateSwitch append = chr->logappend; + + if (chrSourcePriv->logfd) { + path = qemuFDPassGetPath(chrSourcePriv->logfd); + append = VIR_TRISTATE_SWITCH_ON; + } + + if (virJSONValueObjectAdd(props, + "s:logfile", path, + "T:logappend", append, + NULL) < 0) + return -1; + } + + if (!commandline) { + /* The 'chardev-add' QMP command uses two extra layers of wrapping in + * comparison to what the '-chardev' command syntax has */ + g_autoptr(virJSONValue) backend = g_steal_pointer(props); + g_autoptr(virJSONValue) backendWrap = NULL; + + /* the 'data' field of the wrapper below must be present per QMP schema */ + if (!backend) + backend = virJSONValueNewObject(); + + if (virJSONValueObjectAdd(&backendWrap, + "s:type", *backendType, + "a:data", &backend, + NULL) < 0) + return -1; + + /* We now replace the value in the variable we're about to return */ + if (virJSONValueObjectAdd(props, + "s:id", alias, + "a:backend", &backendWrap, + NULL) < 0) + return -1; + } + + return 0; +} + + +int +qemuChardevBuildCommandline(virCommand *cmd, + const virDomainChrSourceDef *dev, + const char *charAlias, + virQEMUCaps *qemuCaps) +{ + g_autoptr(virJSONValue) props = NULL; + g_autofree char *arg = NULL; + /* BEWARE: '-chardev' is not yet accepting JSON syntax. + * QEMU_CAPS_CHARDEV_JSON is asserted just from tests */ + bool useJSON = virQEMUCapsGet(qemuCaps, QEMU_CAPS_CHARDEV_JSON); + const char *backendType = NULL; + + if (qemuChardevGetBackendProps(dev, !useJSON, charAlias, &backendType, &props) < 0) + return -1; + + if (useJSON) { + if (!(arg = virJSONValueToString(props, false))) + return -1; + } else { + g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; + + virBufferAsprintf(&buf, "%s,id=%s", backendType, charAlias); + + if (props) { + virBufferAddLit(&buf, ","); + + if (virQEMUBuildCommandLineJSON(props, &buf, NULL, NULL) < 0) + return -1; + } + + arg = virBufferContentAndReset(&buf); + } + + virCommandAddArgList(cmd, "-chardev", arg, NULL); + return 0; +} diff --git a/src/qemu/qemu_chardev.h b/src/qemu/qemu_chardev.h new file mode 100644 index 0000000000..0381e0df65 --- /dev/null +++ b/src/qemu/qemu_chardev.h @@ -0,0 +1,22 @@ +/* + * SPDX-License-Identifier: LGPL-2.1-or-later + */ + +#pragma once + +#include "domain_conf.h" +#include "qemu_capabilities.h" +#include "vircommand.h" + +int +qemuChardevBuildCommandline(virCommand *cmd, + const virDomainChrSourceDef *dev, + const char *charAlias, + virQEMUCaps *qemuCaps); + +int +qemuChardevGetBackendProps(const virDomainChrSourceDef *chr, + bool commandline, + const char *alias, + const char **backendType, + virJSONValue **props); diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 1b992d8eed..c2d65645c4 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -31,6 +31,7 @@ #include "qemu_slirp.h" #include "qemu_block.h" #include "qemu_fd.h" +#include "qemu_chardev.h" #include "viralloc.h" #include "virlog.h" #include "virarch.h" @@ -1292,203 +1293,6 @@ qemuBuildTLSx509CommandLine(virCommand *cmd, } -static void -qemuBuildChrChardevReconnectStr(virBuffer *buf, - const virDomainChrSourceReconnectDef *def) -{ - if (def->enabled == VIR_TRISTATE_BOOL_YES) { - virBufferAsprintf(buf, ",reconnect=%u", def->timeout); - } else if (def->enabled == VIR_TRISTATE_BOOL_NO) { - virBufferAddLit(buf, ",reconnect=0"); - } -} - - -static char * -qemuBuildChardevStr(const virDomainChrSourceDef *dev, - const char *charAlias) -{ - - qemuDomainChrSourcePrivate *chrSourcePriv = QEMU_DOMAIN_CHR_SOURCE_PRIVATE(dev); - g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; - const char *path; - virTristateSwitch append; - - switch ((virDomainChrType) dev->type) { - case VIR_DOMAIN_CHR_TYPE_NULL: - virBufferAsprintf(&buf, "null,id=%s", charAlias); - break; - - case VIR_DOMAIN_CHR_TYPE_VC: - virBufferAsprintf(&buf, "vc,id=%s", charAlias); - break; - - case VIR_DOMAIN_CHR_TYPE_PTY: - virBufferAsprintf(&buf, "pty,id=%s", charAlias); - break; - - case VIR_DOMAIN_CHR_TYPE_DEV: { - const char *backend = "serial"; - - if (STRPREFIX(charAlias, "charparallel")) - backend = "parallel"; - - virBufferAsprintf(&buf, "%s,id=%s,path=", backend, charAlias); - virQEMUBuildBufferEscapeComma(&buf, dev->data.file.path); - break; - } - - case VIR_DOMAIN_CHR_TYPE_FILE: - virBufferAsprintf(&buf, "file,id=%s", charAlias); - path = dev->data.file.path; - append = dev->data.file.append; - - if (chrSourcePriv->sourcefd) { - path = qemuFDPassGetPath(chrSourcePriv->sourcefd); - append = VIR_TRISTATE_SWITCH_ON; - } - - virBufferAddLit(&buf, ",path="); - virQEMUBuildBufferEscapeComma(&buf, path); - if (append != VIR_TRISTATE_SWITCH_ABSENT) { - virBufferAsprintf(&buf, ",append=%s", - virTristateSwitchTypeToString(append)); - } - break; - - case VIR_DOMAIN_CHR_TYPE_PIPE: - virBufferAsprintf(&buf, "pipe,id=%s,path=", charAlias); - virQEMUBuildBufferEscapeComma(&buf, dev->data.file.path); - break; - - case VIR_DOMAIN_CHR_TYPE_STDIO: - virBufferAsprintf(&buf, "stdio,id=%s", charAlias); - break; - - case VIR_DOMAIN_CHR_TYPE_UDP: { - const char *connectHost = dev->data.udp.connectHost; - const char *bindHost = dev->data.udp.bindHost; - const char *bindService = dev->data.udp.bindService; - - if (connectHost == NULL) - connectHost = ""; - if (bindHost == NULL) - bindHost = ""; - if (bindService == NULL) - bindService = "0"; - - virBufferAsprintf(&buf, - "udp,id=%s,host=%s,port=%s,localaddr=%s," - "localport=%s", - charAlias, - connectHost, - dev->data.udp.connectService, - bindHost, bindService); - break; - } - - case VIR_DOMAIN_CHR_TYPE_TCP: - virBufferAsprintf(&buf, - "socket,id=%s,host=%s,port=%s", - charAlias, - dev->data.tcp.host, - dev->data.tcp.service); - - if (dev->data.tcp.protocol == VIR_DOMAIN_CHR_TCP_PROTOCOL_TELNET) - virBufferAddLit(&buf, ",telnet=on"); - - if (dev->data.tcp.listen) { - virBufferAddLit(&buf, ",server=on"); - if (!chrSourcePriv->wait) - virBufferAddLit(&buf, ",wait=off"); - } - - qemuBuildChrChardevReconnectStr(&buf, &dev->data.tcp.reconnect); - - if (chrSourcePriv->tlsCredsAlias) - virBufferAsprintf(&buf, ",tls-creds=%s", chrSourcePriv->tlsCredsAlias); - break; - - case VIR_DOMAIN_CHR_TYPE_UNIX: - virBufferAsprintf(&buf, "socket,id=%s", charAlias); - if (chrSourcePriv->directfd) { - virBufferAsprintf(&buf, ",fd=%s", qemuFDPassDirectGetPath(chrSourcePriv->directfd)); - } else { - virBufferAddLit(&buf, ",path="); - virQEMUBuildBufferEscapeComma(&buf, dev->data.nix.path); - } - - if (dev->data.nix.listen) { - virBufferAddLit(&buf, ",server=on"); - if (!chrSourcePriv->wait) - virBufferAddLit(&buf, ",wait=off"); - } - - qemuBuildChrChardevReconnectStr(&buf, &dev->data.nix.reconnect); - break; - - case VIR_DOMAIN_CHR_TYPE_SPICEVMC: - virBufferAsprintf(&buf, "spicevmc,id=%s,name=%s", charAlias, - virDomainChrSpicevmcTypeToString(dev->data.spicevmc)); - break; - - case VIR_DOMAIN_CHR_TYPE_SPICEPORT: - virBufferAsprintf(&buf, "spiceport,id=%s,name=%s", charAlias, - dev->data.spiceport.channel); - break; - - case VIR_DOMAIN_CHR_TYPE_QEMU_VDAGENT: - virBufferAsprintf(&buf, "qemu-vdagent,id=%s,name=vdagent", - charAlias); - if (dev->data.qemuVdagent.clipboard != VIR_TRISTATE_BOOL_ABSENT) - virBufferAsprintf(&buf, ",clipboard=%s", - virTristateSwitchTypeToString(dev->data.qemuVdagent.clipboard)); - switch (dev->data.qemuVdagent.mouse) { - case VIR_DOMAIN_MOUSE_MODE_CLIENT: - virBufferAddLit(&buf, ",mouse=on"); - break; - case VIR_DOMAIN_MOUSE_MODE_SERVER: - virBufferAddLit(&buf, ",mouse=off"); - break; - case VIR_DOMAIN_MOUSE_MODE_DEFAULT: - case VIR_DOMAIN_MOUSE_MODE_LAST: - default: - break; - } - break; - - case VIR_DOMAIN_CHR_TYPE_DBUS: - virBufferAsprintf(&buf, "dbus,id=%s,name=%s", charAlias, - dev->data.dbus.channel); - break; - - case VIR_DOMAIN_CHR_TYPE_NMDM: - case VIR_DOMAIN_CHR_TYPE_LAST: - default: - break; - } - - if (dev->logfile) { - path = dev->logfile; - append = dev->logappend; - - if (chrSourcePriv->logfd) { - path = qemuFDPassGetPath(chrSourcePriv->logfd); - append = VIR_TRISTATE_SWITCH_ON; - } - - virBufferAddLit(&buf, ",logfile="); - virQEMUBuildBufferEscapeComma(&buf, path); - if (append != VIR_TRISTATE_SWITCH_ABSENT) { - virBufferAsprintf(&buf, ",logappend=%s", - virTristateSwitchTypeToString(append)); - } - } - - return virBufferContentAndReset(&buf); -} - - static int qemuBuildChardevCommand(virCommand *cmd, const virDomainChrSourceDef *dev, @@ -1564,11 +1368,9 @@ qemuBuildChardevCommand(virCommand *cmd, qemuFDPassTransferCommand(chrSourcePriv->logfd, cmd); - if (!(charstr = qemuBuildChardevStr(dev, charAlias))) + if (qemuChardevBuildCommandline(cmd, dev, charAlias, qemuCaps) < 0) return -1; - virCommandAddArgList(cmd, "-chardev", charstr, NULL); - qemuDomainChrSourcePrivateClearFDPass(chrSourcePriv); return 0; -- 2.46.0
On a Wednesday in 2024, Peter Krempa wrote:
Similarly to how we approach the generators for -device/-object/-blockdev/-netdev rewrite the generator of -chardev to be unified with the generator for the monitor.
Unfortunately with -chardev it will be a bit more quirky when compared to the others as the generator itself will need to know whether it generates command line output or not as few field names change and data
s/few/a few/
is nested differently.
This first step adds the generator and uses it only for command line generation. This was possible to achieve without changing any of the output in tests.
In further patches the same generator will then be used also in the monitor code replacing the both.
s/the//
As basis for the generator I took the monitor code but modified it to have the same field order as the commandline code and extended it further to support all backend types, even those which are not hotpluggable.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/meson.build | 1 + src/qemu/qemu_chardev.c | 488 ++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_chardev.h | 22 ++ src/qemu/qemu_command.c | 202 +---------------- 4 files changed, 513 insertions(+), 200 deletions(-) create mode 100644 src/qemu/qemu_chardev.c create mode 100644 src/qemu/qemu_chardev.h
diff --git a/src/qemu/meson.build b/src/qemu/meson.build index 57356451e4..1d904bbc68 100644 --- a/src/qemu/meson.build +++ b/src/qemu/meson.build @@ -6,6 +6,7 @@ qemu_driver_sources = [ 'qemu_blockjob.c', 'qemu_capabilities.c', 'qemu_cgroup.c', + 'qemu_chardev.c', 'qemu_checkpoint.c', 'qemu_command.c', 'qemu_conf.c', diff --git a/src/qemu/qemu_chardev.c b/src/qemu/qemu_chardev.c new file mode 100644 index 0000000000..cc6ba33f51 --- /dev/null +++ b/src/qemu/qemu_chardev.c @@ -0,0 +1,488 @@ +/* + * SPDX-License-Identifier: LGPL-2.1-or-later + */ + +#include <config.h> + +#include "qemu_capabilities.h" +#include "qemu_domain.h" +#include "qemu_fd.h" + +#include "vircommand.h" +#include "virlog.h" +#include "virqemu.h" + +#include "domain_conf.h" + +#include "qemu_chardev.h" + +#define VIR_FROM_THIS VIR_FROM_QEMU + +VIR_LOG_INIT("qemu.qemu_command"); + +static int +qemuChardevBackendAddSocketAddressInet(virJSONValue **backendData, + const char *backendFieldName, + bool commandline, + const char *commandlinePrefix, + const char *host, + const char *port) +{ + if (commandline) { + g_autofree char *hostField = NULL; + g_autofree char *portField = NULL; + + if (!commandlinePrefix) { + hostField = g_strdup("s:host"); + portField = g_strdup("s:port"); + } else { + hostField = g_strdup_printf("s:%saddr", commandlinePrefix); + portField = g_strdup_printf("s:%sport", commandlinePrefix); + } + + if (virJSONValueObjectAdd(backendData, + hostField, host, + portField, port, + NULL) < 0) + return -1; + } else { + g_autoptr(virJSONValue) addr = NULL; + g_autoptr(virJSONValue) data = NULL; + g_autofree char *datafiled = g_strdup_printf("a:%s", backendFieldName);
s/filed/field/
+ + if (virJSONValueObjectAdd(&data, + "s:host", host, + "s:port", port, + NULL) < 0) + return -1; + + if (virJSONValueObjectAdd(&addr, + "s:type", "inet", + "a:data", &data, + NULL) < 0) + return -1; + + if (virJSONValueObjectAdd(backendData, + datafiled, &addr, + NULL) < 0) + return -1; + } + + return 0; +} + + +static int +qemuChardevBackendAddSocketAddressFD(virJSONValue **backendData, + const char *backendFieldName, + bool commandline, + const char *fdname) +{ + if (commandline) { + if (virJSONValueObjectAdd(backendData, + "s:fd", fdname, + NULL) < 0) + return -1; + } else { + g_autoptr(virJSONValue) addr = NULL; + g_autoptr(virJSONValue) data = NULL; + g_autofree char *datafiled = g_strdup_printf("a:%s", backendFieldName);
s/filed/field/
+ + if (virJSONValueObjectAdd(&data, "s:str", fdname, NULL) < 0) + return -1; + + if (virJSONValueObjectAdd(&addr, + "s:type", "fd", + "a:data", &data, NULL) < 0) + return -1; + + if (virJSONValueObjectAdd(backendData, + datafiled, &addr, + NULL) < 0) + return -1; + } + + return 0; +} +
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
While qemu doesn't yet support JSON args for chardev, we can at least for test purposes of schema validation plumb it to the '-chardev' command as it's easier to create test cases via XML than to write them into code in 'qemuhotplugtest'. Additionally once this becomes available and if e.g. the syntax is fixed we'll be able to also catch the differences early. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/qemuxmlconftest.c | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/qemuxmlconftest.c b/tests/qemuxmlconftest.c index 6fc8d5eabd..914326bc3c 100644 --- a/tests/qemuxmlconftest.c +++ b/tests/qemuxmlconftest.c @@ -504,6 +504,7 @@ static const struct testValidateSchemaCommandData commands[] = { { "-netdev", "netdev_add", false }, { "-object", "object-add", false }, { "-device", "device_add", true }, + { "-chardev", "chardev-add", false }, }; static int -- 2.46.0
On a Wednesday in 2024, Peter Krempa wrote:
While qemu doesn't yet support JSON args for chardev, we can at least for test purposes of schema validation plumb it to the '-chardev' command as it's easier to create test cases via XML than to write them into code in 'qemuhotplugtest'.
Additionally once this becomes available and if e.g. the syntax is fixed we'll be able to also catch the differences early.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/qemuxmlconftest.c | 1 + 1 file changed, 1 insertion(+)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Use the 'chardev-backends' test data as symlink to invoke the test case again asserting QEMU_CAPS_CHARDEV_JSON which will make the commandline generator use the JSON representation of the -chardev backend instead allowing us to validate it agains the QMP schema. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- .../chardev-backends-json.x86_64-latest.args | 79 +++++++++++++++++++ .../chardev-backends-json.x86_64-latest.xml | 1 + .../qemuxmlconfdata/chardev-backends-json.xml | 1 + tests/qemuxmlconftest.c | 4 + 4 files changed, 85 insertions(+) create mode 100644 tests/qemuxmlconfdata/chardev-backends-json.x86_64-latest.args create mode 120000 tests/qemuxmlconfdata/chardev-backends-json.x86_64-latest.xml create mode 120000 tests/qemuxmlconfdata/chardev-backends-json.xml diff --git a/tests/qemuxmlconfdata/chardev-backends-json.x86_64-latest.args b/tests/qemuxmlconfdata/chardev-backends-json.x86_64-latest.args new file mode 100644 index 0000000000..98929ab39f --- /dev/null +++ b/tests/qemuxmlconfdata/chardev-backends-json.x86_64-latest.args @@ -0,0 +1,79 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1 \ +USER=test \ +LOGNAME=test \ +XDG_DATA_HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1/.local/share \ +XDG_CACHE_HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1/.cache \ +XDG_CONFIG_HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1/.config \ +/usr/bin/qemu-system-x86_64 \ +-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 \ +-accel tcg \ +-cpu qemu64 \ +-m size=219136k \ +-object '{"qom-type":"memory-backend-ram","id":"pc.ram","size":224395264}' \ +-overcommit mem-lock=off \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-no-user-config \ +-nodefaults \ +-chardev '{"id":"charmonitor","backend":{"type":"socket","data":{"addr":{"type":"fd","data":{"str":"1729"}},"server":true,"wait":false}}}' \ +-mon chardev=charmonitor,id=monitor,mode=control \ +-rtc base=utc \ +-no-shutdown \ +-boot strict=on \ +-device '{"driver":"piix3-usb-uhci","id":"usb","bus":"pci.0","addr":"0x1.0x2"}' \ +-device '{"driver":"virtio-serial-pci","id":"virtio-serial0","bus":"pci.0","addr":"0x3"}' \ +-chardev '{"id":"charparallel0","backend":{"type":"parallel","data":{"device":"/dev/parport0"}}}' \ +-device '{"driver":"isa-parallel","chardev":"charparallel0","id":"parallel0"}' \ +-add-fd set=0,fd=1751,opaque=channel0-log \ +-chardev '{"id":"charchannel0","backend":{"type":"null","data":{"logfile":"/dev/fdset/0","logappend":true}}}' \ +-device '{"driver":"virtserialport","bus":"virtio-serial0.0","nr":1,"chardev":"charchannel0","id":"channel0","name":"chardev-null"}' \ +-add-fd set=1,fd=1751,opaque=channel1-log \ +-chardev '{"id":"charchannel1","backend":{"type":"vc","data":{"logfile":"/dev/fdset/1","logappend":true}}}' \ +-device '{"driver":"virtserialport","bus":"virtio-serial0.0","nr":2,"chardev":"charchannel1","id":"channel1","name":"chardev-vc"}' \ +-chardev '{"id":"charchannel2","backend":{"type":"pty","data":{}}}' \ +-device '{"driver":"virtserialport","bus":"virtio-serial0.0","nr":3,"chardev":"charchannel2","id":"channel2","name":"chardev-pty"}' \ +-chardev '{"id":"charchannel3","backend":{"type":"stdio","data":{}}}' \ +-device '{"driver":"virtserialport","bus":"virtio-serial0.0","nr":4,"chardev":"charchannel3","id":"channel3","name":"chardev-stdio"}' \ +-add-fd set=2,fd=1750,opaque=channel4-source \ +-chardev '{"id":"charchannel4","backend":{"type":"file","data":{"out":"/dev/fdset/2","append":true}}}' \ +-device '{"driver":"virtserialport","bus":"virtio-serial0.0","nr":5,"chardev":"charchannel4","id":"channel4","name":"chardev-file"}' \ +-chardev '{"id":"charchannel5","backend":{"type":"pipe","data":{"device":"/path/to/pipe"}}}' \ +-device '{"driver":"virtserialport","bus":"virtio-serial0.0","nr":6,"chardev":"charchannel5","id":"channel5","name":"chardev-pipe"}' \ +-chardev '{"id":"charchannel6","backend":{"type":"serial","data":{"device":"/path/to/device"}}}' \ +-device '{"driver":"virtserialport","bus":"virtio-serial0.0","nr":7,"chardev":"charchannel6","id":"channel6","name":"chardev-dev"}' \ +-chardev '{"id":"charchannel7","backend":{"type":"socket","data":{"addr":{"type":"fd","data":{"str":"1729"}},"server":true,"wait":false}}}' \ +-device '{"driver":"virtserialport","bus":"virtio-serial0.0","nr":8,"chardev":"charchannel7","id":"channel7","name":"chardev-unix-listen"}' \ +-chardev '{"id":"charchannel8","backend":{"type":"socket","data":{"addr":{"type":"unix","data":{"path":"/path/to/unix-listen"}},"server":false,"reconnect":2}}}' \ +-device '{"driver":"virtserialport","bus":"virtio-serial0.0","nr":9,"chardev":"charchannel8","id":"channel8","name":"chardev-unix-connect"}' \ +-chardev '{"id":"charchannel9","backend":{"type":"socket","data":{"addr":{"type":"inet","data":{"host":"1.2.3.4","port":"5678"}},"telnet":false,"server":true,"wait":false}}}' \ +-device '{"driver":"virtserialport","bus":"virtio-serial0.0","nr":10,"chardev":"charchannel9","id":"channel9","name":"chardev-tcp-listen-raw"}' \ +-chardev '{"id":"charchannel10","backend":{"type":"socket","data":{"addr":{"type":"inet","data":{"host":"1.2.3.4","port":"5679"}},"telnet":true,"server":true,"wait":false}}}' \ +-device '{"driver":"virtserialport","bus":"virtio-serial0.0","nr":11,"chardev":"charchannel10","id":"channel10","name":"chardev-tcp-listen-telnet"}' \ +-object '{"qom-type":"tls-creds-x509","id":"objcharchannel11_tls0","dir":"/etc/pki/libvirt-chardev","endpoint":"client","verify-peer":true}' \ +-chardev '{"id":"charchannel11","backend":{"type":"socket","data":{"addr":{"type":"inet","data":{"host":"1.2.3.4","port":"5678"}},"telnet":false,"server":false,"reconnect":2,"tls-creds":"objcharchannel11_tls0"}}}' \ +-device '{"driver":"virtserialport","bus":"virtio-serial0.0","nr":12,"chardev":"charchannel11","id":"channel11","name":"chardev-tcp-connect-raw"}' \ +-object '{"qom-type":"tls-creds-x509","id":"objcharchannel12_tls0","dir":"/etc/pki/libvirt-chardev","endpoint":"client","verify-peer":true}' \ +-chardev '{"id":"charchannel12","backend":{"type":"socket","data":{"addr":{"type":"inet","data":{"host":"hostname.global.","port":"5679"}},"telnet":true,"server":false,"reconnect":2,"tls-creds":"objcharchannel12_tls0"}}}' \ +-device '{"driver":"virtserialport","bus":"virtio-serial0.0","nr":13,"chardev":"charchannel12","id":"channel12","name":"chardev-tcp-connect-telnet"}' \ +-chardev '{"id":"charchannel13","backend":{"type":"udp","data":{"remote":{"type":"inet","data":{"host":"127.0.0.1","port":"2222"}}}}}' \ +-device '{"driver":"virtserialport","bus":"virtio-serial0.0","nr":14,"chardev":"charchannel13","id":"channel13","name":"chardev-udp-nobind"}' \ +-chardev '{"id":"charchannel14","backend":{"type":"udp","data":{"remote":{"type":"inet","data":{"host":"127.0.0.1","port":"2222"}},"local":{"type":"inet","data":{"host":"127.0.0.1","port":"1111"}}}}}' \ +-device '{"driver":"virtserialport","bus":"virtio-serial0.0","nr":15,"chardev":"charchannel14","id":"channel14","name":"chardev-udp-bind"}' \ +-chardev '{"id":"charchannel15","backend":{"type":"spicevmc","data":{"type":"vdagent"}}}' \ +-device '{"driver":"virtserialport","bus":"virtio-serial0.0","nr":16,"chardev":"charchannel15","id":"channel15","name":"com.redhat.spice.0"}' \ +-chardev '{"id":"charchannel16","backend":{"type":"qemu-vdagent","data":{"clipboard":false,"mouse":false}}}' \ +-device '{"driver":"virtserialport","bus":"virtio-serial0.0","nr":17,"chardev":"charchannel16","id":"channel16","name":"chardev-vdagent"}' \ +-chardev '{"id":"charchannel17","backend":{"type":"dbus","data":{"name":"test.channel.0"}}}' \ +-device '{"driver":"virtserialport","bus":"virtio-serial0.0","nr":18,"chardev":"charchannel17","id":"channel17","name":"chardev-dbus"}' \ +-chardev '{"id":"charchannel18","backend":{"type":"spiceport","data":{"fqdn":"test.channel.0"}}}' \ +-device '{"driver":"virtserialport","bus":"virtio-serial0.0","nr":19,"chardev":"charchannel18","id":"channel18","name":"chardev-spiceport"}' \ +-audiodev '{"id":"audio1","driver":"spice"}' \ +-spice port=5901,addr=0.0.0.0,seamless-migration=on \ +-device '{"driver":"cirrus-vga","id":"video0","bus":"pci.0","addr":"0x2"}' \ +-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \ +-msg timestamp=on diff --git a/tests/qemuxmlconfdata/chardev-backends-json.x86_64-latest.xml b/tests/qemuxmlconfdata/chardev-backends-json.x86_64-latest.xml new file mode 120000 index 0000000000..39fa243d0a --- /dev/null +++ b/tests/qemuxmlconfdata/chardev-backends-json.x86_64-latest.xml @@ -0,0 +1 @@ +chardev-backends.x86_64-latest.xml \ No newline at end of file diff --git a/tests/qemuxmlconfdata/chardev-backends-json.xml b/tests/qemuxmlconfdata/chardev-backends-json.xml new file mode 120000 index 0000000000..b6ebea2ae3 --- /dev/null +++ b/tests/qemuxmlconfdata/chardev-backends-json.xml @@ -0,0 +1 @@ +chardev-backends.xml \ No newline at end of file diff --git a/tests/qemuxmlconftest.c b/tests/qemuxmlconftest.c index 914326bc3c..08dc8a10e3 100644 --- a/tests/qemuxmlconftest.c +++ b/tests/qemuxmlconftest.c @@ -1832,6 +1832,10 @@ mymain(void) DO_TEST_CAPS_LATEST("console-compat-crash"); DO_TEST_CAPS_LATEST("chardev-backends"); + /* As qemu doesn't yet support JSON syntax for -chardev we use the + * QEMU_CAPS_CHARDEV_JSON capability just in the tests to have QMP schema + * validation also for the QMP mode of the -chardev props generator */ + DO_TEST_CAPS_ARCH_LATEST_FULL("chardev-backends-json", "x86_64", ARG_QEMU_CAPS, QEMU_CAPS_CHARDEV_JSON, QEMU_CAPS_LAST); DO_TEST_CAPS_LATEST("serial-vc-chardev"); DO_TEST_CAPS_LATEST("serial-pty-chardev"); -- 2.46.0
On a Wednesday in 2024, Peter Krempa wrote:
Use the 'chardev-backends' test data as symlink to invoke the test case again asserting QEMU_CAPS_CHARDEV_JSON which will make the commandline generator use the JSON representation of the -chardev backend instead allowing us to validate it agains the QMP schema.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- .../chardev-backends-json.x86_64-latest.args | 79 +++++++++++++++++++ .../chardev-backends-json.x86_64-latest.xml | 1 + .../qemuxmlconfdata/chardev-backends-json.xml | 1 + tests/qemuxmlconftest.c | 4 + 4 files changed, 85 insertions(+) create mode 100644 tests/qemuxmlconfdata/chardev-backends-json.x86_64-latest.args create mode 120000 tests/qemuxmlconfdata/chardev-backends-json.x86_64-latest.xml create mode 120000 tests/qemuxmlconfdata/chardev-backends-json.xml
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
The upcoming refactor of the monitor code will make the hotplug code paths use the same generator we have for commandline -chardev backends which doesn't refuse to format certain backends which can't be hotplugged. To prepare for this we add a check to qemuHotplugChardevAttach() refusing such hotplug and remove 'qemumonitorjsontest' test cases which will not make sense any more. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_hotplug.c | 29 +++++++++++++++++++++++++++++ tests/qemumonitorjsontest.c | 6 ------ 2 files changed, 29 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 75b97cf736..a58c1446e9 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -243,6 +243,35 @@ qemuHotplugChardevAttach(qemuMonitor *mon, const char *alias, virDomainChrSourceDef *def) { + switch ((virDomainChrType) def->type) { + case VIR_DOMAIN_CHR_TYPE_NULL: + case VIR_DOMAIN_CHR_TYPE_VC: + case VIR_DOMAIN_CHR_TYPE_PTY: + case VIR_DOMAIN_CHR_TYPE_FILE: + case VIR_DOMAIN_CHR_TYPE_DEV: + case VIR_DOMAIN_CHR_TYPE_UNIX: + case VIR_DOMAIN_CHR_TYPE_TCP: + case VIR_DOMAIN_CHR_TYPE_UDP: + case VIR_DOMAIN_CHR_TYPE_SPICEVMC: + case VIR_DOMAIN_CHR_TYPE_QEMU_VDAGENT: + case VIR_DOMAIN_CHR_TYPE_DBUS: + break; + + case VIR_DOMAIN_CHR_TYPE_SPICEPORT: + case VIR_DOMAIN_CHR_TYPE_PIPE: + case VIR_DOMAIN_CHR_TYPE_STDIO: + case VIR_DOMAIN_CHR_TYPE_NMDM: + virReportError(VIR_ERR_OPERATION_FAILED, + _("Hotplug unsupported for char device type '%1$s'"), + virDomainChrTypeToString(def->type)); + return -1; + + case VIR_DOMAIN_CHR_TYPE_LAST: + default: + virReportEnumRangeError(virDomainChrType, def->type); + return -1; + } + return qemuMonitorAttachCharDev(mon, alias, def); } diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c index 66d0c127ca..2249787ba7 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c @@ -652,12 +652,6 @@ qemuMonitorJSONTestAttachChardev(virDomainXMLOption *xmlopt, "{'id':'alias','backend':{'type':'spicevmc'," "'data':{'type':'vdagent'}}}"); - chr->type = VIR_DOMAIN_CHR_TYPE_PIPE; - CHECK("pipe", true, NULL); - - chr->type = VIR_DOMAIN_CHR_TYPE_STDIO; - CHECK("stdio", true, NULL); - chr->type = VIR_DOMAIN_CHR_TYPE_PTY; CHECK("pty missing path", true, "{'id':'alias','backend':{'type':'pty','data':{}}}"); -- 2.46.0
On a Wednesday in 2024, Peter Krempa wrote:
The upcoming refactor of the monitor code will make the hotplug code paths use the same generator we have for commandline -chardev backends which doesn't refuse to format certain backends which can't be hotplugged.
To prepare for this we add a check to qemuHotplugChardevAttach() refusing such hotplug and remove 'qemumonitorjsontest' test cases which will not make sense any more.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_hotplug.c | 29 +++++++++++++++++++++++++++++ tests/qemumonitorjsontest.c | 6 ------ 2 files changed, 29 insertions(+), 6 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Now that we have a unified generator of chardev backend which is also validated against the QMP schema we can replace the old generator with it. This patch modifies the monitor code to take virJSONValue 'props' instead of the chardev definition and adds the conversion from the chardev definition to JSON on higher levels. The monitor code now also attempts to extract the returned 'pty' if returned from qemu, so higher level code needs to report the error if the path is needed and missing. The current monitor generator is for now abandoned in place and will be removed later. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_block.c | 9 ++++++++- src/qemu/qemu_hotplug.c | 22 +++++++++++++++++++++- src/qemu/qemu_monitor.c | 8 +++----- src/qemu/qemu_monitor.h | 4 ++-- src/qemu/qemu_monitor_json.c | 32 ++++++++++---------------------- src/qemu/qemu_monitor_json.h | 4 ++-- tests/qemumonitorjsontest.c | 17 +++++++++++------ 7 files changed, 57 insertions(+), 39 deletions(-) diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index 6e90bae9f2..9bdec92697 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -24,6 +24,7 @@ #include "qemu_alias.h" #include "qemu_security.h" #include "qemu_process.h" +#include "qemu_chardev.h" #include "storage_source.h" #include "viralloc.h" @@ -1682,7 +1683,13 @@ qemuBlockStorageSourceAttachApply(qemuMonitor *mon, return -1; if (data->chardevDef) { - if (qemuMonitorAttachCharDev(mon, data->chardevAlias, data->chardevDef) < 0) + g_autoptr(virJSONValue) props = NULL; + + if (qemuChardevGetBackendProps(data->chardevDef, false, + data->chardevAlias, NULL, &props) < 0) + return -1; + + if (qemuMonitorAttachCharDev(mon, &props, NULL) < 0) return -1; data->chardevAdded = true; diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index a58c1446e9..c1070d2562 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -37,6 +37,7 @@ #include "qemu_block.h" #include "qemu_snapshot.h" #include "qemu_virtiofs.h" +#include "qemu_chardev.h" #include "domain_audit.h" #include "domain_cgroup.h" #include "domain_interface.h" @@ -243,6 +244,9 @@ qemuHotplugChardevAttach(qemuMonitor *mon, const char *alias, virDomainChrSourceDef *def) { + g_autoptr(virJSONValue) props = NULL; + g_autofree char *ptypath = NULL; + switch ((virDomainChrType) def->type) { case VIR_DOMAIN_CHR_TYPE_NULL: case VIR_DOMAIN_CHR_TYPE_VC: @@ -272,7 +276,23 @@ qemuHotplugChardevAttach(qemuMonitor *mon, return -1; } - return qemuMonitorAttachCharDev(mon, alias, def); + if (qemuChardevGetBackendProps(def, false, alias, NULL, &props) < 0) + return -1; + + if (qemuMonitorAttachCharDev(mon, &props, &ptypath) < 0) + return -1; + + if (def->type == VIR_DOMAIN_CHR_TYPE_PTY) { + if (!ptypath) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("chardev-add reply was missing pty path")); + return -1; + } + + def->data.file.path = g_steal_pointer(&ptypath); + } + + return 0; } diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 7f65c23748..ada3de474f 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -3523,14 +3523,12 @@ qemuMonitorGetTPMTypes(qemuMonitor *mon, int qemuMonitorAttachCharDev(qemuMonitor *mon, - const char *chrID, - virDomainChrSourceDef *chr) + virJSONValue **props, + char **ptypath) { - VIR_DEBUG("chrID=%s chr=%p", chrID, chr); - QEMU_CHECK_MONITOR(mon); - return qemuMonitorJSONAttachCharDev(mon, chrID, chr); + return qemuMonitorJSONAttachCharDev(mon, props, ptypath); } diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 57d1b45bf5..2bb64dd53f 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -1212,8 +1212,8 @@ int qemuMonitorGetTPMTypes(qemuMonitor *mon, char ***tpmtypes); int qemuMonitorAttachCharDev(qemuMonitor *mon, - const char *chrID, - virDomainChrSourceDef *chr); + virJSONValue **props, + char **ptypath); int qemuMonitorDetachCharDev(qemuMonitor *mon, const char *chrID); diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 2db38c1007..f3a353a13b 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -6342,7 +6342,7 @@ int qemuMonitorJSONGetTPMTypes(qemuMonitor *mon, } -static virJSONValue * +static G_GNUC_UNUSED virJSONValue * qemuMonitorJSONAttachCharDevGetProps(const char *chrID, const virDomainChrSourceDef *chr) { @@ -6571,41 +6571,29 @@ qemuMonitorJSONAttachCharDevGetProps(const char *chrID, int qemuMonitorJSONAttachCharDev(qemuMonitor *mon, - const char *chrID, - virDomainChrSourceDef *chr) + virJSONValue **props, + char **ptypath) { g_autoptr(virJSONValue) cmd = NULL; g_autoptr(virJSONValue) reply = NULL; - g_autoptr(virJSONValue) props = NULL; - - if (!(props = qemuMonitorJSONAttachCharDevGetProps(chrID, chr))) - return -1; + virJSONValue *data; - if (!(cmd = qemuMonitorJSONMakeCommandInternal("chardev-add", &props))) + if (!(cmd = qemuMonitorJSONMakeCommandInternal("chardev-add", props))) return -1; if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) return -1; - if (chr->type == VIR_DOMAIN_CHR_TYPE_PTY) { - virJSONValue *data; - - if (!(data = qemuMonitorJSONGetReply(cmd, reply, VIR_JSON_TYPE_OBJECT))) - return -1; + if (!(data = qemuMonitorJSONGetReply(cmd, reply, VIR_JSON_TYPE_OBJECT))) + return -1; - if (!(chr->data.file.path = g_strdup(virJSONValueObjectGetString(data, "pty")))) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("chardev-add reply was missing pty path")); - return -1; - } - } else { - if (qemuMonitorJSONCheckError(cmd, reply) < 0) - return -1; - } + if (ptypath) + *ptypath = g_strdup(virJSONValueObjectGetString(data, "pty")); return 0; } + int qemuMonitorJSONDetachCharDev(qemuMonitor *mon, const char *chrID) diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index 921dd34ed2..fef81fd911 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -584,8 +584,8 @@ qemuMonitorJSONGetTPMTypes(qemuMonitor *mon, int qemuMonitorJSONAttachCharDev(qemuMonitor *mon, - const char *chrID, - virDomainChrSourceDef *chr); + virJSONValue **props, + char **ptypath); int qemuMonitorJSONDetachCharDev(qemuMonitor *mon, const char *chrID); diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c index 2249787ba7..ab2c445289 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c @@ -28,6 +28,7 @@ #include "qemu/qemu_monitor_json.h" #include "qemu/qemu_qapi.h" #include "qemu/qemu_alias.h" +#include "qemu/qemu_chardev.h" #include "virerror.h" #include "cpu/cpu.h" #include "qemu/qemu_monitor.h" @@ -553,6 +554,8 @@ testQemuMonitorJSONAttachChardev(const void *opaque) { const struct qemuMonitorJSONTestAttachChardevData *data = opaque; g_autoptr(qemuMonitorTest) test = qemuMonitorTestNewSchema(data->xmlopt, data->schema); + g_autoptr(virJSONValue) props = NULL; + g_autofree char *ptypath = NULL; int rc; if (!test) @@ -575,20 +578,20 @@ testQemuMonitorJSONAttachChardev(const void *opaque) return -1; } - if ((rc = qemuMonitorAttachCharDev(qemuMonitorTestGetMonitor(test), - "alias", data->chr)) < 0) + if (qemuChardevGetBackendProps(data->chr, false, "alias", NULL, &props) < 0) + return -1; + + if ((rc = qemuMonitorAttachCharDev(qemuMonitorTestGetMonitor(test), &props, &ptypath)) < 0) goto cleanup; if (data->chr->type == VIR_DOMAIN_CHR_TYPE_PTY) { - if (STRNEQ_NULLABLE(data->expectPty, data->chr->data.file.path)) { + if (STRNEQ_NULLABLE(data->expectPty, ptypath)) { virReportError(VIR_ERR_INTERNAL_ERROR, "expected PTY path: %s got: %s", NULLSTR(data->expectPty), NULLSTR(data->chr->data.file.path)); rc = -1; } - - VIR_FREE(data->chr->data.file.path); } cleanup: @@ -653,7 +656,9 @@ qemuMonitorJSONTestAttachChardev(virDomainXMLOption *xmlopt, "'data':{'type':'vdagent'}}}"); chr->type = VIR_DOMAIN_CHR_TYPE_PTY; - CHECK("pty missing path", true, + /* Higher level code regards the missing path as error, but we simply + * check here what we've parsed */ + CHECK("pty missing path", false, "{'id':'alias','backend':{'type':'pty','data':{}}}"); if (qemuMonitorJSONTestAttachOneChardev(xmlopt, schema, "pty", chr, "{'id':'alias'," -- 2.46.0
On a Wednesday in 2024, Peter Krempa wrote:
Now that we have a unified generator of chardev backend which is also validated against the QMP schema we can replace the old generator with it.
This patch modifies the monitor code to take virJSONValue 'props' instead of the chardev definition and adds the conversion from the chardev definition to JSON on higher levels.
The monitor code now also attempts to extract the returned 'pty' if returned from qemu, so higher level code needs to report the error if the path is needed and missing.
The current monitor generator is for now abandoned in place and will be removed later.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_block.c | 9 ++++++++- src/qemu/qemu_hotplug.c | 22 +++++++++++++++++++++- src/qemu/qemu_monitor.c | 8 +++----- src/qemu/qemu_monitor.h | 4 ++-- src/qemu/qemu_monitor_json.c | 32 ++++++++++---------------------- src/qemu/qemu_monitor_json.h | 4 ++-- tests/qemumonitorjsontest.c | 17 +++++++++++------ 7 files changed, 57 insertions(+), 39 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_monitor_json.c | 243 ----------------------------------- 1 file changed, 243 deletions(-) diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index f3a353a13b..75a5b03024 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -6173,22 +6173,6 @@ qemuMonitorJSONBuildUnixSocketAddress(const char *path) } -static virJSONValue * -qemuMonitorJSONBuildFDSocketAddress(const char *fdname) -{ - g_autoptr(virJSONValue) addr = NULL; - g_autoptr(virJSONValue) data = NULL; - - if (virJSONValueObjectAdd(&data, "s:str", fdname, NULL) < 0) - return NULL; - - if (virJSONValueObjectAdd(&addr, - "s:type", "fd", - "a:data", &data, NULL) < 0) - return NULL; - - return g_steal_pointer(&addr); -} int qemuMonitorJSONNBDServerStart(qemuMonitor *mon, const virStorageNetHostDef *server, @@ -6342,233 +6326,6 @@ int qemuMonitorJSONGetTPMTypes(qemuMonitor *mon, } -static G_GNUC_UNUSED virJSONValue * -qemuMonitorJSONAttachCharDevGetProps(const char *chrID, - const virDomainChrSourceDef *chr) -{ - qemuDomainChrSourcePrivate *chrSourcePriv = QEMU_DOMAIN_CHR_SOURCE_PRIVATE(chr); - g_autoptr(virJSONValue) props = NULL; - g_autoptr(virJSONValue) backend = NULL; - g_autoptr(virJSONValue) backendData = virJSONValueNewObject(); - const char *backendType = NULL; - - switch ((virDomainChrType)chr->type) { - case VIR_DOMAIN_CHR_TYPE_NULL: - case VIR_DOMAIN_CHR_TYPE_VC: - case VIR_DOMAIN_CHR_TYPE_PTY: - backendType = virDomainChrTypeToString(chr->type); - break; - - case VIR_DOMAIN_CHR_TYPE_FILE: { - const char *path = chr->data.file.path; - virTristateSwitch append = chr->data.file.append; - backendType = "file"; - - if (chrSourcePriv->sourcefd) { - path = qemuFDPassGetPath(chrSourcePriv->sourcefd); - append = VIR_TRISTATE_SWITCH_ON; - } - - if (virJSONValueObjectAdd(&backendData, - "s:out", path, - "T:append", append, - NULL) < 0) - return NULL; - } - break; - - case VIR_DOMAIN_CHR_TYPE_DEV: - if (STRPREFIX(chrID, "parallel")) - backendType = "parallel"; - else - backendType = "serial"; - - if (virJSONValueObjectAdd(&backendData, - "s:device", chr->data.file.path, - NULL) < 0) - return NULL; - - break; - - case VIR_DOMAIN_CHR_TYPE_UNIX: - case VIR_DOMAIN_CHR_TYPE_TCP: { - const char *tlsalias = NULL; - g_autoptr(virJSONValue) addr = NULL; - virTristateBool waitval = VIR_TRISTATE_BOOL_ABSENT; - virTristateBool telnet = VIR_TRISTATE_BOOL_ABSENT; - bool server = false; - int reconnect = -1; - - backendType = "socket"; - - if (chr->type == VIR_DOMAIN_CHR_TYPE_TCP) { - telnet = virTristateBoolFromBool(chr->data.tcp.protocol == VIR_DOMAIN_CHR_TCP_PROTOCOL_TELNET); - - if (chr->data.tcp.listen) { - server = true; - waitval = VIR_TRISTATE_BOOL_NO; - } - - tlsalias = chrSourcePriv->tlsCredsAlias; - - if (!(addr = qemuMonitorJSONBuildInetSocketAddress(chr->data.tcp.host, - chr->data.tcp.service))) - return NULL; - - if (chr->data.tcp.reconnect.enabled == VIR_TRISTATE_BOOL_YES) - reconnect = chr->data.tcp.reconnect.timeout; - else if (chr->data.tcp.reconnect.enabled == VIR_TRISTATE_BOOL_NO) - reconnect = 0; - } else { - if (chr->data.nix.listen) { - server = true; - waitval = VIR_TRISTATE_BOOL_NO; - } - - if (chrSourcePriv->directfd) { - if (!(addr = qemuMonitorJSONBuildFDSocketAddress(qemuFDPassDirectGetPath(chrSourcePriv->directfd)))) - return NULL; - } else { - if (!(addr = qemuMonitorJSONBuildUnixSocketAddress(chr->data.nix.path))) - return NULL; - - if (chr->data.nix.reconnect.enabled == VIR_TRISTATE_BOOL_YES) - reconnect = chr->data.nix.reconnect.timeout; - else if (chr->data.nix.reconnect.enabled == VIR_TRISTATE_BOOL_NO) - reconnect = 0; - } - } - - if (virJSONValueObjectAdd(&backendData, - "a:addr", &addr, - "T:wait", waitval, - "T:telnet", telnet, - "b:server", server, - "S:tls-creds", tlsalias, - "k:reconnect", reconnect, - NULL) < 0) - return NULL; - } - break; - - case VIR_DOMAIN_CHR_TYPE_UDP: { - g_autoptr(virJSONValue) local = NULL; - g_autoptr(virJSONValue) remote = NULL; - - backendType = "udp"; - - if (!(remote = qemuMonitorJSONBuildInetSocketAddress(NULLSTR_EMPTY(chr->data.udp.connectHost), - chr->data.udp.connectService))) - return NULL; - - if (chr->data.udp.bindHost || chr->data.udp.bindService) { - if (!(local = qemuMonitorJSONBuildInetSocketAddress(NULLSTR_EMPTY(chr->data.udp.bindHost), - NULLSTR_EMPTY(chr->data.udp.bindService)))) - return NULL; - } - - if (virJSONValueObjectAdd(&backendData, - "a:remote", &remote, - "A:local", &local, - NULL) < 0) - return NULL; - } - break; - - case VIR_DOMAIN_CHR_TYPE_SPICEVMC: - backendType = "spicevmc"; - - if (virJSONValueObjectAdd(&backendData, - "s:type", virDomainChrSpicevmcTypeToString(chr->data.spicevmc), - NULL) < 0) - return NULL; - - break; - - case VIR_DOMAIN_CHR_TYPE_QEMU_VDAGENT: { - virTristateBool mouse = VIR_TRISTATE_BOOL_ABSENT; - switch (chr->data.qemuVdagent.mouse) { - case VIR_DOMAIN_MOUSE_MODE_CLIENT: - mouse = VIR_TRISTATE_BOOL_YES; - break; - case VIR_DOMAIN_MOUSE_MODE_SERVER: - mouse = VIR_TRISTATE_BOOL_NO; - break; - case VIR_DOMAIN_MOUSE_MODE_DEFAULT: - break; - case VIR_DOMAIN_MOUSE_MODE_LAST: - default: - virReportEnumRangeError(virDomainMouseMode, - chr->data.qemuVdagent.mouse); - return NULL; - } - backendType = "qemu-vdagent"; - - if (virJSONValueObjectAdd(&backendData, - "T:clipboard", chr->data.qemuVdagent.clipboard, - "T:mouse", mouse, - NULL) < 0) - return NULL; - break; - } - - case VIR_DOMAIN_CHR_TYPE_DBUS: - backendType = "dbus"; - - if (virJSONValueObjectAdd(&backendData, - "s:name", chr->data.dbus.channel, - NULL) < 0) - return NULL; - - break; - - case VIR_DOMAIN_CHR_TYPE_SPICEPORT: - case VIR_DOMAIN_CHR_TYPE_PIPE: - case VIR_DOMAIN_CHR_TYPE_STDIO: - case VIR_DOMAIN_CHR_TYPE_NMDM: - virReportError(VIR_ERR_OPERATION_FAILED, - _("Hotplug unsupported for char device type '%1$s'"), - virDomainChrTypeToString(chr->type)); - return NULL; - - case VIR_DOMAIN_CHR_TYPE_LAST: - default: - virReportEnumRangeError(virDomainChrType, chr->type); - return NULL; - } - - if (chr->logfile) { - const char *path = chr->logfile; - virTristateSwitch append = chr->logappend; - - if (chrSourcePriv->logfd) { - path = qemuFDPassGetPath(chrSourcePriv->logfd); - append = VIR_TRISTATE_SWITCH_ON; - } - - if (virJSONValueObjectAdd(&backendData, - "s:logfile", path, - "T:logappend", append, - NULL) < 0) - return NULL; - } - - if (virJSONValueObjectAdd(&backend, - "s:type", backendType, - "A:data", &backendData, - NULL) < 0) - return NULL; - - if (virJSONValueObjectAdd(&props, - "s:id", chrID, - "a:backend", &backend, - NULL) < 0) - return NULL; - - return g_steal_pointer(&props); -} - - int qemuMonitorJSONAttachCharDev(qemuMonitor *mon, virJSONValue **props, -- 2.46.0
On a Wednesday in 2024, Peter Krempa wrote:
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_monitor_json.c | 243 ----------------------------------- 1 file changed, 243 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
participants (2)
-
Ján Tomko -
Peter Krempa