From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:40701) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TSY1Y-0007xo-18 for qemu-devel@nongnu.org; Sun, 28 Oct 2012 14:55:53 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TSY1W-0006bi-5M for qemu-devel@nongnu.org; Sun, 28 Oct 2012 14:55:51 -0400 Received: from mail-lb0-f173.google.com ([209.85.217.173]:35281) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TSY1V-0006bb-P2 for qemu-devel@nongnu.org; Sun, 28 Oct 2012 14:55:50 -0400 Received: by mail-lb0-f173.google.com with SMTP id gj3so2551245lbb.4 for ; Sun, 28 Oct 2012 11:55:48 -0700 (PDT) Message-ID: <508D7FAF.1040707@gmail.com> Date: Sun, 28 Oct 2012 22:55:43 +0400 From: Igor Mitsyanko MIME-Version: 1.0 References: <1350669429-26354-1-git-send-email-peter.maydell@linaro.org> In-Reply-To: <1350669429-26354-1-git-send-email-peter.maydell@linaro.org> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v3] hw/qdev-properties.c: Improve diagnostic for setting property after realize List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Maydell Cc: Paolo Bonzini , Anthony Liguori , qemu-devel@nongnu.org, patches@linaro.org 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 > --- > 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; > } > >