Re: [Qemu-devel] [PATCH v2] qom: Introduce object_realize_nofail()

[Paolo Bonzini]

Il 12/04/2012 23:08, Andreas Färber ha scritto:
> Reality with SysBus is multi-stage constructors:
> A = qdev_create()
> A.a = x
> qdev_init_nofail(A) -> A_initfn() -> B = qdev_create()
>                                      B.b = y
>                                      qdev_init_nofail(B) -> B_initfn()
> ...
> 
> while you are dreaming of a flat hierarchy:
> 
> A = qdev_create()
> B = qdev_create()
> ...
> A.a = x
> B.b = y
> ...
> object_property_set_bool(qdev_get_machine(), "realized", true)
> 
> But getting there requires more than denying me object_realize()

I'm pretty sure we're talking past each other, so I'm rewinding before
the last few messages.

If you look at the initialization of a typical qdev object, it goes like
this:

    dev = qdev_create(NULL, "mv88w8618_eth");
    qdev_set_nic_properties(dev, &nd_table[0]);
    qdev_init_nofail(dev);
    sysbus_mmio_map(sysbus_from_qdev(dev), 0, MP_ETH_BASE);
    sysbus_connect_irq(sysbus_from_qdev(dev), 0, pic[MP_ETH_IRQ]);

Now, there is no good reason to have that qdev_init_nofail.  It is there
because sysbus_init_irq and sysbus_init_mmio are called in the device
init function (mv88w8618_eth_init):

    static int mv88w8618_eth_init(SysBusDevice *dev)
    {
        mv88w8618_eth_state *s = FROM_SYSBUS(mv88w8618_eth_state, dev);

        sysbus_init_irq(dev, &s->irq);
        s->nic = qemu_new_nic(&net_mv88w8618_info, &s->conf,
                              object_get_typename(OBJECT(dev)),
                              dev->qdev.id, s);
        memory_region_init_io(&s->iomem, &mv88w8618_eth_ops, s,
                              "mv88w8618-eth", MP_ETH_SIZE);
        sysbus_init_mmio(dev, &s->iomem);
        return 0;
    }

... and those calls are prerequisites for sysbus_mmio_map and
sysbus_connect_irq.

Similarly, there is no reason why qdev_get_gpio_in can only be called
after qdev_init_nofail.  It's only like that because qdev_init_gpio_in
is called later than in the instance_init function.  Only a handful of
devices need a dynamic number of GPIO pins, everything else can call it
in instance_init.

On the other hand, qemu_new_nic for example needs to be done at realize
time (or at least at first access).  Converting a device to remove those
qdev_init_nofail seems to be little more work than just moving bits of
the sysbus init function to instance_init.  We can also introduce new
functions sysbus_new_simple etc. to replace sysbus_create_simple.  It
does not even need to occur a board at a time, it can be done a device
at a time.

There may be indeed cases where you need multi-stage construction.  However:

1) if the construction resembles the composition tree, which should
usually be the case, the construction _can_ happen in the realize callback.

2) if you also need lazy construction of the composition tree before
realization (i.e. initialize it on first use), we can have tools to make
it simple.  See the example from above of gpio pins whose number depends
on properties.

Until we're there, nobody is denying anyone anything.  You already have
qdev_init_nofail, and I said clearly that adding cpu_init_nofail is fine
for me; all I object to is to adding a generic function.

> initfn is unable to handle errors btw, which is another reason to do
> object creations in a second-stage constructor.

What error handling do you need specifically?  You could add an Error **
argument to object_{new,initialize{,_with_type} too as soon as you have
a use for that.

Paolo