aboutsummaryrefslogtreecommitdiff
path: root/chardev/char-socket.c
AgeCommit message (Collapse)Author
2021-09-27qapi: Convert simple union SocketAddressLegacy to flat oneMarkus Armbruster
Simple unions predate flat unions. Having both complicates the QAPI schema language and the QAPI generator. We haven't been using simple unions in new code for a long time, because they are less flexible and somewhat awkward on the wire. To prepare for their removal, convert simple union SocketAddressLegacy to an equivalent flat one, with existing enum SocketAddressType replacing implicit enum type SocketAddressLegacyKind. Adds some boilerplate to the schema, which is a bit ugly, but a lot easier to maintain than the simple union feature. Cc: "Daniel P. Berrangé" <berrange@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-Id: <20210917143134.412106-9-armbru@redhat.com>
2021-08-04chardev/socket: print a more correct command-line addressMarc-André Lureau
Better reflect the command line version of the socket address arguments, following the now recommended long-form opt=on syntax. Complement/fixes commit 9d902d51 "chardev: do not use short form boolean options in non-QemuOpts character device descriptions". Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
2021-06-29chardev/socket: Use qcrypto_tls_creds_check_endpoint()Philippe Mathieu-Daudé
Avoid accessing QCryptoTLSCreds internals by using the qcrypto_tls_creds_check_endpoint() helper. Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
2021-04-01chardev: Fix yank with the chardev-change caseLukas Straub
When changing from chardev-socket (which supports yank) to chardev-socket again, it fails, because the new chardev attempts to register a new yank instance. This in turn fails, as there still is the yank instance from the current chardev. Also, the old chardev shouldn't unregister the yank instance when it is freed. To fix this, now the new chardev only registers a yank instance if the current chardev doesn't support yank and thus hasn't registered one already. Also, when the old chardev is freed, it now only unregisters the yank instance if the new chardev doesn't need it. If the initialization of the new chardev fails, it still has chr->handover_yank_instance set and won't unregister the yank instance when it is freed. s->registered_yank is always true here, as chardev-change only works on user-visible chardevs and those are guraranteed to register a yank instance as they are initialized via chardev_new() qemu_char_open() cc->open() (qmp_chardev_open_socket()). Signed-off-by: Lukas Straub <lukasstraub2@web.de> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> Tested-by: Li Zhang <li.zhang@cloud.ionos.com> Message-Id: <9637888d7591d2971975188478bb707299a1dc04.1617127849.git.lukasstraub2@web.de>
2021-04-01yank: Remove dependency on qiochannelLukas Straub
Remove dependency on qiochannel by removing yank_generic_iochannel and letting migration and chardev use their own yank function for iochannel. Signed-off-by: Lukas Straub <lukasstraub2@web.de> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> Message-Id: <20ff143fc2db23e27cd41d38043e481376c9cec1.1616521341.git.lukasstraub2@web.de>
2021-03-18chardev: reject use of 'wait' flag for socket client chardevsDaniel P. Berrangé
This only makes sense conceptually when used with listener chardevs. Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
2021-03-06chardev: add nodelay optionPaolo Bonzini
The "delay" option was introduced as a way to enable Nagle's algorithm with ",nodelay". Since the short form for boolean options has now been deprecated, introduce a more properly named "nodelay" option. The "delay" option remains as an undocumented option. "delay" and "nodelay" are mutually exclusive. Because the check is done at consumption time, the code also rejects them if one of the two is specified via -set. Based-on: <20210226080526.651705-1-pbonzini@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2021-02-25chardev: do not use short form boolean options in non-QemuOpts character ↵Paolo Bonzini
device descriptions Options such as "-gdb" or "-serial" accept a part-QemuOpts part-parsed-by-hand character device description. Do not use short form boolean options in the QemuOpts part. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2021-02-25char: don't fail when client is not connectedPavel Dovgalyuk
This patch checks that ioc is not null before using it in tcp socket tcp_chr_add_watch function. The failure occurs in replay mode of the execution, when monitor and serial port are tcp servers, and there are no clients connected to them: -monitor tcp:127.0.0.1:8081,server,nowait -serial tcp:127.0.0.1:8082,server,nowait Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgalyuk@ispras.ru> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> Message-Id: <161284977034.741841.12565530923825663110.stgit@pasha-ThinkPad-X280> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2021-02-04chardev: check if the chardev is registered for yankingMarc-André Lureau
Not all chardevs are created via qmp_chardev_open_socket(), and those should not call the yank function registration, as this will eventually assert() not being registered. Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> Message-Id: <20210204105232.834642-20-marcandre.lureau@redhat.com> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
2021-01-13chardev/char-socket.c: Add yank featureLukas Straub
Register a yank function to shutdown the socket on yank. Signed-off-by: Lukas Straub <lukasstraub2@web.de> Acked-by: Stefan Hajnoczi <stefanha@redhat.com> Message-Id: <1f4eeed1d066c6cbb8d05ffa9585f6e87b34aac6.1609167865.git.lukasstraub2@web.de> Acked-by: Marc-André Lureau <marcandre.lureau@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
2020-11-03sockets: Make abstract UnixSocketAddress depend on CONFIG_LINUXMarkus Armbruster
The abstract socket namespace is a non-portable Linux extension. An attempt to use it elsewhere should fail with ENOENT (the abstract address looks like a "" pathname, which does not resolve). We report this failure like Failed to connect socket abc: No such file or directory Tolerable, although ENOTSUP would be better. However, introspection lies: it has @abstract regardless of host support. Easy enough to fix: since Linux provides them since 2.2, 'if': 'defined(CONFIG_LINUX)' should do. The above failure becomes Parameter 'backend.data.addr.data.abstract' is unexpected I consider this an improvement. Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
2020-11-03char-socket: Fix qemu_chr_socket_address() for abstract socketsMarkus Armbruster
Commit 776b97d360 "qemu-sockets: add abstract UNIX domain socket support" neglected to update qemu_chr_socket_address(). It shows shows neither @abstract nor @tight. Fix that. Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
2020-11-03sockets: Fix default of UnixSocketAddress member @tightMarkus Armbruster
An optional bool member of a QAPI struct can be false, true, or absent. The previous commit demonstrated that socket_listen() and socket_connect() are broken for absent @tight, and indeed QMP chardev- add also defaults absent member @tight to false instead of true. In C, QAPI members are represented by two fields, has_MEMBER and MEMBER. We have: has_MEMBER MEMBER false true false true true true absent false false/ignore When has_MEMBER is false, MEMBER should be set to false on write, and ignored on read. For QMP, the QAPI visitors handle absent @tight by setting both @has_tight and @tight to false. unix_listen_saddr() and unix_connect_saddr() however use @tight only, disregarding @has_tight. This is wrong and means that absent @tight defaults to false whereas it should default to true. The same is true for @has_abstract, though @abstract defaults to false and therefore has the same behavior for all of QMP, HMP and CLI. Fix unix_listen_saddr() and unix_connect_saddr() to check @has_abstract/@has_tight, and to default absent @tight to true. However, this is only half of the story. HMP chardev-add and CLI -chardev so far correctly defaulted @tight to true, but defaults to false again with the above fix for HMP and CLI. In fact, the "tight" and "abstract" options now break completely. Digging deeper, we find that qemu_chr_parse_socket() also ignores @has_tight, leaving it false when it sets @tight. That is also wrong, but the two wrongs cancelled out. Fix qemu_chr_parse_socket() to set @has_tight and @has_abstract; writing testcases for HMP and CLI is left for another day. Fixes: 776b97d3605ed0fc94443048fdf988c7725e38a9 Reported-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
2020-09-09Use DECLARE_*CHECKER* macrosEduardo Habkost
Generated using: $ ./scripts/codeconverter/converter.py -i \ --pattern=TypeCheckMacro $(git grep -l '' -- '*.[ch]') Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Reviewed-by: Juan Quintela <quintela@redhat.com> Message-Id: <20200831210740.126168-12-ehabkost@redhat.com> Reviewed-by: Juan Quintela <quintela@redhat.com> Message-Id: <20200831210740.126168-13-ehabkost@redhat.com> Message-Id: <20200831210740.126168-14-ehabkost@redhat.com> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
2020-09-09Move QOM typedefs and add missing includesEduardo Habkost
Some typedefs and macros are defined after the type check macros. This makes it difficult to automatically replace their definitions with OBJECT_DECLARE_TYPE. Patch generated using: $ ./scripts/codeconverter/converter.py -i \ --pattern=QOMStructTypedefSplit $(git grep -l '' -- '*.[ch]') which will split "typdef struct { ... } TypedefName" declarations. Followed by: $ ./scripts/codeconverter/converter.py -i --pattern=MoveSymbols \ $(git grep -l '' -- '*.[ch]') which will: - move the typedefs and #defines above the type check macros - add missing #include "qom/object.h" lines if necessary Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Reviewed-by: Juan Quintela <quintela@redhat.com> Message-Id: <20200831210740.126168-9-ehabkost@redhat.com> Reviewed-by: Juan Quintela <quintela@redhat.com> Message-Id: <20200831210740.126168-10-ehabkost@redhat.com> Message-Id: <20200831210740.126168-11-ehabkost@redhat.com> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
2020-07-13char: fix use-after-free with dup chardev & reconnectMarc-André Lureau
With a reconnect socket, qemu_char_open() will start a background thread. It should keep a reference on the chardev. Fixes invalid read: READ of size 8 at 0x6040000ac858 thread T7 #0 0x5555598d37b8 in unix_connect_saddr /home/elmarco/src/qq/util/qemu-sockets.c:954 #1 0x5555598d4751 in socket_connect /home/elmarco/src/qq/util/qemu-sockets.c:1109 #2 0x555559707c34 in qio_channel_socket_connect_sync /home/elmarco/src/qq/io/channel-socket.c:145 #3 0x5555596adebb in tcp_chr_connect_client_task /home/elmarco/src/qq/chardev/char-socket.c:1104 #4 0x555559723d55 in qio_task_thread_worker /home/elmarco/src/qq/io/task.c:123 #5 0x5555598a6731 in qemu_thread_start /home/elmarco/src/qq/util/qemu-thread-posix.c:519 #6 0x7ffff40d4431 in start_thread (/lib64/libpthread.so.0+0x9431) #7 0x7ffff40029d2 in __clone (/lib64/libc.so.6+0x1019d2) Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Message-Id: <20200420112012.567284-1-marcandre.lureau@redhat.com>
2020-07-13char-socket: initialize reconnect timer only when the timer doesn't startLi Feng
When the disconnect event is triggered in the connecting stage, the tcp_chr_disconnect_locked may be called twice. The first call: #0 qemu_chr_socket_restart_timer (chr=0x55555582ee90) at chardev/char-socket.c:120 #1 0x000055555558e38c in tcp_chr_disconnect_locked (chr=<optimized out>) at chardev/char-socket.c:490 #2 0x000055555558e3cd in tcp_chr_disconnect (chr=0x55555582ee90) at chardev/char-socket.c:497 #3 0x000055555558ea32 in tcp_chr_new_client (chr=chr@entry=0x55555582ee90, sioc=sioc@entry=0x55555582f0b0) at chardev/char-socket.c:892 #4 0x000055555558eeb8 in qemu_chr_socket_connected (task=0x55555582f300, opaque=<optimized out>) at chardev/char-socket.c:1090 #5 0x0000555555574352 in qio_task_complete (task=task@entry=0x55555582f300) at io/task.c:196 #6 0x00005555555745f4 in qio_task_thread_result (opaque=0x55555582f300) at io/task.c:111 #7 qio_task_wait_thread (task=0x55555582f300) at io/task.c:190 #8 0x000055555558f17e in tcp_chr_wait_connected (chr=0x55555582ee90, errp=0x555555802a08 <error_abort>) at chardev/char-socket.c:1013 #9 0x0000555555567cbd in char_socket_client_reconnect_test (opaque=0x5555557fe020 <client8unix>) at tests/test-char.c:1152 The second call: #0 0x00007ffff5ac3277 in raise () from /lib64/libc.so.6 #1 0x00007ffff5ac4968 in abort () from /lib64/libc.so.6 #2 0x00007ffff5abc096 in __assert_fail_base () from /lib64/libc.so.6 #3 0x00007ffff5abc142 in __assert_fail () from /lib64/libc.so.6 #4 0x000055555558d10a in qemu_chr_socket_restart_timer (chr=0x55555582ee90) at chardev/char-socket.c:125 #5 0x000055555558df0c in tcp_chr_disconnect_locked (chr=<optimized out>) at chardev/char-socket.c:490 #6 0x000055555558df4d in tcp_chr_disconnect (chr=0x55555582ee90) at chardev/char-socket.c:497 #7 0x000055555558e5b2 in tcp_chr_new_client (chr=chr@entry=0x55555582ee90, sioc=sioc@entry=0x55555582f0b0) at chardev/char-socket.c:892 #8 0x000055555558e93a in tcp_chr_connect_client_sync (chr=chr@entry=0x55555582ee90, errp=errp@entry=0x7fffffffd178) at chardev/char-socket.c:944 #9 0x000055555558ec78 in tcp_chr_wait_connected (chr=0x55555582ee90, errp=0x555555802a08 <error_abort>) at chardev/char-socket.c:1035 #10 0x000055555556804b in char_socket_client_test (opaque=0x5555557fe020 <client8unix>) at tests/test-char.c:1023 Run test/test-char to reproduce this issue. test-char: chardev/char-socket.c:125: qemu_chr_socket_restart_timer: Assertion `!s->reconnect_timer' failed. Signed-off-by: Li Feng <fengli@smartx.com> Acked-by: Marc-André Lureau <marcandre.lureau@redhat.com> Message-Id: <20200522025554.41063-1-fengli@smartx.com>
2020-07-02Clean up some calls to ignore Error objects the right wayMarkus Armbruster
Receiving the error in a local variable only to free it is less clear (and also less efficient) than passing NULL. Clean up. Cc: Daniel P. Berrange <berrange@redhat.com> Cc: Jerome Forissier <jerome@forissier.org> CC: Greg Kurz <groug@kaod.org> Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Greg Kurz <groug@kaod.org> Message-Id: <20200630090351.1247703-4-armbru@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
2020-07-02chardev/tcp: Fix error message double free errorlichun
Errors are already freed by error_report_err, so we only need to call error_free when that function is not called. Cc: qemu-stable@nongnu.org Signed-off-by: lichun <lichun@ruijie.com.cn> Message-Id: <20200621213017.17978-1-lichun@ruijie.com.cn> Reviewed-by: Markus Armbruster <armbru@redhat.com> [Commit message improved, cc: qemu-stable] Signed-off-by: Markus Armbruster <armbru@redhat.com>
2020-06-12Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream' into stagingPeter Maydell
* Miscellaneous fixes and feature enablement (many) * SEV refactoring (David) * Hyper-V initial support (Jon) * i386 TCG fixes (x87 and SSE, Joseph) * vmport cleanup and improvements (Philippe, Liran) * Use-after-free with vCPU hot-unplug (Nengyuan) * run-coverity-scan improvements (myself) * Record/replay fixes (Pavel) * -machine kernel_irqchip=split improvements for INTx (Peter) * Code cleanups (Philippe) * Crash and security fixes (PJP) * HVF cleanups (Roman) # gpg: Signature made Fri 12 Jun 2020 16:57:04 BST # gpg: using RSA key F13338574B662389866C7682BFFBD25F78C7AE83 # gpg: issuer "pbonzini@redhat.com" # gpg: Good signature from "Paolo Bonzini <bonzini@gnu.org>" [full] # gpg: aka "Paolo Bonzini <pbonzini@redhat.com>" [full] # Primary key fingerprint: 46F5 9FBD 57D6 12E7 BFD4 E2F7 7E15 100C CD36 69B1 # Subkey fingerprint: F133 3857 4B66 2389 866C 7682 BFFB D25F 78C7 AE83 * remotes/bonzini/tags/for-upstream: (116 commits) target/i386: Remove obsolete TODO file stubs: move Xen stubs to accel/ replay: fix replay shutdown for console mode exec/cpu-common: Move MUSB specific typedefs to 'hw/usb/hcd-musb.h' hw/usb: Move device-specific declarations to new 'hcd-musb.h' header exec/memory: Remove unused MemoryRegionMmio type checkpatch: reversed logic with acpi test checks target/i386: sev: Unify SEVState and SevGuestState target/i386: sev: Remove redundant handle field target/i386: sev: Remove redundant policy field target/i386: sev: Remove redundant cbitpos and reduced_phys_bits fields target/i386: sev: Partial cleanup to sev_state global target/i386: sev: Embed SEVState in SevGuestState target/i386: sev: Rename QSevGuestInfo target/i386: sev: Move local structure definitions into .c file target/i386: sev: Remove unused QSevGuestInfoClass xen: fix build without pci passthrough i386: hvf: Drop HVFX86EmulatorState i386: hvf: Move mmio_buf into CPUX86State i386: hvf: Move lazy_flags into CPUX86State ... Signed-off-by: Peter Maydell <peter.maydell@linaro.org> # Conflicts: # hw/i386/acpi-build.c
2020-06-10chardev/char-socket: Properly make qio connections non blockingSai Pavan Boddu
In tcp_chr_sync_read function, there is a possibility of socket disconnection during blocking read, then tcp_chr_hup function would clean up the qio channel pointers(i.e ioc, sioc). Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com> Message-Id: <1587289900-29485-1-git-send-email-sai.pavan.boddu@xilinx.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2020-06-09char-socket: return -1 in case of disconnect during tcp_chr_writeDima Stepanov
During testing of the vhost-user-blk reconnect functionality the qemu SIGSEGV was triggered: start qemu as: x86_64-softmmu/qemu-system-x86_64 -m 1024M -M q35 \ -object memory-backend-file,id=ram-node0,size=1024M,mem-path=/dev/shm/qemu,share=on \ -numa node,cpus=0,memdev=ram-node0 \ -chardev socket,id=chardev0,path=./vhost.sock,noserver,reconnect=1 \ -device vhost-user-blk-pci,chardev=chardev0,num-queues=4 --enable-kvm start vhost-user-blk daemon: ./vhost-user-blk -s ./vhost.sock -b test-img.raw If vhost-user-blk will be killed during the vhost initialization process, for instance after getting VHOST_SET_VRING_CALL command, then QEMU will fail with the following backtrace: Thread 1 "qemu-system-x86" received signal SIGSEGV, Segmentation fault. 0x00005555559272bb in vhost_user_read (dev=0x7fffef2d53e0, msg=0x7fffffffd5b0) at ./hw/virtio/vhost-user.c:260 260 CharBackend *chr = u->user->chr; #0 0x00005555559272bb in vhost_user_read (dev=0x7fffef2d53e0, msg=0x7fffffffd5b0) at ./hw/virtio/vhost-user.c:260 #1 0x000055555592acb8 in vhost_user_get_config (dev=0x7fffef2d53e0, config=0x7fffef2d5394 "", config_len=60) at ./hw/virtio/vhost-user.c:1645 #2 0x0000555555925525 in vhost_dev_get_config (hdev=0x7fffef2d53e0, config=0x7fffef2d5394 "", config_len=60) at ./hw/virtio/vhost.c:1490 #3 0x00005555558cc46b in vhost_user_blk_device_realize (dev=0x7fffef2d51a0, errp=0x7fffffffd8f0) at ./hw/block/vhost-user-blk.c:429 #4 0x0000555555920090 in virtio_device_realize (dev=0x7fffef2d51a0, errp=0x7fffffffd948) at ./hw/virtio/virtio.c:3615 #5 0x0000555555a9779c in device_set_realized (obj=0x7fffef2d51a0, value=true, errp=0x7fffffffdb88) at ./hw/core/qdev.c:891 ... The problem is that vhost_user_write doesn't get an error after disconnect and try to call vhost_user_read(). The tcp_chr_write() routine should return -1 in case of disconnect. Indicate the EIO error if this routine is called in the disconnected state. Signed-off-by: Dima Stepanov <dimastep@yandex-team.ru> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> Message-Id: <aeb7806bfc945faadf09f64dcfa30f59de3ac053.1590396396.git.dimastep@yandex-team.ru> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
2020-05-27error: Use error_reportf_err() where appropriateMarkus Armbruster
Replace error_report("...: %s", ..., error_get_pretty(err)); by error_reportf_err(err, "...: ", ...); One of the replaced messages lacked a colon. Add it. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Message-Id: <20200505101908.6207-6-armbru@redhat.com>
2020-05-20qemu-sockets: add abstract UNIX domain socket supportxiaoqiang zhao
unix_listen/connect_saddr now support abstract address types two aditional BOOL switches are introduced: tight: whether to set @addrlen to the minimal string length, or the maximum sun_path length. default is TRUE abstract: whether we use abstract address. default is FALSE cli example: -monitor unix:/tmp/unix.socket,abstract,tight=off OR -chardev socket,path=/tmp/unix.socket,id=unix1,abstract,tight=on Signed-off-by: xiaoqiang zhao <zxq_yx_007@163.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
2020-05-15qom: Drop parameter @errp of object_property_add() & friendsMarkus Armbruster
The only way object_property_add() can fail is when a property with the same name already exists. Since our property names are all hardcoded, failure is a programming error, and the appropriate way to handle it is passing &error_abort. Same for its variants, except for object_property_add_child(), which additionally fails when the child already has a parent. Parentage is also under program control, so this is a programming error, too. We have a bit over 500 callers. Almost half of them pass &error_abort, slightly fewer ignore errors, one test case handles errors, and the remaining few callers pass them to their own callers. The previous few commits demonstrated once again that ignoring programming errors is a bad idea. Of the few ones that pass on errors, several violate the Error API. The Error ** argument must be NULL, &error_abort, &error_fatal, or a pointer to a variable containing NULL. Passing an argument of the latter kind twice without clearing it in between is wrong: if the first call sets an error, it no longer points to NULL for the second call. ich9_pm_add_properties(), sparc32_ledma_realize(), sparc32_dma_realize(), xilinx_axidma_realize(), xilinx_enet_realize() are wrong that way. When the one appropriate choice of argument is &error_abort, letting users pick the argument is a bad idea. Drop parameter @errp and assert the preconditions instead. There's one exception to "duplicate property name is a programming error": the way object_property_add() implements the magic (and undocumented) "automatic arrayification". Don't drop @errp there. Instead, rename object_property_add() to object_property_try_add(), and add the obvious wrapper object_property_add(). Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> Message-Id: <20200505152926.18877-15-armbru@redhat.com> [Two semantic rebase conflicts resolved]
2019-09-03socket: Add num connections to qio_net_listener_open_sync()Juan Quintela
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Signed-off-by: Juan Quintela <quintela@redhat.com>
2019-08-21char-socket: Lock tcp_chr_disconnect() and socket_reconnect_timeout()Alberto Garcia
There's a race condition in which the tcp_chr_read() ioc handler can close a connection that is being written to from another thread. Running iotest 136 in a loop triggers this problem and crashes QEMU. (gdb) bt #0 0x00005558b842902d in object_get_class (obj=0x0) at qom/object.c:860 #1 0x00005558b84f92db in qio_channel_writev_full (ioc=0x0, iov=0x7ffc355decf0, niov=1, fds=0x0, nfds=0, errp=0x0) at io/channel.c:76 #2 0x00005558b84e0e9e in io_channel_send_full (ioc=0x0, buf=0x5558baf5beb0, len=138, fds=0x0, nfds=0) at chardev/char-io.c:123 #3 0x00005558b84e4a69 in tcp_chr_write (chr=0x5558ba460380, buf=0x5558baf5beb0 "...", len=138) at chardev/char-socket.c:135 #4 0x00005558b84dca55 in qemu_chr_write_buffer (s=0x5558ba460380, buf=0x5558baf5beb0 "...", len=138, offset=0x7ffc355dedd0, write_all=false) at chardev/char.c:112 #5 0x00005558b84dcbc2 in qemu_chr_write (s=0x5558ba460380, buf=0x5558baf5beb0 "...", len=138, write_all=false) at chardev/char.c:147 #6 0x00005558b84dfb26 in qemu_chr_fe_write (be=0x5558ba476610, buf=0x5558baf5beb0 "...", len=138) at chardev/char-fe.c:42 #7 0x00005558b8088c86 in monitor_flush_locked (mon=0x5558ba476610) at monitor.c:406 #8 0x00005558b8088e8c in monitor_puts (mon=0x5558ba476610, str=0x5558ba921e49 "") at monitor.c:449 #9 0x00005558b8089178 in qmp_send_response (mon=0x5558ba476610, rsp=0x5558bb161600) at monitor.c:498 #10 0x00005558b808920c in monitor_qapi_event_emit (event=QAPI_EVENT_SHUTDOWN, qdict=0x5558bb161600) at monitor.c:526 #11 0x00005558b8089307 in monitor_qapi_event_queue_no_reenter (event=QAPI_EVENT_SHUTDOWN, qdict=0x5558bb161600) at monitor.c:551 #12 0x00005558b80896c0 in qapi_event_emit (event=QAPI_EVENT_SHUTDOWN, qdict=0x5558bb161600) at monitor.c:626 #13 0x00005558b855f23b in qapi_event_send_shutdown (guest=false, reason=SHUTDOWN_CAUSE_HOST_QMP_QUIT) at qapi/qapi-events-run-state.c:43 #14 0x00005558b81911ef in qemu_system_shutdown (cause=SHUTDOWN_CAUSE_HOST_QMP_QUIT) at vl.c:1837 #15 0x00005558b8191308 in main_loop_should_exit () at vl.c:1885 #16 0x00005558b819140d in main_loop () at vl.c:1924 #17 0x00005558b8198c84 in main (argc=18, argv=0x7ffc355df3f8, envp=0x7ffc355df490) at vl.c:4665 This patch adds a lock to protect tcp_chr_disconnect() and socket_reconnect_timeout() Signed-off-by: Alberto Garcia <berto@igalia.com> Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com> Message-Id: <1565625509-404969-3-git-send-email-andrey.shinkevich@virtuozzo.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2019-06-12Include qemu/module.h where needed, drop it from qemu-common.hMarkus Armbruster
Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20190523143508.25387-4-armbru@redhat.com> [Rebased with conflicts resolved automatically, except for hw/usb/dev-hub.c hw/misc/exynos4210_rng.c hw/misc/bcm2835_rng.c hw/misc/aspeed_scu.c hw/display/virtio-vga.c hw/arm/stm32f205_soc.c; ui/cocoa.m fixed up]
2019-04-16socket: allow wait=false for client socketMarc-André Lureau
Commit 767abe7 ("chardev: forbid 'wait' option with client sockets") is a bit too strict. Current libvirt always set wait=false, and will thus fail to add client chardev. Make the code more permissive, allowing wait=false with client socket chardevs. Deprecate usage of 'wait' with client sockets. Fixes: 767abe7f49e8be14d29da5db3527817b5d696a52 Cc: Daniel P. Berrangé <berrange@redhat.com> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Message-id: 20190415163337.2795-1-marcandre.lureau@redhat.com Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
2019-03-11chardev: add support for authorization for TLS clientsDaniel P. Berrange
Currently any client which can complete the TLS handshake is able to use a chardev server. The server admin can turn on the 'verify-peer' option for the x509 creds to require the client to provide a x509 certificate. This means the client will have to acquire a certificate from the CA before they are permitted to use the chardev server. This is still a fairly low bar. This adds a 'tls-authz=OBJECT-ID' option to the socket chardev backend which takes the ID of a previously added 'QAuthZ' object instance. This will be used to validate the client's x509 distinguished name. Clients failing the check will not be permitted to use the chardev server. For example to setup authorization that only allows connection from a client whose x509 certificate distinguished name contains 'CN=fred', you would use: $QEMU -object tls-creds-x509,id=tls0,dir=/home/berrange/qemutls,\ endpoint=server,verify-peer=yes \ -object authz-simple,id=authz0,identity=CN=laptop.example.com,,\ O=Example Org,,L=London,,ST=London,,C=GB \ -chardev socket,host=127.0.0.1,port=9000,server,\ tls-creds=tls0,tls-authz=authz0 \ ...other qemu args... Signed-off-by: Daniel P. Berrange <berrange@redhat.com> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
2019-03-07chardev-socket: do not blindly reset handlers when switching GMainContextPaolo Bonzini
If the socket is connecting or connected, tcp_chr_update_read_handler will be called but it should not set the NetListener's callbacks again. Otherwise, tcp_chr_accept is invoked while the socket is in connected state and you get an assertion failure. Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2019-02-12chardev: fix race with client connections in tcp_chr_wait_connectedDaniel P. Berrangé
When the 'reconnect' option is given for a client connection, the qmp_chardev_open_socket_client method will run an asynchronous connection attempt. The QIOChannel socket executes this is a single use background thread, so the connection will succeed immediately (assuming the server is listening). The chardev, however, won't get the result from this background thread until the main loop starts running and processes idle callbacks. Thus when tcp_chr_wait_connected is run s->ioc will be NULL, but the state will be TCP_CHARDEV_STATE_CONNECTING, and there may already be an established connection that will be associated with the chardev by the pending idle callback. tcp_chr_wait_connected doesn't check the state, only s->ioc, so attempts to establish another connection synchronously. If the server allows multiple connections this is unhelpful but not a fatal problem as the duplicate connection will get ignored by the tcp_chr_new_client method when it sees the state is already connected. If the server only supports a single connection, however, the tcp_chr_wait_connected method will hang forever because the server will not accept its synchronous connection attempt until the first connection is closed. To deal with this tcp_chr_wait_connected needs to synchronize with the completion of the background connection task. To do this it needs to create the QIOTask directly and use the qio_task_wait_thread method. This will cancel the pending idle callback and directly dispatch the task completion callback, allowing the connection to be associated with the chardev. If the background connection failed, it can still attempt a new synchronous connection. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> Message-Id: <20190211182442.8542-15-berrange@redhat.com> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
2019-02-12chardev: disallow TLS/telnet/websocket with tcp_chr_wait_connectedDaniel P. Berrangé
In the previous commit commit 1dc8a6695c731abb7461c637b2512c3670d82be4 Author: Marc-André Lureau <marcandre.lureau@redhat.com> Date: Tue Aug 16 12:33:32 2016 +0400 char: fix waiting for TLS and telnet connection the tcp_chr_wait_connected() method was changed to check for a non-NULL 's->ioc' as a sign that there is already a connection present, as opposed to checking the "connected" flag to supposedly fix handling of TLS/telnet connections. The original code would repeatedly call tcp_chr_wait_connected creating many connections as 'connected' would never become true. The changed code would still repeatedly call tcp_chr_wait_connected busy waiting because s->ioc is set but the chardev will never see CHR_EVENT_OPENED. IOW, the code is still broken with TLS/telnet, but in a different way. Checking for a non-NULL 's->ioc' does not mean that a CHR_EVENT_OPENED will be ready for a TLS/telnet connection. These protocols (and the websocket protocol) all require the main loop to be running in order to complete the protocol handshake before emitting CHR_EVENT_OPENED. The tcp_chr_wait_connected() method is only used during early startup before a main loop is running, so TLS/telnet/websock connections can never complete initialization. Making this work would require changing tcp_chr_wait_connected to run a main loop. This is quite complex since we must not allow GSource's that other parts of QEMU have registered to run yet. The current callers of tcp_chr_wait_connected do not require use of the TLS/telnet/websocket protocols, so the simplest option is to just forbid this combination completely for now. Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> Message-Id: <20190211182442.8542-14-berrange@redhat.com> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
2019-02-12chardev: honour the reconnect setting in tcp_chr_wait_connectedDaniel P. Berrangé
If establishing a client connection fails, the tcp_chr_wait_connected method should sleep for the reconnect timeout and then retry the attempt. This ensures the callers don't immediately abort with an error when the initial connection fails. Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> Message-Id: <20190211182442.8542-13-berrange@redhat.com> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
2019-02-12chardev: use a state machine for socket connection stateDaniel P. Berrangé
The socket connection state is indicated via the 'bool connected' field in the SocketChardev struct. This variable is somewhat misleading though, as it is only set to true once the connection has completed all required handshakes (eg for TLS, telnet or websockets). IOW there is a period of time in which the socket is connected, but the "connected" flag is still false. The socket chardev really has three states that it can be in, disconnected, connecting and connected and those should be tracked explicitly. Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> Message-Id: <20190211182442.8542-12-berrange@redhat.com> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
2019-02-12chardev: split up qmp_chardev_open_socket connection codeDaniel P. Berrangé
In qmp_chardev_open_socket the code for connecting client chardevs is split across two conditionals far apart with some server chardev code in the middle. Split up the method so that code for client connection setup is separate from code for server connection setup. Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> Message-Id: <20190211182442.8542-11-berrange@redhat.com> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
2019-02-12chardev: split tcp_chr_wait_connected into two methodsDaniel P. Berrangé
The tcp_chr_wait_connected method can deal with either server or client chardevs, but some callers only care about one of these possibilities. The tcp_chr_wait_connected method will also need some refactoring to reliably deal with its primary goal of allowing a device frontend to wait for an established connection, which will interfere with other callers. Split it into two methods, one responsible for server initiated connections, the other responsible for client initiated connections. In doing this split the tcp_char_connect_async() method is renamed to become consistent with naming of the new methods. Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> Message-Id: <20190211182442.8542-10-berrange@redhat.com> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
2019-02-12chardev: remove unused 'sioc' variable & cleanup pathsDaniel P. Berrangé
The 'sioc' variable in qmp_chardev_open_socket was unused since commit 3e7d4d20d3a528b1ed10b1dc3d83119bfb0c5f24 Author: Peter Xu <peterx@redhat.com> Date: Tue Mar 6 13:33:17 2018 +0800 chardev: use chardev's gcontext for async connect Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> Reviewed-by: Thomas Huth <thuth@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> Message-Id: <20190211182442.8542-9-berrange@redhat.com> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
2019-02-12chardev: remove many local variables in qemu_chr_parse_socketDaniel P. Berrangé
Now that all validation is separated off into a separate method, we can directly populate the ChardevSocket struct from the QemuOpts values, avoiding many local variables. Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> Message-Id: <20190211182442.8542-7-berrange@redhat.com> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
2019-02-12chardev: forbid 'wait' option with client socketsDaniel P. Berrangé
The 'wait'/'nowait' parameter is used to tell server sockets whether to block until a client is accepted during initialization. Client chardevs have always silently ignored this option. Various tests were mistakenly passing this option for their client chardevs. Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> Reviewed-by: Thomas Huth <thuth@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> Message-Id: <20190211182442.8542-6-berrange@redhat.com> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
2019-02-12chardev: forbid 'reconnect' option with server socketsDaniel P. Berrangé
The 'reconnect' option is used to give the sleep time, in seconds, before a client socket attempts to re-establish a connection to the server. It does not make sense to set this for server sockets, as they will always accept a new client connection immediately after the previous one went away. Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> Reviewed-by: Thomas Huth <thuth@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> Message-Id: <20190211182442.8542-5-berrange@redhat.com> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
2019-02-12chardev: fix validation of options for QMP created chardevsDaniel P. Berrangé
The TLS creds option is not valid with certain address types. The user config was only checked for errors when parsing legacy QemuOpts, thus the user could pass unsupported values via QMP. Pull all code for validating options out into a new method qmp_chardev_validate_socket, that is called from the main qmp_chardev_open_socket method. This adds a missing check for rejecting TLS creds with the vsock address type. Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> Message-Id: <20190211182442.8542-4-berrange@redhat.com> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
2018-11-01chardev: Add websocket supportJulia Suvorova
New option "websocket" added to allow using WebSocket protocol for chardev socket backend. Example: -chardev socket,websocket,server,id=... Signed-off-by: Julia Suvorova <jusual@mail.ru> Message-Id: <20181018223501.21683-3-jusual@mail.ru> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
2018-11-01chardev/char-socket: Function headers refactoringJulia Suvorova
Upcoming websocket support requires additional parameters in function headers that are already overloaded. This patch replaces the bunch of parameters with a single structure pointer. Signed-off-by: Julia Suvorova <jusual@mail.ru> Message-Id: <20181018223501.21683-2-jusual@mail.ru> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
2018-11-01char-socket: make 'fd' incompatible with 'reconnect'Marc-André Lureau
A chardev socket created with the 'fd=' argument is not going to handle reconnection properly by recycling the same fd (or not in a supported way). Let's forbid this case. Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
2018-11-01char-socket: correctly set has_reconnect when parsing QemuOptsMarc-André Lureau
qemu_chr_parse_socket() fills all ChardevSocket fields, but that doesn't reflect correctly the arguments given with the options / on the command line. "reconnect" takes a number as argument, and the default value is 0, which doesn't help to identify the missing option. The other arguments have default values that are less problematic, leave them set by default for now. Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
2018-10-03chardev: avoid crash if no associated addressMarc-André Lureau
A socket chardev may not have associated address (when adding client fd manually for example). But on disconnect, updating socket filename expects an address and may lead to this crash: Thread 1 "qemu-system-x86" received signal SIGSEGV, Segmentation fault. 0x0000555555d8c70c in SocketAddress_to_str (prefix=0x555556043062 "disconnected:", addr=0x0, is_listen=false, is_telnet=false) at /home/elmarco/src/qq/chardev/char-socket.c:388 388 switch (addr->type) { (gdb) bt #0 0x0000555555d8c70c in SocketAddress_to_str (prefix=0x555556043062 "disconnected:", addr=0x0, is_listen=false, is_telnet=false) at /home/elmarco/src/qq/chardev/char-socket.c:388 #1 0x0000555555d8c8aa in update_disconnected_filename (s=0x555556b1ed00) at /home/elmarco/src/qq/chardev/char-socket.c:419 #2 0x0000555555d8c959 in tcp_chr_disconnect (chr=0x555556b1ed00) at /home/elmarco/src/qq/chardev/char-socket.c:438 #3 0x0000555555d8cba1 in tcp_chr_hup (channel=0x555556b75690, cond=G_IO_HUP, opaque=0x555556b1ed00) at /home/elmarco/src/qq/chardev/char-socket.c:482 #4 0x0000555555da596e in qio_channel_fd_source_dispatch (source=0x555556bb68b0, callback=0x555555d8cb58 <tcp_chr_hup>, user_data=0x555556b1ed00) at /home/elmarco/src/qq/io/channel-watch.c:84 Replace filename with a generic "disconnected:socket" in this case. Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
2018-10-02char-socket: update all ioc handlers when changing contextMarc-André Lureau
So far, tcp_chr_update_read_handler() only updated the read handler. Let's also update the hup handler. Factorize the code while at it. (note that s->ioc != NULL when s->connected) Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> Message-Id: <20180817135224.22971-4-marcandre.lureau@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2018-10-02Revert "chardev: tcp: postpone async connection setup"Marc-André Lureau
This reverts commit 25679e5d58e258e9950685ffbd0cae4cd40d9cc2. This commit broke "reconnect socket" chardev that are created after "machine_done": they no longer try to connect. It broke also vhost-user-test that uses chardev while there is no "machine_done" event. The goal of this patch was to move the "connect" source to the frontend context. chr->gcontext is set with qemu_chr_fe_set_handlers(). But there is no guarantee that it will be called, so we can't delay connection until then: the chardev should still attempt to connect during open(). qemu_chr_fe_set_handlers() is eventually called later and will update the context. Unless there is a good reason to not use initially the default context, I think we should revert to the previous state to fix the regressions. Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> Message-Id: <20180817135224.22971-3-marcandre.lureau@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
generated by cgit v1.2.3 (git 2.26.2) at 2024-11-22 20:21:12 +0000