Re: [Qemu-devel] [PATCH v6 02/27] bootindex: add del_boot_device_path function

[Eduardo Habkost]

On Fri, Sep 05, 2014 at 12:44:56AM +0000, Gonglei (Arei) wrote:
[...]
> > can we have a wrapper to reduce code
> > duplication? e.g. a:
> >   void device_add_bootindex_property(DeviceState *dev, int32_t *field, const
> > char *suffix)
> > function.
> > 
> > Then instead of reimplementing set/get/finalize functions, device code
> > could just call something like:
> >   device_add_bootindex_property(dev, &n->nic_conf.bootindex,
> >                                 "/ethernet-phy@0");
> > 
> 
> This way we cannot attach our target that changing bootindex and take effect
> during vm rebooting.

I don't understand what you mean, here. Whatever you are planning to do on the
device-specific setter/getters, if they all look the same, you can write a
common getter/setter to do exactly the same steps, and register it inside
device_add_bootindex_property(). This way, patches 08/27 to 26/27 can become
one-liners, instead of adding more 20 lines each.

I mean something like this:

typedef struct BootIndexProperty
{
    int32_t *field;
    const char *suffix;
} BootIndexProperty;

static void device_get_bootindex(Object *obj, Visitor *v, void *opaque,
                                 const char *name, Error **errp)
{
    BootIndexProperty *prop = opaque;
    visit_type_int32(v, prop->field, name, errp);
}

static void device_set_bootindex(Object *obj, Visitor *v, void *opaque,
                                 const char *name, Error **errp)
{
    DeviceState *dev = DEVICE(obj);
    BootIndexProperty *prop = opaque;
    int32_t boot_index;
    Error *local_err = NULL;

    visit_type_int32(v, &boot_index, name, &local_err);
    if (local_err) {
        goto out;
    }

    check_boot_index(boot_index, &local_err);
    if (local_err) {
        goto out;
    }

    *prop->field = boot_index;
    add_boot_device_path(boot_index, dev, prop->suffix);

out:
    if (local_err) {
        error_propagate(errp, local_err);
    }
}

static void property_release_bootindex(Object *obj, const char *name,
                                       void *opaque)

{
    BootIndexProperty *prop = opaque;
    DeviceState *dev = DEVICE(obj);
    del_boot_device_path(dev);
    g_free(prop);
}

void device_add_bootindex_property(DeviceState *dev, int32_t *field,
                                   const char *suffix, Error **errp)
{
    Error *local_err = NULL;
    BootIndexProperty *prop = g_malloc0(sizeof(*prop));

    prop->field = field;
    prop->suffix = suffix;
    object_property_add(OBJECT(dev), "bootindex", "int32",
                        device_get_bootindex,
                        device_set_bootindex,
                        property_release_bootindex,
                        prop, &local_err);

    if (local_err) {
        error_propagate(errp, local_err);
        g_free(prop);
    }
}

-- 
Eduardo