From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:44797) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SCC9c-0000XF-JO for qemu-devel@nongnu.org; Mon, 26 Mar 2012 11:48:25 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SCC9V-0003sc-ME for qemu-devel@nongnu.org; Mon, 26 Mar 2012 11:48:20 -0400 Received: from cantor2.suse.de ([195.135.220.15]:52440 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SCC9V-0003s9-Ca for qemu-devel@nongnu.org; Mon, 26 Mar 2012 11:48:13 -0400 Message-ID: <4F708FB9.6000507@suse.de> Date: Mon, 26 Mar 2012 17:48:09 +0200 From: =?ISO-8859-15?Q?Andreas_F=E4rber?= MIME-Version: 1.0 References: <1332775847-29202-1-git-send-email-peter.maydell@linaro.org> In-Reply-To: <1332775847-29202-1-git-send-email-peter.maydell@linaro.org> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH] qerror: Add QERR_PROPERTY_SET_AFTER_REALIZE List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org Cc: Luiz Capitulino , Peter Maydell , Anthony Liguori , Paolo Bonzini , patches@linaro.org Am 26.03.2012 17:30, schrieb Peter Maydell: > Add a new QError QERR_PROPERTY_SET_AFTER_REALIZE for attempts > to set a QOM or qdev property after the object/device has been > realized. This allows a slightly more informative diagnostic > than the previous "Insufficient permission" message. >=20 > Signed-off-by: Peter Maydell > --- > We've talked about the rather unhelpful nature of the "Insufficient > permission" diagnostic before: > http://lists.gnu.org/archive/html/qemu-devel/2012-02/msg02368.html > and I finally got round to writing a patch to improve it... Not aware of the submission, I suggested to append this to my series, since upstream does not yet have "realize" as such. > hw/qdev-properties.c | 22 +++++++++++----------- > qerror.c | 4 ++++ > qerror.h | 3 +++ > 3 files changed, 18 insertions(+), 11 deletions(-) >=20 > diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c > index bff9152..627c6ba 100644 > --- a/hw/qdev-properties.c > +++ b/hw/qdev-properties.c > @@ -54,7 +54,7 @@ static void set_bit(Object *obj, Visitor *v, void *op= aque, > bool value; > =20 > if (dev->state !=3D DEV_STATE_CREATED) { > - error_set(errp, QERR_PERMISSION_DENIED); > + error_set(errp, QERR_PROPERTY_SET_AFTER_REALIZE, dev->id?:"", = name); I suggested spaces around ?: but was told this was the surrounding style - leave or fix? > return; > } > =20 > @@ -98,7 +98,7 @@ static void set_int8(Object *obj, Visitor *v, void *o= paque, > int64_t value; > =20 > if (dev->state !=3D DEV_STATE_CREATED) { > - error_set(errp, QERR_PERMISSION_DENIED); > + error_set(errp, QERR_PROPERTY_SET_AFTER_REALIZE, dev->id?:"", = name); > return; > } > =20 > @@ -184,7 +184,7 @@ static void set_int16(Object *obj, Visitor *v, void= *opaque, > int64_t value; > =20 > if (dev->state !=3D DEV_STATE_CREATED) { > - error_set(errp, QERR_PERMISSION_DENIED); > + error_set(errp, QERR_PROPERTY_SET_AFTER_REALIZE, dev->id?:"", = name); > return; > } > =20 > @@ -234,7 +234,7 @@ static void set_int32(Object *obj, Visitor *v, void= *opaque, > int64_t value; > =20 > if (dev->state !=3D DEV_STATE_CREATED) { > - error_set(errp, QERR_PERMISSION_DENIED); > + error_set(errp, QERR_PROPERTY_SET_AFTER_REALIZE, dev->id?:"", = name); > return; > } > =20 > @@ -324,7 +324,7 @@ static void set_int64(Object *obj, Visitor *v, void= *opaque, > int64_t *ptr =3D qdev_get_prop_ptr(dev, prop); > =20 > if (dev->state !=3D DEV_STATE_CREATED) { > - error_set(errp, QERR_PERMISSION_DENIED); > + error_set(errp, QERR_PROPERTY_SET_AFTER_REALIZE, dev->id?:"", = name); > return; > } > =20 > @@ -412,7 +412,7 @@ static void set_string(Object *obj, Visitor *v, voi= d *opaque, > char *str; > =20 > if (dev->state !=3D DEV_STATE_CREATED) { > - error_set(errp, QERR_PERMISSION_DENIED); > + error_set(errp, QERR_PROPERTY_SET_AFTER_REALIZE, dev->id?:"", = name); > return; > } > =20 > @@ -490,7 +490,7 @@ static void set_pointer(Object *obj, Visitor *v, Pr= operty *prop, > int ret; > =20 > if (dev->state !=3D DEV_STATE_CREATED) { > - error_set(errp, QERR_PERMISSION_DENIED); > + error_set(errp, QERR_PROPERTY_SET_AFTER_REALIZE, dev->id?:"", = name); > return; > } > =20 > @@ -659,7 +659,7 @@ static void set_vlan(Object *obj, Visitor *v, void = *opaque, > VLANState *vlan; > =20 > if (dev->state !=3D DEV_STATE_CREATED) { > - error_set(errp, QERR_PERMISSION_DENIED); > + error_set(errp, QERR_PROPERTY_SET_AFTER_REALIZE, dev->id?:"", = name); > return; > } > =20 > @@ -729,7 +729,7 @@ static void set_mac(Object *obj, Visitor *v, void *= opaque, > char *str, *p; > =20 > if (dev->state !=3D DEV_STATE_CREATED) { > - error_set(errp, QERR_PERMISSION_DENIED); > + error_set(errp, QERR_PROPERTY_SET_AFTER_REALIZE, dev->id?:"", = name); > return; > } > =20 > @@ -797,7 +797,7 @@ static void set_enum(Object *obj, Visitor *v, void = *opaque, > int *ptr =3D qdev_get_prop_ptr(dev, prop); > =20 > if (dev->state !=3D DEV_STATE_CREATED) { > - error_set(errp, QERR_PERMISSION_DENIED); > + error_set(errp, QERR_PROPERTY_SET_AFTER_REALIZE, dev->id?:"", = name); > return; > } > =20 > @@ -828,7 +828,7 @@ static void set_pci_devfn(Object *obj, Visitor *v, = void *opaque, > char *str =3D (char *)""; > =20 > if (dev->state !=3D DEV_STATE_CREATED) { > - error_set(errp, QERR_PERMISSION_DENIED); > + error_set(errp, QERR_PROPERTY_SET_AFTER_REALIZE, dev->id?:"", = name); > return; > } > =20 > diff --git a/qerror.c b/qerror.c > index 41c729a..7d9aa4f 100644 > --- a/qerror.c > +++ b/qerror.c > @@ -229,6 +229,10 @@ static const QErrorStringTable qerror_table[] =3D = { > .desc =3D "Property '%(device).%(property)' not found", > }, > { > + .error_fmt =3D QERR_PROPERTY_SET_AFTER_REALIZE, > + .desc =3D "Property '%(device).%(property)' cannot be set= after realize", > + }, > + { > .error_fmt =3D QERR_PROPERTY_VALUE_BAD, > .desc =3D "Property '%(device).%(property)' doesn't take = value '%(value)'", > }, > diff --git a/qerror.h b/qerror.h > index e16f9c2..266a04a 100644 > --- a/qerror.h > +++ b/qerror.h > @@ -193,6 +193,9 @@ QError *qobject_to_qerror(const QObject *obj); > #define QERR_PROPERTY_NOT_FOUND \ > "{ 'class': 'PropertyNotFound', 'data': { 'device': %s, 'property'= : %s } }" > =20 > +#define QERR_PROPERTY_SET_AFTER_REALIZE \ > + "{ 'class': 'PropertySetAfterRealize', 'data': { 'device': %s, 'pr= operty': %s } }" Properties are no longer a device concept. Can we start using a different key name here, e.g., 'type'? Andreas > + > #define QERR_PROPERTY_VALUE_BAD \ > "{ 'class': 'PropertyValueBad', 'data': { 'device': %s, 'property'= : %s, 'value': %s } }" > =20 --=20 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=FCrnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imend=F6rffer; HRB 16746 AG N=FCrnbe= rg