[v3,02/13] qdev: enhance global_prop_list_add()

Originally it can only alloc new entries and insert. Let it be smarter
that it can update existing fields, and even delete elements. This is
preparation of (finally) the replacement of x86_cpu_apply_props(). If
you see x86_cpu_apply_props(), it allows to skip entries when value is
NULL. Here, it works just like deleting an existing entry.

Signed-off-by: Peter Xu <peterx@redhat.com>
---
 hw/core/qdev-properties.c    | 28 +++++++++++++++++++++++++++-
 include/hw/qdev-properties.h | 10 ++++++++++
 2 files changed, 37 insertions(+), 1 deletion(-)

On Mon, Jun 19, 2017 at 08:49:37PM +0800, Peter Xu wrote:
> Originally it can only alloc new entries and insert. Let it be smarter
> that it can update existing fields, and even delete elements. This is
> preparation of (finally) the replacement of x86_cpu_apply_props(). If
> you see x86_cpu_apply_props(), it allows to skip entries when value is
> NULL. Here, it works just like deleting an existing entry.
> 
> Signed-off-by: Peter Xu <peterx@redhat.com>

This makes the global property API much more complex, making the
interactions between different callers much more subtle.

Currently, the global property list was always an append-only
list, and we had only two possible sources of global properties:
 1) MachineClass::compat_props;
 2) User-provided globals (including -global and other
   command-line options that are simply translated to globals).
So it is easy to calculate the results of a given QEMU
configuration: just append both lists.

With this patch, the rules become more complex, and calculating
the actual results of multiple register_compat_prop() calls is
not trivial.  Tracking the actual register_compat_prop() calls
scattered around the code (and figuring out the order in which
they are called) makes it even more difficult.  IMO, this defeats
the purpose of introducing a AccelClass::global_props field.

I believe the main source of this complexity are the
x86_cpu_change_kvm_default() calls in pc_piix.c, and the rules
those calls represent are really more complex.  But I would like
to find a simpler solution to represent those rules inside either
MachineClass::compat_props or AccelClass::global_props.  If we
can't do that, I think we should keep that complexity inside the
PC/x86 code instead of exposing it to all users of the global
properties API.

I suggest we do this, to keep things simple in the first version:

1) Introduce AccelClass::global_props without this patch.
2) Move: kvmclock, kvm-nopiodelay, kvm-asyncpf, kvm-steal-time,
   kvmclock-stable-bit, acpi, monitor to AccelClass::global_props
   in kvm-accel.
3) Move: vme=off to AccelClass::global_props in tcg-accel.
4) Keep svm, x2apic and kvm-pv-eoi inside kvm_default_props in
   x86 code, because their rules are more complex.
5) Look for a simpler way to represent the rules from (4) later.


> ---
>  hw/core/qdev-properties.c    | 28 +++++++++++++++++++++++++++-
>  include/hw/qdev-properties.h | 10 ++++++++++
>  2 files changed, 37 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
> index 4b74382..dc3b0ac 100644
> --- a/hw/core/qdev-properties.c
> +++ b/hw/core/qdev-properties.c
> @@ -1045,7 +1045,33 @@ static GList *global_props;
>  GList *global_prop_list_add(GList *list, const char *driver,
>                              const char *property, const char *value)
>  {
> -    GlobalProperty *p = g_new0(GlobalProperty, 1);
> +    GList *l;
> +    GlobalProperty *p;
> +
> +    /* Look up the (property, value) first in the list */
> +    for (l = list; l; l = l->next) {
> +        p = l->data;
> +        if (!strcmp(driver, p->driver) && !strcmp(property, p->property)) {
> +            if (value) {
> +                /* Modify existing value */
> +                p->value = value;
> +            } else {
> +                /* Delete this entry entirely */
> +                list = g_list_remove_link(list, l);
> +                g_free(p);
> +                g_list_free(l);
> +            }
> +            return list;
> +        }
> +    }
> +
> +    /* Entry not exist. If we are deleting one entry, we're done. */
> +    if (!value) {
> +        return list;
> +    }
> +
> +    /* If not found and value is set, we try to create a new entry */
> +    p = g_new0(GlobalProperty, 1);
>  
>      /* These properties cannot fail the apply */
>      p->errp = &error_abort;
> diff --git a/include/hw/qdev-properties.h b/include/hw/qdev-properties.h
> index 15ee6ba..55ad507 100644
> --- a/include/hw/qdev-properties.h
> +++ b/include/hw/qdev-properties.h
> @@ -201,6 +201,16 @@ 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);
>  
> +/*
> + * Register global property on the list. Elements of list should be
> + * GlobalProperty.
> + *
> + * - If (driver, property) is not existing on the list, create a new
> + *   one and link to the list.
> + * - If (driver, property) exists on the list, then:
> + *   - if value != NULL, update with new value
> + *   - if value == NULL, delete the entry
> + */
>  GList *global_prop_list_add(GList *list, const char *driver,
>                              const char *property, const char *value);
>  void register_compat_prop(const char *driver, const char *property,
> -- 
> 2.7.4
>