Re: [Qemu-devel] [PATCH 2/7] Convert pc cpu to qdev

[Andreas Färber]

Am 13.03.2012 10:32, schrieb Lai Jiangshan:
> On 02/16/2012 08:51 PM, Anthony Liguori wrote:
>> On 02/16/2012 06:01 AM, Jan Kiszka wrote:
>>> On 2012-02-16 00:16, Igor Mammedov wrote:
>>>> +static ICCBusDeviceInfo cpu_device_info = {
>>>> +    .qdev.name = "cpu-pc",
>>>> +    .qdev.size = sizeof(CPUPC),
>>>> +    .qdev.reset = cpu_device_reset,
>>>> +    .init = cpu_device_init,
>>>> +    .qdev.props = (Property[]) {
>>>> +        DEFINE_PROP_STRING("model", CPUPC, model),
>>>
>>> And how do you pass in feature flags? Or the core layout? Basically both
>>> -cpu and -smp need to be expressible via multiple "-device cpu-x86,xxx"
>>> (not "pc") commands.
>>
>> The approach that I'd recommend is:
>>
>> 1) convert CPU_COMMON_STATE to a structure named CPUCommonState, 
>> query/replace all references to members of CPU_COMMON_STATE appropriately.
> 
> I just tried this for several days, it will result a huge patch,
> it is hard for human, any suggestion?
> (I used Semantic patches script: http://coccinelle.lip6.fr/, it is still hard,
> it still leaves huge part which needs manual conversion/fix)
> 
> I will take part in implementing cpu-hotplug future for qemu,
> Could you give me some tips/suggestions? What approach should I take?

In short: Don't!

Please instead provide feedback on my series which already does
something like that for you, available since multiple weeks. If a
different naming is preferred, I can change it.

The approach I have taken is:
1. CPUState -> CPU{X86,...}State, CPUArchState as alias where necessary
2. Embed CPU$archState in $archCPU
3. Step by step move CPU_COMMON_STATE from CPUArchState into CPUState

Steps 1+2 are done, 2 is partially on the list (e.g., [1]), the rest on
my qom-cpu-wip branch [2].
Step 3 is shown for icount on qom-cpu-wip as well as for multiple
target-specific fields such as target-arm features [3]. Contributions
welcome.

Whatever naming and order we decide on, this WILL touch a lot of code.
If you start doing this in some random different way we'll end up with
conflicting series on the list, wasting each other's time.

Thanks,
Andreas

[1] http://patchwork.ozlabs.org/patch/145800/
[2] http://repo.or.cz/w/qemu/afaerber.git/shortlog/refs/heads/qom-cpu-wip
[3] http://patchwork.ozlabs.org/patch/145874/

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg