On Thu, Feb 18, 2016 at 1:47 PM, Paolo Bonzini <pbonz...@redhat.com> wrote: > > > On 18/02/2016 10:56, Markus Armbruster wrote: >> Alistair Francis <alistair.fran...@xilinx.com> writes: >> >>> If the device being added when running qdev_device_add() has >>> a reset function, register it so that it can be called. >>> >>> Signed-off-by: Alistair Francis <alistair.fran...@xilinx.com> >>> --- >>> >>> qdev-monitor.c | 2 ++ >>> 1 file changed, 2 insertions(+) >>> >>> diff --git a/qdev-monitor.c b/qdev-monitor.c >>> index 81e3ff3..0a99d01 100644 >>> --- a/qdev-monitor.c >>> +++ b/qdev-monitor.c >>> @@ -561,6 +561,8 @@ DeviceState *qdev_device_add(QemuOpts *opts, Error >>> **errp) >>> >>> if (bus) { >>> qdev_set_parent_bus(dev, bus); >>> + } else if (dc->reset) { >>> + qemu_register_reset((void (*)(void *))dc->reset, dev); >>> } >>> >>> id = qemu_opts_id(opts); >> >> This looks wrong to me. >> >> You stuff all the device reset methods into the global reset_handlers >> list, where they get called in some semi-random order. This breaks when >> there are reset order dependencies between devices, e.g. between a >> device and the bus it plugs into. > > There is no bus here, see the "if" above the one that's being added. > > However, what devices have done so far is to register/unregister the > reset in the realize/unrealize methods, and I suggest doing the same. >
Does this assume the device itself knows whether it is bus-connected or not? This way has the advantage of catchall-ing devices that have no bus connected and the device may or may not know whether it is bus-connected (nor should it need to know). Probably doesn't have in tree precedent yet, but I thought we wanted to move away from qdev/qbus managing the device-tree. So ideally, the new else should become unconditional long term once we debusify (and properly QOMify) the reset tree (and the if goes away). > If you really want to add the magic qemu_register_reset, you should at > least do one of the following: > > * add a matching unregister (no idea where) > You could add a boolean flag to DeviceState that is set by this registration. It can then be checked at unrealize to remove reset handler. Regards, Peter > * assert that the device is not hot-unpluggable, otherwise hot-unplug > would leave a dangling pointer. > > Paolo > >> Propagating the reset signal to all the devices is a qdev problem. >> Copying Andreas for further insight.