> From: Eduardo Habkost [mailto:ehabk...@redhat.com]
> Sent: Friday, September 05, 2014 10:20 AM
> Subject: Re: [PATCH v6 02/27] bootindex: add del_boot_device_path function
>
> 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:
>
OK. Thanks for explanation. :)
I have two questions:
1) virtio-net-pci device do need special handle in device_set_bootindex().
2) isa-fdc' property is bootindexA and bootindexB, maybe we
can add more a parameter for device_add_bootindex_property()?
Best regards,
-Gonglei
> 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
