[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-ppc] [PATCH 27/28] sysbus: apic: ioapic: convert to QEMU Objec

From: Paolo Bonzini
Subject: Re: [Qemu-ppc] [PATCH 27/28] sysbus: apic: ioapic: convert to QEMU Object Model
Date: Wed, 25 Jan 2012 12:15:59 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:9.0) Gecko/20111222 Thunderbird/9.0

On 01/25/2012 11:27 AM, Jan Kiszka wrote:
>  I agree with Anthony, this would get really ugly where you are calling
>  the functions and doing the class initialization.

I think we need to try it first.

There is a lot of repetition, and that gets boring at beat and ugly at
worst when doing it for hundreds of devices - compared to the number of
base classes we will have.

I think we have to trust Anthony's judgement that he tried it and didn't like it. Perhaps he didn't try _exactly_ the version that you gave, but I think it's not fair to say "please try redoing it this way---maybe" a month after the first version has been posted.

So far, we were always able to reach a compromise between Anthony's taste and others', and also on the order with which to do things. I understand this is a pretty major issue, but it's going to be one more such compromise (and one that I thought had already been settled).

Note that it's not impossible to change directions! QOM started with the idea of converting devices _last_. We are converting them _first_. That was a very early change in plans, and very much against Anthony. It's fair and natural that the balance swings in the other direction now. As a reviewer, the later you join the party, the harder it will be to swallow the pill.

>  It's a different
>  style from what we're used to, granted, but the difference in code size
>  is not relevant (not enough to introduce a level of macro magic, at
>  least) and the diffstat in this series is misleading because qdev is
>  left with temporary duplication for now.

I was looking at the final version Anthony pointed at.

There _are_ things that leave something to be desired in there. For example I don't like accessing the base class with a different name, as in:

    DeviceClass *dc = DEVICE_CLASS(klass);
    PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);

    dc->props = e100_properties;
    dc->desc = info->desc;
    k->vendor_id = PCI_VENDOR_ID_INTEL;
    k->romfile = "pxe-eepro100.rom";

and a few more things that we'll discuss as soon as he posts the third instalment. I know we're all busy, but the only way to solve these problems is prompt review.


reply via email to

[Prev in Thread] Current Thread [Next in Thread]