Re: [Qemu-devel] [PATCH 05/10] vmport: convert to qdev

[Markus Armbruster]

Blue Swirl <address@hidden> writes:

> On Tue, Feb 15, 2011 at 12:22 PM, Markus Armbruster <address@hidden> wrote:
>> Blue Swirl <address@hidden> writes:
>>
>>> On Sat, Feb 12, 2011 at 6:57 PM, Markus Armbruster <address@hidden> wrote:
[...]
>>>> Old code has pc_init1() call vmport_init().  Where does your code create
>>>> qdev "vmport"?  And what's happening with port_state?  It's still used
>>>> by vmport_register(), but no longer connected to the I/O ports.  Can't
>>>> see how vmport_register() has any effect anymore.
>>>
>>> I fixed it in the committed version.
>>
>> Did you post v2 to the list for review?
>
> No, since v1 got no review.

*Please* don't do that.

>>>> Have you tested this?
>>>
>>> Sure.
>>
>> Here's how your v2 creates and initializes the qdev:
[...]
>> Maybe I'm dense, but I can't see how this can work.
>
> Good catch, it doesn't. Probably vmport_register() should take
> ISADevice* parameter to provide the state, instead of using static
> state (which would be easy one-line change).
>
> But if all this is going to be thrown into ps2.c, it may not be
> necessary. The whole concept of registration may become useless.

Yes.