[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 01/15] qdev: do not reset a device until the par
From: |
Paolo Bonzini |
Subject: |
Re: [Qemu-devel] [PATCH 01/15] qdev: do not reset a device until the parent has been initialized |
Date: |
Mon, 17 Dec 2012 18:06:32 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/17.0 Thunderbird/17.0 |
Il 17/12/2012 17:53, Michael S. Tsirkin ha scritto:
>>> This happens when hotplugging a usb-storage device. Without this patch,
>>> initialization of a hotplugged usb-storage device would run in pre-order.
>>> Initialization of a coldplugged usb-storage device would run according to
>>> qdev_reset_all semantics (pre-order right now, post-order later in the
>>> series).
>>
>> Is this a problem or just not pretty?
Not pretty. The child device doesn't exist until the parent has been
initialized, it doesn't make sense to reset it.
I can add an assertion to make it a problem, of course. :)
>>> This patch makes things consistent.
>>>
>>> Signed-off-by: Paolo Bonzini <address@hidden>
>>> ---
>>> hw/qdev.c | 5 +++--
>>> 1 file changed, 3 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/hw/qdev.c b/hw/qdev.c
>>> index 599382c..2fdf4ac 100644
>>> --- a/hw/qdev.c
>>> +++ b/hw/qdev.c
>>> @@ -176,8 +176,9 @@ int qdev_init(DeviceState *dev)
>>> dev->alias_required_for_version);
>>> }
>>> dev->state = DEV_STATE_INITIALIZED;
>>> - if (dev->hotplugged) {
>>> - device_reset(dev);
>>> + if (dev->hotplugged &&
>>> + dev->parent_bus->parent->state == DEV_STATE_INITIALIZED) {
>>> + qdev_reset_all(dev);
>>
>> Let's add a comment explaining the why of this test, and when
>> will reset happen if it does not trigger here.
>
> Also - shouldn't device init be delayed as well?
> What do you think?
Initialization should run in pre-order (unlike reset): the parent needs
to be ready in order to start the child. What you typically have, is
that the parent device initializes itself, and then calls
qdev_create/qdev_init on the child before returning. So it is already
being delayed. The delay is explicit in program flow, not hidden in
qdev semantics.
Paolo
>>> }
>>> return 0;
>>> }
>>> --
>>> 1.8.0.2
>>>
- [Qemu-devel] [PATCH 00/15] qdev: make reset semantics more clear and consistent, reset qbuses under virtio devices, Paolo Bonzini, 2012/12/17
- [Qemu-devel] [PATCH 03/15] pci: clean up resetting of IRQs, Paolo Bonzini, 2012/12/17
- [Qemu-devel] [PATCH 05/15] virtio-s390: add a reset function to virtio-s390 devices, Paolo Bonzini, 2012/12/17
- [Qemu-devel] [PATCH 06/15] qdev: add qbus_reset_all, Paolo Bonzini, 2012/12/17
- [Qemu-devel] [PATCH 04/15] virtio-pci: reset device before PCI layer, Paolo Bonzini, 2012/12/17
- [Qemu-devel] [PATCH 08/15] lsi: use qbus_reset_all to reset SCSI bus, Paolo Bonzini, 2012/12/17
- [Qemu-devel] [PATCH 07/15] pci: do not export pci_bus_reset, Paolo Bonzini, 2012/12/17
- [Qemu-devel] [PATCH 09/15] qdev: allow both pre- and post-order vists in qdev walking functions, Paolo Bonzini, 2012/12/17
- [Qemu-devel] [PATCH 10/15] qdev: switch reset to post-order, Paolo Bonzini, 2012/12/17
- [Qemu-devel] [PATCH 11/15] qdev: remove device_reset, Paolo Bonzini, 2012/12/17