Re: [Qemu-devel] [RFC] QDev explicit constructors & destructors

[Liviu Ionescu]

after some more experimenting, here are the conclusions:

- I added a new .construct pointer to all my classes:

    void (*construct)(Object *obj, void *data);

the 'data' pointer is optional, for qdev calls, parameters should be passed via 
properties.

- constructors should manually call their parent constructor

static void stm32_mcu_construct_callback(Object *obj, void *data)
{
    /* Call parent constructor */
    CORTEXM_MCU_GET_CLASS(obj)->construct(obj, NULL);

    ...
}

this is intentional, to allow the caller to set additional properties:

static void stm32_mcus_construct_callback(Object *obj, void *data)
{
    STM32DeviceClass *st_class = STM32_DEVICE_GET_CLASS(obj);
    STM32PartInfo *part_info = st_class->part_info;
    DeviceState *dev = DEVICE(obj);
    /*
     * Set additional constructor parameters, that were passed via
     * the .class_data and copied to custom class member.
     */
    qdev_prop_set_ptr(dev, "param-cortexm-capabilities",
            (void *) &part_info->cortexm);
    qdev_prop_set_ptr(dev, "param-stm32-capabilities",
            (void *) part_info->stm32);

    STM32_MCU_GET_CLASS(obj)->construct(obj, NULL);

    ...
}

- using this mechanism is pretty simple:

    DeviceState *mcu = qdev_alloc(NULL, TYPE_STM32F103RB);
    {
        qdev_prop_set_ptr(mcu, "param-machine", machine);
        STM32_DEVICE_GET_CLASS(mcu)->construct(OBJECT(mcu), NULL);

        /* Set the board specific oscillator frequencies. */
        qdev_prop_set_uint32(mcu, "hse-freq-hz", 8000000); /* 8.0 MHz */
        qdev_prop_set_uint32(mcu, "lse-freq-hz", 32768); /* 32 KHz */
    }
    qdev_realize(mcu);


- once the explicit constructors are in place, the .realize callbacks can be 
freed of other duties and return to their intended function, to propagate the 
"realized" event to parents and children:

static void stm32_mcu_realize_callback(DeviceState *dev, Error **errp)
{
    /* Call parent realize(). */
    if (!qdev_parent_realize(dev, errp, TYPE_STM32_MCU)) {
        return;
    }

    STM32MCUState *state = STM32_MCU_STATE(dev);

    /* Propagate realize() to children. */

    /* RCC */
    qdev_realize(DEVICE(state->rcc));

    /* FLASH */
    qdev_realize(DEVICE(state->flash));

    /* GPIO[A-G] */
    for (int i = 0; i < STM32_MAX_GPIO; ++i) {
        /* Realize all initialised GPIOs */
        if (state->gpio[i]) {
            qdev_realize(DEVICE(state->gpio[i]));
        }
    }
}


- as already mentioned, in my current implementation the .construct member is 
added to each class, but this is not needed as redundant, and should be added 
once to ObjectClass or at least to DeviceClass.

this would greatly simplify setting it and calling the constructor:

        object_construct(obj, data);

        qdev_prop_set_ptr(dev, "param-machine", machine);       
        qdev_construct(dev);

(for qdev the params are passed via properties, the optional data pointer is 
not used)


Peter, could you consider this proposal?

should I prepare a patch for it? 

adding this extra field in the class structure does not break compatibility 
with existing code, but would help reduce the mess with existing .realize, 
which is abused to do whatever initialisations were not possible in 
.instance_init; I expect that this will make future code easier to understand 
and maintain.


regards,

Liviu