* [Qemu-devel] [PATCH v2 0/8] qom: introduce non-legacy static properties
@ 2011-12-16 16:47 Paolo Bonzini
2011-12-16 16:47 ` [Qemu-devel] [PATCH v2 1/8] qapi: protect against NULL QObject in qmp_input_get_object Paolo Bonzini
` (7 more replies)
0 siblings, 8 replies; 15+ messages in thread
From: Paolo Bonzini @ 2011-12-16 16:47 UTC (permalink / raw)
To: qemu-devel
QOM right now does not have a way to communicate values for qdev
properties except as strings. This is bad.
This patch improves the Property implementation so that properties
export a visitor-based interface in addition to the string-based
interface. The new interface can then be registered as a "static"
property. It's called static because it uses a struct field for
storage and, as such, should be present in all objects of a given
class.
Patches 1-2 are bugfixes and patches 3-4 are cleanups.
Example using qmp-shell:
x86_64-softmmu/qemu-system-x86_64 \
-hda ~/test.img -snapshot -S \
-qmp unix:/tmp/server.sock,server,nowait \
-netdev type=user,id=net -device virtio-net-pci,netdev=net,id=net \
-net user,vlan=1 -device virtio-net-pci,id=net2,vlan=1 \
-chardev id=stdio,backend=stdio -device isa-serial,chardev=stdio,id=serial
Boolean properties:
(QEMU) qom-get path=/i440fx/piix3 property=command_serr_enable
{u'return': True}
(QEMU) qom-get path=/i440fx/piix3 property=legacy<command_serr_enable>
{u'return': u'on'}
PCI address properties (perhaps will disappear later, but not yet):
(QEMU) qom-get path=/i440fx/piix3 property=addr
{u'return': u'01.0'}
(QEMU) qom-get path=/i440fx/piix3 property=legacy<addr>
{u'return': u'01.0'}
String properties (QObject does not have NULL):
(QEMU) qom-get path=/vga property=romfile
{u'return': u'vgabios-cirrus.bin'}
(QEMU) qom-get path=/vga property=legacy<romfile>
{u'return': u'"vgabios-cirrus.bin"'}
(QEMU) qom-get path=/i440fx/piix3 property=romfile
{u'return': u''}
(QEMU) qom-get path=/i440fx/piix3 property=legacy<romfile>
{u'return': u'<null>'}
MAC properties:
(QEMU) qom-get path=/peripheral/net property=mac
{u'return': u'52:54:00:12:34:56'}
(QEMU) qom-get path=/peripheral/net property=legacy<mac>
{u'return': u'52:54:00:12:34:56'}
(QEMU) qom-set path=/peripheral/net property=mac value=52-54-00-12-34-57
{u'error': {u'data': {}, u'class': u'PermissionDenied', u'desc': u'Insufficient permission to perform this operation'}}
Network properties:
(QEMU) qom-get path=/peripheral/net property=netdev
{u'return': u'net'}
(QEMU) qom-get path=/peripheral/net property=legacy<netdev>
{u'return': u'net'}
VLAN properties:
(QEMU) qom-get path=/peripheral/net property=vlan
{u'return': -1}
(QEMU) qom-get path=/peripheral/net property=legacy<vlan>
{u'return': u'<null>'}
(QEMU) qom-get path=/peripheral/net2 property=vlan
{u'return': 1}
(QEMU) qom-get path=/peripheral/net2 property=legacy<vlan>
{u'return': u'1'}
Chardev properties:
(QEMU) qom-get path=/peripheral/serial property=chardev
{u'return': u'stdio'}
(QEMU) qom-get path=/peripheral/serial property=legacy<chardev>
{u'return': u'stdio'}
Legacy hex32 properties:
(QEMU) qom-get path=/peripheral/serial property=iobase
{u'return': 1016}
(QEMU) qom-get path=/peripheral/serial property=legacy<iobase>
{u'return': u'0x3f8'}
Examples of setting properties (after disabling the DEV_STATE_CREATED
check for testing only):
(QEMU) qom-set path=/peripheral/net2 property=vlan value=-1
{u'return': {}}
(QEMU) qom-get path=/peripheral/net2 property=vlan
{u'return': -1}
(QEMU) qom-get path=/peripheral/net2 property=legacy<vlan>
{u'return': u'<null>'}
(QEMU) qom-set path=/peripheral/serial property=iobase value=760
{u'return': {}}
(QEMU) qom-get path=/peripheral/serial property=iobase
{u'return': 760}
(QEMU) qom-get path=/peripheral/serial property=legacy<iobase>
{u'return': u'0x2f8'}
v1->v2:
New "qom: interpret the return value when setting legacy
properties". Always pass a value to the visitor when there
is no error. Handle empty strings as NULL. Did not change
QERR_PROPERTY_VALUE_OUT_OF_RANGE because it is consistent
with other QERR_PROPERTY_* errors, now used by QOM too.
Paolo Bonzini (8):
qapi: protect against NULL QObject in qmp_input_get_object
qom: fix swapped parameters
qom: push permission checks up into qdev_property_add_legacy
qom: interpret the return value when setting legacy properties
qom: introduce QERR_PROPERTY_VALUE_OUT_OF_RANGE
qom: introduce get/set methods for Property
qom: distinguish "legacy" property type name from QOM type name
qom: register qdev properties also as non-legacy properties
hw/qdev-addr.c | 41 +++++
hw/qdev-properties.c | 406 +++++++++++++++++++++++++++++++++++++++++++---
hw/qdev.c | 84 ++++++----
hw/qdev.h | 14 ++-
qapi/qmp-input-visitor.c | 10 +-
qerror.c | 5 +
qerror.h | 3 +
7 files changed, 502 insertions(+), 61 deletions(-)
--
1.7.7.1
^ permalink raw reply [flat|nested] 15+ messages in thread
* [Qemu-devel] [PATCH v2 1/8] qapi: protect against NULL QObject in qmp_input_get_object
2011-12-16 16:47 [Qemu-devel] [PATCH v2 0/8] qom: introduce non-legacy static properties Paolo Bonzini
@ 2011-12-16 16:47 ` Paolo Bonzini
2011-12-16 16:47 ` [Qemu-devel] [PATCH v2 2/8] qom: fix swapped parameters Paolo Bonzini
` (6 subsequent siblings)
7 siblings, 0 replies; 15+ messages in thread
From: Paolo Bonzini @ 2011-12-16 16:47 UTC (permalink / raw)
To: qemu-devel
A NULL qobj can occur when a parameter is fetched via qdict_get, but
the parameter is not in the command. By returning NULL, the caller can
choose whether to raise a missing parameter error, an invalid parameter
type error, or use a default value. For example, qom-set could can
use this to reset a property to its default value, though at this time
it will fail with "Invalid parameter type". In any case, anything is
better than crashing!
Reviewed-by: Anthony Liguori <anthony@codemonkey.ws>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
qapi/qmp-input-visitor.c | 10 ++++++----
1 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c
index 8cbc0ab..c78022b 100644
--- a/qapi/qmp-input-visitor.c
+++ b/qapi/qmp-input-visitor.c
@@ -49,10 +49,12 @@ static const QObject *qmp_input_get_object(QmpInputVisitor *qiv,
qobj = qiv->stack[qiv->nb_stack - 1].obj;
}
- if (name && qobject_type(qobj) == QTYPE_QDICT) {
- return qdict_get(qobject_to_qdict(qobj), name);
- } else if (qiv->nb_stack > 0 && qobject_type(qobj) == QTYPE_QLIST) {
- return qlist_entry_obj(qiv->stack[qiv->nb_stack - 1].entry);
+ if (qobj) {
+ if (name && qobject_type(qobj) == QTYPE_QDICT) {
+ return qdict_get(qobject_to_qdict(qobj), name);
+ } else if (qiv->nb_stack > 0 && qobject_type(qobj) == QTYPE_QLIST) {
+ return qlist_entry_obj(qiv->stack[qiv->nb_stack - 1].entry);
+ }
}
return qobj;
--
1.7.7.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [Qemu-devel] [PATCH v2 2/8] qom: fix swapped parameters
2011-12-16 16:47 [Qemu-devel] [PATCH v2 0/8] qom: introduce non-legacy static properties Paolo Bonzini
2011-12-16 16:47 ` [Qemu-devel] [PATCH v2 1/8] qapi: protect against NULL QObject in qmp_input_get_object Paolo Bonzini
@ 2011-12-16 16:47 ` Paolo Bonzini
2011-12-16 16:47 ` [Qemu-devel] [PATCH v2 3/8] qom: push permission checks up into qdev_property_add_legacy Paolo Bonzini
` (5 subsequent siblings)
7 siblings, 0 replies; 15+ messages in thread
From: Paolo Bonzini @ 2011-12-16 16:47 UTC (permalink / raw)
To: qemu-devel
Reviewed-by: Anthony Liguori <anthony@codemonkey.ws>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
hw/qdev.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/hw/qdev.c b/hw/qdev.c
index 83913c7..bda8d6c 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -1110,7 +1110,7 @@ void qdev_property_set(DeviceState *dev, Visitor *v, const char *name,
if (!prop->set) {
error_set(errp, QERR_PERMISSION_DENIED);
} else {
- prop->set(dev, prop->opaque, v, name, errp);
+ prop->set(dev, v, prop->opaque, name, errp);
}
}
--
1.7.7.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [Qemu-devel] [PATCH v2 3/8] qom: push permission checks up into qdev_property_add_legacy
2011-12-16 16:47 [Qemu-devel] [PATCH v2 0/8] qom: introduce non-legacy static properties Paolo Bonzini
2011-12-16 16:47 ` [Qemu-devel] [PATCH v2 1/8] qapi: protect against NULL QObject in qmp_input_get_object Paolo Bonzini
2011-12-16 16:47 ` [Qemu-devel] [PATCH v2 2/8] qom: fix swapped parameters Paolo Bonzini
@ 2011-12-16 16:47 ` Paolo Bonzini
2011-12-16 16:47 ` [Qemu-devel] [PATCH v2 4/8] qom: interpret the return value when setting legacy properties Paolo Bonzini
` (4 subsequent siblings)
7 siblings, 0 replies; 15+ messages in thread
From: Paolo Bonzini @ 2011-12-16 16:47 UTC (permalink / raw)
To: qemu-devel
qdev_property_get and qdev_property_set can generate permission
denied errors themselves. Do not duplicate this functionality in
qdev_get/set_legacy_property, and clean up excessive indentation.
Reviewed-by: Anthony Liguori <anthony@codemonkey.ws>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
hw/qdev.c | 46 +++++++++++++++++++---------------------------
1 files changed, 19 insertions(+), 27 deletions(-)
diff --git a/hw/qdev.c b/hw/qdev.c
index bda8d6c..c020a6f 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -1135,46 +1135,38 @@ static void qdev_get_legacy_property(DeviceState *dev, Visitor *v, void *opaque,
{
Property *prop = opaque;
- if (prop->info->print) {
- char buffer[1024];
- char *ptr = buffer;
+ char buffer[1024];
+ char *ptr = buffer;
- prop->info->print(dev, prop, buffer, sizeof(buffer));
- visit_type_str(v, &ptr, name, errp);
- } else {
- error_set(errp, QERR_PERMISSION_DENIED);
- }
+ prop->info->print(dev, prop, buffer, sizeof(buffer));
+ visit_type_str(v, &ptr, name, errp);
}
static void qdev_set_legacy_property(DeviceState *dev, Visitor *v, void *opaque,
const char *name, Error **errp)
{
Property *prop = opaque;
+ Error *local_err = NULL;
+ char *ptr = NULL;
+ int ret;
if (dev->state != DEV_STATE_CREATED) {
error_set(errp, QERR_PERMISSION_DENIED);
return;
}
- if (prop->info->parse) {
- Error *local_err = NULL;
- char *ptr = NULL;
+ visit_type_str(v, &ptr, name, &local_err);
+ if (local_err) {
+ error_propagate(errp, local_err);
+ return;
+ }
- visit_type_str(v, &ptr, name, &local_err);
- if (!local_err) {
- int ret;
- ret = prop->info->parse(dev, prop, ptr);
- if (ret != 0) {
- error_set(errp, QERR_INVALID_PARAMETER_VALUE,
- name, prop->info->name);
- }
- g_free(ptr);
- } else {
- error_propagate(errp, local_err);
- }
- } else {
- error_set(errp, QERR_PERMISSION_DENIED);
+ ret = prop->info->parse(dev, prop, ptr);
+ if (ret != 0) {
+ error_set(errp, QERR_INVALID_PARAMETER_VALUE,
+ name, prop->info->name);
}
+ g_free(ptr);
}
/**
@@ -1194,8 +1186,8 @@ void qdev_property_add_legacy(DeviceState *dev, Property *prop,
type = g_strdup_printf("legacy<%s>", prop->info->name);
qdev_property_add(dev, prop->name, type,
- qdev_get_legacy_property,
- qdev_set_legacy_property,
+ prop->info->print ? qdev_get_legacy_property : NULL,
+ prop->info->parse ? qdev_set_legacy_property : NULL,
NULL,
prop, errp);
--
1.7.7.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [Qemu-devel] [PATCH v2 4/8] qom: interpret the return value when setting legacy properties
2011-12-16 16:47 [Qemu-devel] [PATCH v2 0/8] qom: introduce non-legacy static properties Paolo Bonzini
` (2 preceding siblings ...)
2011-12-16 16:47 ` [Qemu-devel] [PATCH v2 3/8] qom: push permission checks up into qdev_property_add_legacy Paolo Bonzini
@ 2011-12-16 16:47 ` Paolo Bonzini
2011-12-16 17:00 ` Anthony Liguori
2011-12-16 16:47 ` [Qemu-devel] [PATCH v2 5/8] qom: introduce QERR_PROPERTY_VALUE_OUT_OF_RANGE Paolo Bonzini
` (3 subsequent siblings)
7 siblings, 1 reply; 15+ messages in thread
From: Paolo Bonzini @ 2011-12-16 16:47 UTC (permalink / raw)
To: qemu-devel
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
hw/qdev-properties.c | 39 ++++++++++++++++++++++++---------------
hw/qdev.c | 3 +--
hw/qdev.h | 2 ++
3 files changed, 27 insertions(+), 17 deletions(-)
diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
index f0b811c..76ecb38 100644
--- a/hw/qdev-properties.c
+++ b/hw/qdev-properties.c
@@ -614,6 +614,26 @@ int qdev_prop_exists(DeviceState *dev, const char *name)
return qdev_prop_find(dev, name) ? true : false;
}
+void qdev_prop_error(Error **errp, int ret,
+ DeviceState *dev, Property *prop, const char *value)
+{
+ switch (ret) {
+ case -EEXIST:
+ error_set(errp, QERR_PROPERTY_VALUE_IN_USE,
+ dev->info->name, prop->name, value);
+ break;
+ default:
+ case -EINVAL:
+ error_set(errp, QERR_PROPERTY_VALUE_BAD,
+ dev->info->name, prop->name, value);
+ break;
+ case -ENOENT:
+ error_set(errp, QERR_PROPERTY_VALUE_NOT_FOUND,
+ dev->info->name, prop->name, value);
+ break;
+ }
+}
+
int qdev_prop_parse(DeviceState *dev, const char *name, const char *value)
{
Property *prop;
@@ -632,21 +652,10 @@ int qdev_prop_parse(DeviceState *dev, const char *name, const char *value)
}
ret = prop->info->parse(dev, prop, value);
if (ret < 0) {
- switch (ret) {
- case -EEXIST:
- qerror_report(QERR_PROPERTY_VALUE_IN_USE,
- dev->info->name, name, value);
- break;
- default:
- case -EINVAL:
- qerror_report(QERR_PROPERTY_VALUE_BAD,
- dev->info->name, name, value);
- break;
- case -ENOENT:
- qerror_report(QERR_PROPERTY_VALUE_NOT_FOUND,
- dev->info->name, name, value);
- break;
- }
+ Error *err;
+ qdev_prop_error(&err, ret, dev, prop, value);
+ qerror_report_err(err);
+ error_free(err);
return -1;
}
return 0;
diff --git a/hw/qdev.c b/hw/qdev.c
index c020a6f..c8ab7b7 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -1163,8 +1163,7 @@ static void qdev_set_legacy_property(DeviceState *dev, Visitor *v, void *opaque,
ret = prop->info->parse(dev, prop, ptr);
if (ret != 0) {
- error_set(errp, QERR_INVALID_PARAMETER_VALUE,
- name, prop->info->name);
+ qdev_prop_error(errp, ret, dev, prop, ptr);
}
g_free(ptr);
}
diff --git a/hw/qdev.h b/hw/qdev.h
index 6e18427..828d811 100644
--- a/hw/qdev.h
+++ b/hw/qdev.h
@@ -370,6 +370,8 @@ void qdev_prop_set_defaults(DeviceState *dev, Property *props);
void qdev_prop_register_global_list(GlobalProperty *props);
void qdev_prop_set_globals(DeviceState *dev);
+void qdev_prop_error(Error **errp, int ret, DeviceState *name,
+ Property *prop, const char *value);
static inline const char *qdev_fw_name(DeviceState *dev)
{
--
1.7.7.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [Qemu-devel] [PATCH v2 5/8] qom: introduce QERR_PROPERTY_VALUE_OUT_OF_RANGE
2011-12-16 16:47 [Qemu-devel] [PATCH v2 0/8] qom: introduce non-legacy static properties Paolo Bonzini
` (3 preceding siblings ...)
2011-12-16 16:47 ` [Qemu-devel] [PATCH v2 4/8] qom: interpret the return value when setting legacy properties Paolo Bonzini
@ 2011-12-16 16:47 ` Paolo Bonzini
2011-12-16 17:00 ` Anthony Liguori
2011-12-16 16:47 ` [Qemu-devel] [PATCH v2 6/8] qom: introduce get/set methods for Property Paolo Bonzini
` (2 subsequent siblings)
7 siblings, 1 reply; 15+ messages in thread
From: Paolo Bonzini @ 2011-12-16 16:47 UTC (permalink / raw)
To: qemu-devel
This will be used when reject invalid values for integer fields that
are less than 64-bits wide.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
qerror.c | 5 +++++
qerror.h | 3 +++
2 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/qerror.c b/qerror.c
index adde8a5..9a75d06 100644
--- a/qerror.c
+++ b/qerror.c
@@ -206,6 +206,11 @@ static const QErrorStringTable qerror_table[] = {
.desc = "Property '%(device).%(property)' can't find value '%(value)'",
},
{
+ .error_fmt = QERR_PROPERTY_VALUE_OUT_OF_RANGE,
+ .desc = "Property '%(device).%(property)' doesn't take "
+ "value %(value) (minimum: %(min), maximum: %(max)'",
+ },
+ {
.error_fmt = QERR_QMP_BAD_INPUT_OBJECT,
.desc = "Expected '%(expected)' in QMP input",
},
diff --git a/qerror.h b/qerror.h
index 9190b02..efda232 100644
--- a/qerror.h
+++ b/qerror.h
@@ -171,6 +171,9 @@ QError *qobject_to_qerror(const QObject *obj);
#define QERR_PROPERTY_VALUE_NOT_FOUND \
"{ 'class': 'PropertyValueNotFound', 'data': { 'device': %s, 'property': %s, 'value': %s } }"
+#define QERR_PROPERTY_VALUE_OUT_OF_RANGE \
+ "{ 'class': 'PropertyValueOutOfRange', 'data': { 'device': %s, 'property': %s, 'value': %"PRId64", 'min': %"PRId64", 'max': %"PRId64" } }"
+
#define QERR_QMP_BAD_INPUT_OBJECT \
"{ 'class': 'QMPBadInputObject', 'data': { 'expected': %s } }"
--
1.7.7.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [Qemu-devel] [PATCH v2 6/8] qom: introduce get/set methods for Property
2011-12-16 16:47 [Qemu-devel] [PATCH v2 0/8] qom: introduce non-legacy static properties Paolo Bonzini
` (4 preceding siblings ...)
2011-12-16 16:47 ` [Qemu-devel] [PATCH v2 5/8] qom: introduce QERR_PROPERTY_VALUE_OUT_OF_RANGE Paolo Bonzini
@ 2011-12-16 16:47 ` Paolo Bonzini
2011-12-16 17:02 ` Anthony Liguori
2011-12-16 16:47 ` [Qemu-devel] [PATCH v2 7/8] qom: distinguish "legacy" property type name from QOM type name Paolo Bonzini
2011-12-16 16:47 ` [Qemu-devel] [PATCH v2 8/8] qom: register qdev properties also as non-legacy properties Paolo Bonzini
7 siblings, 1 reply; 15+ messages in thread
From: Paolo Bonzini @ 2011-12-16 16:47 UTC (permalink / raw)
To: qemu-devel
This patch adds a visitor interface to Property. This way, QOM will be
able to expose Properties that access a fixed field in a struct without
exposing also the everything-is-a-string "feature" of qdev properties.
Whenever the printed representation in both QOM and qdev (which is
typically the case for device backends), parse/print code can be reused
via get_generic/set_generic. Dually, whenever multiple PropertyInfos
have the same representation in both the struct and the visitors the
code can be reused (for example among all of int32/uint32/hex32).
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
hw/qdev-addr.c | 41 ++++++
hw/qdev-properties.c | 355 ++++++++++++++++++++++++++++++++++++++++++++++++++
hw/qdev.h | 4 +
3 files changed, 400 insertions(+), 0 deletions(-)
diff --git a/hw/qdev-addr.c b/hw/qdev-addr.c
index 305c2d3..5ddda2d 100644
--- a/hw/qdev-addr.c
+++ b/hw/qdev-addr.c
@@ -18,12 +18,53 @@ static int print_taddr(DeviceState *dev, Property *prop, char *dest, size_t len)
return snprintf(dest, len, "0x" TARGET_FMT_plx, *ptr);
}
+static void get_taddr(DeviceState *dev, Visitor *v, void *opaque,
+ const char *name, Error **errp)
+{
+ Property *prop = opaque;
+ target_phys_addr_t *ptr = qdev_get_prop_ptr(dev, prop);
+ int64_t value;
+
+ value = *ptr;
+ visit_type_int(v, &value, name, errp);
+}
+
+static void set_taddr(DeviceState *dev, Visitor *v, void *opaque,
+ const char *name, Error **errp)
+{
+ Property *prop = opaque;
+ target_phys_addr_t *ptr = qdev_get_prop_ptr(dev, prop);
+ Error *local_err = NULL;
+ int64_t value;
+
+ if (dev->state != DEV_STATE_CREATED) {
+ error_set(errp, QERR_PERMISSION_DENIED);
+ return;
+ }
+
+ visit_type_int(v, &value, name, &local_err);
+ if (local_err) {
+ error_propagate(errp, local_err);
+ return;
+ }
+ if ((uint64_t)value <= (uint64_t) ~(target_phys_addr_t)0) {
+ *ptr = value;
+ } else {
+ error_set(errp, QERR_PROPERTY_VALUE_OUT_OF_RANGE,
+ dev->id?:"", name, value, (uint64_t) 0,
+ (uint64_t) ~(target_phys_addr_t)0);
+ }
+}
+
+
PropertyInfo qdev_prop_taddr = {
.name = "taddr",
.type = PROP_TYPE_TADDR,
.size = sizeof(target_phys_addr_t),
.parse = parse_taddr,
.print = print_taddr,
+ .get = get_taddr,
+ .set = set_taddr,
};
void qdev_prop_set_taddr(DeviceState *dev, const char *name, target_phys_addr_t value)
diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
index 76ecb38..f7aa3cb 100644
--- a/hw/qdev-properties.c
+++ b/hw/qdev-properties.c
@@ -55,12 +55,44 @@ static int print_bit(DeviceState *dev, Property *prop, char *dest, size_t len)
return snprintf(dest, len, (*p & qdev_get_prop_mask(prop)) ? "on" : "off");
}
+static void get_bit(DeviceState *dev, Visitor *v, void *opaque,
+ const char *name, Error **errp)
+{
+ Property *prop = opaque;
+ uint32_t *p = qdev_get_prop_ptr(dev, prop);
+ bool value = (*p & qdev_get_prop_mask(prop)) != 0;
+
+ visit_type_bool(v, &value, name, errp);
+}
+
+static void set_bit(DeviceState *dev, Visitor *v, void *opaque,
+ const char *name, Error **errp)
+{
+ Property *prop = opaque;
+ Error *local_err = NULL;
+ bool value;
+
+ if (dev->state != DEV_STATE_CREATED) {
+ error_set(errp, QERR_PERMISSION_DENIED);
+ return;
+ }
+
+ visit_type_bool(v, &value, name, &local_err);
+ if (local_err) {
+ error_propagate(errp, local_err);
+ return;
+ }
+ bit_prop_set(dev, prop, value);
+}
+
PropertyInfo qdev_prop_bit = {
.name = "on/off",
.type = PROP_TYPE_BIT,
.size = sizeof(uint32_t),
.parse = parse_bit,
.print = print_bit,
+ .get = get_bit,
+ .set = set_bit,
};
/* --- 8bit integer --- */
@@ -85,12 +117,54 @@ static int print_uint8(DeviceState *dev, Property *prop, char *dest, size_t len)
return snprintf(dest, len, "%" PRIu8, *ptr);
}
+static void get_int8(DeviceState *dev, Visitor *v, void *opaque,
+ const char *name, Error **errp)
+{
+ Property *prop = opaque;
+ int8_t *ptr = qdev_get_prop_ptr(dev, prop);
+ int64_t value;
+
+ value = *ptr;
+ visit_type_int(v, &value, name, errp);
+}
+
+static void set_int8(DeviceState *dev, Visitor *v, void *opaque,
+ const char *name, Error **errp)
+{
+ Property *prop = opaque;
+ int8_t *ptr = qdev_get_prop_ptr(dev, prop);
+ Error *local_err = NULL;
+ int64_t value;
+
+ if (dev->state != DEV_STATE_CREATED) {
+ error_set(errp, QERR_PERMISSION_DENIED);
+ return;
+ }
+
+ visit_type_int(v, &value, name, &local_err);
+ if (local_err) {
+ error_propagate(errp, local_err);
+ return;
+ }
+ if (value > prop->info->min && value <= prop->info->max) {
+ *ptr = value;
+ } else {
+ error_set(errp, QERR_PROPERTY_VALUE_OUT_OF_RANGE,
+ dev->id?:"", name, value, prop->info->min,
+ prop->info->max);
+ }
+}
+
PropertyInfo qdev_prop_uint8 = {
.name = "uint8",
.type = PROP_TYPE_UINT8,
.size = sizeof(uint8_t),
.parse = parse_uint8,
.print = print_uint8,
+ .get = get_int8,
+ .set = set_int8,
+ .min = 0,
+ .max = 255,
};
/* --- 8bit hex value --- */
@@ -120,6 +194,10 @@ PropertyInfo qdev_prop_hex8 = {
.size = sizeof(uint8_t),
.parse = parse_hex8,
.print = print_hex8,
+ .get = get_int8,
+ .set = set_int8,
+ .min = 0,
+ .max = 255,
};
/* --- 16bit integer --- */
@@ -144,12 +222,54 @@ static int print_uint16(DeviceState *dev, Property *prop, char *dest, size_t len
return snprintf(dest, len, "%" PRIu16, *ptr);
}
+static void get_int16(DeviceState *dev, Visitor *v, void *opaque,
+ const char *name, Error **errp)
+{
+ Property *prop = opaque;
+ int16_t *ptr = qdev_get_prop_ptr(dev, prop);
+ int64_t value;
+
+ value = *ptr;
+ visit_type_int(v, &value, name, errp);
+}
+
+static void set_int16(DeviceState *dev, Visitor *v, void *opaque,
+ const char *name, Error **errp)
+{
+ Property *prop = opaque;
+ int16_t *ptr = qdev_get_prop_ptr(dev, prop);
+ Error *local_err = NULL;
+ int64_t value;
+
+ if (dev->state != DEV_STATE_CREATED) {
+ error_set(errp, QERR_PERMISSION_DENIED);
+ return;
+ }
+
+ visit_type_int(v, &value, name, &local_err);
+ if (local_err) {
+ error_propagate(errp, local_err);
+ return;
+ }
+ if (value > prop->info->min && value <= prop->info->max) {
+ *ptr = value;
+ } else {
+ error_set(errp, QERR_PROPERTY_VALUE_OUT_OF_RANGE,
+ dev->id?:"", name, value, prop->info->min,
+ prop->info->max);
+ }
+}
+
PropertyInfo qdev_prop_uint16 = {
.name = "uint16",
.type = PROP_TYPE_UINT16,
.size = sizeof(uint16_t),
.parse = parse_uint16,
.print = print_uint16,
+ .get = get_int16,
+ .set = set_int16,
+ .min = 0,
+ .max = 65535,
};
/* --- 32bit integer --- */
@@ -174,12 +294,54 @@ static int print_uint32(DeviceState *dev, Property *prop, char *dest, size_t len
return snprintf(dest, len, "%" PRIu32, *ptr);
}
+static void get_int32(DeviceState *dev, Visitor *v, void *opaque,
+ const char *name, Error **errp)
+{
+ Property *prop = opaque;
+ int32_t *ptr = qdev_get_prop_ptr(dev, prop);
+ int64_t value;
+
+ value = *ptr;
+ visit_type_int(v, &value, name, errp);
+}
+
+static void set_int32(DeviceState *dev, Visitor *v, void *opaque,
+ const char *name, Error **errp)
+{
+ Property *prop = opaque;
+ int32_t *ptr = qdev_get_prop_ptr(dev, prop);
+ Error *local_err = NULL;
+ int64_t value;
+
+ if (dev->state != DEV_STATE_CREATED) {
+ error_set(errp, QERR_PERMISSION_DENIED);
+ return;
+ }
+
+ visit_type_int(v, &value, name, &local_err);
+ if (local_err) {
+ error_propagate(errp, local_err);
+ return;
+ }
+ if (value > prop->info->min && value <= prop->info->max) {
+ *ptr = value;
+ } else {
+ error_set(errp, QERR_PROPERTY_VALUE_OUT_OF_RANGE,
+ dev->id?:"", name, value, prop->info->min,
+ prop->info->max);
+ }
+}
+
PropertyInfo qdev_prop_uint32 = {
.name = "uint32",
.type = PROP_TYPE_UINT32,
.size = sizeof(uint32_t),
.parse = parse_uint32,
.print = print_uint32,
+ .get = get_int32,
+ .set = set_int32,
+ .min = 0,
+ .max = 0xFFFFFFFFULL,
};
static int parse_int32(DeviceState *dev, Property *prop, const char *str)
@@ -207,6 +369,10 @@ PropertyInfo qdev_prop_int32 = {
.size = sizeof(int32_t),
.parse = parse_int32,
.print = print_int32,
+ .get = get_int32,
+ .set = set_int32,
+ .min = -0x80000000LL,
+ .max = 0x7FFFFFFFLL,
};
/* --- 32bit hex value --- */
@@ -236,6 +402,10 @@ PropertyInfo qdev_prop_hex32 = {
.size = sizeof(uint32_t),
.parse = parse_hex32,
.print = print_hex32,
+ .get = get_int32,
+ .set = set_int32,
+ .min = 0,
+ .max = 0xFFFFFFFFULL,
};
/* --- 64bit integer --- */
@@ -260,12 +430,37 @@ static int print_uint64(DeviceState *dev, Property *prop, char *dest, size_t len
return snprintf(dest, len, "%" PRIu64, *ptr);
}
+static void get_int64(DeviceState *dev, Visitor *v, void *opaque,
+ const char *name, Error **errp)
+{
+ Property *prop = opaque;
+ int64_t *ptr = qdev_get_prop_ptr(dev, prop);
+
+ visit_type_int(v, ptr, name, errp);
+}
+
+static void set_int64(DeviceState *dev, Visitor *v, void *opaque,
+ const char *name, Error **errp)
+{
+ Property *prop = opaque;
+ int64_t *ptr = qdev_get_prop_ptr(dev, prop);
+
+ if (dev->state != DEV_STATE_CREATED) {
+ error_set(errp, QERR_PERMISSION_DENIED);
+ return;
+ }
+
+ visit_type_int(v, ptr, name, errp);
+}
+
PropertyInfo qdev_prop_uint64 = {
.name = "uint64",
.type = PROP_TYPE_UINT64,
.size = sizeof(uint64_t),
.parse = parse_uint64,
.print = print_uint64,
+ .get = get_int64,
+ .set = set_int64,
};
/* --- 64bit hex value --- */
@@ -295,6 +490,8 @@ PropertyInfo qdev_prop_hex64 = {
.size = sizeof(uint64_t),
.parse = parse_hex64,
.print = print_hex64,
+ .get = get_int64,
+ .set = set_int64,
};
/* --- string --- */
@@ -322,6 +519,48 @@ static int print_string(DeviceState *dev, Property *prop, char *dest, size_t len
return snprintf(dest, len, "\"%s\"", *ptr);
}
+static void get_string(DeviceState *dev, Visitor *v, void *opaque,
+ const char *name, Error **errp)
+{
+ Property *prop = opaque;
+ char **ptr = qdev_get_prop_ptr(dev, prop);
+
+ if (!*ptr) {
+ char *str = (char *)"";
+ visit_type_str(v, &str, name, errp);
+ } else {
+ visit_type_str(v, ptr, name, errp);
+ }
+}
+
+static void set_string(DeviceState *dev, Visitor *v, void *opaque,
+ const char *name, Error **errp)
+{
+ Property *prop = opaque;
+ char **ptr = qdev_get_prop_ptr(dev, prop);
+ Error *local_err = NULL;
+ char *str;
+
+ if (dev->state != DEV_STATE_CREATED) {
+ error_set(errp, QERR_PERMISSION_DENIED);
+ return;
+ }
+
+ visit_type_str(v, &str, name, &local_err);
+ if (local_err) {
+ error_propagate(errp, local_err);
+ return;
+ }
+ if (!*str) {
+ g_free(str);
+ str = NULL;
+ }
+ if (*ptr) {
+ g_free(*ptr);
+ }
+ *ptr = str;
+}
+
PropertyInfo qdev_prop_string = {
.name = "string",
.type = PROP_TYPE_STRING,
@@ -329,6 +568,8 @@ PropertyInfo qdev_prop_string = {
.parse = parse_string,
.print = print_string,
.free = free_string,
+ .get = get_string,
+ .set = set_string,
};
/* --- drive --- */
@@ -364,12 +605,59 @@ static int print_drive(DeviceState *dev, Property *prop, char *dest, size_t len)
*ptr ? bdrv_get_device_name(*ptr) : "<null>");
}
+static void get_generic(DeviceState *dev, Visitor *v, void *opaque,
+ const char *name, Error **errp)
+{
+ Property *prop = opaque;
+ void **ptr = qdev_get_prop_ptr(dev, prop);
+ char buffer[1024];
+ char *p = buffer;
+
+ buffer[0] = 0;
+ if (*ptr) {
+ prop->info->print(dev, prop, buffer, sizeof(buffer));
+ }
+ visit_type_str(v, &p, name, errp);
+}
+
+static void set_generic(DeviceState *dev, Visitor *v, void *opaque,
+ const char *name, Error **errp)
+{
+ Property *prop = opaque;
+ Error *local_err = NULL;
+ char *str;
+ int ret;
+
+ if (dev->state != DEV_STATE_CREATED) {
+ error_set(errp, QERR_PERMISSION_DENIED);
+ return;
+ }
+
+ visit_type_str(v, &str, name, &local_err);
+ if (local_err) {
+ error_propagate(errp, local_err);
+ return;
+ }
+ if (!*str) {
+ g_free(str);
+ qdev_prop_error(errp, EINVAL, dev, prop, str);
+ return;
+ }
+ ret = prop->info->parse(dev, prop, str);
+ if (ret != 0) {
+ qdev_prop_error(errp, ret, dev, prop, str);
+ }
+ g_free(str);
+}
+
PropertyInfo qdev_prop_drive = {
.name = "drive",
.type = PROP_TYPE_DRIVE,
.size = sizeof(BlockDriverState *),
.parse = parse_drive,
.print = print_drive,
+ .get = get_generic,
+ .set = set_generic,
.free = free_drive,
};
@@ -407,6 +695,8 @@ PropertyInfo qdev_prop_chr = {
.size = sizeof(CharDriverState*),
.parse = parse_chr,
.print = print_chr,
+ .get = get_generic,
+ .set = set_generic,
};
/* --- netdev device --- */
@@ -441,6 +731,8 @@ PropertyInfo qdev_prop_netdev = {
.size = sizeof(VLANClientState*),
.parse = parse_netdev,
.print = print_netdev,
+ .get = get_generic,
+ .set = set_generic,
};
/* --- vlan --- */
@@ -469,12 +761,57 @@ static int print_vlan(DeviceState *dev, Property *prop, char *dest, size_t len)
}
}
+static void get_vlan(DeviceState *dev, Visitor *v, void *opaque,
+ const char *name, Error **errp)
+{
+ Property *prop = opaque;
+ VLANState **ptr = qdev_get_prop_ptr(dev, prop);
+ int64_t id;
+
+ id = *ptr ? (*ptr)->id : -1;
+ visit_type_int(v, &id, name, errp);
+}
+
+static void set_vlan(DeviceState *dev, Visitor *v, void *opaque,
+ const char *name, Error **errp)
+{
+ Property *prop = opaque;
+ VLANState **ptr = qdev_get_prop_ptr(dev, prop);
+ Error *local_err = NULL;
+ int64_t id;
+ VLANState *vlan;
+
+ if (dev->state != DEV_STATE_CREATED) {
+ error_set(errp, QERR_PERMISSION_DENIED);
+ return;
+ }
+
+ visit_type_int(v, &id, name, &local_err);
+ if (local_err) {
+ error_propagate(errp, local_err);
+ return;
+ }
+ if (id == -1) {
+ *ptr = NULL;
+ return;
+ }
+ vlan = qemu_find_vlan(id, 1);
+ if (!vlan) {
+ error_set(errp, QERR_INVALID_PARAMETER_VALUE,
+ name, prop->info->name);
+ return;
+ }
+ *ptr = vlan;
+}
+
PropertyInfo qdev_prop_vlan = {
.name = "vlan",
.type = PROP_TYPE_VLAN,
.size = sizeof(VLANClientState*),
.parse = parse_vlan,
.print = print_vlan,
+ .get = get_vlan,
+ .set = set_vlan,
};
/* --- pointer --- */
@@ -531,6 +867,8 @@ PropertyInfo qdev_prop_macaddr = {
.size = sizeof(MACAddr),
.parse = parse_mac,
.print = print_mac,
+ .get = get_generic,
+ .set = set_generic,
};
/* --- pci address --- */
@@ -570,12 +908,29 @@ static int print_pci_devfn(DeviceState *dev, Property *prop, char *dest, size_t
}
}
+static void get_pci_devfn(DeviceState *dev, Visitor *v, void *opaque,
+ const char *name, Error **errp)
+{
+ Property *prop = opaque;
+ uint32_t *ptr = qdev_get_prop_ptr(dev, prop);
+ char buffer[32];
+ char *p = buffer;
+
+ buffer[0] = 0;
+ if (*ptr != -1) {
+ snprintf(buffer, sizeof(buffer), "%02x.%x", *ptr >> 3, *ptr & 7);
+ }
+ visit_type_str(v, &p, name, errp);
+}
+
PropertyInfo qdev_prop_pci_devfn = {
.name = "pci-devfn",
.type = PROP_TYPE_UINT32,
.size = sizeof(uint32_t),
.parse = parse_pci_devfn,
.print = print_pci_devfn,
+ .get = get_pci_devfn,
+ .set = set_generic,
};
/* --- public helpers --- */
diff --git a/hw/qdev.h b/hw/qdev.h
index 828d811..a1cce37 100644
--- a/hw/qdev.h
+++ b/hw/qdev.h
@@ -158,9 +158,13 @@ struct PropertyInfo {
const char *name;
size_t size;
enum PropertyType type;
+ int64_t min;
+ int64_t max;
int (*parse)(DeviceState *dev, Property *prop, const char *str);
int (*print)(DeviceState *dev, Property *prop, char *dest, size_t len);
void (*free)(DeviceState *dev, Property *prop);
+ DevicePropertyAccessor *get;
+ DevicePropertyAccessor *set;
};
typedef struct GlobalProperty {
--
1.7.7.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [Qemu-devel] [PATCH v2 7/8] qom: distinguish "legacy" property type name from QOM type name
2011-12-16 16:47 [Qemu-devel] [PATCH v2 0/8] qom: introduce non-legacy static properties Paolo Bonzini
` (5 preceding siblings ...)
2011-12-16 16:47 ` [Qemu-devel] [PATCH v2 6/8] qom: introduce get/set methods for Property Paolo Bonzini
@ 2011-12-16 16:47 ` Paolo Bonzini
2011-12-16 17:03 ` Anthony Liguori
2011-12-16 16:47 ` [Qemu-devel] [PATCH v2 8/8] qom: register qdev properties also as non-legacy properties Paolo Bonzini
7 siblings, 1 reply; 15+ messages in thread
From: Paolo Bonzini @ 2011-12-16 16:47 UTC (permalink / raw)
To: qemu-devel
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
hw/qdev-properties.c | 12 ++++++++----
hw/qdev.c | 9 ++++++---
hw/qdev.h | 1 +
3 files changed, 15 insertions(+), 7 deletions(-)
diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
index f7aa3cb..dd41e5a 100644
--- a/hw/qdev-properties.c
+++ b/hw/qdev-properties.c
@@ -86,7 +86,8 @@ static void set_bit(DeviceState *dev, Visitor *v, void *opaque,
}
PropertyInfo qdev_prop_bit = {
- .name = "on/off",
+ .name = "boolean",
+ .legacy_name = "on/off",
.type = PROP_TYPE_BIT,
.size = sizeof(uint32_t),
.parse = parse_bit,
@@ -189,7 +190,8 @@ static int print_hex8(DeviceState *dev, Property *prop, char *dest, size_t len)
}
PropertyInfo qdev_prop_hex8 = {
- .name = "hex8",
+ .name = "uint8",
+ .legacy_name = "hex8",
.type = PROP_TYPE_UINT8,
.size = sizeof(uint8_t),
.parse = parse_hex8,
@@ -397,7 +399,8 @@ static int print_hex32(DeviceState *dev, Property *prop, char *dest, size_t len)
}
PropertyInfo qdev_prop_hex32 = {
- .name = "hex32",
+ .name = "uint32",
+ .legacy_name = "hex32",
.type = PROP_TYPE_UINT32,
.size = sizeof(uint32_t),
.parse = parse_hex32,
@@ -485,7 +488,8 @@ static int print_hex64(DeviceState *dev, Property *prop, char *dest, size_t len)
}
PropertyInfo qdev_prop_hex64 = {
- .name = "hex64",
+ .name = "uint64",
+ .legacy_name = "hex64",
.type = PROP_TYPE_UINT64,
.size = sizeof(uint64_t),
.parse = parse_hex64,
diff --git a/hw/qdev.c b/hw/qdev.c
index c8ab7b7..426ea71 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -218,13 +218,15 @@ int qdev_device_help(QemuOpts *opts)
if (!prop->info->parse) {
continue; /* no way to set it, don't show */
}
- error_printf("%s.%s=%s\n", info->name, prop->name, prop->info->name);
+ error_printf("%s.%s=%s\n", info->name, prop->name,
+ prop->info->legacy_name ?: prop->info->name);
}
for (prop = info->bus_info->props; prop && prop->name; prop++) {
if (!prop->info->parse) {
continue; /* no way to set it, don't show */
}
- error_printf("%s.%s=%s\n", info->name, prop->name, prop->info->name);
+ error_printf("%s.%s=%s\n", info->name, prop->name,
+ prop->info->legacy_name ?: prop->info->name);
}
return 1;
}
@@ -1182,7 +1184,8 @@ void qdev_property_add_legacy(DeviceState *dev, Property *prop,
{
gchar *type;
- type = g_strdup_printf("legacy<%s>", prop->info->name);
+ type = g_strdup_printf("legacy<%s>",
+ prop->info->legacy_name ?: prop->info->name);
qdev_property_add(dev, prop->name, type,
prop->info->print ? qdev_get_legacy_property : NULL,
diff --git a/hw/qdev.h b/hw/qdev.h
index a1cce37..8002644 100644
--- a/hw/qdev.h
+++ b/hw/qdev.h
@@ -156,6 +156,7 @@ enum PropertyType {
struct PropertyInfo {
const char *name;
+ const char *legacy_name;
size_t size;
enum PropertyType type;
int64_t min;
--
1.7.7.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [Qemu-devel] [PATCH v2 8/8] qom: register qdev properties also as non-legacy properties
2011-12-16 16:47 [Qemu-devel] [PATCH v2 0/8] qom: introduce non-legacy static properties Paolo Bonzini
` (6 preceding siblings ...)
2011-12-16 16:47 ` [Qemu-devel] [PATCH v2 7/8] qom: distinguish "legacy" property type name from QOM type name Paolo Bonzini
@ 2011-12-16 16:47 ` Paolo Bonzini
2011-12-16 17:05 ` Anthony Liguori
7 siblings, 1 reply; 15+ messages in thread
From: Paolo Bonzini @ 2011-12-16 16:47 UTC (permalink / raw)
To: qemu-devel
Push legacy properties into a legacy<...> namespace, and make them
available with correct types too.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
hw/qdev.c | 28 +++++++++++++++++++++++++---
hw/qdev.h | 7 +++----
2 files changed, 28 insertions(+), 7 deletions(-)
diff --git a/hw/qdev.c b/hw/qdev.c
index 426ea71..2f7defc 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -80,6 +80,9 @@ static DeviceInfo *qdev_find_info(BusInfo *bus_info, const char *name)
return NULL;
}
+static void qdev_property_add_legacy(DeviceState *dev, Property *prop,
+ Error **errp);
+
static DeviceState *qdev_create_from_info(BusState *bus, DeviceInfo *info)
{
DeviceState *dev;
@@ -104,10 +107,12 @@ static DeviceState *qdev_create_from_info(BusState *bus, DeviceInfo *info)
for (prop = dev->info->props; prop && prop->name; prop++) {
qdev_property_add_legacy(dev, prop, NULL);
+ qdev_property_add_static(dev, prop, NULL);
}
for (prop = dev->info->bus_info->props; prop && prop->name; prop++) {
qdev_property_add_legacy(dev, prop, NULL);
+ qdev_property_add_static(dev, prop, NULL);
}
qdev_property_add_str(dev, "type", qdev_get_type, NULL, NULL);
@@ -1174,7 +1179,7 @@ static void qdev_set_legacy_property(DeviceState *dev, Visitor *v, void *opaque,
* @qdev_add_legacy_property - adds a legacy property
*
* Do not use this is new code! Properties added through this interface will
- * be given types in the "legacy<>" type namespace.
+ * be given names and types in the "legacy<>" type namespace.
*
* Legacy properties are always processed as strings. The format of the string
* depends on the property type.
@@ -1182,18 +1187,35 @@ static void qdev_set_legacy_property(DeviceState *dev, Visitor *v, void *opaque,
void qdev_property_add_legacy(DeviceState *dev, Property *prop,
Error **errp)
{
- gchar *type;
+ gchar *name, *type;
+ name = g_strdup_printf("legacy<%s>", prop->name);
type = g_strdup_printf("legacy<%s>",
prop->info->legacy_name ?: prop->info->name);
- qdev_property_add(dev, prop->name, type,
+ qdev_property_add(dev, name, type,
prop->info->print ? qdev_get_legacy_property : NULL,
prop->info->parse ? qdev_set_legacy_property : NULL,
NULL,
prop, errp);
g_free(type);
+ g_free(name);
+}
+
+/**
+ * @qdev_property_add_static - add a @Property to a device.
+ *
+ * Static properties access data in a struct. The actual type of the
+ * property and the field depends on the property type.
+ */
+void qdev_property_add_static(DeviceState *dev, Property *prop,
+ Error **errp)
+{
+ qdev_property_add(dev, prop->name, prop->info->name,
+ prop->info->get, prop->info->set,
+ NULL,
+ prop, errp);
}
DeviceState *qdev_get_root(void)
diff --git a/hw/qdev.h b/hw/qdev.h
index 8002644..3410e77 100644
--- a/hw/qdev.h
+++ b/hw/qdev.h
@@ -487,11 +487,10 @@ const char *qdev_property_get_type(DeviceState *dev, const char *name,
Error **errp);
/**
- * @qdev_property_add_legacy - add a legacy @Property to a device
- *
- * DO NOT USE THIS IN NEW CODE!
+ * @qdev_property_add_static - add a @Property to a device referencing a
+ * field in a struct.
*/
-void qdev_property_add_legacy(DeviceState *dev, Property *prop, Error **errp);
+void qdev_property_add_static(DeviceState *dev, Property *prop, Error **errp);
/**
* @qdev_get_root - returns the root device of the composition tree
--
1.7.7.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH v2 4/8] qom: interpret the return value when setting legacy properties
2011-12-16 16:47 ` [Qemu-devel] [PATCH v2 4/8] qom: interpret the return value when setting legacy properties Paolo Bonzini
@ 2011-12-16 17:00 ` Anthony Liguori
2011-12-16 17:19 ` Paolo Bonzini
0 siblings, 1 reply; 15+ messages in thread
From: Anthony Liguori @ 2011-12-16 17:00 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel
On 12/16/2011 10:47 AM, Paolo Bonzini wrote:
> Signed-off-by: Paolo Bonzini<pbonzini@redhat.com>
> ---
> hw/qdev-properties.c | 39 ++++++++++++++++++++++++---------------
> hw/qdev.c | 3 +--
> hw/qdev.h | 2 ++
> 3 files changed, 27 insertions(+), 17 deletions(-)
>
> diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
> index f0b811c..76ecb38 100644
> --- a/hw/qdev-properties.c
> +++ b/hw/qdev-properties.c
> @@ -614,6 +614,26 @@ int qdev_prop_exists(DeviceState *dev, const char *name)
> return qdev_prop_find(dev, name) ? true : false;
> }
>
> +void qdev_prop_error(Error **errp, int ret,
> + DeviceState *dev, Property *prop, const char *value)
> +{
> + switch (ret) {
> + case -EEXIST:
> + error_set(errp, QERR_PROPERTY_VALUE_IN_USE,
> + dev->info->name, prop->name, value);
> + break;
> + default:
> + case -EINVAL:
> + error_set(errp, QERR_PROPERTY_VALUE_BAD,
> + dev->info->name, prop->name, value);
> + break;
> + case -ENOENT:
> + error_set(errp, QERR_PROPERTY_VALUE_NOT_FOUND,
> + dev->info->name, prop->name, value);
> + break;
> + }
> +}
> +
> int qdev_prop_parse(DeviceState *dev, const char *name, const char *value)
> {
> Property *prop;
> @@ -632,21 +652,10 @@ int qdev_prop_parse(DeviceState *dev, const char *name, const char *value)
> }
> ret = prop->info->parse(dev, prop, value);
> if (ret< 0) {
> - switch (ret) {
> - case -EEXIST:
> - qerror_report(QERR_PROPERTY_VALUE_IN_USE,
> - dev->info->name, name, value);
> - break;
> - default:
> - case -EINVAL:
> - qerror_report(QERR_PROPERTY_VALUE_BAD,
> - dev->info->name, name, value);
> - break;
> - case -ENOENT:
> - qerror_report(QERR_PROPERTY_VALUE_NOT_FOUND,
> - dev->info->name, name, value);
> - break;
> - }
> + Error *err;
> + qdev_prop_error(&err, ret, dev, prop, value);
> + qerror_report_err(err);
> + error_free(err);
> return -1;
> }
> return 0;
> diff --git a/hw/qdev.c b/hw/qdev.c
> index c020a6f..c8ab7b7 100644
> --- a/hw/qdev.c
> +++ b/hw/qdev.c
> @@ -1163,8 +1163,7 @@ static void qdev_set_legacy_property(DeviceState *dev, Visitor *v, void *opaque,
>
> ret = prop->info->parse(dev, prop, ptr);
> if (ret != 0) {
> - error_set(errp, QERR_INVALID_PARAMETER_VALUE,
> - name, prop->info->name);
> + qdev_prop_error(errp, ret, dev, prop, ptr);
> }
> g_free(ptr);
> }
> diff --git a/hw/qdev.h b/hw/qdev.h
> index 6e18427..828d811 100644
> --- a/hw/qdev.h
> +++ b/hw/qdev.h
> @@ -370,6 +370,8 @@ void qdev_prop_set_defaults(DeviceState *dev, Property *props);
>
> void qdev_prop_register_global_list(GlobalProperty *props);
> void qdev_prop_set_globals(DeviceState *dev);
> +void qdev_prop_error(Error **errp, int ret, DeviceState *name,
> + Property *prop, const char *value);
s/name/dev
And perhaps it would make more sense to return Error * and make the function
name be a constructor:
Error *error_from_qdev_prop_err(int ret, DeviceState *dev,
Property *prop, const char *value);
I was fairly confused about what was going on here at first.
Reards,
Anthony Liguori
>
> static inline const char *qdev_fw_name(DeviceState *dev)
> {
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH v2 5/8] qom: introduce QERR_PROPERTY_VALUE_OUT_OF_RANGE
2011-12-16 16:47 ` [Qemu-devel] [PATCH v2 5/8] qom: introduce QERR_PROPERTY_VALUE_OUT_OF_RANGE Paolo Bonzini
@ 2011-12-16 17:00 ` Anthony Liguori
0 siblings, 0 replies; 15+ messages in thread
From: Anthony Liguori @ 2011-12-16 17:00 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel
On 12/16/2011 10:47 AM, Paolo Bonzini wrote:
> This will be used when reject invalid values for integer fields that
> are less than 64-bits wide.
>
> Signed-off-by: Paolo Bonzini<pbonzini@redhat.com>
VALUE_OUT_OF_RANGE?
Regards,
Anthony Liguori
> ---
> qerror.c | 5 +++++
> qerror.h | 3 +++
> 2 files changed, 8 insertions(+), 0 deletions(-)
>
> diff --git a/qerror.c b/qerror.c
> index adde8a5..9a75d06 100644
> --- a/qerror.c
> +++ b/qerror.c
> @@ -206,6 +206,11 @@ static const QErrorStringTable qerror_table[] = {
> .desc = "Property '%(device).%(property)' can't find value '%(value)'",
> },
> {
> + .error_fmt = QERR_PROPERTY_VALUE_OUT_OF_RANGE,
> + .desc = "Property '%(device).%(property)' doesn't take "
> + "value %(value) (minimum: %(min), maximum: %(max)'",
> + },
> + {
> .error_fmt = QERR_QMP_BAD_INPUT_OBJECT,
> .desc = "Expected '%(expected)' in QMP input",
> },
> diff --git a/qerror.h b/qerror.h
> index 9190b02..efda232 100644
> --- a/qerror.h
> +++ b/qerror.h
> @@ -171,6 +171,9 @@ QError *qobject_to_qerror(const QObject *obj);
> #define QERR_PROPERTY_VALUE_NOT_FOUND \
> "{ 'class': 'PropertyValueNotFound', 'data': { 'device': %s, 'property': %s, 'value': %s } }"
>
> +#define QERR_PROPERTY_VALUE_OUT_OF_RANGE \
> + "{ 'class': 'PropertyValueOutOfRange', 'data': { 'device': %s, 'property': %s, 'value': %"PRId64", 'min': %"PRId64", 'max': %"PRId64" } }"
> +
> #define QERR_QMP_BAD_INPUT_OBJECT \
> "{ 'class': 'QMPBadInputObject', 'data': { 'expected': %s } }"
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH v2 6/8] qom: introduce get/set methods for Property
2011-12-16 16:47 ` [Qemu-devel] [PATCH v2 6/8] qom: introduce get/set methods for Property Paolo Bonzini
@ 2011-12-16 17:02 ` Anthony Liguori
0 siblings, 0 replies; 15+ messages in thread
From: Anthony Liguori @ 2011-12-16 17:02 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel
On 12/16/2011 10:47 AM, Paolo Bonzini wrote:
> This patch adds a visitor interface to Property. This way, QOM will be
> able to expose Properties that access a fixed field in a struct without
> exposing also the everything-is-a-string "feature" of qdev properties.
>
> Whenever the printed representation in both QOM and qdev (which is
> typically the case for device backends), parse/print code can be reused
> via get_generic/set_generic. Dually, whenever multiple PropertyInfos
> have the same representation in both the struct and the visitors the
> code can be reused (for example among all of int32/uint32/hex32).
>
> Signed-off-by: Paolo Bonzini<pbonzini@redhat.com>
Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>
Regards,
Anthony Liguori
> ---
> hw/qdev-addr.c | 41 ++++++
> hw/qdev-properties.c | 355 ++++++++++++++++++++++++++++++++++++++++++++++++++
> hw/qdev.h | 4 +
> 3 files changed, 400 insertions(+), 0 deletions(-)
>
> diff --git a/hw/qdev-addr.c b/hw/qdev-addr.c
> index 305c2d3..5ddda2d 100644
> --- a/hw/qdev-addr.c
> +++ b/hw/qdev-addr.c
> @@ -18,12 +18,53 @@ static int print_taddr(DeviceState *dev, Property *prop, char *dest, size_t len)
> return snprintf(dest, len, "0x" TARGET_FMT_plx, *ptr);
> }
>
> +static void get_taddr(DeviceState *dev, Visitor *v, void *opaque,
> + const char *name, Error **errp)
> +{
> + Property *prop = opaque;
> + target_phys_addr_t *ptr = qdev_get_prop_ptr(dev, prop);
> + int64_t value;
> +
> + value = *ptr;
> + visit_type_int(v,&value, name, errp);
> +}
> +
> +static void set_taddr(DeviceState *dev, Visitor *v, void *opaque,
> + const char *name, Error **errp)
> +{
> + Property *prop = opaque;
> + target_phys_addr_t *ptr = qdev_get_prop_ptr(dev, prop);
> + Error *local_err = NULL;
> + int64_t value;
> +
> + if (dev->state != DEV_STATE_CREATED) {
> + error_set(errp, QERR_PERMISSION_DENIED);
> + return;
> + }
> +
> + visit_type_int(v,&value, name,&local_err);
> + if (local_err) {
> + error_propagate(errp, local_err);
> + return;
> + }
> + if ((uint64_t)value<= (uint64_t) ~(target_phys_addr_t)0) {
> + *ptr = value;
> + } else {
> + error_set(errp, QERR_PROPERTY_VALUE_OUT_OF_RANGE,
> + dev->id?:"", name, value, (uint64_t) 0,
> + (uint64_t) ~(target_phys_addr_t)0);
> + }
> +}
> +
> +
> PropertyInfo qdev_prop_taddr = {
> .name = "taddr",
> .type = PROP_TYPE_TADDR,
> .size = sizeof(target_phys_addr_t),
> .parse = parse_taddr,
> .print = print_taddr,
> + .get = get_taddr,
> + .set = set_taddr,
> };
>
> void qdev_prop_set_taddr(DeviceState *dev, const char *name, target_phys_addr_t value)
> diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
> index 76ecb38..f7aa3cb 100644
> --- a/hw/qdev-properties.c
> +++ b/hw/qdev-properties.c
> @@ -55,12 +55,44 @@ static int print_bit(DeviceState *dev, Property *prop, char *dest, size_t len)
> return snprintf(dest, len, (*p& qdev_get_prop_mask(prop)) ? "on" : "off");
> }
>
> +static void get_bit(DeviceState *dev, Visitor *v, void *opaque,
> + const char *name, Error **errp)
> +{
> + Property *prop = opaque;
> + uint32_t *p = qdev_get_prop_ptr(dev, prop);
> + bool value = (*p& qdev_get_prop_mask(prop)) != 0;
> +
> + visit_type_bool(v,&value, name, errp);
> +}
> +
> +static void set_bit(DeviceState *dev, Visitor *v, void *opaque,
> + const char *name, Error **errp)
> +{
> + Property *prop = opaque;
> + Error *local_err = NULL;
> + bool value;
> +
> + if (dev->state != DEV_STATE_CREATED) {
> + error_set(errp, QERR_PERMISSION_DENIED);
> + return;
> + }
> +
> + visit_type_bool(v,&value, name,&local_err);
> + if (local_err) {
> + error_propagate(errp, local_err);
> + return;
> + }
> + bit_prop_set(dev, prop, value);
> +}
> +
> PropertyInfo qdev_prop_bit = {
> .name = "on/off",
> .type = PROP_TYPE_BIT,
> .size = sizeof(uint32_t),
> .parse = parse_bit,
> .print = print_bit,
> + .get = get_bit,
> + .set = set_bit,
> };
>
> /* --- 8bit integer --- */
> @@ -85,12 +117,54 @@ static int print_uint8(DeviceState *dev, Property *prop, char *dest, size_t len)
> return snprintf(dest, len, "%" PRIu8, *ptr);
> }
>
> +static void get_int8(DeviceState *dev, Visitor *v, void *opaque,
> + const char *name, Error **errp)
> +{
> + Property *prop = opaque;
> + int8_t *ptr = qdev_get_prop_ptr(dev, prop);
> + int64_t value;
> +
> + value = *ptr;
> + visit_type_int(v,&value, name, errp);
> +}
> +
> +static void set_int8(DeviceState *dev, Visitor *v, void *opaque,
> + const char *name, Error **errp)
> +{
> + Property *prop = opaque;
> + int8_t *ptr = qdev_get_prop_ptr(dev, prop);
> + Error *local_err = NULL;
> + int64_t value;
> +
> + if (dev->state != DEV_STATE_CREATED) {
> + error_set(errp, QERR_PERMISSION_DENIED);
> + return;
> + }
> +
> + visit_type_int(v,&value, name,&local_err);
> + if (local_err) {
> + error_propagate(errp, local_err);
> + return;
> + }
> + if (value> prop->info->min&& value<= prop->info->max) {
> + *ptr = value;
> + } else {
> + error_set(errp, QERR_PROPERTY_VALUE_OUT_OF_RANGE,
> + dev->id?:"", name, value, prop->info->min,
> + prop->info->max);
> + }
> +}
> +
> PropertyInfo qdev_prop_uint8 = {
> .name = "uint8",
> .type = PROP_TYPE_UINT8,
> .size = sizeof(uint8_t),
> .parse = parse_uint8,
> .print = print_uint8,
> + .get = get_int8,
> + .set = set_int8,
> + .min = 0,
> + .max = 255,
> };
>
> /* --- 8bit hex value --- */
> @@ -120,6 +194,10 @@ PropertyInfo qdev_prop_hex8 = {
> .size = sizeof(uint8_t),
> .parse = parse_hex8,
> .print = print_hex8,
> + .get = get_int8,
> + .set = set_int8,
> + .min = 0,
> + .max = 255,
> };
>
> /* --- 16bit integer --- */
> @@ -144,12 +222,54 @@ static int print_uint16(DeviceState *dev, Property *prop, char *dest, size_t len
> return snprintf(dest, len, "%" PRIu16, *ptr);
> }
>
> +static void get_int16(DeviceState *dev, Visitor *v, void *opaque,
> + const char *name, Error **errp)
> +{
> + Property *prop = opaque;
> + int16_t *ptr = qdev_get_prop_ptr(dev, prop);
> + int64_t value;
> +
> + value = *ptr;
> + visit_type_int(v,&value, name, errp);
> +}
> +
> +static void set_int16(DeviceState *dev, Visitor *v, void *opaque,
> + const char *name, Error **errp)
> +{
> + Property *prop = opaque;
> + int16_t *ptr = qdev_get_prop_ptr(dev, prop);
> + Error *local_err = NULL;
> + int64_t value;
> +
> + if (dev->state != DEV_STATE_CREATED) {
> + error_set(errp, QERR_PERMISSION_DENIED);
> + return;
> + }
> +
> + visit_type_int(v,&value, name,&local_err);
> + if (local_err) {
> + error_propagate(errp, local_err);
> + return;
> + }
> + if (value> prop->info->min&& value<= prop->info->max) {
> + *ptr = value;
> + } else {
> + error_set(errp, QERR_PROPERTY_VALUE_OUT_OF_RANGE,
> + dev->id?:"", name, value, prop->info->min,
> + prop->info->max);
> + }
> +}
> +
> PropertyInfo qdev_prop_uint16 = {
> .name = "uint16",
> .type = PROP_TYPE_UINT16,
> .size = sizeof(uint16_t),
> .parse = parse_uint16,
> .print = print_uint16,
> + .get = get_int16,
> + .set = set_int16,
> + .min = 0,
> + .max = 65535,
> };
>
> /* --- 32bit integer --- */
> @@ -174,12 +294,54 @@ static int print_uint32(DeviceState *dev, Property *prop, char *dest, size_t len
> return snprintf(dest, len, "%" PRIu32, *ptr);
> }
>
> +static void get_int32(DeviceState *dev, Visitor *v, void *opaque,
> + const char *name, Error **errp)
> +{
> + Property *prop = opaque;
> + int32_t *ptr = qdev_get_prop_ptr(dev, prop);
> + int64_t value;
> +
> + value = *ptr;
> + visit_type_int(v,&value, name, errp);
> +}
> +
> +static void set_int32(DeviceState *dev, Visitor *v, void *opaque,
> + const char *name, Error **errp)
> +{
> + Property *prop = opaque;
> + int32_t *ptr = qdev_get_prop_ptr(dev, prop);
> + Error *local_err = NULL;
> + int64_t value;
> +
> + if (dev->state != DEV_STATE_CREATED) {
> + error_set(errp, QERR_PERMISSION_DENIED);
> + return;
> + }
> +
> + visit_type_int(v,&value, name,&local_err);
> + if (local_err) {
> + error_propagate(errp, local_err);
> + return;
> + }
> + if (value> prop->info->min&& value<= prop->info->max) {
> + *ptr = value;
> + } else {
> + error_set(errp, QERR_PROPERTY_VALUE_OUT_OF_RANGE,
> + dev->id?:"", name, value, prop->info->min,
> + prop->info->max);
> + }
> +}
> +
> PropertyInfo qdev_prop_uint32 = {
> .name = "uint32",
> .type = PROP_TYPE_UINT32,
> .size = sizeof(uint32_t),
> .parse = parse_uint32,
> .print = print_uint32,
> + .get = get_int32,
> + .set = set_int32,
> + .min = 0,
> + .max = 0xFFFFFFFFULL,
> };
>
> static int parse_int32(DeviceState *dev, Property *prop, const char *str)
> @@ -207,6 +369,10 @@ PropertyInfo qdev_prop_int32 = {
> .size = sizeof(int32_t),
> .parse = parse_int32,
> .print = print_int32,
> + .get = get_int32,
> + .set = set_int32,
> + .min = -0x80000000LL,
> + .max = 0x7FFFFFFFLL,
> };
>
> /* --- 32bit hex value --- */
> @@ -236,6 +402,10 @@ PropertyInfo qdev_prop_hex32 = {
> .size = sizeof(uint32_t),
> .parse = parse_hex32,
> .print = print_hex32,
> + .get = get_int32,
> + .set = set_int32,
> + .min = 0,
> + .max = 0xFFFFFFFFULL,
> };
>
> /* --- 64bit integer --- */
> @@ -260,12 +430,37 @@ static int print_uint64(DeviceState *dev, Property *prop, char *dest, size_t len
> return snprintf(dest, len, "%" PRIu64, *ptr);
> }
>
> +static void get_int64(DeviceState *dev, Visitor *v, void *opaque,
> + const char *name, Error **errp)
> +{
> + Property *prop = opaque;
> + int64_t *ptr = qdev_get_prop_ptr(dev, prop);
> +
> + visit_type_int(v, ptr, name, errp);
> +}
> +
> +static void set_int64(DeviceState *dev, Visitor *v, void *opaque,
> + const char *name, Error **errp)
> +{
> + Property *prop = opaque;
> + int64_t *ptr = qdev_get_prop_ptr(dev, prop);
> +
> + if (dev->state != DEV_STATE_CREATED) {
> + error_set(errp, QERR_PERMISSION_DENIED);
> + return;
> + }
> +
> + visit_type_int(v, ptr, name, errp);
> +}
> +
> PropertyInfo qdev_prop_uint64 = {
> .name = "uint64",
> .type = PROP_TYPE_UINT64,
> .size = sizeof(uint64_t),
> .parse = parse_uint64,
> .print = print_uint64,
> + .get = get_int64,
> + .set = set_int64,
> };
>
> /* --- 64bit hex value --- */
> @@ -295,6 +490,8 @@ PropertyInfo qdev_prop_hex64 = {
> .size = sizeof(uint64_t),
> .parse = parse_hex64,
> .print = print_hex64,
> + .get = get_int64,
> + .set = set_int64,
> };
>
> /* --- string --- */
> @@ -322,6 +519,48 @@ static int print_string(DeviceState *dev, Property *prop, char *dest, size_t len
> return snprintf(dest, len, "\"%s\"", *ptr);
> }
>
> +static void get_string(DeviceState *dev, Visitor *v, void *opaque,
> + const char *name, Error **errp)
> +{
> + Property *prop = opaque;
> + char **ptr = qdev_get_prop_ptr(dev, prop);
> +
> + if (!*ptr) {
> + char *str = (char *)"";
> + visit_type_str(v,&str, name, errp);
> + } else {
> + visit_type_str(v, ptr, name, errp);
> + }
> +}
> +
> +static void set_string(DeviceState *dev, Visitor *v, void *opaque,
> + const char *name, Error **errp)
> +{
> + Property *prop = opaque;
> + char **ptr = qdev_get_prop_ptr(dev, prop);
> + Error *local_err = NULL;
> + char *str;
> +
> + if (dev->state != DEV_STATE_CREATED) {
> + error_set(errp, QERR_PERMISSION_DENIED);
> + return;
> + }
> +
> + visit_type_str(v,&str, name,&local_err);
> + if (local_err) {
> + error_propagate(errp, local_err);
> + return;
> + }
> + if (!*str) {
> + g_free(str);
> + str = NULL;
> + }
> + if (*ptr) {
> + g_free(*ptr);
> + }
> + *ptr = str;
> +}
> +
> PropertyInfo qdev_prop_string = {
> .name = "string",
> .type = PROP_TYPE_STRING,
> @@ -329,6 +568,8 @@ PropertyInfo qdev_prop_string = {
> .parse = parse_string,
> .print = print_string,
> .free = free_string,
> + .get = get_string,
> + .set = set_string,
> };
>
> /* --- drive --- */
> @@ -364,12 +605,59 @@ static int print_drive(DeviceState *dev, Property *prop, char *dest, size_t len)
> *ptr ? bdrv_get_device_name(*ptr) : "<null>");
> }
>
> +static void get_generic(DeviceState *dev, Visitor *v, void *opaque,
> + const char *name, Error **errp)
> +{
> + Property *prop = opaque;
> + void **ptr = qdev_get_prop_ptr(dev, prop);
> + char buffer[1024];
> + char *p = buffer;
> +
> + buffer[0] = 0;
> + if (*ptr) {
> + prop->info->print(dev, prop, buffer, sizeof(buffer));
> + }
> + visit_type_str(v,&p, name, errp);
> +}
> +
> +static void set_generic(DeviceState *dev, Visitor *v, void *opaque,
> + const char *name, Error **errp)
> +{
> + Property *prop = opaque;
> + Error *local_err = NULL;
> + char *str;
> + int ret;
> +
> + if (dev->state != DEV_STATE_CREATED) {
> + error_set(errp, QERR_PERMISSION_DENIED);
> + return;
> + }
> +
> + visit_type_str(v,&str, name,&local_err);
> + if (local_err) {
> + error_propagate(errp, local_err);
> + return;
> + }
> + if (!*str) {
> + g_free(str);
> + qdev_prop_error(errp, EINVAL, dev, prop, str);
> + return;
> + }
> + ret = prop->info->parse(dev, prop, str);
> + if (ret != 0) {
> + qdev_prop_error(errp, ret, dev, prop, str);
> + }
> + g_free(str);
> +}
> +
> PropertyInfo qdev_prop_drive = {
> .name = "drive",
> .type = PROP_TYPE_DRIVE,
> .size = sizeof(BlockDriverState *),
> .parse = parse_drive,
> .print = print_drive,
> + .get = get_generic,
> + .set = set_generic,
> .free = free_drive,
> };
>
> @@ -407,6 +695,8 @@ PropertyInfo qdev_prop_chr = {
> .size = sizeof(CharDriverState*),
> .parse = parse_chr,
> .print = print_chr,
> + .get = get_generic,
> + .set = set_generic,
> };
>
> /* --- netdev device --- */
> @@ -441,6 +731,8 @@ PropertyInfo qdev_prop_netdev = {
> .size = sizeof(VLANClientState*),
> .parse = parse_netdev,
> .print = print_netdev,
> + .get = get_generic,
> + .set = set_generic,
> };
>
> /* --- vlan --- */
> @@ -469,12 +761,57 @@ static int print_vlan(DeviceState *dev, Property *prop, char *dest, size_t len)
> }
> }
>
> +static void get_vlan(DeviceState *dev, Visitor *v, void *opaque,
> + const char *name, Error **errp)
> +{
> + Property *prop = opaque;
> + VLANState **ptr = qdev_get_prop_ptr(dev, prop);
> + int64_t id;
> +
> + id = *ptr ? (*ptr)->id : -1;
> + visit_type_int(v,&id, name, errp);
> +}
> +
> +static void set_vlan(DeviceState *dev, Visitor *v, void *opaque,
> + const char *name, Error **errp)
> +{
> + Property *prop = opaque;
> + VLANState **ptr = qdev_get_prop_ptr(dev, prop);
> + Error *local_err = NULL;
> + int64_t id;
> + VLANState *vlan;
> +
> + if (dev->state != DEV_STATE_CREATED) {
> + error_set(errp, QERR_PERMISSION_DENIED);
> + return;
> + }
> +
> + visit_type_int(v,&id, name,&local_err);
> + if (local_err) {
> + error_propagate(errp, local_err);
> + return;
> + }
> + if (id == -1) {
> + *ptr = NULL;
> + return;
> + }
> + vlan = qemu_find_vlan(id, 1);
> + if (!vlan) {
> + error_set(errp, QERR_INVALID_PARAMETER_VALUE,
> + name, prop->info->name);
> + return;
> + }
> + *ptr = vlan;
> +}
> +
> PropertyInfo qdev_prop_vlan = {
> .name = "vlan",
> .type = PROP_TYPE_VLAN,
> .size = sizeof(VLANClientState*),
> .parse = parse_vlan,
> .print = print_vlan,
> + .get = get_vlan,
> + .set = set_vlan,
> };
>
> /* --- pointer --- */
> @@ -531,6 +867,8 @@ PropertyInfo qdev_prop_macaddr = {
> .size = sizeof(MACAddr),
> .parse = parse_mac,
> .print = print_mac,
> + .get = get_generic,
> + .set = set_generic,
> };
>
> /* --- pci address --- */
> @@ -570,12 +908,29 @@ static int print_pci_devfn(DeviceState *dev, Property *prop, char *dest, size_t
> }
> }
>
> +static void get_pci_devfn(DeviceState *dev, Visitor *v, void *opaque,
> + const char *name, Error **errp)
> +{
> + Property *prop = opaque;
> + uint32_t *ptr = qdev_get_prop_ptr(dev, prop);
> + char buffer[32];
> + char *p = buffer;
> +
> + buffer[0] = 0;
> + if (*ptr != -1) {
> + snprintf(buffer, sizeof(buffer), "%02x.%x", *ptr>> 3, *ptr& 7);
> + }
> + visit_type_str(v,&p, name, errp);
> +}
> +
> PropertyInfo qdev_prop_pci_devfn = {
> .name = "pci-devfn",
> .type = PROP_TYPE_UINT32,
> .size = sizeof(uint32_t),
> .parse = parse_pci_devfn,
> .print = print_pci_devfn,
> + .get = get_pci_devfn,
> + .set = set_generic,
> };
>
> /* --- public helpers --- */
> diff --git a/hw/qdev.h b/hw/qdev.h
> index 828d811..a1cce37 100644
> --- a/hw/qdev.h
> +++ b/hw/qdev.h
> @@ -158,9 +158,13 @@ struct PropertyInfo {
> const char *name;
> size_t size;
> enum PropertyType type;
> + int64_t min;
> + int64_t max;
> int (*parse)(DeviceState *dev, Property *prop, const char *str);
> int (*print)(DeviceState *dev, Property *prop, char *dest, size_t len);
> void (*free)(DeviceState *dev, Property *prop);
> + DevicePropertyAccessor *get;
> + DevicePropertyAccessor *set;
> };
>
> typedef struct GlobalProperty {
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH v2 7/8] qom: distinguish "legacy" property type name from QOM type name
2011-12-16 16:47 ` [Qemu-devel] [PATCH v2 7/8] qom: distinguish "legacy" property type name from QOM type name Paolo Bonzini
@ 2011-12-16 17:03 ` Anthony Liguori
0 siblings, 0 replies; 15+ messages in thread
From: Anthony Liguori @ 2011-12-16 17:03 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel
On 12/16/2011 10:47 AM, Paolo Bonzini wrote:
> Signed-off-by: Paolo Bonzini<pbonzini@redhat.com>
Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>
Regards,
Anthony Liguori
> ---
> hw/qdev-properties.c | 12 ++++++++----
> hw/qdev.c | 9 ++++++---
> hw/qdev.h | 1 +
> 3 files changed, 15 insertions(+), 7 deletions(-)
>
> diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
> index f7aa3cb..dd41e5a 100644
> --- a/hw/qdev-properties.c
> +++ b/hw/qdev-properties.c
> @@ -86,7 +86,8 @@ static void set_bit(DeviceState *dev, Visitor *v, void *opaque,
> }
>
> PropertyInfo qdev_prop_bit = {
> - .name = "on/off",
> + .name = "boolean",
> + .legacy_name = "on/off",
> .type = PROP_TYPE_BIT,
> .size = sizeof(uint32_t),
> .parse = parse_bit,
> @@ -189,7 +190,8 @@ static int print_hex8(DeviceState *dev, Property *prop, char *dest, size_t len)
> }
>
> PropertyInfo qdev_prop_hex8 = {
> - .name = "hex8",
> + .name = "uint8",
> + .legacy_name = "hex8",
> .type = PROP_TYPE_UINT8,
> .size = sizeof(uint8_t),
> .parse = parse_hex8,
> @@ -397,7 +399,8 @@ static int print_hex32(DeviceState *dev, Property *prop, char *dest, size_t len)
> }
>
> PropertyInfo qdev_prop_hex32 = {
> - .name = "hex32",
> + .name = "uint32",
> + .legacy_name = "hex32",
> .type = PROP_TYPE_UINT32,
> .size = sizeof(uint32_t),
> .parse = parse_hex32,
> @@ -485,7 +488,8 @@ static int print_hex64(DeviceState *dev, Property *prop, char *dest, size_t len)
> }
>
> PropertyInfo qdev_prop_hex64 = {
> - .name = "hex64",
> + .name = "uint64",
> + .legacy_name = "hex64",
> .type = PROP_TYPE_UINT64,
> .size = sizeof(uint64_t),
> .parse = parse_hex64,
> diff --git a/hw/qdev.c b/hw/qdev.c
> index c8ab7b7..426ea71 100644
> --- a/hw/qdev.c
> +++ b/hw/qdev.c
> @@ -218,13 +218,15 @@ int qdev_device_help(QemuOpts *opts)
> if (!prop->info->parse) {
> continue; /* no way to set it, don't show */
> }
> - error_printf("%s.%s=%s\n", info->name, prop->name, prop->info->name);
> + error_printf("%s.%s=%s\n", info->name, prop->name,
> + prop->info->legacy_name ?: prop->info->name);
> }
> for (prop = info->bus_info->props; prop&& prop->name; prop++) {
> if (!prop->info->parse) {
> continue; /* no way to set it, don't show */
> }
> - error_printf("%s.%s=%s\n", info->name, prop->name, prop->info->name);
> + error_printf("%s.%s=%s\n", info->name, prop->name,
> + prop->info->legacy_name ?: prop->info->name);
> }
> return 1;
> }
> @@ -1182,7 +1184,8 @@ void qdev_property_add_legacy(DeviceState *dev, Property *prop,
> {
> gchar *type;
>
> - type = g_strdup_printf("legacy<%s>", prop->info->name);
> + type = g_strdup_printf("legacy<%s>",
> + prop->info->legacy_name ?: prop->info->name);
>
> qdev_property_add(dev, prop->name, type,
> prop->info->print ? qdev_get_legacy_property : NULL,
> diff --git a/hw/qdev.h b/hw/qdev.h
> index a1cce37..8002644 100644
> --- a/hw/qdev.h
> +++ b/hw/qdev.h
> @@ -156,6 +156,7 @@ enum PropertyType {
>
> struct PropertyInfo {
> const char *name;
> + const char *legacy_name;
> size_t size;
> enum PropertyType type;
> int64_t min;
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH v2 8/8] qom: register qdev properties also as non-legacy properties
2011-12-16 16:47 ` [Qemu-devel] [PATCH v2 8/8] qom: register qdev properties also as non-legacy properties Paolo Bonzini
@ 2011-12-16 17:05 ` Anthony Liguori
0 siblings, 0 replies; 15+ messages in thread
From: Anthony Liguori @ 2011-12-16 17:05 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel
On 12/16/2011 10:47 AM, Paolo Bonzini wrote:
> Push legacy properties into a legacy<...> namespace, and make them
> available with correct types too.
>
> Signed-off-by: Paolo Bonzini<pbonzini@redhat.com>
> ---
> hw/qdev.c | 28 +++++++++++++++++++++++++---
> hw/qdev.h | 7 +++----
> 2 files changed, 28 insertions(+), 7 deletions(-)
>
> diff --git a/hw/qdev.c b/hw/qdev.c
> index 426ea71..2f7defc 100644
> --- a/hw/qdev.c
> +++ b/hw/qdev.c
> @@ -80,6 +80,9 @@ static DeviceInfo *qdev_find_info(BusInfo *bus_info, const char *name)
> return NULL;
> }
>
> +static void qdev_property_add_legacy(DeviceState *dev, Property *prop,
> + Error **errp);
> +
> static DeviceState *qdev_create_from_info(BusState *bus, DeviceInfo *info)
> {
> DeviceState *dev;
> @@ -104,10 +107,12 @@ static DeviceState *qdev_create_from_info(BusState *bus, DeviceInfo *info)
>
> for (prop = dev->info->props; prop&& prop->name; prop++) {
> qdev_property_add_legacy(dev, prop, NULL);
> + qdev_property_add_static(dev, prop, NULL);
> }
>
> for (prop = dev->info->bus_info->props; prop&& prop->name; prop++) {
> qdev_property_add_legacy(dev, prop, NULL);
> + qdev_property_add_static(dev, prop, NULL);
> }
>
> qdev_property_add_str(dev, "type", qdev_get_type, NULL, NULL);
> @@ -1174,7 +1179,7 @@ static void qdev_set_legacy_property(DeviceState *dev, Visitor *v, void *opaque,
> * @qdev_add_legacy_property - adds a legacy property
> *
> * Do not use this is new code! Properties added through this interface will
> - * be given types in the "legacy<>" type namespace.
> + * be given names and types in the "legacy<>" type namespace.
> *
> * Legacy properties are always processed as strings. The format of the string
> * depends on the property type.
> @@ -1182,18 +1187,35 @@ static void qdev_set_legacy_property(DeviceState *dev, Visitor *v, void *opaque,
> void qdev_property_add_legacy(DeviceState *dev, Property *prop,
> Error **errp)
> {
> - gchar *type;
> + gchar *name, *type;
>
> + name = g_strdup_printf("legacy<%s>", prop->name);
Okay, let's make this name legacy-%s to make it clear that it's not type.
Otherwise, the patch looks good.
Regards,
Anthony Liguori
> type = g_strdup_printf("legacy<%s>",
> prop->info->legacy_name ?: prop->info->name);
>
> - qdev_property_add(dev, prop->name, type,
> + qdev_property_add(dev, name, type,
> prop->info->print ? qdev_get_legacy_property : NULL,
> prop->info->parse ? qdev_set_legacy_property : NULL,
> NULL,
> prop, errp);
>
> g_free(type);
> + g_free(name);
> +}
> +
> +/**
> + * @qdev_property_add_static - add a @Property to a device.
> + *
> + * Static properties access data in a struct. The actual type of the
> + * property and the field depends on the property type.
> + */
> +void qdev_property_add_static(DeviceState *dev, Property *prop,
> + Error **errp)
> +{
> + qdev_property_add(dev, prop->name, prop->info->name,
> + prop->info->get, prop->info->set,
> + NULL,
> + prop, errp);
> }
>
> DeviceState *qdev_get_root(void)
> diff --git a/hw/qdev.h b/hw/qdev.h
> index 8002644..3410e77 100644
> --- a/hw/qdev.h
> +++ b/hw/qdev.h
> @@ -487,11 +487,10 @@ const char *qdev_property_get_type(DeviceState *dev, const char *name,
> Error **errp);
>
> /**
> - * @qdev_property_add_legacy - add a legacy @Property to a device
> - *
> - * DO NOT USE THIS IN NEW CODE!
> + * @qdev_property_add_static - add a @Property to a device referencing a
> + * field in a struct.
> */
> -void qdev_property_add_legacy(DeviceState *dev, Property *prop, Error **errp);
> +void qdev_property_add_static(DeviceState *dev, Property *prop, Error **errp);
>
> /**
> * @qdev_get_root - returns the root device of the composition tree
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH v2 4/8] qom: interpret the return value when setting legacy properties
2011-12-16 17:00 ` Anthony Liguori
@ 2011-12-16 17:19 ` Paolo Bonzini
0 siblings, 0 replies; 15+ messages in thread
From: Paolo Bonzini @ 2011-12-16 17:19 UTC (permalink / raw)
To: Anthony Liguori; +Cc: qemu-devel
[-- Attachment #1: Type: text/plain, Size: 636 bytes --]
On 12/16/2011 06:00 PM, Anthony Liguori wrote:
>
> And perhaps it would make more sense to return Error * and make the
> function name be a constructor:
>
> Error *error_from_qdev_prop_err(int ret, DeviceState *dev,
> Property *prop, const char *value);
That doesn't work too well when calling it from setters. However,
error_set_from_qdev_prop_error is definitely an improvement (no matter
what a mouthful it is).
I need to rush now. I placed this at git://github.com/qemu/bonzini.git
qom-props (commit 263e8c5), so you can play on top of it or pull it. I
attach the interdiff from v2.
Paolo
[-- Attachment #2: interdiff.patch --]
[-- Type: text/x-patch, Size: 3619 bytes --]
diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
index dd41e5a..80115b3 100644
--- a/hw/qdev-properties.c
+++ b/hw/qdev-properties.c
@@ -644,13 +644,11 @@ static void set_generic(DeviceState *dev, Visitor *v, void *opaque,
}
if (!*str) {
g_free(str);
- qdev_prop_error(errp, EINVAL, dev, prop, str);
+ error_set_from_qdev_prop_error(errp, EINVAL, dev, prop, str);
return;
}
ret = prop->info->parse(dev, prop, str);
- if (ret != 0) {
- qdev_prop_error(errp, ret, dev, prop, str);
- }
+ error_set_from_qdev_prop_error(errp, ret, dev, prop, str);
g_free(str);
}
@@ -973,8 +972,8 @@ int qdev_prop_exists(DeviceState *dev, const char *name)
return qdev_prop_find(dev, name) ? true : false;
}
-void qdev_prop_error(Error **errp, int ret,
- DeviceState *dev, Property *prop, const char *value)
+void error_set_from_qdev_prop_error(Error **errp, int ret, DeviceState *dev,
+ Property *prop, const char *value)
{
switch (ret) {
case -EEXIST:
@@ -990,6 +989,10 @@ void qdev_prop_error(Error **errp, int ret,
error_set(errp, QERR_PROPERTY_VALUE_NOT_FOUND,
dev->info->name, prop->name, value);
break;
+ case 0:
+ break;
+ default:
+ abort();
}
}
@@ -1012,7 +1015,7 @@ int qdev_prop_parse(DeviceState *dev, const char *name, const char *value)
ret = prop->info->parse(dev, prop, value);
if (ret < 0) {
Error *err;
- qdev_prop_error(&err, ret, dev, prop, value);
+ error_set_from_qdev_prop_error(&err, ret, dev, prop, value);
qerror_report_err(err);
error_free(err);
return -1;
diff --git a/hw/qdev.c b/hw/qdev.c
index 2f7defc..0465632 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -1169,9 +1169,7 @@ static void qdev_set_legacy_property(DeviceState *dev, Visitor *v, void *opaque,
}
ret = prop->info->parse(dev, prop, ptr);
- if (ret != 0) {
- qdev_prop_error(errp, ret, dev, prop, ptr);
- }
+ error_set_from_qdev_prop_error(errp, ret, dev, prop, ptr);
g_free(ptr);
}
@@ -1179,7 +1177,7 @@ static void qdev_set_legacy_property(DeviceState *dev, Visitor *v, void *opaque,
* @qdev_add_legacy_property - adds a legacy property
*
* Do not use this is new code! Properties added through this interface will
- * be given names and types in the "legacy<>" type namespace.
+ * be given names and types in the "legacy" namespace.
*
* Legacy properties are always processed as strings. The format of the string
* depends on the property type.
@@ -1189,7 +1187,7 @@ void qdev_property_add_legacy(DeviceState *dev, Property *prop,
{
gchar *name, *type;
- name = g_strdup_printf("legacy<%s>", prop->name);
+ name = g_strdup_printf("legacy-%s", prop->name);
type = g_strdup_printf("legacy<%s>",
prop->info->legacy_name ?: prop->info->name);
diff --git a/hw/qdev.h b/hw/qdev.h
index 3410e77..d5896be 100644
--- a/hw/qdev.h
+++ b/hw/qdev.h
@@ -375,8 +375,8 @@ void qdev_prop_set_defaults(DeviceState *dev, Property *props);
void qdev_prop_register_global_list(GlobalProperty *props);
void qdev_prop_set_globals(DeviceState *dev);
-void qdev_prop_error(Error **errp, int ret, DeviceState *name,
- Property *prop, const char *value);
+void error_set_from_qdev_prop_error(Error **errp, int ret, DeviceState *dev,
+ Property *prop, const char *value);
static inline const char *qdev_fw_name(DeviceState *dev)
{
^ permalink raw reply related [flat|nested] 15+ messages in thread
end of thread, other threads:[~2011-12-16 17:19 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-12-16 16:47 [Qemu-devel] [PATCH v2 0/8] qom: introduce non-legacy static properties Paolo Bonzini
2011-12-16 16:47 ` [Qemu-devel] [PATCH v2 1/8] qapi: protect against NULL QObject in qmp_input_get_object Paolo Bonzini
2011-12-16 16:47 ` [Qemu-devel] [PATCH v2 2/8] qom: fix swapped parameters Paolo Bonzini
2011-12-16 16:47 ` [Qemu-devel] [PATCH v2 3/8] qom: push permission checks up into qdev_property_add_legacy Paolo Bonzini
2011-12-16 16:47 ` [Qemu-devel] [PATCH v2 4/8] qom: interpret the return value when setting legacy properties Paolo Bonzini
2011-12-16 17:00 ` Anthony Liguori
2011-12-16 17:19 ` Paolo Bonzini
2011-12-16 16:47 ` [Qemu-devel] [PATCH v2 5/8] qom: introduce QERR_PROPERTY_VALUE_OUT_OF_RANGE Paolo Bonzini
2011-12-16 17:00 ` Anthony Liguori
2011-12-16 16:47 ` [Qemu-devel] [PATCH v2 6/8] qom: introduce get/set methods for Property Paolo Bonzini
2011-12-16 17:02 ` Anthony Liguori
2011-12-16 16:47 ` [Qemu-devel] [PATCH v2 7/8] qom: distinguish "legacy" property type name from QOM type name Paolo Bonzini
2011-12-16 17:03 ` Anthony Liguori
2011-12-16 16:47 ` [Qemu-devel] [PATCH v2 8/8] qom: register qdev properties also as non-legacy properties Paolo Bonzini
2011-12-16 17:05 ` Anthony Liguori
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).