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 <peterx@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: */
+    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);
 
+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

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 <peterx@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).


> +    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
 */

> +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

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 <peterx@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