| Message ID | 1350669429-26354-1-git-send-email-peter.maydell@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
Ping? (slightly eager ping but softfreeze is not far off...) -- PMM On 19 October 2012 18:57, Peter Maydell <peter.maydell@linaro.org> wrote: > Now we have error_setg() we can improve the error message emitted if > you attempt to set a property of a device after the device is realized > (the previous message was "permission denied" which was not very > informative). > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > This is v3 of this patch (v2 was sent back in July); I think the > error_setg() version is nicer and allows us to be a bit more > helpful with the message text... > > hw/qdev-properties.c | 42 ++++++++++++++++++++++++++++-------------- > 1 file changed, 28 insertions(+), 14 deletions(-) > > diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c > index 8aca0d4..b7aa386 100644 > --- a/hw/qdev-properties.c > +++ b/hw/qdev-properties.c > @@ -5,6 +5,20 @@ > #include "hw/block-common.h" > #include "net/hub.h" > > +static void report_already_realized(Error **errp, DeviceState *dev, > + const char *name) > +{ > + if (dev->id) { > + error_setg(errp, "Attempt to set property '%s' on device '%s' " > + "(type '%s') after it was realized", name, dev->id, > + object_get_typename(OBJECT(dev))); > + } else { > + error_setg(errp, "Attempt to set property '%s' on anonymous device " > + "(type '%s') after it was realized", name, > + object_get_typename(OBJECT(dev))); > + } > +} > + > void *qdev_get_prop_ptr(DeviceState *dev, Property *prop) > { > void *ptr = dev; > @@ -36,7 +50,7 @@ static void set_pointer(Object *obj, Visitor *v, Property *prop, > int ret; > > if (dev->state != DEV_STATE_CREATED) { > - error_set(errp, QERR_PERMISSION_DENIED); > + report_already_realized(errp, dev, name); > return; > } > > @@ -74,7 +88,7 @@ static void set_enum(Object *obj, Visitor *v, void *opaque, > int *ptr = qdev_get_prop_ptr(dev, prop); > > if (dev->state != DEV_STATE_CREATED) { > - error_set(errp, QERR_PERMISSION_DENIED); > + report_already_realized(errp, dev, name); > return; > } > > @@ -126,7 +140,7 @@ static void set_bit(Object *obj, Visitor *v, void *opaque, > bool value; > > if (dev->state != DEV_STATE_CREATED) { > - error_set(errp, QERR_PERMISSION_DENIED); > + report_already_realized(errp, dev, name); > return; > } > > @@ -166,7 +180,7 @@ static void set_uint8(Object *obj, Visitor *v, void *opaque, > uint8_t *ptr = qdev_get_prop_ptr(dev, prop); > > if (dev->state != DEV_STATE_CREATED) { > - error_set(errp, QERR_PERMISSION_DENIED); > + report_already_realized(errp, dev, name); > return; > } > > @@ -233,7 +247,7 @@ static void set_uint16(Object *obj, Visitor *v, void *opaque, > uint16_t *ptr = qdev_get_prop_ptr(dev, prop); > > if (dev->state != DEV_STATE_CREATED) { > - error_set(errp, QERR_PERMISSION_DENIED); > + report_already_realized(errp, dev, name); > return; > } > > @@ -266,7 +280,7 @@ static void set_uint32(Object *obj, Visitor *v, void *opaque, > uint32_t *ptr = qdev_get_prop_ptr(dev, prop); > > if (dev->state != DEV_STATE_CREATED) { > - error_set(errp, QERR_PERMISSION_DENIED); > + report_already_realized(errp, dev, name); > return; > } > > @@ -291,7 +305,7 @@ static void set_int32(Object *obj, Visitor *v, void *opaque, > int32_t *ptr = qdev_get_prop_ptr(dev, prop); > > if (dev->state != DEV_STATE_CREATED) { > - error_set(errp, QERR_PERMISSION_DENIED); > + report_already_realized(errp, dev, name); > return; > } > > @@ -364,7 +378,7 @@ static void set_uint64(Object *obj, Visitor *v, void *opaque, > uint64_t *ptr = qdev_get_prop_ptr(dev, prop); > > if (dev->state != DEV_STATE_CREATED) { > - error_set(errp, QERR_PERMISSION_DENIED); > + report_already_realized(errp, dev, name); > return; > } > > @@ -452,7 +466,7 @@ static void set_string(Object *obj, Visitor *v, void *opaque, > char *str; > > if (dev->state != DEV_STATE_CREATED) { > - error_set(errp, QERR_PERMISSION_DENIED); > + report_already_realized(errp, dev, name); > return; > } > > @@ -666,7 +680,7 @@ static void set_vlan(Object *obj, Visitor *v, void *opaque, > NetClientState *hubport; > > if (dev->state != DEV_STATE_CREATED) { > - error_set(errp, QERR_PERMISSION_DENIED); > + report_already_realized(errp, dev, name); > return; > } > > @@ -737,7 +751,7 @@ static void set_mac(Object *obj, Visitor *v, void *opaque, > char *str, *p; > > if (dev->state != DEV_STATE_CREATED) { > - error_set(errp, QERR_PERMISSION_DENIED); > + report_already_realized(errp, dev, name); > return; > } > > @@ -825,7 +839,7 @@ static void set_pci_devfn(Object *obj, Visitor *v, void *opaque, > char *str; > > if (dev->state != DEV_STATE_CREATED) { > - error_set(errp, QERR_PERMISSION_DENIED); > + report_already_realized(errp, dev, name); > return; > } > > @@ -895,7 +909,7 @@ static void set_blocksize(Object *obj, Visitor *v, void *opaque, > const int64_t max = 32768; > > if (dev->state != DEV_STATE_CREATED) { > - error_set(errp, QERR_PERMISSION_DENIED); > + report_already_realized(errp, dev, name); > return; > } > > @@ -963,7 +977,7 @@ static void set_pci_host_devaddr(Object *obj, Visitor *v, void *opaque, > unsigned int slot = 0, func = 0; > > if (dev->state != DEV_STATE_CREATED) { > - error_set(errp, QERR_PERMISSION_DENIED); > + report_already_realized(errp, dev, name); > return; > } > > -- > 1.7.9.5
On 10/19/2012 9:57 PM, Peter Maydell wrote: > Now we have error_setg() we can improve the error message emitted if > you attempt to set a property of a device after the device is realized > (the previous message was "permission denied" which was not very > informative). > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > This is v3 of this patch (v2 was sent back in July); I think the > error_setg() version is nicer and allows us to be a bit more > helpful with the message text... > > hw/qdev-properties.c | 42 ++++++++++++++++++++++++++++-------------- > 1 file changed, 28 insertions(+), 14 deletions(-) > > diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c > index 8aca0d4..b7aa386 100644 > --- a/hw/qdev-properties.c > +++ b/hw/qdev-properties.c > @@ -5,6 +5,20 @@ > #include "hw/block-common.h" > #include "net/hub.h" > > +static void report_already_realized(Error **errp, DeviceState *dev, > + const char *name) > +{ > + if (dev->id) { > + error_setg(errp, "Attempt to set property '%s' on device '%s' " > + "(type '%s') after it was realized", name, dev->id, Hi, Peter. Maybe "after it was initialized" would be better, "realized" sounds too much like an "inside" information (also, we call qdev_init(), not qdev_realize). And there is also set_taddr() in hw/qdev-addr.c > + object_get_typename(OBJECT(dev))); > + } else { > + error_setg(errp, "Attempt to set property '%s' on anonymous device " > + "(type '%s') after it was realized", name, > + object_get_typename(OBJECT(dev))); > + } > +} > + > void *qdev_get_prop_ptr(DeviceState *dev, Property *prop) > { > void *ptr = dev; > @@ -36,7 +50,7 @@ static void set_pointer(Object *obj, Visitor *v, Property *prop, > int ret; > > if (dev->state != DEV_STATE_CREATED) { > - error_set(errp, QERR_PERMISSION_DENIED); > + report_already_realized(errp, dev, name); > return; > } > > @@ -74,7 +88,7 @@ static void set_enum(Object *obj, Visitor *v, void *opaque, > int *ptr = qdev_get_prop_ptr(dev, prop); > > if (dev->state != DEV_STATE_CREATED) { > - error_set(errp, QERR_PERMISSION_DENIED); > + report_already_realized(errp, dev, name); > return; > } > > @@ -126,7 +140,7 @@ static void set_bit(Object *obj, Visitor *v, void *opaque, > bool value; > > if (dev->state != DEV_STATE_CREATED) { > - error_set(errp, QERR_PERMISSION_DENIED); > + report_already_realized(errp, dev, name); > return; > } > > @@ -166,7 +180,7 @@ static void set_uint8(Object *obj, Visitor *v, void *opaque, > uint8_t *ptr = qdev_get_prop_ptr(dev, prop); > > if (dev->state != DEV_STATE_CREATED) { > - error_set(errp, QERR_PERMISSION_DENIED); > + report_already_realized(errp, dev, name); > return; > } > > @@ -233,7 +247,7 @@ static void set_uint16(Object *obj, Visitor *v, void *opaque, > uint16_t *ptr = qdev_get_prop_ptr(dev, prop); > > if (dev->state != DEV_STATE_CREATED) { > - error_set(errp, QERR_PERMISSION_DENIED); > + report_already_realized(errp, dev, name); > return; > } > > @@ -266,7 +280,7 @@ static void set_uint32(Object *obj, Visitor *v, void *opaque, > uint32_t *ptr = qdev_get_prop_ptr(dev, prop); > > if (dev->state != DEV_STATE_CREATED) { > - error_set(errp, QERR_PERMISSION_DENIED); > + report_already_realized(errp, dev, name); > return; > } > > @@ -291,7 +305,7 @@ static void set_int32(Object *obj, Visitor *v, void *opaque, > int32_t *ptr = qdev_get_prop_ptr(dev, prop); > > if (dev->state != DEV_STATE_CREATED) { > - error_set(errp, QERR_PERMISSION_DENIED); > + report_already_realized(errp, dev, name); > return; > } > > @@ -364,7 +378,7 @@ static void set_uint64(Object *obj, Visitor *v, void *opaque, > uint64_t *ptr = qdev_get_prop_ptr(dev, prop); > > if (dev->state != DEV_STATE_CREATED) { > - error_set(errp, QERR_PERMISSION_DENIED); > + report_already_realized(errp, dev, name); > return; > } > > @@ -452,7 +466,7 @@ static void set_string(Object *obj, Visitor *v, void *opaque, > char *str; > > if (dev->state != DEV_STATE_CREATED) { > - error_set(errp, QERR_PERMISSION_DENIED); > + report_already_realized(errp, dev, name); > return; > } > > @@ -666,7 +680,7 @@ static void set_vlan(Object *obj, Visitor *v, void *opaque, > NetClientState *hubport; > > if (dev->state != DEV_STATE_CREATED) { > - error_set(errp, QERR_PERMISSION_DENIED); > + report_already_realized(errp, dev, name); > return; > } > > @@ -737,7 +751,7 @@ static void set_mac(Object *obj, Visitor *v, void *opaque, > char *str, *p; > > if (dev->state != DEV_STATE_CREATED) { > - error_set(errp, QERR_PERMISSION_DENIED); > + report_already_realized(errp, dev, name); > return; > } > > @@ -825,7 +839,7 @@ static void set_pci_devfn(Object *obj, Visitor *v, void *opaque, > char *str; > > if (dev->state != DEV_STATE_CREATED) { > - error_set(errp, QERR_PERMISSION_DENIED); > + report_already_realized(errp, dev, name); > return; > } > > @@ -895,7 +909,7 @@ static void set_blocksize(Object *obj, Visitor *v, void *opaque, > const int64_t max = 32768; > > if (dev->state != DEV_STATE_CREATED) { > - error_set(errp, QERR_PERMISSION_DENIED); > + report_already_realized(errp, dev, name); > return; > } > > @@ -963,7 +977,7 @@ static void set_pci_host_devaddr(Object *obj, Visitor *v, void *opaque, > unsigned int slot = 0, func = 0; > > if (dev->state != DEV_STATE_CREATED) { > - error_set(errp, QERR_PERMISSION_DENIED); > + report_already_realized(errp, dev, name); > return; > } > >
diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c index 8aca0d4..b7aa386 100644 --- a/hw/qdev-properties.c +++ b/hw/qdev-properties.c @@ -5,6 +5,20 @@ #include "hw/block-common.h" #include "net/hub.h" +static void report_already_realized(Error **errp, DeviceState *dev, + const char *name) +{ + if (dev->id) { + error_setg(errp, "Attempt to set property '%s' on device '%s' " + "(type '%s') after it was realized", name, dev->id, + object_get_typename(OBJECT(dev))); + } else { + error_setg(errp, "Attempt to set property '%s' on anonymous device " + "(type '%s') after it was realized", name, + object_get_typename(OBJECT(dev))); + } +} + void *qdev_get_prop_ptr(DeviceState *dev, Property *prop) { void *ptr = dev; @@ -36,7 +50,7 @@ static void set_pointer(Object *obj, Visitor *v, Property *prop, int ret; if (dev->state != DEV_STATE_CREATED) { - error_set(errp, QERR_PERMISSION_DENIED); + report_already_realized(errp, dev, name); return; } @@ -74,7 +88,7 @@ static void set_enum(Object *obj, Visitor *v, void *opaque, int *ptr = qdev_get_prop_ptr(dev, prop); if (dev->state != DEV_STATE_CREATED) { - error_set(errp, QERR_PERMISSION_DENIED); + report_already_realized(errp, dev, name); return; } @@ -126,7 +140,7 @@ static void set_bit(Object *obj, Visitor *v, void *opaque, bool value; if (dev->state != DEV_STATE_CREATED) { - error_set(errp, QERR_PERMISSION_DENIED); + report_already_realized(errp, dev, name); return; } @@ -166,7 +180,7 @@ static void set_uint8(Object *obj, Visitor *v, void *opaque, uint8_t *ptr = qdev_get_prop_ptr(dev, prop); if (dev->state != DEV_STATE_CREATED) { - error_set(errp, QERR_PERMISSION_DENIED); + report_already_realized(errp, dev, name); return; } @@ -233,7 +247,7 @@ static void set_uint16(Object *obj, Visitor *v, void *opaque, uint16_t *ptr = qdev_get_prop_ptr(dev, prop); if (dev->state != DEV_STATE_CREATED) { - error_set(errp, QERR_PERMISSION_DENIED); + report_already_realized(errp, dev, name); return; } @@ -266,7 +280,7 @@ static void set_uint32(Object *obj, Visitor *v, void *opaque, uint32_t *ptr = qdev_get_prop_ptr(dev, prop); if (dev->state != DEV_STATE_CREATED) { - error_set(errp, QERR_PERMISSION_DENIED); + report_already_realized(errp, dev, name); return; } @@ -291,7 +305,7 @@ static void set_int32(Object *obj, Visitor *v, void *opaque, int32_t *ptr = qdev_get_prop_ptr(dev, prop); if (dev->state != DEV_STATE_CREATED) { - error_set(errp, QERR_PERMISSION_DENIED); + report_already_realized(errp, dev, name); return; } @@ -364,7 +378,7 @@ static void set_uint64(Object *obj, Visitor *v, void *opaque, uint64_t *ptr = qdev_get_prop_ptr(dev, prop); if (dev->state != DEV_STATE_CREATED) { - error_set(errp, QERR_PERMISSION_DENIED); + report_already_realized(errp, dev, name); return; } @@ -452,7 +466,7 @@ static void set_string(Object *obj, Visitor *v, void *opaque, char *str; if (dev->state != DEV_STATE_CREATED) { - error_set(errp, QERR_PERMISSION_DENIED); + report_already_realized(errp, dev, name); return; } @@ -666,7 +680,7 @@ static void set_vlan(Object *obj, Visitor *v, void *opaque, NetClientState *hubport; if (dev->state != DEV_STATE_CREATED) { - error_set(errp, QERR_PERMISSION_DENIED); + report_already_realized(errp, dev, name); return; } @@ -737,7 +751,7 @@ static void set_mac(Object *obj, Visitor *v, void *opaque, char *str, *p; if (dev->state != DEV_STATE_CREATED) { - error_set(errp, QERR_PERMISSION_DENIED); + report_already_realized(errp, dev, name); return; } @@ -825,7 +839,7 @@ static void set_pci_devfn(Object *obj, Visitor *v, void *opaque, char *str; if (dev->state != DEV_STATE_CREATED) { - error_set(errp, QERR_PERMISSION_DENIED); + report_already_realized(errp, dev, name); return; } @@ -895,7 +909,7 @@ static void set_blocksize(Object *obj, Visitor *v, void *opaque, const int64_t max = 32768; if (dev->state != DEV_STATE_CREATED) { - error_set(errp, QERR_PERMISSION_DENIED); + report_already_realized(errp, dev, name); return; } @@ -963,7 +977,7 @@ static void set_pci_host_devaddr(Object *obj, Visitor *v, void *opaque, unsigned int slot = 0, func = 0; if (dev->state != DEV_STATE_CREATED) { - error_set(errp, QERR_PERMISSION_DENIED); + report_already_realized(errp, dev, name); return; }
Now we have error_setg() we can improve the error message emitted if you attempt to set a property of a device after the device is realized (the previous message was "permission denied" which was not very informative). Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- This is v3 of this patch (v2 was sent back in July); I think the error_setg() version is nicer and allows us to be a bit more helpful with the message text... hw/qdev-properties.c | 42 ++++++++++++++++++++++++++++-------------- 1 file changed, 28 insertions(+), 14 deletions(-)