On Mon, Sep 16, 2013 at 08:32:13AM +0200, Andreas Färber wrote:
> Am 15.09.2013 19:23, schrieb Michael S. Tsirkin:
> > Add a helper macro for adding read-only properties, that works in the
> > common case where the value is a constant.
> >
> > Signed-off-by: Michael S. Tsirkin <m...@redhat.com>
> > ---
> >
> > I'm using this patch in my acpi work - any objections
> > to applying it on my tree?
>
> Actually yes: Apart from the clang issues raised and the disturbing
> upper-casing of arguments, this is hardcoding "int" type and NULL errp,
> so I don't think it deserves to live in object.h as is. I do agree that
> we could use more helper functions to deal with dynamic properties.
>
> So what about taking bool/string property helpers as example and putting
> intX_t getters into object.c, using a passed-through opaque argument to
> obtain the value? We could then have real object_property_add_int32()
> etc. functions using the appropriate type name, with field/value pointer
> and Error** arguments. A pointer can be assumed to hold up to uint32_t
> values or, to keep the API more general, use a local static const
> variable for non-field values.
This reminds me.
[mst@robin qemu]$ git grep object_property_set_bool
backends/rng.c: object_property_set_bool(OBJECT(s), true, "opened", errp);
backends/tpm.c: object_property_set_bool(OBJECT(s), true, "opened", errp);
hw/core/qdev-properties.c: object_property_set_bool(OBJECT(dev), value,
name, &errp);
hw/core/qdev.c: object_property_set_bool(OBJECT(dev), true, "realized",
&local_err);
hw/core/qdev.c: object_property_set_bool(obj, prop->defval, prop->name,
&local_err);
hw/core/qdev.c: object_property_set_bool(obj, false, "realized", NULL);
hw/i386/pc.c: object_property_set_bool(OBJECT(cpu), true, "realized",
&local_err);
hw/pci-host/prep.c: object_property_set_bool(OBJECT(&s->pci_dev), true,
"realized", errp);
hw/pci-host/versatile.c: object_property_set_bool(OBJECT(&s->pci_dev), true,
"realized", errp)
hw/scsi/scsi-bus.c: object_property_set_bool(OBJECT(dev), true, "realized",
&err);
include/qom/object.h: * object_property_set_bool:
include/qom/object.h:void object_property_set_bool(Object *obj, bool value,
qom/object.c:void object_property_set_bool(Object *obj, bool value,
target-alpha/cpu.c: object_property_set_bool(OBJECT(cpu), true, "realized",
NULL);
target-arm/helper.c: object_property_set_bool(OBJECT(cpu), true, "realized",
NULL);
target-cris/cpu.c: object_property_set_bool(OBJECT(cpu), true, "realized",
NULL);
target-i386/cpu.c: object_property_set_bool(OBJECT(cpu), true, "pmu",
&err);
target-i386/cpu.c: object_property_set_bool(OBJECT(cpu), true, "realized",
&error);
target-lm32/helper.c: object_property_set_bool(OBJECT(cpu), true,
"realized", NULL);
target-m68k/helper.c: object_property_set_bool(OBJECT(cpu), true,
"realized", NULL);
target-microblaze/translate.c: object_property_set_bool(OBJECT(cpu), true,
"realized", NULL);
target-mips/translate.c: object_property_set_bool(OBJECT(cpu), true,
"realized", NULL);
target-moxie/cpu.c: object_property_set_bool(OBJECT(cpu), true, "realized",
NULL);
target-openrisc/cpu.c: object_property_set_bool(OBJECT(cpu), true,
"realized", NULL);
target-ppc/translate_init.c: object_property_set_bool(OBJECT(cpu), true,
"realized", &err);
target-s390x/helper.c: object_property_set_bool(OBJECT(cpu), true,
"realized", NULL);
target-sh4/cpu.c: object_property_set_bool(OBJECT(cpu), true, "realized",
NULL);
target-sparc/cpu.c: object_property_set_bool(OBJECT(cpu), true, "realized",
NULL);
target-unicore32/helper.c: object_property_set_bool(OBJECT(cpu), true,
"realized", NULL);
target-xtensa/helper.c: object_property_set_bool(OBJECT(cpu), true,
"realized", NULL);
Shouldn't we have a constant for the "realized" string?
If there's a typo somewhere it will all fail at runtime
in a hard to debug way, won't it?
> It does touch on the issue I brought up on a KVM call a couple weeks ago
> of how dynamic and static properties are supposed to relate. I
> personally welcome making dynamic properties more easy to deal with; an
> alternative might be to extend qdev-properties.c with
> DEFINE_PROP_READONLY_UINT32() etc. CC'ing Igor, who has dealt with
> dynamic-vs.-static properties for X86CPU.
>
> Regards,
> Andreas
>
> >
> > include/qom/object.h | 21 +++++++++++++++++++++
> > 1 file changed, 21 insertions(+)
> >
> > diff --git a/include/qom/object.h b/include/qom/object.h
> > index 1a7b71a..4787de6 100644
> > --- a/include/qom/object.h
> > +++ b/include/qom/object.h
> > @@ -17,6 +17,7 @@
> > #include <glib.h>
> > #include <stdint.h>
> > #include <stdbool.h>
> > +#include "qemu/typedefs.h"
> > #include "qemu/queue.h"
> >
> > struct Visitor;
> > @@ -792,6 +793,26 @@ void object_property_add(Object *obj, const char
> > *name, const char *type,
> > ObjectPropertyRelease *release,
> > void *opaque, struct Error **errp);
> >
> > +/* Add a property that is an integer constant. */
> > +#define OBJECT_ADD_PROP_CONST(obj, name, value) \
> > + do {
> > \
> > + void OBJECT_ADD_PROP_GET(Object *OBJECT_ADD_PROP_OBJ,
> > \
> > + struct Visitor *OBJECT_ADD_PROP_VISITOR,
> > \
> > + void *OBJECT_ADD_PROP_OPAQUE,
> > \
> > + const char *OBJECT_ADD_PROP_NAME,
> > \
> > + struct Error **OBJECT_ADD_PROP_VALUE_ERR)
> > \
> > + {
> > \
> > + int64_t OBJECT_ADD_PROP_VALUE = value;
> > \
> > +
> > \
> > + visit_type_int64(OBJECT_ADD_PROP_VISITOR,
> > \
> > + &OBJECT_ADD_PROP_VALUE,
> > \
> > + OBJECT_ADD_PROP_NAME,
> > \
> > + OBJECT_ADD_PROP_VALUE_ERR);
> > \
> > + }
> > \
> > + object_property_add(obj, name, "int", OBJECT_ADD_PROP_GET,
> > \
> > + NULL, NULL, NULL, NULL);
> > \
> > + } while (0)
> > +
> > void object_property_del(Object *obj, const char *name, struct Error
> > **errp);
> >
> > /**
> >
>
>
> --
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
