summaryrefslogtreecommitdiffstats
path: root/hw/usb/bus.c
Commit message (Collapse)AuthorAgeFilesLines
* usb/bus: Remove dead assignment in usb_get_fw_dev_path()Chen Qun2020-09-011-2/+2
| | | | | | | | | | | | | Clang static code analyzer show warning: qemu/hw/usb/bus.c:615:13: warning: Value stored to 'pos' is never read pos += snprintf(fw_path + pos, fw_len - pos, "%s@%lx", Reported-by: Euler Robot <euler.robot@huawei.com> Signed-off-by: Chen Qun <kuhn.chenqun@huawei.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Gerd Hoffmann <kraxel@redhat.com> Message-Id: <20200827110311.164316-9-kuhn.chenqun@huawei.com> Signed-off-by: Laurent Vivier <laurent@vivier.eu>
* error: Eliminate error_propagate() manuallyMarkus Armbruster2020-07-101-3/+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 two commits did that for sufficiently simple cases with Coccinelle. Do it for several more manually. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-Id: <20200707160613.848843-37-armbru@redhat.com>
* qdev: Use returned bool to check for qdev_realize() etc. failureMarkus Armbruster2020-07-101-2/+1Star
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Convert foo(..., &err); if (err) { ... } to if (!foo(..., &err)) { ... } for qdev_realize(), qdev_realize_and_unref(), qbus_realize() and their wrappers isa_realize_and_unref(), pci_realize_and_unref(), sysbus_realize(), sysbus_realize_and_unref(), usb_realize_and_unref(). Coccinelle script: @@ identifier fun = { isa_realize_and_unref, pci_realize_and_unref, qbus_realize, qdev_realize, qdev_realize_and_unref, sysbus_realize, sysbus_realize_and_unref, usb_realize_and_unref }; expression list args, args2; typedef Error; Error *err; @@ - fun(args, &err, args2); - if (err) + if (!fun(args, &err, args2)) { ... } Chokes on hw/arm/musicpal.c's lcd_refresh() with the unhelpful error message "no position information". Nothing to convert there; skipped. 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. 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> Reviewed-by: Greg Kurz <groug@kaod.org> Message-Id: <20200707160613.848843-5-armbru@redhat.com>
* qdev: Drop qbus_set_bus_hotplug_handler() parameter @errpMarkus Armbruster2020-07-021-1/+1
| | | | | | | | | | All callers pass &error_abort. Drop the parameter. Cc: Paolo Bonzini <pbonzini@redhat.com> Cc: "Daniel P. Berrangé" <berrange@redhat.com> Cc: Eduardo Habkost <ehabkost@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20200630090351.1247703-14-armbru@redhat.com>
* usb: Eliminate usb_try_create_simple()Markus Armbruster2020-06-151-22/+13Star
| | | | | | | | | | | | | | | usb_try_create_simple() is qdev_try_new() and qdev_realize_and_unref() with more verbose error messages. Of its two users, one ignores errors, and the other asserts they are impossible. Make them use qdev_try_new() and qdev_realize_and_unref() directly, and eliminate usb_try_create_simple Cc: Gerd Hoffmann <kraxel@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Gerd Hoffmann <kraxel@redhat.com> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> Message-Id: <20200610053247.1583243-30-armbru@redhat.com>
* usb: usb_create() is now unused, dropMarkus Armbruster2020-06-151-8/+0Star
| | | | | | | | Cc: Gerd Hoffmann <kraxel@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Gerd Hoffmann <kraxel@redhat.com> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> Message-Id: <20200610053247.1583243-29-armbru@redhat.com>
* usb: Convert uses of usb_create()Markus Armbruster2020-06-151-6/+5Star
| | | | | | | | | | | | | | | | | | | | | | | Replace dev = usb_create(bus, type_name); ... object_property_set_bool(OBJECT(dev), true, "realized", &err); by dev = isa_new(type_name); ... usb_realize_and_unref(dev, bus, &err); Recent commit "qdev: New qdev_new(), qdev_realize(), etc." explains why. Cc: Gerd Hoffmann <kraxel@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Gerd Hoffmann <kraxel@redhat.com> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> Message-Id: <20200610053247.1583243-28-armbru@redhat.com>
* usb: New usb_new(), usb_realize_and_unref()Markus Armbruster2020-06-151-0/+10
| | | | | | | | | | | | | | | | | | I'm converting from qdev_create()/qdev_init_nofail() to qdev_new()/qdev_realize_and_unref(); recent commit "qdev: New qdev_new(), qdev_realize(), etc." explains why. USB devices use qdev_create() through usb_create(). Provide usb_new() and usb_realize_and_unref() for converting USB devices. Cc: Gerd Hoffmann <kraxel@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Gerd Hoffmann <kraxel@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> Message-Id: <20200610053247.1583243-27-armbru@redhat.com>
* qdev: Convert uses of qdev_create() manuallyMarkus Armbruster2020-06-151-4/+4
| | | | | | | | | | Same transformation as in the previous commit. Manual, because convincing Coccinelle to transform these cases is somewhere between not worthwhile and infeasible (at least for me). Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> Message-Id: <20200610053247.1583243-11-armbru@redhat.com>
* qdev: Unrealize must not failMarkus Armbruster2020-05-151-6/+6
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Devices may have component devices and buses. Device realization may fail. Realization is recursive: a device's realize() method realizes its components, and device_set_realized() realizes its buses (which should in turn realize the devices on that bus, except bus_set_realized() doesn't implement that, yet). When realization of a component or bus fails, we need to roll back: unrealize everything we realized so far. If any of these unrealizes failed, the device would be left in an inconsistent state. Must not happen. device_set_realized() lets it happen: it ignores errors in the roll back code starting at label child_realize_fail. Since realization is recursive, unrealization must be recursive, too. But how could a partly failed unrealize be rolled back? We'd have to re-realize, which can fail. This design is fundamentally broken. device_set_realized() does not roll back at all. Instead, it keeps unrealizing, ignoring further errors. It can screw up even for a device with no buses: if the lone dc->unrealize() fails, it still unregisters vmstate, and calls listeners' unrealize() callback. bus_set_realized() does not roll back either. Instead, it stops unrealizing. Fortunately, no unrealize method can fail, as we'll see below. To fix the design error, drop parameter @errp from all the unrealize methods. Any unrealize method that uses @errp now needs an update. This leads us to unrealize() methods that can fail. Merely passing it to another unrealize method cannot cause failure, though. Here are the ones that do other things with @errp: * virtio_serial_device_unrealize() Fails when qbus_set_hotplug_handler() fails, but still does all the other work. On failure, the device would stay realized with its resources completely gone. Oops. Can't happen, because qbus_set_hotplug_handler() can't actually fail here. Pass &error_abort to qbus_set_hotplug_handler() instead. * hw/ppc/spapr_drc.c's unrealize() Fails when object_property_del() fails, but all the other work is already done. On failure, the device would stay realized with its vmstate registration gone. Oops. Can't happen, because object_property_del() can't actually fail here. Pass &error_abort to object_property_del() instead. * spapr_phb_unrealize() Fails and bails out when remove_drcs() fails, but other work is already done. On failure, the device would stay realized with some of its resources gone. Oops. remove_drcs() fails only when chassis_from_bus()'s object_property_get_uint() fails, and it can't here. Pass &error_abort to remove_drcs() instead. Therefore, no unrealize method can fail before this patch. device_set_realized()'s recursive unrealization via bus uses object_property_set_bool(). Can't drop @errp there, so pass &error_abort. We similarly unrealize with object_property_set_bool() elsewhere, always ignoring errors. Pass &error_abort instead. Several unrealize methods no longer handle errors from other unrealize methods: virtio_9p_device_unrealize(), virtio_input_device_unrealize(), scsi_qdev_unrealize(), ... Much of the deleted error handling looks wrong anyway. One unrealize methods no longer ignore such errors: usb_ehci_pci_exit(). Several realize methods no longer ignore errors when rolling back: v9fs_device_realize_common(), pci_qdev_unrealize(), spapr_phb_realize(), usb_qdev_realize(), vfio_ccw_realize(), virtio_device_realize(). Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> Message-Id: <20200505152926.18877-17-armbru@redhat.com>
* qom: Drop parameter @errp of object_property_add() & friendsMarkus Armbruster2020-05-151-4/+2Star
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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]
* qdev: set properties with device_class_set_props()Marc-André Lureau2020-01-241-1/+1
| | | | | | | | | | | | | | | | | | | | | The following patch will need to handle properties registration during class_init time. Let's use a device_class_set_props() setter. spatch --macro-file scripts/cocci-macro-file.h --sp-file ./scripts/coccinelle/qdev-set-props.cocci --keep-comments --in-place --dir . @@ typedef DeviceClass; DeviceClass *d; expression val; @@ - d->props = val + device_class_set_props(d, val) Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> Message-Id: <20200110153039.1379601-20-marcandre.lureau@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
* Include hw/qdev-properties.h lessMarkus Armbruster2019-08-161-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | In my "build everything" tree, changing hw/qdev-properties.h triggers a recompile of some 2700 out of 6600 objects (not counting tests and objects that don't depend on qemu/osdep.h). Many places including hw/qdev-properties.h (directly or via hw/qdev.h) actually need only hw/qdev-core.h. Include hw/qdev-core.h there instead. hw/qdev.h is actually pointless: all it does is include hw/qdev-core.h and hw/qdev-properties.h, which in turn includes hw/qdev-core.h. Replace the remaining uses of hw/qdev.h by hw/qdev-properties.h. While there, delete a few superfluous inclusions of hw/qdev-core.h. Touching hw/qdev-properties.h now recompiles some 1200 objects. Cc: Paolo Bonzini <pbonzini@redhat.com> Cc: "Daniel P. Berrangé" <berrange@redhat.com> Cc: Eduardo Habkost <ehabkost@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eduardo Habkost <ehabkost@redhat.com> Message-Id: <20190812052359.30071-22-armbru@redhat.com>
* Include hw/hw.h exactly where neededMarkus Armbruster2019-08-161-1/+0Star
| | | | | | | | | | | | | | | | In my "build everything" tree, changing hw/hw.h triggers a recompile of some 2600 out of 6600 objects (not counting tests and objects that don't depend on qemu/osdep.h). The previous commits have left only the declaration of hw_error() in hw/hw.h. This permits dropping most of its inclusions. Touching it now recompiles less than 200 objects. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Alistair Francis <alistair.francis@wdc.com> Message-Id: <20190812052359.30071-19-armbru@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
* Include migration/vmstate.h lessMarkus Armbruster2019-08-161-0/+1
| | | | | | | | | | | | | | | | | | In my "build everything" tree, changing migration/vmstate.h triggers a recompile of some 2700 out of 6600 objects (not counting tests and objects that don't depend on qemu/osdep.h). hw/hw.h supposedly includes it for convenience. Several other headers include it just to get VMStateDescription. The previous commit made that unnecessary. Include migration/vmstate.h only where it's still needed. Touching it now recompiles only some 1600 objects. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Alistair Francis <alistair.francis@wdc.com> Message-Id: <20190812052359.30071-16-armbru@redhat.com> Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
* Include qemu/module.h where needed, drop it from qemu-common.hMarkus Armbruster2019-06-121-0/+1
| | | | | | | | | 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]
* hw/usb/bus.c: Handle "no speed matched" case in usb_mask_to_str()Peter Maydell2019-04-011-0/+4
| | | | | | | | | | | | | | | | | | | | In usb_mask_to_str() we convert a mask of USB speeds into a human-readable string (like "full+high") for use in tracing and error messages. However the conversion code doesn't do anything to the string buffer if the passed in speedmask doesn't match any of the recognized speeds, which means that the tracing and error messages will end up with random garbage in them. This can happen if we're doing USB device passthrough. Handle the "unrecognized speed" case by using the string "unknown". Fixes: https://bugs.launchpad.net/qemu/+bug/1603785 Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Message-id: 20190328133503.6490-1-peter.maydell@linaro.org Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
* usb: drop unnecessary usb_device_post_load checksJonathan Davies2019-01-081-6/+0Star
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | In usb_device_post_load, certain values of dev->setup_len or dev->setup_index can cause -EINVAL to be returned. One example is when setup_len exceeds 4096, the hard-coded value of sizeof(dev->data_buf). This can happen through legitimate guest activity and will cause all subsequent attempts to migrate the guest to fail in vmstate_load_state. The values of these variables can be set by USB packets originating in the guest. There are two ways in which they can be set: in do_token_setup and in do_parameter in hw/usb/core.c. It is easy to craft a USB packet in a guest that causes do_token_setup to set setup_len to a value larger than 4096. When this has been done once, all subsequent attempts to migrate the VM will fail in usb_device_post_load until the VM is next power-cycled or a smaller-sized USB packet is sent to the device. Sample code for achieving this in a VM started with "-device usb-tablet" running Linux with CONFIG_HIDRAW=y and HID_MAX_BUFFER_SIZE > 4096: #include <sys/types.h> #include <sys/stat.h> #include <fcntl.h> #include <unistd.h> int main() { char buf[4097]; int fd = open("/dev/hidraw0", O_RDWR|O_NONBLOCK); buf[0] = 0x1; write(fd, buf, 4097); return 0; } When this code is run in the VM, qemu will output: usb_generic_handle_packet: ctrl buffer too small (4097 > 4096) A subsequent attempt to migrate the VM will fail and output the following on the destination host: qemu-kvm: error while loading state for instance 0x0 of device '0000:00:06.7/1/usb-ptr' qemu-kvm: load of migration failed: Invalid argument The idea behind checking the values of setup_len and setup_index before they are used is correct, but doing it in usb_device_post_load feels arbitrary, and will cause unnecessary migration failures. Indeed, none of the commit messages for c60174e8, 9f8e9895 and 719ffe1f justify why post_load is the right place to do these checks. They correctly point out that the important thing to protect is the usb_packet_copy. Instead, the right place to do the checks is in do_token_setup and do_parameter. Indeed, there are already some checks here. We can examine each of the disjuncts currently tested in usb_device_post_load to see whether any need adding to do_token_setup or do_parameter to improve safety there: * dev->setup_index < 0 - This test is not needed because setup_index is explicitly set to 0 in do_token_setup and do_parameter. * dev->setup_len < 0 - In both do_token_setup and do_parameter, the value of setup_len is computed by (s->setup_buf[7] << 8) | s->setup_buf[6]. Since s->setup_buf is a byte array and setup_len is an int32_t, it's impossible for this arithmetic to set setup_len's top bit, so it can never be negative. * dev->setup_index > dev->setup_len - Since setup_index is 0, this is equivalent to the previous test, so is redundant. * dev->setup_len > sizeof(dev->data_buf) - This condition is already explicitly checked in both do_token_setup and do_parameter. Hence there is no need to bolster the existing checks in do_token_setup or do_parameter, and we can safely remove these checks from usb_device_post_load without reducing safety but allowing migrations to proceed regardless of what USB packets have been generated by the guest. Signed-off-by: Jonathan Davies <jonathan.davies@nutanix.com> Message-Id: <20190107175117.23769-1-jonathan.davies@nutanix.com> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
* error: Fix use of error_prepend() with &error_fatal, &error_abortMarkus Armbruster2018-10-191-2/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | | From include/qapi/error.h: * Pass an existing error to the caller with the message modified: * error_propagate(errp, err); * error_prepend(errp, "Could not frobnicate '%s': ", name); Fei Li pointed out that doing error_propagate() first doesn't work well when @errp is &error_fatal or &error_abort: the error_prepend() is never reached. Since I doubt fixing the documentation will stop people from getting it wrong, introduce error_propagate_prepend(), in the hope that it lures people away from using its constituents in the wrong order. Update the instructions in error.h accordingly. Convert existing error_prepend() next to error_propagate to error_propagate_prepend(). If any of these get reached with &error_fatal or &error_abort, the error messages improve. I didn't check whether that's the case anywhere. Cc: Fei Li <fli@suse.com> Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-Id: <20181017082702.5581-2-armbru@redhat.com>
* hmp-commands: Remove the deprecated usb_add and usb_delThomas Huth2017-12-141-22/+0Star
| | | | | | | | | | | | It's easy to use device_add and device_del as replacement instead. The usb_add and usb_del commands are deprecated since QEMU 2.10, and nobody complained that they are still needed, so let's get rid of them now to make the HMP interface a little bit less overloaded. Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> Signed-off-by: Thomas Huth <thuth@redhat.com> Message-Id: <1512073140-17672-1-git-send-email-thuth@redhat.com> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
* hw/usb/bus: Remove bad object_unparent() from usb_try_create_simple()Thomas Huth2017-09-291-3/+1Star
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Valgrind detects an invalid read operation when hot-plugging of an USB device fails: $ valgrind x86_64-softmmu/qemu-system-x86_64 -device usb-ehci -nographic -S ==30598== Memcheck, a memory error detector ==30598== Copyright (C) 2002-2015, and GNU GPL'd, by Julian Seward et al. ==30598== Using Valgrind-3.12.0 and LibVEX; rerun with -h for copyright info ==30598== Command: x86_64-softmmu/qemu-system-x86_64 -device usb-ehci -nographic -S ==30598== QEMU 2.10.50 monitor - type 'help' for more information (qemu) device_add usb-tablet (qemu) device_add usb-tablet (qemu) device_add usb-tablet (qemu) device_add usb-tablet (qemu) device_add usb-tablet (qemu) device_add usb-tablet ==30598== Invalid read of size 8 ==30598== at 0x60EF50: object_unparent (object.c:445) ==30598== by 0x580F0D: usb_try_create_simple (bus.c:346) ==30598== by 0x581BEB: usb_claim_port (bus.c:451) ==30598== by 0x582310: usb_qdev_realize (bus.c:257) ==30598== by 0x4CB399: device_set_realized (qdev.c:914) ==30598== by 0x60E26D: property_set_bool (object.c:1886) ==30598== by 0x61235E: object_property_set_qobject (qom-qobject.c:27) ==30598== by 0x61000F: object_property_set_bool (object.c:1162) ==30598== by 0x4567C3: qdev_device_add (qdev-monitor.c:630) ==30598== by 0x456D52: qmp_device_add (qdev-monitor.c:807) ==30598== by 0x470A99: hmp_device_add (hmp.c:1933) ==30598== by 0x3679C3: handle_hmp_command (monitor.c:3123) The object_unparent() here is not necessary anymore since commit 69382d8b3e8600b3 ("qdev: Fix object reference leak in case device.realize() fails"), so let's remove it now. Suggested-by: Paolo Bonzini <pbonzini@redhat.com> Signed-off-by: Thomas Huth <thuth@redhat.com> Message-id: 1506526106-30971-1-git-send-email-thuth@redhat.com Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
* usb: Fix build with newer gccEric Blake2017-07-201-2/+4
| | | | | | | | | | | | | | | | | | | | | | | | | | | gcc 7 is pickier about our sources: hw/usb/bus.c: In function ‘usb_port_location’: hw/usb/bus.c:410:66: error: ‘%d’ directive output may be truncated writing between 1 and 11 bytes into a region of size between 0 and 15 [-Werror=format-truncation=] snprintf(downstream->path, sizeof(downstream->path), "%s.%d", ^~ hw/usb/bus.c:410:9: note: ‘snprintf’ output between 3 and 28 bytes into a destination of size 16 snprintf(downstream->path, sizeof(downstream->path), "%s.%d", ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ upstream->path, portnr); ~~~~~~~~~~~~~~~~~~~~~~~ But we know that there are at most 5 levels of USB hubs, with at most two digits per level; that plus the separating dots means we use at most 15 bytes (including trailing NUL) of our 16-byte field. Adding an assertion to show gcc that we checked for truncation is enough to shut up the false-positive warning. Inspired by an idea by Dr. David Alan Gilbert <dgilbert@redhat.com>. Signed-off-by: Eric Blake <eblake@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Message-id: 20170717151334.17954-1-eblake@redhat.com Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
* error: Apply error_propagate_null.cocci againFam Zheng2017-04-241-3/+1Star
| | | | | | | | Signed-off-by: Fam Zheng <famz@redhat.com> Message-Id: <20170421122710.15373-15-famz@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
* migrate: Introduce a 'dc->vmsd' check to avoid segfault for --only-migratableAshijeet Acharya2017-02-281-19/+0Star
| | | | | | | | | | | | | | | | | | | | Commit a3a3d8c7 introduced a segfault bug while checking for 'dc->vmsd->unmigratable' which caused QEMU to crash when trying to add devices which do no set their 'dc->vmsd' yet while initialization. Place a 'dc->vmsd' check prior to it so that we do not segfault for such devices. NOTE: This doesn't compromise the functioning of --only-migratable option as all the unmigratable devices do set their 'dc->vmsd'. Introduce a new function check_migratable() and move the only_migratable check inside it, also use stubs to avoid user-mode qemu build failures. Signed-off-by: Ashijeet Acharya <ashijeetacharya@gmail.com> Message-Id: <1487009088-23891-1-git-send-email-ashijeetacharya@gmail.com> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
* usb: replace handle_destroy with unrealizeMarc-André Lureau2017-02-231-4/+5
| | | | | | | | | | | Curiously, unrealize() is not being used, but it seems more appropriate than handle_destroy() together with realize(). It is more ubiquitous destroy name in qemu code base and may throw errors. Cc: Gerd Hoffmann <kraxel@redhat.com> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> Message-id: 20170221141451.28305-25-marcandre.lureau@redhat.com Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
* migration: Allow "device add" options to only add migratable devicesAshijeet Acharya2017-01-241-0/+19
| | | | | | | | | | | | | | Introduce checks for the unmigratable flag in the VMStateDescription structs of respective devices when user attempts to add them. If the "--only-migratable" was specified, all unmigratable devices will rightly fail to add. This feature is made compatible for both "-device" and "-usbdevice" command line options and covers their hmp and qmp counterparts as well. Signed-off-by: Ashijeet Acharya <ashijeetacharya@gmail.com> Message-Id: <1484566314-3987-4-git-send-email-ashijeetacharya@gmail.com> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
* usb: free USBDevice.stringsMarc-André Lureau2016-08-071-0/+7
| | | | | | | | The list is created during instance init and further populated with usb_desc_set_string(). Clear it when unrealizing the device. Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>
* usb: Add QOM property "attached".Gerd Hoffmann2016-06-221-0/+43
| | | | | | | | | | | | | | | | USB devices in attached state are visible to the guest. This patch adds a QOM property for this. Write access is opt-in per device. Some devices manage attached state automatically (usb-host, usb-serial, usb-redir), so we can't enable write access universally but have to do it on a case by case base. So far, no device opts in. Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Message-id: 1465984019-28963-4-git-send-email-kraxel@redhat.com [ minor codestyle fix ] Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
* usb: make USBDevice->attached boolGerd Hoffmann2016-06-221-4/+4
| | | | | | Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Message-id: 1465984019-28963-3-git-send-email-kraxel@redhat.com
* util: move declarations out of qemu-common.hVeronia Bahaa2016-03-221-0/+1
| | | | | | | | | | Move declarations out of qemu-common.h for functions declared in utils/ files: e.g. include/qemu/path.h for utils/path.c. Move inline functions out of qemu-common.h and into new files (e.g. include/qemu/bcd.h) Signed-off-by: Veronia Bahaa <veroniabahaa@gmail.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
* include/qemu/osdep.h: Don't include qapi/error.hMarkus Armbruster2016-03-221-0/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Commit 57cb38b included qapi/error.h into qemu/osdep.h to get the Error typedef. Since then, we've moved to include qemu/osdep.h everywhere. Its file comment explains: "To avoid getting into possible circular include dependencies, this file should not include any other QEMU headers, with the exceptions of config-host.h, compiler.h, os-posix.h and os-win32.h, all of which are doing a similar job to this file and are under similar constraints." qapi/error.h doesn't do a similar job, and it doesn't adhere to similar constraints: it includes qapi-types.h. That's in excess of 100KiB of crap most .c files don't actually need. Add the typedef to qemu/typedefs.h, and include that instead of qapi/error.h. Include qapi/error.h in .c files that need it and don't get it now. Include qapi-types.h in qom/object.h for uint16List. Update scripts/clean-includes accordingly. Update it further to match reality: replace config.h by config-target.h, add sysemu/os-posix.h, sysemu/os-win32.h. Update the list of includes in the qemu/osdep.h comment quoted above similarly. This reduces the number of objects depending on qapi/error.h from "all of them" to less than a third. Unfortunately, the number depending on qapi-types.h shrinks only a little. More work is needed for that one. Signed-off-by: Markus Armbruster <armbru@redhat.com> [Fix compilation without the spice devel packages. - Paolo] Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
* usb: Clean up includesPeter Maydell2016-01-291-0/+1
| | | | | | | | | | Clean up includes so that osdep.h is included first and headers which it implies are not included manually. This commit was created with scripts/clean-includes. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Message-id: 1453832250-766-20-git-send-email-peter.maydell@linaro.org
* error: Use error_prepend() where it makes obvious senseMarkus Armbruster2016-01-131-3/+3
| | | | | | | | | | | | | | | | | | | | | | | | | Done with this Coccinelle semantic patch @@ expression FMT, E1, E2; expression list ARGS; @@ - error_setg(E1, FMT, ARGS, error_get_pretty(E2)); + error_propagate(E1, E2);/*###*/ + error_prepend(E1, FMT/*@@@*/, ARGS); followed by manual cleanup, first because I can't figure out how to make Coccinelle transform strings, and second to get rid of now superfluous error_propagate(). We now use or propagate the original error whole instead of just its message obtained with error_get_pretty(). This avoids suppressing its hint (see commit 50b7b00), but I can't see how the errors touched in this commit could come with hints. It also improves the message printed with &error_abort when we screw up (see commit 1e9b65b). Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com>
* error: Use error_reportf_err() where it makes obvious senseMarkus Armbruster2016-01-131-3/+2Star
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Done with this Coccinelle semantic patch @@ expression FMT, E, S; expression list ARGS; @@ - error_report(FMT, ARGS, error_get_pretty(E)); + error_reportf_err(E, FMT/*@@@*/, ARGS); ( - error_free(E); | exit(S); | abort(); ) followed by a replace of '%s"/*@@@*/' by '"' and some line rewrapping, because I can't figure out how to make Coccinelle transform strings. We now use the error whole instead of just its message obtained with error_get_pretty(). This avoids suppressing its hint (see commit 50b7b00), but I can't see how the errors touched in this commit could come with hints. Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <1450452927-8346-12-git-send-email-armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com>
* usb: print device id in "info usb" monitor commandGerd Hoffmann2015-10-201-3/+6
| | | | Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
* qerror: Move #include out of qerror.hMarkus Armbruster2015-06-221-0/+1
| | | | | | | Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Luiz Capitulino <lcapitulino@redhat.com>
* usb: Improve companion configuration error messagesMarkus Armbruster2015-03-171-10/+8Star
| | | | | | | | | The previous commit broke the additional messages explaining the error messages. Improve the error messages, so they don't need explaining so much. Helps QMP users as well, unlike additional explanations. Signed-off-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
* usb: Propagate errors through usb_register_companion()Markus Armbruster2015-03-171-7/+10
| | | | | | | | This loses the messages explaining the error printed with error_printf_unless_qmp(). The next commit will make up for the loss. Signed-off-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
* Merge remote-tracking branch 'remotes/armbru/tags/pull-monitor-2015-02-18' ↵Peter Maydell2015-02-251-1/+1
|\ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | into staging hmp: Normalize HMP command handler names # gpg: Signature made Wed Feb 18 10:59:44 2015 GMT using RSA key ID EB918653 # gpg: Good signature from "Markus Armbruster <armbru@redhat.com>" # gpg: aka "Markus Armbruster <armbru@pond.sub.org>" * remotes/armbru/tags/pull-monitor-2015-02-18: hmp: Name HMP info handler functions hmp_info_SUBCOMMAND() hmp: Name HMP command handler functions hmp_COMMAND() hmp: Clean up declarations for long-gone info handlers Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
| * hmp: Name HMP info handler functions hmp_info_SUBCOMMAND()Markus Armbruster2015-02-181-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Some are called do_info_SUBCOMMAND() (old ones, usually), some hmp_info_SUBCOMMAND(), some SUBCOMMAND_info(), sometimes SUBCOMMAND pointlessly differs in spelling. Normalize to hmp_info_SUBCOMMAND(), where SUBCOMMAND is exactly the subcommand name with '-' replaced by '_'. Exceptions: * sun4m_irq_info(), sun4m_pic_info() renamed to sun4m_hmp_info_irq(), sun4m_hmp_info_pic(). * lm32_irq_info(), lm32_pic_info() renamed to lm32_hmp_info_irq(), lm32_hmp_info_pic(). Signed-off-by: Markus Armbruster <armbru@redhat.com>
* | usb: Change usb_create_simple() to abort on failureMarkus Armbruster2015-02-181-8/+1Star
| | | | | | | | | | | | | | | | | | Instead of returning null pointer. Matches pci_create_simple(), isa_create_simple(), sysbus_create_simple(). It's unused since the previous commit, but I'll put it to use again shortly. Signed-off-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
* | usb: Suppress bogus error when automatic usb-hub creation failsMarkus Armbruster2015-02-181-7/+28
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | USBDevice's realize method usb_qdev_realize() automatically creates a usb-hub when only one port is left. Creating devices in realize methods is questionable, but works. If usb-hub creation fails, an error is reported to stderr, but the failure is otherwise ignored. We then create the actual device using the last port, which may well succeed. Example: $ qemu -nodefaults -S -display none -machine usb=on -monitor stdio QEMU 2.2.50 monitor - type 'help' for more information (qemu) device_add usb-mouse [Repeat 36 times] (qemu) info usb Device 0.0, Port 1, Speed 12 Mb/s, Product QEMU USB Mouse Device 0.0, Port 2, Speed 12 Mb/s, Product QEMU USB Hub Device 0.0, Port 2.1, Speed 12 Mb/s, Product QEMU USB Mouse [More mice and hubs omitted...] Device 0.0, Port 2.8.8.8.8.7, Speed 12 Mb/s, Product QEMU USB Mouse (qemu) device_add usb-mouse usb hub chain too deep Failed to initialize USB device 'usb-hub' (qemu) info usb [...] Device 0.0, Port 2.8.8.8.8.7, Speed 12 Mb/s, Product QEMU USB Mouse Device 0.0, Port 2.8.8.8.8.8, Speed 12 Mb/s, Product QEMU USB Mouse Despite the "Failed" message, the command actually succeeded. In QMP, it's worse. When adding the 37th mouse via QMP, the command fails with {"error": {"class": "GenericError", "desc": "usb hub chain too deep"}} Additionally, "Failed to initialize USB device 'usb-hub'" is reported on stderr. Despite the command failure, the device was created. This is wrong. Fix by avoiding qdev_init() for usb-hub creation, so we can ignore errors cleanly. Signed-off-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
* | usb: Do not prefix error_setg() messages with "Error: "Markus Armbruster2015-02-181-2/+2
| | | | | | | | | | | | | | | | | | | | Because it produces beauties like (qemu) usb_add mouse Failed to initialize USB device 'usb-mouse': Error: tried to attach usb device QEMU USB Mouse to a bus with no free ports Signed-off-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
* | usb: Improve -usbdevice error reporting a bitMarkus Armbruster2015-02-181-3/+19
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Most LegacyUSBFactory usbdevice_init() methods realize with qdev_init_nofail(), even though their caller usbdevice_create() can handle failure. Okay if it really can't fail (I didn't check), but somewhat brittle. usb_msd_init() and usb_bt_init() call qdev_init(). The latter additionally reports an error when qdev_init() fails. Realization failure produces multiple error reports: a specific one from qdev_init(), and generic ones from usb_bt_init(), usb_create_simple(), usbdevice_create() and usb_parse(). Remove realization from the usbdevice_init() methods. Realize in usbdevice_create(), and produce exactly one error message there. You still get another one from usb_parse(). Signed-off-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
* | usb: usb_create() can't fail, drop useless error handlingMarkus Armbruster2015-02-181-4/+0Star
|/ | | | | Signed-off-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
* usb: Convert usb devices to hotplug handler APIIgor Mammedov2014-10-151-2/+7
| | | | | Signed-off-by: Igor Mammedov <imammedo@redhat.com> Signed-off-by: Andreas Färber <afaerber@suse.de>
* usb-bus: introduce a wrapper function to check speedGonglei2014-09-231-1/+13
| | | | | | | | | | In this way, we can check speed directly, don't need call usb_device_attach(), which has other conditions, such as checking the chardev is open. Signed-off-by: Gonglei <arei.gonglei@huawei.com> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
* usb-bus: remove "init" from USBDeviceClass structGonglei2014-09-231-2/+0Star
| | | | | | | | | All usb-bus devices are realized by realize(), remove init callback function from USBDeviceClass struct. Signed-off-by: Gonglei <arei.gonglei@huawei.com> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
* usb-bus: convert USBDeviceClass init to realizeGonglei2014-09-231-39/+42
| | | | | | | | | | | | | Add "realize/unrealize" in USBDeviceClass, which has errp as a parameter. So all the implementations now use error_setg instead of error_report for reporting error. Note: this patch still keep "init" in USBDeviceClass, and call kclass->init in usb_device_realize(), avoid breaking git bisect. After realize all usb devices, will be removed. Signed-off-by: Gonglei <arei.gonglei@huawei.com> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
* usb: add usb_bus_release functionGonglei2014-08-291-0/+7
| | | | | | | | add global variables releasing logic when the usb buses were removed or hot-unpluged. Signed-off-by: Gonglei <arei.gonglei@huawei.com> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
generated by cgit v1.2.3-55-g7522 (git 2.39.0) at 2025-01-15 20:42:47 +0100