On Thu, 06/29 12:40, Igor Mammedov wrote:
> On Thu, 29 Jun 2017 16:04:49 +0800
> Fam Zheng <f...@redhat.com> wrote:
>
> > This property can be used to replace the object_property_add_link in
> > device code, to add a link to other objects, which is a common pattern.
> >
> > Signed-off-by: Fam Zheng <f...@redhat.com>
> > ---
> > hw/core/qdev-properties.c | 16 ++++++++++++++++
> > include/hw/qdev-core.h | 5 +++++
> > include/hw/qdev-properties.h | 11 +++++++++++
> > 3 files changed, 32 insertions(+)
> >
> > diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
> > index 68cd653..7c11eb8 100644
> > --- a/hw/core/qdev-properties.c
> > +++ b/hw/core/qdev-properties.c
> > @@ -1192,3 +1192,19 @@ PropertyInfo qdev_prop_size = {
> > .set = set_size,
> > .set_default_value = set_default_value_uint,
> > };
> > +
> > +/* --- object link property --- */
> > +
> > +static void create_link_property(Object *obj, Property *prop, Error **errp)
> > +{
> > + Object **child = qdev_get_prop_ptr(DEVICE(obj), prop);
> > +
> > + object_property_add_link(obj, prop->name, prop->link_type,
> > + child, prop->link.check,
> > + prop->link.flags, errp);
> > +}
> > +
> > +PropertyInfo qdev_prop_link = {
> > + .name = "link",
> > + .create = create_link_property,
> > +};
> > diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
> > index 33518ee..40afb3d 100644
> > --- a/include/hw/qdev-core.h
> > +++ b/include/hw/qdev-core.h
> > @@ -5,6 +5,7 @@
> > #include "qemu/option.h"
> > #include "qemu/bitmap.h"
> > #include "qom/object.h"
> > +#include "qom/link-property.h"
> > #include "hw/irq.h"
> > #include "hw/hotplug.h"
> >
> > @@ -233,6 +234,10 @@ struct Property {
> > int arrayoffset;
> > PropertyInfo *arrayinfo;
> > int arrayfieldsize;
> > + /* Only @check and @flags are used; @child is unuseful because we need
> > a
> > + * dynamic pointer in @obj as derived from @offset. */
> @check, @flags, @child, @obj are not fields of struct Property so it's
> not clear what doc comments talks about. Maybe adding prefixes would help,
> for example:
> @link.child
Good point, I was being too stingy with words.
>
> > + LinkProperty link;
> > + const char *link_type;
> > };
> >
> > struct PropertyInfo {
> > diff --git a/include/hw/qdev-properties.h b/include/hw/qdev-properties.h
> > index 39bf4b2..767c10b 100644
> > --- a/include/hw/qdev-properties.h
> > +++ b/include/hw/qdev-properties.h
> > @@ -30,6 +30,7 @@ extern PropertyInfo qdev_prop_pci_devfn;
> > extern PropertyInfo qdev_prop_blocksize;
> > extern PropertyInfo qdev_prop_pci_host_devaddr;
> > extern PropertyInfo qdev_prop_arraylen;
> > +extern PropertyInfo qdev_prop_link;
> >
> > #define DEFINE_PROP(_name, _state, _field, _prop, _type) { \
> > .name = (_name), \
> > @@ -117,6 +118,16 @@ extern PropertyInfo qdev_prop_arraylen;
> > .arrayoffset = offsetof(_state, _arrayfield), \
> > }
> >
> > +#define DEFINE_PROP_LINK(_name, _state, _field, _type, _check, _flags) {\
> > + .name = (_name), \
> > + .info = &(qdev_prop_link), \
> > + .offset = offsetof(_state, _field) \
> > + + type_check(Object *, typeof_field(_state, _field)), \
> > + .link.check = _check, \
> > + .link.flags = _flags, \
> maybe we shouldn't have custom _check and _flags fields as majority of devices
> use qdev_prop_allow_set_link_before_realize + OBJ_PROP_LINK_UNREF_ON_RELEASE
> policies. That will save us ~2LOC per property of boiler plate code.
>
> I've looked at current device link usage and there is only few that actually
> use or need custom check/flags and several are misusing
> object_property_allow_set_link()
> where they should use qdev_prop_allow_set_link_before_realize().
>
> We could leave alone exceptions that require custom check/flags as is
> or provide DEFINE_PROP_LINK_CUSTOM() for a few that actually need it and fix
> ones that misuse it.
>
> There is a bunch of board code that uses links but that's not for Device
> so it's not related to this macro anyways.
OK, I wasn't sure about this when Paolo pointed out in v1, but since you've now
taken a look, I will simplify it.
I will also try to cover more devices in v3. Thanks!
Fam
