Am 16.09.2013 14:33, schrieb Michael S. Tsirkin: > 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);
These look like two distinct properties used once each. [...] > hw/core/qdev.c: object_property_set_bool(OBJECT(dev), true, "realized", > &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); [...] > 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); Leaving the bulk for "realized". > > Shouldn't we have a constant for the "realized" string? That's a two-sided sword: We actually shouldn't be setting realized = true manually but once on machine init - in that case we wouldn't strictly need a constant. I pushed to get that central infrastructure in place to spare me/us the repetitive realized = true setting, but Paolo shot it down, asking for a full-fledged solver to make ordering guarantees. > If there's a typo somewhere it will all fail at runtime > in a hard to debug way, won't it? It would. However, this is typically executed once on startup, so with proper error handling we should notice this immediately. My qom-test (that Anthony didn't take for 1.6 and I still need to respin) served to test them, with focus on SysBusDevices. You are cordially invited to add trivial qtests covering instantiation of PCI/virtio devices you care about. :) Andreas -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg