From: Igor Mitsyanko <i.mitsyanko@gmail.com>
To: Peter Maydell <peter.maydell@linaro.org>
Cc: Paolo Bonzini <pbonzini@redhat.com>,
Anthony Liguori <aliguori@us.ibm.com>,
qemu-devel@nongnu.org, patches@linaro.org
Subject: Re: [Qemu-devel] [PATCH v3] hw/qdev-properties.c: Improve diagnostic for setting property after realize
Date: Sun, 28 Oct 2012 22:55:43 +0400 [thread overview]
Message-ID: <508D7FAF.1040707@gmail.com> (raw)
In-Reply-To: <1350669429-26354-1-git-send-email-peter.maydell@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 <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;
> }
>
>
prev parent reply other threads:[~2012-10-28 18:55 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-10-19 17:57 [Qemu-devel] [PATCH v3] hw/qdev-properties.c: Improve diagnostic for setting property after realize Peter Maydell
2012-10-26 17:30 ` Peter Maydell
2012-10-28 18:55 ` Igor Mitsyanko [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=508D7FAF.1040707@gmail.com \
--to=i.mitsyanko@gmail.com \
--cc=aliguori@us.ibm.com \
--cc=patches@linaro.org \
--cc=pbonzini@redhat.com \
--cc=peter.maydell@linaro.org \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.