On Fri, Jun 23, 2017 at 06:24:32PM -0300, Eduardo Habkost wrote:
> On Fri, Jun 23, 2017 at 12:46:37PM +0800, Peter Xu wrote:
> > We have HW_COMPAT_*, however that's only bound to machines, not other
> > things (like accelerators). Behind it, it was register_compat_prop()
> > that played the trick. Let's export the function for further use
> > outside HW_COMPAT_* magic.
> >
> > Meanwhile, move it to qdev-properties.c where seems more proper (since
> > it'll be used not only in machine codes).
> >
> > Signed-off-by: Peter Xu <pet...@redhat.com>
> > ---
> > hw/core/machine.c | 13 -------------
> > hw/core/qdev-properties.c | 13 +++++++++++++
> > include/hw/qdev-properties.h | 3 +++
> > 3 files changed, 16 insertions(+), 13 deletions(-)
> >
> > diff --git a/hw/core/machine.c b/hw/core/machine.c
> > index 2e7e977..ecb5552 100644
> > --- a/hw/core/machine.c
> > +++ b/hw/core/machine.c
> > @@ -770,19 +770,6 @@ static void machine_class_finalize(ObjectClass *klass,
> > void *data)
> > g_free(mc->name);
> > }
> >
> > -static void register_compat_prop(const char *driver,
> > - const char *property,
> > - const char *value)
> > -{
> > - GlobalProperty *p = g_new0(GlobalProperty, 1);
> > - /* Machine compat_props must never cause errors: */
> > - p->errp = &error_abort;
> > - p->driver = driver;
> > - p->property = property;
> > - p->value = value;
> > - qdev_prop_register_global(p);
> > -}
> > -
> > static void machine_register_compat_for_subclass(ObjectClass *oc, void
> > *opaque)
> > {
> > GlobalProperty *p = opaque;
> > diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
> > index 9f1a497..6ff1ac3 100644
> > --- a/hw/core/qdev-properties.c
> > +++ b/hw/core/qdev-properties.c
> > @@ -1047,6 +1047,19 @@ void qdev_prop_register_global(GlobalProperty *prop)
> > global_props = g_list_append(global_props, prop);
> > }
> >
> > +void register_compat_prop(const char *driver,
> > + const char *property,
> > + const char *value)
> > +{
> > + GlobalProperty *p = g_new0(GlobalProperty, 1);
> > + /* Machine compat_props must never cause errors: */
>
> Now this won't be used only for machine compat_props, so this comment
> needs to be deleted or updated (see suggestion below).
Yes. Fixing.
>
>
> > + p->errp = &error_abort;
> > + p->driver = driver;
> > + p->property = property;
> > + p->value = value;
> > + qdev_prop_register_global(p);
> > +}
> > +
> > void qdev_prop_register_global_list(GlobalProperty *props)
> > {
> > int i;
> > diff --git a/include/hw/qdev-properties.h b/include/hw/qdev-properties.h
> > index d206fc9..1722ca4 100644
> > --- a/include/hw/qdev-properties.h
> > +++ b/include/hw/qdev-properties.h
> > @@ -201,6 +201,9 @@ void qdev_prop_set_globals(DeviceState *dev);
> > void error_set_from_qdev_prop_error(Error **errp, int ret, DeviceState
> > *dev,
> > Property *prop, const char *value);
> >
>
> As this function is very easy to be misused, I suggest including the
> following documentation:
>
> /**
> * register_compat_prop:
> *
> * Register internal (not user-provided) global property, changing the
> * default value of a given property in a device type. This can be used
> * for enabling machine-type compatibility or for enabling
> * accelerator-specific defaults in devices.
> *
> * The property values set using this function must be always valid and
> * never report setter errors, as they property have
> * GlobalProperty::errp set to &error_abort.
> *
> * User-provided global properties should override internal global
> * properties, so callers of this function should ensure that it is
> * called before user-provided global properties are registered.
> *
> * @driver: Device type to be affected
> * @property: Property whose default value is going to be changed
> * @value: New default value for the property
> */
Sure. Will take it directly. Thanks!
>
> > +void register_compat_prop(const char *driver, const char *property,
> > + const char *value);
> > +
> > /**
> > * qdev_property_add_static:
> > * @dev: Device to add the property to.
> > --
> > 2.7.4
> >
> >
>
> --
> Eduardo
--
Peter Xu
