Re: [Qemu-devel] [PATCH 00/10] AHCI emulation support v2

[Kevin Wolf]

Am 19.11.2010 14:08, schrieb Alexander Graf:
> 
> On 19.11.2010, at 10:15, Kevin Wolf wrote:
> 
>> Am 18.11.2010 19:43, schrieb Alexander Graf:
>>>> Then I believe that core.c is now a mixture of some generic ATA code
>>>> (that is also used by SATA) and the Legacy IDE code. SATA doesn't seem
>>>> to interact with the generic code through clean interfaces, but by
>>>> accessing internal data structures and calls to somewhere in the middle
>>>> of the existing IDE emultion. I think we should get a clean abstraction
>>>> there and have a clean split between SATA, PATA and common code, with
>>>> each of them sitting in its own file in hw/ide.
>>>>
>>>> I haven't reviewed the patches in detail but just had a quick look at
>>>> them, so my impressions might be wrong. If so, please correct me.
>>>
>>> No, you're completely right. We're in a chicken and egg situation. We don't 
>>> have ahci, but the ide code is ugly. We would probably do a bad job at 
>>> refactoring the ata code if we don't know which interfaces to design for.
>>
>> That problem is solved. You have posted patches, so you're aware what
>> interfaces you need for AHCI. This awareness doesn't come from putting
>> the code into git master.
> 
> I guess you should go back and read the "this doesn't work yet" list. There 
> is a lot of stuff that I'm not sure we have all correctly sorted out. The 
> most intrusive one on that side is the legacy IDE compatibility. I don't know 
> what interfaces and what changes we will need for that to become realistic.

Fair enough. I'll accept that we can't get it sorted out now, but let's
try to do the part that we can do. Let's change the split to SATA
(ahci.c), Legacy IDE (ide.c?), common code (ata.c) and "don't know yet"
(core.c).

A start for that would be if in Patch 2 you moved the function to ata.c
instead of just reindenting. We're also probably pretty sure that, for
example, the I/O port handling won't need to be shared and can be moved
to ide.c. Whenever it becomes clear for a part in which category it
belongs, we would move it out of core.c and eventually, I hope, core.c
could be removed.

> Also to catch up on Gerd's point - whatever refactoring we do, we will 
> basically have to break migration. There is no way we can change all the 
> internal state and structure and maintain binary compatibility with the old 
> save states.

Hm, breaking migration isn't really an option. I'm not familiar with
migration code, but maybe Juan can contribute some magic?

Speaking of migration, that seems to be missing for the AHCI yet, too.
Are you planning to complete the functionality first before you start
with that?

Kevin