Re: [Qemu-devel] [PATCH] qdev: Reset hotplugged devices

2010-08-26 Thread Isaku Yamahata
On Wed, Aug 25, 2010 at 11:49:19AM -0500, Anthony Liguori wrote: On 08/25/2010 10:17 AM, Isaku Yamahata wrote: On Wed, Aug 25, 2010 at 07:55:27AM -0500, Anthony Liguori wrote: Maybe we can merge the patches. As for your patch, I have some comment. - bus itself may want its own handler.

Re: [Qemu-devel] [PATCH] qdev: Reset hotplugged devices

2010-08-26 Thread Anthony Liguori
On 08/26/2010 03:38 AM, Isaku Yamahata wrote: I think that starts by understanding exactly what's guaranteed and understanding what the use cases are for it. Fair enough. How about the followings? Thanks for enumerating. This is just a starting point. I borrowed terminology

Re: [Qemu-devel] [PATCH] qdev: Reset hotplugged devices

2010-08-26 Thread Anthony Liguori
On 08/26/2010 03:38 AM, Isaku Yamahata wrote: QEMU_RESET_COLD BTW, just from a implementation perspective, I'd rather have multiple reset callbacks in qdev instead of having a single callback with a type flag. A type flag implies that every callback has to handle all cases

Re: [Qemu-devel] [PATCH] qdev: Reset hotplugged devices

2010-08-26 Thread Avi Kivity
On 08/25/2010 03:55 PM, Anthony Liguori wrote: Maybe we can merge the patches. As for your patch, I have some comment. - bus itself may want its own handler. At lease pci bus needs it. And propagating reset signal to children is up to the bus controller. I disagree. Reset should be

Re: [Qemu-devel] [PATCH] qdev: Reset hotplugged devices

2010-08-26 Thread Anthony Liguori
On 08/26/2010 08:15 AM, Avi Kivity wrote: On 08/25/2010 03:55 PM, Anthony Liguori wrote: Maybe we can merge the patches. As for your patch, I have some comment. - bus itself may want its own handler. At lease pci bus needs it. And propagating reset signal to children is up to the bus

Re: [Qemu-devel] [PATCH] qdev: Reset hotplugged devices

2010-08-26 Thread Avi Kivity
On 08/26/2010 04:25 PM, Anthony Liguori wrote: On 08/26/2010 08:15 AM, Avi Kivity wrote: On 08/25/2010 03:55 PM, Anthony Liguori wrote: Maybe we can merge the patches. As for your patch, I have some comment. - bus itself may want its own handler. At lease pci bus needs it. And

Re: [Qemu-devel] [PATCH] qdev: Reset hotplugged devices

2010-08-26 Thread Blue Swirl
On Thu, Aug 26, 2010 at 2:29 PM, Avi Kivity a...@redhat.com wrote:  On 08/26/2010 04:25 PM, Anthony Liguori wrote: On 08/26/2010 08:15 AM, Avi Kivity wrote:  On 08/25/2010 03:55 PM, Anthony Liguori wrote: Maybe we can merge the patches. As for your patch, I have some comment. - bus itself

Re: [Qemu-devel] [PATCH] qdev: Reset hotplugged devices

2010-08-26 Thread Isaku Yamahata
I added CC for those who might be interested in this discussion. On Thu, Aug 26, 2010 at 08:02:38AM -0500, Anthony Liguori wrote: On 08/26/2010 03:38 AM, Isaku Yamahata wrote: I think that starts by understanding exactly what's guaranteed and understanding what the use cases are for it.

Re: [Qemu-devel] [PATCH] qdev: Reset hotplugged devices

2010-08-25 Thread Anthony Liguori
On 08/24/2010 10:07 PM, Isaku Yamahata wrote: On Fri, Aug 20, 2010 at 10:56:57AM -0500, Anthony Liguori wrote: 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

Re: [Qemu-devel] [PATCH] qdev: Reset hotplugged devices

2010-08-25 Thread Isaku Yamahata
On Wed, Aug 25, 2010 at 07:55:27AM -0500, Anthony Liguori wrote: Maybe we can merge the patches. As for your patch, I have some comment. - bus itself may want its own handler. At lease pci bus needs it. And propagating reset signal to children is up to the bus controller. I disagree.

Re: [Qemu-devel] [PATCH] qdev: Reset hotplugged devices

2010-08-25 Thread Anthony Liguori
On 08/25/2010 10:17 AM, Isaku Yamahata wrote: On Wed, Aug 25, 2010 at 07:55:27AM -0500, Anthony Liguori wrote: Maybe we can merge the patches. As for your patch, I have some comment. - bus itself may want its own handler. At lease pci bus needs it. And propagating reset signal to

Re: [Qemu-devel] [PATCH] qdev: Reset hotplugged devices

2010-08-24 Thread Isaku Yamahata
On Fri, Aug 20, 2010 at 10:56:57AM -0500, Anthony Liguori wrote: 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

Re: [Qemu-devel] [PATCH] qdev: Reset hotplugged devices

2010-08-23 Thread Avi Kivity
On 08/20/2010 12:00 PM, Markus Armbruster wrote: Alex Williamsonalex.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

Re: [Qemu-devel] [PATCH] qdev: Reset hotplugged devices

2010-08-23 Thread Anthony Liguori
On 08/23/2010 07:00 AM, Avi Kivity wrote: Off-topic, but what's the reason for dev-hotplugged's existence? A device is either plugged or not, it is either hotpluggable or not, but is there a way to tell, from looking at a plugged device, whether it has been hotplugged in the past? AFAICT it

Re: [Qemu-devel] [PATCH] qdev: Reset hotplugged devices

2010-08-21 Thread Markus Armbruster
Anthony Liguori anth...@codemonkey.ws writes: On 08/20/2010 11:14 AM, Markus Armbruster wrote: 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

Re: [Qemu-devel] [PATCH] qdev: Reset hotplugged devices

2010-08-21 Thread Anthony Liguori
On 08/21/2010 05:07 AM, Markus Armbruster wrote: diff --git a/vl.c b/vl.c index b3e3676..5de1688 100644 --- a/vl.c +++ b/vl.c @@ -2968,6 +2968,9 @@ int main(int argc, char **argv, char **envp) } qemu_system_reset(); + +qemu_register_reset((void *)qbus_reset_all,

Re: [Qemu-devel] [PATCH] qdev: Reset hotplugged devices

2010-08-20 Thread Markus Armbruster
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

Re: [Qemu-devel] [PATCH] qdev: Reset hotplugged devices

2010-08-20 Thread Markus Armbruster
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

Re: [Qemu-devel] [PATCH] qdev: Reset hotplugged devices

2010-08-20 Thread Markus Armbruster
Anthony Liguori anth...@codemonkey.ws writes: On 08/20/2010 10:47 AM, Markus Armbruster wrote: Alex Williamsonalex.william...@redhat.com writes: On Fri, 2010-08-20 at 11:00 +0200, Markus Armbruster wrote: Alex Williamsonalex.william...@redhat.com writes: Several

Re: [Qemu-devel] [PATCH] qdev: Reset hotplugged devices

2010-08-20 Thread Anthony Liguori
On 08/20/2010 11:14 AM, Markus Armbruster wrote: 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

Re: [Qemu-devel] [PATCH] qdev: Reset hotplugged devices

2010-08-20 Thread Alex Williamson
On Fri, 2010-08-20 at 13:12 -0500, Anthony Liguori wrote: On 08/20/2010 11:14 AM, Markus Armbruster wrote: 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

[Qemu-devel] [PATCH] qdev: Reset hotplugged devices

2010-08-03 Thread Alex Williamson
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