summaryrefslogtreecommitdiffstats
path: root/net
Commit message (Collapse)AuthorAgeFilesLines
...
* Use OBJECT_DECLARE_SIMPLE_TYPE when possibleEduardo Habkost2020-09-185-15/+5Star
| | | | | | | | | | | | | This converts existing DECLARE_INSTANCE_CHECKER usage to OBJECT_DECLARE_SIMPLE_TYPE when possible. $ ./scripts/codeconverter/converter.py -i \ --pattern=AddObjectDeclareSimpleType $(git grep -l '' -- '*.[ch]') Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> Acked-by: Paul Durrant <paul@xen.org> Message-Id: <20200916182519.415636-6-ehabkost@redhat.com> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
* util: rename qemu_open() to qemu_open_old()Daniel P. Berrangé2020-09-161-1/+1
| | | | | | | | | | | We want to introduce a new version of qemu_open() that uses an Error object for reporting problems and make this it the preferred interface. Rename the existing method to release the namespace for the new impl. Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
* Merge remote-tracking branch ↵Peter Maydell2020-09-122-13/+7Star
|\ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 'remotes/vivier2/tags/trivial-branch-for-5.2-pull-request' into staging trivial patches pull request 20200911 # gpg: Signature made Fri 11 Sep 2020 20:32:27 BST # gpg: using RSA key CD2F75DDC8E3A4DC2E4F5173F30C38BD3F2FBE3C # gpg: issuer "laurent@vivier.eu" # gpg: Good signature from "Laurent Vivier <lvivier@redhat.com>" [full] # gpg: aka "Laurent Vivier <laurent@vivier.eu>" [full] # gpg: aka "Laurent Vivier (Red Hat) <lvivier@redhat.com>" [full] # Primary key fingerprint: CD2F 75DD C8E3 A4DC 2E4F 5173 F30C 38BD 3F2F BE3C * remotes/vivier2/tags/trivial-branch-for-5.2-pull-request: target/i386/kvm: Add missing fallthrough comment util/hexdump: Reorder qemu_hexdump() arguments util/hexdump: Convert to take a void pointer argument hw/arm/pxa2xx: Add missing fallthrough comment target/i386/kvm: Rename host_tsx_blacklisted() as host_tsx_broken() test-vmstate: remove unnecessary code in match_interval_mapping_node hw: hyperv: vmbus: Fix 32bit compilation kconfig: fix comment referring to old Makefiles meson.build: tweak sdl-image error message hw/net/e1000e: Remove duplicated write handler for FLSWDATA register hw/net/e1000e: Remove overwritten read handler for STATUS register Makefile: Skip the meson subdir in cscope/TAGS/ctags Makefile: Drop extra phony cscope hw/gpio/max7310: Replace disabled printf() by qemu_log_mask(UNIMP) hw/gpio/omap_gpio: Replace fprintf() by qemu_log_mask(GUEST_ERROR) hw/acpi/tco: Remove unused definitions hw/isa/isa-bus: Replace hw_error() by assert() hw/mips/fuloong2e: Convert pointless error message to an assert() Signed-off-by: Peter Maydell <peter.maydell@linaro.org> # Conflicts: # net/colo-compare.c
| * util/hexdump: Reorder qemu_hexdump() argumentsPhilippe Mathieu-Daudé2020-09-112-13/+13
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | qemu_hexdump()'s pointer to the buffer and length of the buffer are closely related arguments but are widely separated in the argument list order (also, the format of <stdio.h> function prototypes is usually to have the FILE* argument coming first). Reorder the arguments as "fp, prefix, buf, size" which is more logical. Suggested-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com> Reviewed-by: Stefano Garzarella <sgarzare@redhat.com> Message-Id: <20200822180950.1343963-3-f4bug@amsat.org> Signed-off-by: Laurent Vivier <laurent@vivier.eu>
| * util/hexdump: Convert to take a void pointer argumentPhilippe Mathieu-Daudé2020-09-112-7/+7
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Most uses of qemu_hexdump() do not take an array of char as input, forcing use of cast. Since we can use this helper to dump any kind of buffer, use a pointer to void argument instead. Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Alistair Francis <alistair.francis@wdc.com> Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com> Reviewed-by: Li Qiang <liq3ea@gmail.com> Reviewed-by: Stefano Garzarella <sgarzare@redhat.com> Message-Id: <20200822180950.1343963-2-f4bug@amsat.org> Signed-off-by: Laurent Vivier <laurent@vivier.eu>
* | Merge remote-tracking branch ↵Peter Maydell2020-09-117-39/+47
|\ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 'remotes/ehabkost/tags/machine-next-pull-request' into staging QOM boilerplate cleanup Documentation build fix: * memory: Remove kernel-doc comment marker (Eduardo Habkost) QOM cleanups: * Rename QOM macros for consistency between TYPE_* and type checking constants (Eduardo Habkost) QOM new macros: * OBJECT_DECLARE_* and OBJECT_DEFINE_* macros (Daniel P. Berrangé) * DECLARE_*_CHECKER macros (Eduardo Habkost) Automated QOM boilerplate changes: * Automated changes to use DECLARE_*_CHECKER (Eduardo Habkost * Automated changes to use OBJECT_DECLARE* (Eduardo Habkost) # gpg: Signature made Thu 10 Sep 2020 19:17:49 BST # gpg: using RSA key 5A322FD5ABC4D3DBACCFD1AA2807936F984DC5A6 # gpg: issuer "ehabkost@redhat.com" # gpg: Good signature from "Eduardo Habkost <ehabkost@redhat.com>" [full] # Primary key fingerprint: 5A32 2FD5 ABC4 D3DB ACCF D1AA 2807 936F 984D C5A6 * remotes/ehabkost/tags/machine-next-pull-request: (33 commits) virtio-vga: Use typedef name for instance_size vhost-user-vga: Use typedef name for instance_size xilinx_axienet: Use typedef name for instance_size lpc_ich9: Use typedef name for instance_size omap_intc: Use typedef name for instance_size xilinx_axidma: Use typedef name for instance_size tusb6010: Rename TUSB to TUSB6010 pc87312: Rename TYPE_PC87312_SUPERIO to TYPE_PC87312 vfio: Rename PCI_VFIO to VFIO_PCI usb: Rename USB_SERIAL_DEV to USB_SERIAL sabre: Rename SABRE_DEVICE to SABRE rs6000_mc: Rename RS6000MC_DEVICE to RS6000MC filter-rewriter: Rename FILTER_COLO_REWRITER to FILTER_REWRITER esp: Rename ESP_STATE to ESP ahci: Rename ICH_AHCI to ICH9_AHCI vmgenid: Rename VMGENID_DEVICE to TYPE_VMGENID vfio: Rename VFIO_AP_DEVICE_TYPE to TYPE_VFIO_AP_DEVICE dev-smartcard-reader: Rename CCID_DEV_NAME to TYPE_USB_CCID_DEV ap-device: Rename AP_DEVICE_TYPE to TYPE_AP_DEVICE gpex: Fix type checking function name ... Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
| * | filter-rewriter: Rename FILTER_COLO_REWRITER to FILTER_REWRITEREduardo Habkost2020-09-091-9/+9
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Make the type checking macro name consistent with the TYPE_* constant. Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> Reviewed-by: Zhang Chen <chen.zhang@intel.com> Message-Id: <20200902224311.1321159-41-ehabkost@redhat.com> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
| * | Use DECLARE_*CHECKER* macrosEduardo Habkost2020-09-097-16/+16
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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>
| * | Move QOM typedefs and add missing includesEduardo Habkost2020-09-097-15/+23
| |/ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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>
* / net/colo: Match is-enabled probe to tracepointRoman Bolshakov2020-09-093-24/+30
|/ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Build of QEMU with dtrace fails on macOS: LINK x86_64-softmmu/qemu-system-x86_64 error: probe colo_compare_miscompare doesn't exist error: Could not register probes ld: error creating dtrace DOF section for architecture x86_64 The reason of the error is explained by Adam Leventhal [1]: Note that is-enabled probes don't have the stability magic so I'm not sure how things would work if only is-enabled probes were used. net/colo code uses is-enabled probes to determine if other probes should be used but colo_compare_miscompare itself is not used explicitly. Linker doesn't include the symbol and build fails. The issue can be resolved if is-enabled probe matches the actual trace point that is used inside the test. Packet dump toggle is replaced with a compile-time conditional definition. 1. http://markmail.org/message/6grq2ygr5nwdwsnb Fixes: f4b618360e ("colo-compare: add TCP, UDP, ICMP packet comparison") Cc: Philippe Mathieu-Daudé <philmd@redhat.com> Cc: Cameron Esfahani <dirty@apple.com> Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com> Reviewed-by: Zhang Chen <chen.zhang@intel.com> Message-id: 20200717093517.73397-5-r.bolshakov@yadro.com Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
* meson: convert net directory to MesonMarc-André Lureau2020-08-214-34/+45
| | | | | Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
* trace: switch position of headers to what Meson requiresPaolo Bonzini2020-08-211-0/+1
| | | | | | | | | | | | | | | | | Meson doesn't enjoy the same flexibility we have with Make in choosing the include path. In particular the tracing headers are using $(build_root)/$(<D). In order to keep the include directives unchanged, the simplest solution is to generate headers with patterns like "trace/trace-audio.h" and place forwarding headers in the source tree such that for example "audio/trace.h" includes "trace/trace-audio.h". This patch is too ugly to be applied to the Makefiles now. It's only a way to separate the changes to the tracing header files from the Meson rewrite of the tracing logic. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
* colo-compare: Remove superfluous NULL-pointer checks for s->iothreadLukas Straub2020-08-041-6/+2Star
| | | | | | | | | | | | s->iothread is checked for NULL on object creation in colo_compare_complete, so it's guaranteed not to be NULL. This resolves a false alert from Coverity (CID 1429969). Signed-off-by: Lukas Straub <lukasstraub2@web.de> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Reviewed-by: Li Qiang <liq3ea@gmail.com> Reviewed-by: Zhang Chen <chen.zhang@intel.com> Signed-off-by: Jason Wang <jasowang@redhat.com>
* net: forbid the reentrant RXJason Wang2020-07-281-0/+3
| | | | | | | | | | | | The memory API allows DMA into NIC's MMIO area. This means the NIC's RX routine must be reentrant. Instead of auditing all the NIC, we can simply detect the reentrancy and return early. The queue->delivering is set and cleared by qemu_net_queue_deliver() for other queue helpers to know whether the delivering in on going (NIC's receive is being called). We can check it and return early in qemu_net_queue_flush() to forbid reentrant RX. Signed-off-by: Jason Wang <jasowang@redhat.com>
* qom: Change object_get_canonical_path_component() not to mallocMarkus Armbruster2020-07-211-4/+2Star
| | | | | | | | | | | | | | | | | | | object_get_canonical_path_component() returns a malloced copy of a property name on success, null on failure. 19 of its 25 callers immediately free the returned copy. Change object_get_canonical_path_component() to return the property name directly. Since modifying the name would be wrong, adjust the return type to const char *. Drop the free from the 19 callers become simpler, add the g_strdup() to the other six. Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20200714160202.3121879-4-armbru@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Reviewed-by: Li Qiang <liq3ea@gmail.com>
* net: detect errors from probing vnet hdr flag for TAP devicesDaniel P. Berrange2020-07-156-12/+29
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When QEMU sets up a tap based network device backend, it mostly ignores errors reported from various ioctl() calls it makes, assuming the TAP file descriptor is valid. This assumption can easily be violated when the user is passing in a pre-opened file descriptor. At best, the ioctls may fail with a -EBADF, but if the user passes in a bogus FD number that happens to clash with a FD number that QEMU has opened internally for another reason, a wide variety of errnos may result, as the TUNGETIFF ioctl number may map to a completely different command on a different type of file. By ignoring all these errors, QEMU sets up a zombie network backend that will never pass any data. Even worse, when QEMU shuts down, or that network backend is hot-removed, it will close this bogus file descriptor, which could belong to another QEMU device backend. There's no obvious guaranteed reliable way to detect that a FD genuinely is a TAP device, as opposed to a UNIX socket, or pipe, or something else. Checking the errno from probing vnet hdr flag though, does catch the big common cases. ie calling TUNGETIFF will return EBADF for an invalid FD, and ENOTTY when FD is a UNIX socket, or pipe which catches accidental collisions with FDs used for stdio, or monitor socket. Previously the example below where bogus fd 9 collides with the FD used for the chardev saw: $ ./x86_64-softmmu/qemu-system-x86_64 -netdev tap,id=hostnet0,fd=9 \ -chardev socket,id=charchannel0,path=/tmp/qga,server,nowait \ -monitor stdio -vnc :0 qemu-system-x86_64: -netdev tap,id=hostnet0,fd=9: TUNGETIFF ioctl() failed: Inappropriate ioctl for device TUNSETOFFLOAD ioctl() failed: Bad address QEMU 2.9.1 monitor - type 'help' for more information (qemu) Warning: netdev hostnet0 has no peer which gives a running QEMU with a zombie network backend. With this change applied we get an error message and QEMU immediately exits before carrying on and making a bigger disaster: $ ./x86_64-softmmu/qemu-system-x86_64 -netdev tap,id=hostnet0,fd=9 \ -chardev socket,id=charchannel0,path=/tmp/qga,server,nowait \ -monitor stdio -vnc :0 qemu-system-x86_64: -netdev tap,id=hostnet0,vhost=on,fd=9: Unable to query TUNGETIFF on FD 9: Inappropriate ioctl for device Reported-by: Dr. David Alan Gilbert <dgilbert@redhat.com> Signed-off-by: Daniel P. Berrange <berrange@redhat.com> Tested-by: Dr. David Alan Gilbert <dgilbert@redhat.com> Message-id: 20171027085548.3472-1-berrange@redhat.com [lv: to simplify, don't check on EINVAL with TUNGETIFF as it exists since v2.6.27] Signed-off-by: Laurent Vivier <lvivier@redhat.com> Signed-off-by: Jason Wang <jasowang@redhat.com>
* net: check if the file descriptor is valid before using itLaurent Vivier2020-07-152-6/+28
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | qemu_set_nonblock() checks that the file descriptor can be used and, if not, crashes QEMU. An assert() is used for that. The use of assert() is used to detect programming error and the coredump will allow to debug the problem. But in the case of the tap device, this assert() can be triggered by a misconfiguration by the user. At startup, it's not a real problem, but it can also happen during the hot-plug of a new device, and here it's a problem because we can crash a perfectly healthy system. For instance: # ip link add link virbr0 name macvtap0 type macvtap mode bridge # ip link set macvtap0 up # TAP=/dev/tap$(ip -o link show macvtap0 | cut -d: -f1) # qemu-system-x86_64 -machine q35 -device pcie-root-port,id=pcie-root-port-0 -monitor stdio 9<> $TAP (qemu) netdev_add type=tap,id=hostnet0,vhost=on,fd=9 (qemu) device_add driver=virtio-net-pci,netdev=hostnet0,id=net0,bus=pcie-root-port-0 (qemu) device_del net0 (qemu) netdev_del hostnet0 (qemu) netdev_add type=tap,id=hostnet1,vhost=on,fd=9 qemu-system-x86_64: .../util/oslib-posix.c:247: qemu_set_nonblock: Assertion `f != -1' failed. Aborted (core dumped) To avoid that, add a function, qemu_try_set_nonblock(), that allows to report the problem without crashing. In the same way, we also update the function for vhostfd in net_init_tap_one() and for fd in net_init_socket() (both descriptors are provided by the user and can be wrong). Signed-off-by: Laurent Vivier <lvivier@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Signed-off-by: Jason Wang <jasowang@redhat.com>
* net/colo-compare.c: Expose compare "max_queue_size" to usersZhang Chen2020-07-151-1/+42
| | | | | | | | This patch allow users to set the "max_queue_size" according to their environment. Signed-off-by: Zhang Chen <chen.zhang@intel.com> Signed-off-by: Jason Wang <jasowang@redhat.com>
* error: Eliminate error_propagate() with Coccinelle, part 2Markus Armbruster2020-07-101-2/+1Star
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When all we do with an Error we receive into a local variable is propagating to somewhere else, we can just as well receive it there right away. The previous commit did that with a Coccinelle script I consider fairly trustworthy. This commit uses the same script with the matching of return taken out, i.e. we convert if (!foo(..., &err)) { ... error_propagate(errp, err); ... } to if (!foo(..., errp)) { ... ... } This is unsound: @err could still be read between afterwards. I don't know how to express "no read of @err without an intervening write" in Coccinelle. Instead, I manually double-checked for uses of @err. Suboptimal line breaks tweaked manually. qdev_realize() simplified further to placate scripts/checkpatch.pl. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-Id: <20200707160613.848843-36-armbru@redhat.com>
* error: Eliminate error_propagate() with Coccinelle, part 1Markus Armbruster2020-07-104-14/+5Star
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When all we do with an Error we receive into a local variable is propagating to somewhere else, we can just as well receive it there right away. Convert if (!foo(..., &err)) { ... error_propagate(errp, err); ... return ... } to if (!foo(..., errp)) { ... ... return ... } where nothing else needs @err. Coccinelle script: @rule1 forall@ identifier fun, err, errp, lbl; expression list args, args2; binary operator op; constant c1, c2; symbol false; @@ if ( ( - fun(args, &err, args2) + fun(args, errp, args2) | - !fun(args, &err, args2) + !fun(args, errp, args2) | - fun(args, &err, args2) op c1 + fun(args, errp, args2) op c1 ) ) { ... when != err when != lbl: when strict - error_propagate(errp, err); ... when != err ( return; | return c2; | return false; ) } @rule2 forall@ identifier fun, err, errp, lbl; expression list args, args2; expression var; binary operator op; constant c1, c2; symbol false; @@ - var = fun(args, &err, args2); + var = fun(args, errp, args2); ... when != err if ( ( var | !var | var op c1 ) ) { ... when != err when != lbl: when strict - error_propagate(errp, err); ... when != err ( return; | return c2; | return false; | return var; ) } @depends on rule1 || rule2@ identifier err; @@ - Error *err = NULL; ... when != err Not exactly elegant, I'm afraid. The "when != lbl:" is necessary to avoid transforming if (fun(args, &err)) { goto out } ... out: error_propagate(errp, err); even though other paths to label out still need the error_propagate(). For an actual example, see sclp_realize(). Without the "when strict", Coccinelle transforms vfio_msix_setup(), incorrectly. I don't know what exactly "when strict" does, only that it helps here. The match of return is narrower than what I want, but I can't figure out how to express "return where the operand doesn't use @err". For an example where it's too narrow, see vfio_intx_enable(). Silently fails to convert hw/arm/armsse.c, because Coccinelle gets confused by ARMSSE being used both as typedef and function-like macro there. Converted manually. Line breaks tidied up manually. One nested declaration of @local_err deleted manually. Preexisting unwanted blank line dropped in hw/riscv/sifive_e.c. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-Id: <20200707160613.848843-35-armbru@redhat.com>
* error: Avoid unnecessary error_propagate() after error_setg()Markus Armbruster2020-07-103-24/+16Star
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Replace error_setg(&err, ...); error_propagate(errp, err); by error_setg(errp, ...); Related pattern: if (...) { error_setg(&err, ...); goto out; } ... out: error_propagate(errp, err); return; When all paths to label out are that way, replace by if (...) { error_setg(errp, ...); return; } and delete the label along with the error_propagate(). When we have at most one other path that actually needs to propagate, and maybe one at the end that where propagation is unnecessary, e.g. foo(..., &err); if (err) { goto out; } ... bar(..., &err); out: error_propagate(errp, err); return; move the error_propagate() to where it's needed, like if (...) { foo(..., &err); error_propagate(errp, err); return; } ... bar(..., errp); return; and transform the error_setg() as above. In some places, the transformation results in obviously unnecessary error_propagate(). The next few commits will eliminate them. Bonus: the elimination of gotos will make later patches in this series easier to review. Candidates for conversion tracked down with this Coccinelle script: @@ identifier err, errp; expression list args; @@ - error_setg(&err, args); + error_setg(errp, args); ... when != err error_propagate(errp, err); Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-Id: <20200707160613.848843-34-armbru@redhat.com>
* qom: Put name parameter before value / visitor parameterMarkus Armbruster2020-07-102-2/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The object_property_set_FOO() setters take property name and value in an unusual order: void object_property_set_FOO(Object *obj, FOO_TYPE value, const char *name, Error **errp) Having to pass value before name feels grating. Swap them. Same for object_property_set(), object_property_get(), and object_property_parse(). Convert callers with this Coccinelle script: @@ identifier fun = { object_property_get, object_property_parse, object_property_set_str, object_property_set_link, object_property_set_bool, object_property_set_int, object_property_set_uint, object_property_set, object_property_set_qobject }; expression obj, v, name, errp; @@ - fun(obj, v, name, errp) + fun(obj, name, v, errp) Chokes on hw/arm/musicpal.c's lcd_refresh() with the unhelpful error message "no position information". Convert that one manually. Fails to convert hw/arm/armsse.c, because Coccinelle gets confused by ARMSSE being used both as typedef and function-like macro there. Convert manually. Fails to convert hw/rx/rx-gdbsim.c, because Coccinelle gets confused by RXCPU being used both as typedef and function-like macro there. Convert manually. The other files using RXCPU that way don't need conversion. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20200707160613.848843-27-armbru@redhat.com> [Straightforwad conflict with commit 2336172d9b "audio: set default value for pcspk.iobase property" resolved]
* qapi: Use returned bool to check for failure, manual partMarkus Armbruster2020-07-101-6/+2Star
| | | | | | | | | | | | The previous commit used Coccinelle to convert from checking the Error object to checking the return value. Convert a few more manually. Also tweak control flow in places to conform to the conventional "if error bail out" pattern. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20200707160613.848843-20-armbru@redhat.com>
* qapi: Use returned bool to check for failure, Coccinelle partMarkus Armbruster2020-07-103-8/+4Star
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The previous commit enables conversion of visit_foo(..., &err); if (err) { ... } to if (!visit_foo(..., errp)) { ... } for visitor functions that now return true / false on success / error. Coccinelle script: @@ identifier fun =~ "check_list|input_type_enum|lv_start_struct|lv_type_bool|lv_type_int64|lv_type_str|lv_type_uint64|output_type_enum|parse_type_bool|parse_type_int64|parse_type_null|parse_type_number|parse_type_size|parse_type_str|parse_type_uint64|print_type_bool|print_type_int64|print_type_null|print_type_number|print_type_size|print_type_str|print_type_uint64|qapi_clone_start_alternate|qapi_clone_start_list|qapi_clone_start_struct|qapi_clone_type_bool|qapi_clone_type_int64|qapi_clone_type_null|qapi_clone_type_number|qapi_clone_type_str|qapi_clone_type_uint64|qapi_dealloc_start_list|qapi_dealloc_start_struct|qapi_dealloc_type_anything|qapi_dealloc_type_bool|qapi_dealloc_type_int64|qapi_dealloc_type_null|qapi_dealloc_type_number|qapi_dealloc_type_str|qapi_dealloc_type_uint64|qobject_input_check_list|qobject_input_check_struct|qobject_input_start_alternate|qobject_input_start_list|qobject_input_start_struct|qobject_input_type_any|qobject_input_type_bool|qobject_input_type_bool_keyval|qobject_input_type_int64|qobject_input_type_int64_keyval|qobject_input_type_null|qobject_input_type_number|qobject_input_type_number_keyval|qobject_input_type_size_keyval|qobject_input_type_str|qobject_input_type_str_keyval|qobject_input_type_uint64|qobject_input_type_uint64_keyval|qobject_output_start_list|qobject_output_start_struct|qobject_output_type_any|qobject_output_type_bool|qobject_output_type_int64|qobject_output_type_null|qobject_output_type_number|qobject_output_type_str|qobject_output_type_uint64|start_list|visit_check_list|visit_check_struct|visit_start_alternate|visit_start_list|visit_start_struct|visit_type_.*"; expression list args; typedef Error; Error *err; @@ - fun(args, &err); - if (err) + if (!fun(args, &err)) { ... } A few line breaks tidied up manually. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20200707160613.848843-19-armbru@redhat.com>
* Merge remote-tracking branch ↵Peter Maydell2020-07-091-0/+1
|\ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 'remotes/vivier2/tags/trivial-branch-for-5.1-pull-request' into staging trivial branch patches 20200707 # gpg: Signature made Tue 07 Jul 2020 11:52:06 BST # gpg: using RSA key CD2F75DDC8E3A4DC2E4F5173F30C38BD3F2FBE3C # gpg: issuer "laurent@vivier.eu" # gpg: Good signature from "Laurent Vivier <lvivier@redhat.com>" [full] # gpg: aka "Laurent Vivier <laurent@vivier.eu>" [full] # gpg: aka "Laurent Vivier (Red Hat) <lvivier@redhat.com>" [full] # Primary key fingerprint: CD2F 75DD C8E3 A4DC 2E4F 5173 F30C 38BD 3F2F BE3C * remotes/vivier2/tags/trivial-branch-for-5.1-pull-request: net/tap-solaris.c: Include qemu-common.h for TFR macro intel_iommu: "aw-bits" error message still refers to "x-aw-bits" util/qemu-option: Document the get_opt_value() function MAINTAINERS: Update Radoslaw Biernacki email address .mailmap: Update Alexander Graf email address trivial: Respect alphabetical order of .o files in Makefile.objs fix the prototype of muls64/mulu64 Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
| * net/tap-solaris.c: Include qemu-common.h for TFR macroPeter Maydell2020-07-071-0/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | In commit a8d2532645cf5ce4 we cleaned up usage of the qemu-common.h header so that it was always included from .c files and never from other .h files. We missed adding it to net/tap-solaris.c (which previously was pulling it in via tap-int.h), which broke building on Solaris hosts. Fixes: a8d2532645cf5ce4 Reported-by: Michele Denber <denber@mindspring.com> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Thomas Huth <thuth@redhat.com> Tested-by: Michele Denber <denber@mindspring.com> Message-Id: <20200704092317.12943-1-peter.maydell@linaro.org> Signed-off-by: Laurent Vivier <laurent@vivier.eu>
* | vhost-vdpa: introduce vhost-vdpa net clientCindy Lu2020-07-074-1/+234
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This patch set introduces a new net client type: vhost-vdpa. vhost-vdpa net client will set up a vDPA device which is specified by a "vhostdev" parameter. Signed-off-by: Lingshan Zhu <lingshan.zhu@intel.com> Signed-off-by: Tiwei Bie <tiwei.bie@intel.com> Signed-off-by: Cindy Lu <lulu@redhat.com> Signed-off-by: Jason Wang <jasowang@redhat.com> Message-Id: <20200701145538.22333-15-lulu@redhat.com> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> Signed-off-by: Michael S. Tsirkin <mst@redhat.com> Acked-by: Jason Wang <jasowang@redhat.com>
* | net: introduce qemu_get_peerCindy Lu2020-07-031-0/+7
|/ | | | | | | | | | | This is a small function that can get the peer from given NetClientState and queue_index Signed-off-by: Cindy Lu <lulu@redhat.com> Message-Id: <20200701145538.22333-2-lulu@redhat.com> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> Signed-off-by: Michael S. Tsirkin <mst@redhat.com> Acked-by: Jason Wang <jasowang@redhat.com>
* net: Drop the NetLegacy structure, always use Netdev insteadThomas Huth2020-06-181-64/+13Star
| | | | | | | | | | | | | | | | | | | | Now that the "name" parameter is gone, there is hardly any difference between NetLegacy and Netdev anymore, so we can drop NetLegacy and always use Netdev to simplify the code quite a bit. The only two differences that were really left between Netdev and NetLegacy: 1) NetLegacy does not allow a "hubport" type. We can continue to block this with a simple check in net_client_init1() for this type. 2) The "id" parameter was optional in NetLegacy (and an internal id was chosen via assign_name() during initialization), but it is mandatory for Netdev. To avoid that the visitor code bails out here, we have to add an internal id to the QemuOpts already earlier now. Signed-off-by: Thomas Huth <thuth@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Jason Wang <jasowang@redhat.com>
* net: Drop the legacy "name" parameter from the -net optionThomas Huth2020-06-181-9/+1Star
| | | | | | | | | It's been deprecated since QEMU v3.1, so it's time to finally remove it. The "id" parameter can simply be used instead. Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Thomas Huth <thuth@redhat.com> Signed-off-by: Jason Wang <jasowang@redhat.com>
* colo-compare: Fix memory leak in packet_enqueue()Derek Su2020-06-182-8/+16
| | | | | | | | | | | | | | The patch is to fix the "pkt" memory leak in packet_enqueue(). The allocated "pkt" needs to be freed if the colo compare primary or secondary queue is too big. Replace the error_report of full queue with a trace event. Signed-off-by: Derek Su <dereksu@qnap.com> Reviewed-by: Zhang Chen <chen.zhang@intel.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Signed-off-by: Zhang Chen <chen.zhang@intel.com> Signed-off-by: Jason Wang <jasowang@redhat.com>
* net/colo-compare.c: Correct ordering in complete and finalizeLukas Straub2020-06-181-22/+23
| | | | | | | | | | | | In colo_compare_complete, insert CompareState into net_compares only after everything has been initialized. In colo_compare_finalize, remove CompareState from net_compares before anything is deinitialized. Signed-off-by: Lukas Straub <lukasstraub2@web.de> Reviewed-by: Zhang Chen <chen.zhang@intel.com> Signed-off-by: Zhang Chen <chen.zhang@intel.com> Signed-off-by: Jason Wang <jasowang@redhat.com>
* net/colo-compare.c: Check that colo-compare is activeLukas Straub2020-06-181-6/+29
| | | | | | | | | | | | | | | | | | | | | | | If the colo-compare object is removed before failover and a checkpoint happens, qemu crashes because it tries to lock the destroyed event_mtx in colo_notify_compares_event. Fix this by checking if everything is initialized by introducing a new variable colo_compare_active which is protected by a new mutex colo_compare_mutex. The new mutex also protects against concurrent access of the net_compares list and makes sure that colo_notify_compares_event isn't active while we destroy event_mtx and event_complete_cond. With this it also is again possible to use colo without colo-compare (periodic mode) and to use multiple colo-compare for multiple network interfaces. Signed-off-by: Lukas Straub <lukasstraub2@web.de> Tested-by: Lukas Straub <lukasstraub2@web.de> Reviewed-by: Zhang Chen <chen.zhang@intel.com> Signed-off-by: Zhang Chen <chen.zhang@intel.com> Signed-off-by: Jason Wang <jasowang@redhat.com>
* net/colo-compare.c: Only hexdump packets if tracing is enabledLukas Straub2020-06-181-4/+6
| | | | | | | | | | | | Else the log will be flooded if there is a lot of network traffic. Signed-off-by: Lukas Straub <lukasstraub2@web.de> Reviewed-by: Zhang Chen <chen.zhang@intel.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com> Signed-off-by: Zhang Chen <chen.zhang@intel.com> Signed-off-by: Jason Wang <jasowang@redhat.com>
* net/colo-compare.c: Fix deadlock in compare_chr_sendLukas Straub2020-06-183-45/+156
| | | | | | | | | | | | | | | | | | | The chr_out chardev is connected to a filter-redirector running in the main loop. qemu_chr_fe_write_all might block here in compare_chr_send if the (socket-)buffer is full. If another filter-redirector in the main loop want's to send data to chr_pri_in it might also block if the buffer is full. This leads to a deadlock because both event loops get blocked. Fix this by converting compare_chr_send to a coroutine and putting the packets in a send queue. Signed-off-by: Lukas Straub <lukasstraub2@web.de> Reviewed-by: Zhang Chen <chen.zhang@intel.com> Tested-by: Zhang Chen <chen.zhang@intel.com> Signed-off-by: Zhang Chen <chen.zhang@intel.com> Signed-off-by: Jason Wang <jasowang@redhat.com>
* net/colo-compare.c: Create event_bh with the right AioContextLukas Straub2020-06-181-1/+2
| | | | | | | | | | | | | | | | | | qemu_bh_new will set the bh to be executed in the main loop. This causes crashes as colo_compare_handle_event assumes that it has exclusive access the queues, which are also concurrently accessed in the iothread. Create the bh with the AioContext of the iothread to fulfill these assumptions and fix the crashes. This is safe, because the bh already takes the appropriate locks. Signed-off-by: Lukas Straub <lukasstraub2@web.de> Reviewed-by: Zhang Chen <chen.zhang@intel.com> Reviewed-by: Derek Su <dereksu@qnap.com> Tested-by: Derek Su <dereksu@qnap.com> Signed-off-by: Zhang Chen <chen.zhang@intel.com> Signed-off-by: Jason Wang <jasowang@redhat.com>
* net: use peer when purging queue in qemu_flush_or_purge_queue_packets()Jason Wang2020-06-181-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | The sender of packet will be checked in the qemu_net_queue_purge() but we use NetClientState not its peer when trying to purge the incoming queue in qemu_flush_or_purge_packets(). This will trigger the assert in virtio_net_reset since we can't pass the sender check: hw/net/virtio-net.c:533: void virtio_net_reset(VirtIODevice *): Assertion `!virtio_net_get_subqueue(nc)->async_tx.elem' failed. #9 0x55a33fa31b78 in virtio_net_reset hw/net/virtio-net.c:533:13 #10 0x55a33fc88412 in virtio_reset hw/virtio/virtio.c:1919:9 #11 0x55a341d82764 in virtio_bus_reset hw/virtio/virtio-bus.c:95:9 #12 0x55a341dba2de in virtio_pci_reset hw/virtio/virtio-pci.c:1824:5 #13 0x55a341db3e02 in virtio_pci_common_write hw/virtio/virtio-pci.c:1252:13 #14 0x55a33f62117b in memory_region_write_accessor memory.c:496:5 #15 0x55a33f6205e4 in access_with_adjusted_size memory.c:557:18 #16 0x55a33f61e177 in memory_region_dispatch_write memory.c:1488:16 Reproducer: https://www.mail-archive.com/qemu-devel@nongnu.org/msg701914.html Fix by using the peer. Reported-by: "Alexander Bulekov" <alxndr@bu.edu> Acked-by: Alexander Bulekov <alxndr@bu.edu> Fixes: ca77d85e1dbf9 ("net: complete all queued packets on VM stop") Cc: qemu-stable@nongnu.org Signed-off-by: Jason Wang <jasowang@redhat.com>
* tap: allow extended virtio header with hash infoYuri Benditovich2020-06-181-1/+2
| | | | | Signed-off-by: Yuri Benditovich <yuri.benditovich@daynix.com> Signed-off-by: Jason Wang <jasowang@redhat.com>
* net: Do not include a newline in the id of -nic devicesThomas Huth2020-06-091-1/+1
| | | | | | | | | | | The '\n' sneaked in by accident here, an "id" string should really not contain a newline character at the end. Fixes: 78cd6f7bf6b ('net: Add a new convenience option "--nic" ...') Signed-off-by: Thomas Huth <thuth@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Message-Id: <20200518074352.23125-1-thuth@redhat.com> Signed-off-by: Laurent Vivier <laurent@vivier.eu>
* qom: Drop parameter @errp of object_property_add() & friendsMarkus Armbruster2020-05-158-35/+24Star
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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]
* net/colo-compare.c: Expose "expired_scan_cycle" to usersZhang Chen2020-03-311-3/+45
| | | | | | | | The "expired_scan_cycle" determines period of scanning expired primary node net packets. Signed-off-by: Zhang Chen <chen.zhang@intel.com> Signed-off-by: Jason Wang <jasowang@redhat.com>
* net/colo-compare.c: Expose "compare_timeout" to usersZhang Chen2020-03-311-2/+45
| | | | | | | | | | | | | | | | | | | The "compare_timeout" determines the maximum time to hold the primary net packet. This patch expose the "compare_timeout", make user have ability to adjest the value according to application scenarios. QMP command demo: { "execute": "qom-get", "arguments": { "path": "/objects/comp0", "property": "compare_timeout" } } { "execute": "qom-set", "arguments": { "path": "/objects/comp0", "property": "compare_timeout", "value": 5000} } Signed-off-by: Zhang Chen <chen.zhang@intel.com> Signed-off-by: Jason Wang <jasowang@redhat.com>
* hw/net/can: Make CanBusClientInfo::can_receive() return a booleanPhilippe Mathieu-Daudé2020-03-311-2/+2
| | | | | | | | | | | The CanBusClientInfo::can_receive handler return whether the device can or can not receive new frames. Make it obvious by returning a boolean type. Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> Reviewed-by: Alistair Francis <alistair.francis@wdc.com> Reviewed-by: Cédric Le Goater <clg@kaod.org> Signed-off-by: Jason Wang <jasowang@redhat.com>
* hw/net: Make NetCanReceive() return a booleanPhilippe Mathieu-Daudé2020-03-312-4/+4
| | | | | | | | | | | | The NetCanReceive handler return whether the device can or can not receive new packets. Make it obvious by returning a boolean type. Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> Acked-by: David Gibson <david@gibson.dropbear.id.au> Reviewed-by: Alistair Francis <alistair.francis@wdc.com> Reviewed-by: Cédric Le Goater <clg@kaod.org> Signed-off-by: Jason Wang <jasowang@redhat.com>
* Merge remote-tracking branch 'remotes/armbru/tags/pull-qapi-2020-03-17' into ↵Peter Maydell2020-03-191-27/+12Star
|\ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | staging QAPI patches for 2020-03-17 # gpg: Signature made Tue 17 Mar 2020 20:50:54 GMT # gpg: using RSA key 354BC8B3D7EB2A6B68674E5F3870B400EB918653 # gpg: issuer "armbru@redhat.com" # gpg: Good signature from "Markus Armbruster <armbru@redhat.com>" [full] # gpg: aka "Markus Armbruster <armbru@pond.sub.org>" [full] # Primary key fingerprint: 354B C8B3 D7EB 2A6B 6867 4E5F 3870 B400 EB91 8653 * remotes/armbru/tags/pull-qapi-2020-03-17: (30 commits) net: Track netdevs in NetClientState rather than QemuOpt net: Complete qapi-fication of netdev_add qmp: constify QmpCommand and list qapi: Mark deprecated QMP parts with feature 'deprecated' qapi: New special feature flag "deprecated" qapi: Replace qmp_dispatch()'s TODO comment by an explanation qapi: Simplify how qmp_dispatch() gets the request ID qapi: Simplify how qmp_dispatch() deals with QCO_NO_SUCCESS_RESP qapi: Inline do_qmp_dispatch() into qmp_dispatch() qapi: Add feature flags to struct members qapi/schema: Call QAPIDoc.connect_member() in just one place qapi/schema: Rename QAPISchemaObjectType{Variant,Variants} qapi/schema: Reorder classes so related ones are together qapi/schema: Change _make_features() to a take feature list qapi/introspect: Factor out _make_tree() qapi/introspect: Rename *qlit* to reduce confusion qapi: Consistently put @features parameter right after @ifcond qapi: Add feature flags to remaining definitions qapi/schema: Clean up around QAPISchemaEntity.connect_doc() tests/test-qmp-event: Check event is actually emitted ... Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
| * net: Track netdevs in NetClientState rather than QemuOptEric Blake2020-03-171-26/+11Star
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | As mentioned in the previous patch, our use of QemuOpt group "netdev" has two purposes: collect the CLI arguments, and serve as a witness for monitor hotplug actions. As the latter didn't use anything but an id, it felt rather unclean to have to touch QemuOpts at all when going through QMP, so let's instead track things with a bool field in NetClientState. Suggested-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <20200317201711.322764-3-eblake@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
| * net: Complete qapi-fication of netdev_addEric Blake2020-03-171-3/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | We've had all the required pieces for doing a type-safe representation of netdev_add as a flat union for quite some time now (since 0e55c381f6 in v2.7.0, released in 2016), but did not make the final switch to using it because of concern about whether a command-line regression in accepting "1" in place of 1 for integer arguments would be problematic. Back then, we did not have the deprecation cycle to allow us to make progress. But now that we have waited so long, other problems have crept in: for example, our desire to add qemu-storage-daemon is hampered by the inability to express net objects, and we are unable to introspect what we actually accept. Additionally, our round-trip through QemuOpts silently eats any argument that expands to an array, rendering dnssearch, hostfwd, and guestfwd useless through QMP: {"execute": "netdev_add", "arguments": { "id": "netdev0", "type": "user", "dnssearch": [ { "str": "8.8.8.8" }, { "str": "8.8.4.4" } ]}} So without further ado, let's turn on proper QAPI. netdev_add() was a trivial wrapper around net_client_init(), which did a few steps prior to calling net_client_init1(); with this patch, we now skip directly to net_client_init1(). In addition to fixing array parameters, the following additional differences occur: - {"execute": "netdev_add", "arguments": {"type": "help"}} no longer attempts to print help to stdout and exit. Bug fix, broken in 547203ead4 'net: List available netdevs with "-netdev help"', v2.12.0. - {"execute": "netdev_add", "arguments': {... "ipv6-net": "..." }} no longer attempts to desugar the undocumented ipv6-net magic string into the proper "ipv6-prefix" and "ipv6-prefixlen". Undocumented misfeature, introduced in commit 7aac531ef2 "qapi-schema, qemu-options & slirp: Adding Qemu options for IPv6 addresses", v2.6.0. - {'execute':'netdev_add', 'arguments':{'id':'net2', 'type':'hubport', 'hubid':"2"}} {"error": {"class": "GenericError", "desc": "Invalid parameter type for 'hubid', expected: integer"}} Used to succeed: since our command line treats everything as strings, our not-so-round-trip conversion from QAPI -> QemuOpts -> QAPI lost the original typing and turned everything into a string; now that we skip the QemuOpts, the JSON input has to match the exact QAPI type. But this stricter QMP is desirable, and introspection is sufficient for any affected applications to make sure they use it correctly. In qmp_netdev_add(), we still have to create a QemuOpts object so that qmp_netdev_del() will be able to remove a hotplugged network device; but the opts->head remains empty since we now manage all parsing through the QAPI object rather than QemuOpts; a separate patch will address the abuse of QemuOpts as a witness for whether a NetClientState is a netdev. In the meantime, our argument that we are okay requires auditing all uses of option group "netdev": - qemu_netdev_opts: option group definition, empty .desc[] - CLI (CLI netdev parsing ends before monitors start, so while monitors can mess with CLI netdevs, CLI cannot mess with monitor netdevs): - main() case QEMU_OPTION_netdev: store CLI definition - main() case QEMU_OPTION_readconfig, case QEMU_OPTION_writeconfig: similar, dealing only with CLI - net_init_clients(): Pass CLI to net_client_init() - Monitor: - hmp_netdev_add(): straightforward parse into net_client_init() - qmp_netdev_add(): subject of this patch, used to add full object to option group, now just adds bare-bones id - qmp_netdev_del(), netdev_del_completion(): check the option group solely for id, as a 'is this a netdev' predicate Reported-by: Alex Kirillov <lekiravi@yandex-team.ru> Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <20200317201711.322764-2-eblake@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> [Commit message typo fixed] Signed-off-by: Markus Armbruster <armbru@redhat.com>
* | misc: Replace zero-length arrays with flexible array member (automatic)Philippe Mathieu-Daudé2020-03-161-1/+1
|/ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Description copied from Linux kernel commit from Gustavo A. R. Silva (see [3]): --v-- description start --v-- The current codebase makes use of the zero-length array language extension to the C90 standard, but the preferred mechanism to declare variable-length types such as these ones is a flexible array member [1], introduced in C99: struct foo { int stuff; struct boo array[]; }; By making use of the mechanism above, we will get a compiler warning in case the flexible array does not occur last in the structure, which will help us prevent some kind of undefined behavior bugs from being unadvertenly introduced [2] to the Linux codebase from now on. --^-- description end --^-- Do the similar housekeeping in the QEMU codebase (which uses C99 since commit 7be41675f7cb). All these instances of code were found with the help of the following Coccinelle script: @@ identifier s, m, a; type t, T; @@ struct s { ... t m; - T a[0]; + T a[]; }; @@ identifier s, m, a; type t, T; @@ struct s { ... t m; - T a[0]; + T a[]; } QEMU_PACKED; [1] https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html [2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=76497732932f [3] https://git.kernel.org/pub/scm/linux/kernel/git/gustavoars/linux.git/commit/?id=17642a2fbd2c1 Inspired-by: Gustavo A. R. Silva <gustavo@embeddedor.com> Reviewed-by: David Hildenbrand <david@redhat.com> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
* net: Remove deprecated [hub_id name] tuple of 'hostfwd_add' / 'hostfwd_remove'Thomas Huth2020-03-093-57/+12Star
| | | | | | | | | It's been deprecated since QEMU v3.1.0. Time to finally remove it now. Signed-off-by: Thomas Huth <thuth@redhat.com> Message-Id: <20191205104109.18680-1-thuth@redhat.com> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com> Reworked Thomas's deprecated.texi to the rst
* net/filter.c: Add Options to insert filters anywhere in the filter listLukas Straub2020-03-031-1/+91
| | | | | | | | | | | | | | | | | | To switch the Secondary to Primary, we need to insert new filters before the filter-rewriter. Add the options insert= and position= to be able to insert filters anywhere in the filter list. position should be "head" or "tail" to insert at the head or tail of the filter list or it should be "id=<id>" to specify the id of another filter. insert should be either "before" or "behind" to specify where to insert the new filter relative to the one specified with position. Signed-off-by: Lukas Straub <lukasstraub2@web.de> Reviewed-by: Zhang Chen <chen.zhang@intel.com> Signed-off-by: Jason Wang <jasowang@redhat.com>