Il 15/10/2012 18:35, Michael Roth ha scritto:
>> - immutable/derived/broken/elsewhere (and the default, let's call it
>> serialized) are really five cases of the same QIDL property. Perhaps
>> this could be enforced in the extended syntax like this:
>>
>> #define q_immutable QIDL(serialize(immutable))
>> #define q_derived QIDL(serialize(derived))
>> #define q_broken QIDL(serialize(broken))
>> #define q_elsewhere QIDL(serialize(elsewhere))
>>
>> I would also make it possible to explicitly specify the fifth state, if
>> only for symmetry.
>
> Agreed, that's a more proper grouping. Though, for consistency with
> QIDL(property, ...), I would do QIDL(serialize, ...)
Sure.
>> - it would be _much_ better if you could automatically derive properties
>> information for embedded structs. For example, Notifiers and qemu_irqs
>> are always q_immutable. NotifierLists probably are always q_elsewhere,
>> because the owner of the notifiers should add themselves back.
>
> q_inherit maybe? Otherwise we're overriding "q_default" in subtle
> ways that may not always be desired.
I think the default should be "whatever makes more sense", which for
QIDL_DECLAREd types means making the member q_immutable if it makes
sense for the type.
It's too fragile to expect all subclasses to know whether their
superclass is immutable or has to be serialized, so q_inherit should be
the default. For atomic types, q_inherit is the same as q_serialized.
That said, an alternative is just to never declare the superclass
q_immutable. That would work as long as you do not restrict QIDL to
DeviceState subclasses---see attached patch.
>> In general, if struct X is QIDL_DECLAREd and only has q_immutable
>> fields, it can be taken as q_immutable. Hence for example the base
>> class should not need any decoration; ISADevice will be seen as
>> q_immutable, but PCIDevice will be seen as serialized. But even if a
>> struct is not QIDL_DECLAREd, it should be possible to apply a tag to a
>> typedef, and have it always applied to the members.
Hmm, this wasn't the best choice of words. What I actually meant was
"to apply a tag to a typedef, and have it always applied to members of
that type in other structs". Like
typedef struct Notifier Notifier q_immutable;
Note that Notifier will never have any serializable state, hence it will
not be QIDL_DECLAREd. It is just a proxy that specifies a function to call.
While in principle it is possible to change the function at run-time,
that's not the way that Notifiers are used. That can still be
documented using q_elsewhere, but I think that sane defaults other than
q_serialized are useful to avoid cluttering the declarations. However,
this is a very minor qualm.
Paolo
>From 0629eab1d8cff0764d135b85052dd3ba47a1a198 Mon Sep 17 00:00:00 2001
From: Paolo Bonzini <pbonz...@redhat.com>
Date: Tue, 16 Oct 2012 09:19:04 +0200
Subject: [PATCH] qidl: allow marking Object as QIDL_DECLARE
Signed-off-by: Paolo Bonzini <pbonz...@redhat.com>
---
hw/isa.h | 8 ++++----
hw/mc146818rtc.c | 2 +-
hw/qdev.h | 28 ++++++++++++++--------------
include/qemu/object.h | 11 ++++++-----
qidl.h | 6 ++----
scripts/qidl.py | 2 +-
6 file modificati, 28 inserzioni(+), 29 rimozioni(-)
diff --git a/hw/isa.h b/hw/isa.h
index 8fb498a..6c7d815 100644
--- a/hw/isa.h
+++ b/hw/isa.h
@@ -31,11 +31,11 @@ struct ISABus {
qemu_irq *irqs;
};
-struct ISADevice {
+QIDL_DECLARE(ISADevice) {
DeviceState qdev;
- uint32_t isairq[2];
- int nirqs;
- int ioport_id;
+ uint32_t q_immutable isairq[2];
+ int q_immutable nirqs;
+ int q_immutable ioport_id;
};
ISABus *isa_bus_new(DeviceState *dev, MemoryRegion *address_space_io);
diff --git a/hw/mc146818rtc.c b/hw/mc146818rtc.c
index f5f4bb4..b29cce5 100644
--- a/hw/mc146818rtc.c
+++ b/hw/mc146818rtc.c
@@ -62,7 +62,7 @@ QIDL_ENABLE()
typedef struct RTCState RTCState;
QIDL_DECLARE(RTCState) {
- ISADevice q_immutable dev;
+ ISADevice dev;
MemoryRegion q_immutable io;
uint8_t cmos_data[128];
uint8_t cmos_index;
diff --git a/hw/qdev.h b/hw/qdev.h
index 6f2c7f2..3dba377 100644
--- a/hw/qdev.h
+++ b/hw/qdev.h
@@ -57,22 +57,22 @@ typedef struct DeviceClass {
/* This structure should not be accessed directly. We declare it here
so that it can be embedded in individual device state structures. */
-struct DeviceState {
+QIDL_DECLARE(DeviceState) {
Object parent_obj;
- const char *id;
- enum DevState state;
- QemuOpts *opts;
- int hotplugged;
- BusState *parent_bus;
- int num_gpio_out;
- qemu_irq *gpio_out;
- int num_gpio_in;
- qemu_irq *gpio_in;
- QLIST_HEAD(, BusState) child_bus;
- int num_child_bus;
- int instance_id_alias;
- int alias_required_for_version;
+ const char q_immutable *id;
+ enum DevState q_immutable state;
+ QemuOpts q_immutable *opts;
+ int q_immutable hotplugged;
+ BusState q_immutable *parent_bus;
+ int q_immutable num_gpio_out;
+ qemu_irq q_immutable *gpio_out;
+ int q_immutable num_gpio_in;
+ qemu_irq q_immutable *gpio_in;
+ QLIST_HEAD(, BusState) q_immutable child_bus;
+ int q_immutable num_child_bus;
+ int q_immutable instance_id_alias;
+ int q_immutable alias_required_for_version;
};
#define TYPE_BUS "bus"
diff --git a/include/qemu/object.h b/include/qemu/object.h
index cc75fee..cb4a89b 100644
--- a/include/qemu/object.h
+++ b/include/qemu/object.h
@@ -18,6 +18,7 @@
#include <stdint.h>
#include <stdbool.h>
#include "qemu-queue.h"
+#include "qidl.h"
struct Visitor;
struct Error;
@@ -257,13 +258,13 @@ struct ObjectClass
* #Object also contains a list of #Interfaces that this object
* implements.
*/
-struct Object
+QIDL_DECLARE(Object)
{
/*< private >*/
- ObjectClass *class;
- QTAILQ_HEAD(, ObjectProperty) properties;
- uint32_t ref;
- Object *parent;
+ ObjectClass q_immutable *class;
+ QTAILQ_HEAD(, ObjectProperty) q_immutable properties;
+ uint32_t q_elsewhere ref;
+ Object q_immutable *parent;
};
/**
diff --git a/qidl.h b/qidl.h
index b4c9b02..abff6ba 100644
--- a/qidl.h
+++ b/qidl.h
@@ -18,8 +18,6 @@
#include <glib.h>
#include "qapi/qapi-visit-core.h"
-#include "qemu/object.h"
-#include "hw/qdev-properties.h"
/* must be "called" in any C files that make use of QIDL-generated code */
#define QIDL_ENABLE()
@@ -42,8 +40,8 @@
static struct { \
void (*visitor)(Visitor *, struct name **, const char *, Error **); \
const char *schema_json_text; \
- Object *schema_obj; \
- Property *properties; \
+ struct Object *schema_obj; \
+ struct Property *properties; \
} qidl_data_##name;
#else
#define QIDL_START(name, ...)
diff --git a/scripts/qidl.py b/scripts/qidl.py
index 4e0880e..2544829 100644
--- a/scripts/qidl.py
+++ b/scripts/qidl.py
@@ -240,7 +240,7 @@ def main(argv=[]):
output_filepath = None
schema_filepath = None
- includes = []
+ includes = ["hw/qdev-properties.h"]
for o, a in opts:
if o in ("-f", "--output-filepath"):
output_filepath = a
--
1.7.12.1
