Anthony Liguori <anth...@codemonkey.ws> writes: > On 08/20/2010 10:47 AM, Markus Armbruster wrote: >> Alex Williamson<alex.william...@redhat.com> writes: >> >> >>> On Fri, 2010-08-20 at 11:00 +0200, Markus Armbruster wrote: >>> >>>> Alex Williamson<alex.william...@redhat.com> writes: >>>> >>>> >>>>> Several devices rely on their reset() function being called to >>>>> initialize device state, e1000 and rtl8139 in particular. When >>>>> the device is hot added, the reset doesn't occur, often leaving >>>>> the device in an unusable state. Adding a call to reset() after >>>>> init() for hotplugged devices puts the device in the expected >>>>> state for the guest. >>>>> >>>>> Signed-off-by: Alex Williamson<alex.william...@redhat.com> >>>>> --- >>>>> >>>>> 0.13 candidate? >>>>> >>>>> hw/qdev.c | 3 +++ >>>>> 1 files changed, 3 insertions(+), 0 deletions(-) >>>>> >>>>> diff --git a/hw/qdev.c b/hw/qdev.c >>>>> index e99c73f..b156272 100644 >>>>> --- a/hw/qdev.c >>>>> +++ b/hw/qdev.c >>>>> @@ -278,6 +278,9 @@ int qdev_init(DeviceState *dev) >>>>> qdev_free(dev); >>>>> return rc; >>>>> } >>>>> + if (dev->hotplugged) { >>>>> + qdev_reset(dev); >>>>> + } >>>>> qemu_register_reset(qdev_reset, dev); >>>>> if (dev->info->vmsd) { >>>>> vmstate_register_with_alias_id(dev, -1, dev->info->vmsd, dev, >>>>> >>>> qdev_reset() isn't necessary when !dev->hotplugged, because then >>>> qemu_system_reset() will run shortly, which will call qdev_reset(). >>>> Correct? >>>> >>> Yes, exactly. >>> >> Hmm. "qdev_init() automatically calls qdev_reset() if hotplug" feels >> needlessly complicated. I'd prefer qdev_init() to call it always or >> never. >> >> If "always", we reset twice for cold-plug. Is that bad? >> >> If "never", we need to reset somewhere else for hot-plug. What about >> do_device_add()? >> > > The real problem is how we do reset. We shouldn't register a reset > handler for every qdev device but rather register a single reset > handler that walks the device tree and calls reset on every reachable > device. > > Then we can always call reset in init() and there's no need to have a > dev->hotplugged check. The qdev device tree reset handler should not > be registered until *after* we call qemu_system_reset() after creating > the device model which will ensure that we don't do a double reset.
Fine with me. But we need to merge something short term (pre 0.13) to fix hot plug of e1000 et al. Use Alex's patch as such a stop-gap?