Re: [Qemu-devel] [PATCH v11 18/28] qerror: more error_setg() usage
Am 11.11.2015 um 07:51 schrieb Eric Blake: > A few uses of error_set(ERROR_CLASS_GENERIC_ERROR) have snuck in > since c6bd8c706. Nuke them. > > Signed-off-by: Eric Blake [...] > diff --git a/qom/object.c b/qom/object.c > index fc6e161..c0decb6 100644 > --- a/qom/object.c > +++ b/qom/object.c > @@ -1330,8 +1330,8 @@ static Object *object_resolve_link(Object *obj, const > char *name, > target = object_resolve_path_type(path, target_type, &ambiguous); > > if (ambiguous) { > -error_set(errp, ERROR_CLASS_GENERIC_ERROR, > - "Path '%s' does not uniquely identify an object", path); > +error_setg(errp, "Path '%s' does not uniquely identify an object", > + path); > } else if (!target) { > target = object_resolve_path(path, &ambiguous); > if (target || ambiguous) { Acked-by: Andreas Färber No idea why it's this way, maybe predated the function? Regards, Andreas -- SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Felix Imendörffer, Jane Smithard, Graham Norton; HRB 21284 (AG Nürnberg)
Re: [Qemu-devel] [PATCH v11 18/28] qerror: more error_setg() usage
Eric Blake writes: > A few uses of error_set(ERROR_CLASS_GENERIC_ERROR) have snuck in > since c6bd8c706. Nuke them. Doesn't really belong to this series, but that's okay. > Signed-off-by: Eric Blake > > --- > v11: new patch > --- > block.c | 3 +-- > docs/writing-qmp-commands.txt | 20 +--- > hw/i386/pc.c | 2 +- > hw/net/rocker/rocker.c| 6 ++ > hw/net/rocker/rocker_of_dpa.c | 12 > qom/object.c | 4 ++-- > 6 files changed, 19 insertions(+), 28 deletions(-) > > diff --git a/block.c b/block.c > index e9f40dc..53a978a 100644 > --- a/block.c > +++ b/block.c > @@ -1795,8 +1795,7 @@ int bdrv_reopen_prepare(BDRVReopenState *reopen_state, > BlockReopenQueue *queue, > > ret = bdrv_flush(reopen_state->bs); > if (ret) { > -error_set(errp, ERROR_CLASS_GENERIC_ERROR, "Error (%s) flushing > drive", > - strerror(-ret)); > +error_setg_errno(errp, -ret, "Error flushing drive"); > goto error; > } > > diff --git a/docs/writing-qmp-commands.txt b/docs/writing-qmp-commands.txt > index 8647cac..59aa77a 100644 > --- a/docs/writing-qmp-commands.txt > +++ b/docs/writing-qmp-commands.txt > @@ -210,7 +210,7 @@ if you don't see these strings, then something went wrong. > === Errors === > > QMP commands should use the error interface exported by the error.h header > -file. Basically, errors are set by calling the error_set() function. > +file. Basically, most errors are set by calling the error_setg() function. > > Let's say we don't accept the string "message" to contain the word "love". If > it does contain it, we want the "hello-world" command to return an error: > @@ -219,8 +219,7 @@ void qmp_hello_world(bool has_message, const char > *message, Error **errp) > { > if (has_message) { > if (strstr(message, "love")) { > -error_set(errp, ERROR_CLASS_GENERIC_ERROR, > - "the word 'love' is not allowed"); > +error_setg(errp, "the word 'love' is not allowed"); > return; > } > printf("%s\n", message); > @@ -229,10 +228,8 @@ void qmp_hello_world(bool has_message, const char > *message, Error **errp) > } > } > > -The first argument to the error_set() function is the Error pointer to > pointer, > -which is passed to all QMP functions. The second argument is a ErrorClass > -value, which should be ERROR_CLASS_GENERIC_ERROR most of the time (more > -details about error classes are given below). The third argument is a human > +The first argument to the error_setg() function is the Error pointer > +to pointer, which is passed to all QMP functions. The next argument is a > human > description of the error, this is a free-form printf-like string. > > Let's test the example above. Build qemu, run it as defined in the "Testing" > @@ -249,8 +246,9 @@ The QMP server's response should be: > } > } > > -As a general rule, all QMP errors should use ERROR_CLASS_GENERIC_ERROR. There > -are two exceptions to this rule: > +As a general rule, all QMP errors should use ERROR_CLASS_GENERIC_ERROR > +(done by default when using error_setg()). There are two exceptions to > +this rule: > > 1. A non-generic ErrorClass value exists* for the failure you want to report > (eg. DeviceNotFound) > @@ -259,8 +257,8 @@ are two exceptions to this rule: > want to report, hence you have to add a new ErrorClass value so that they > can check for it > > -If the failure you want to report doesn't fall in one of the two cases above, > -just report ERROR_CLASS_GENERIC_ERROR. > +If the failure you want to report falls into one of the two cases above, > +use error_set() with a second argument of an ErrorClass value. > > * All existing ErrorClass values are defined in the qapi-schema.json file > Thanks a lot for this doc update! > diff --git a/hw/i386/pc.c b/hw/i386/pc.c > index 0cb8afd..dfb57a8 100644 > --- a/hw/i386/pc.c > +++ b/hw/i386/pc.c > @@ -1795,7 +1795,7 @@ static void pc_machine_set_max_ram_below_4g(Object > *obj, Visitor *v, > return; > } > if (value > (1ULL << 32)) { > -error_set(&error, ERROR_CLASS_GENERIC_ERROR, > +error_setg(&error, >"Machine option 'max-ram-below-4g=%"PRIu64 >"' expects size less than or equal to 4G", value); Indentation is now off. Can tidy up in my tree. > error_propagate(errp, error); [Rest snipped, it looks good]
Re: [Qemu-devel] [PATCH v11 18/28] qerror: more error_setg() usage
Am 11.11.2015 um 15:21 schrieb Markus Armbruster: > Eric Blake writes: > >> A few uses of error_set(ERROR_CLASS_GENERIC_ERROR) have snuck in >> since c6bd8c706. Nuke them. > > Doesn't really belong to this series, but that's okay. It seemed to avoid a GENERIC_ERROR -> GENERICERROR change in the following patch, so it is related. Cheers, Andreas -- SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Felix Imendörffer, Jane Smithard, Graham Norton; HRB 21284 (AG Nürnberg)
Re: [Qemu-devel] [PATCH v11 18/28] qerror: more error_setg() usage
On 11/11/2015 07:23 AM, Andreas Färber wrote: > Am 11.11.2015 um 15:21 schrieb Markus Armbruster: >> Eric Blake writes: >> >>> A few uses of error_set(ERROR_CLASS_GENERIC_ERROR) have snuck in >>> since c6bd8c706. Nuke them. >> >> Doesn't really belong to this series, but that's okay. > > It seemed to avoid a GENERIC_ERROR -> GENERICERROR change in the > following patch, so it is related. Indeed, that's why it ended up here. But I'm also okay if we want to detach this one from the series and apply it now through the qerror tree for 2.5, as it includes a doc update and should be safe even during hard freeze. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH v11 18/28] qerror: more error_setg() usage
On 11/11/2015 07:21 AM, Markus Armbruster wrote: > Eric Blake writes: > >> A few uses of error_set(ERROR_CLASS_GENERIC_ERROR) have snuck in >> since c6bd8c706. Nuke them. > > Doesn't really belong to this series, but that's okay. If you're going to modify this for 2.5 inclusion through your qerror tree, you may want to change the description to: A few uses of error_set(ERROR_CLASS_GENERIC_ERROR) were missed in c6bd8c706, or have snuck in since. Nuke them. >> +++ b/hw/i386/pc.c >> @@ -1795,7 +1795,7 @@ static void pc_machine_set_max_ram_below_4g(Object >> *obj, Visitor *v, >> return; >> } >> if (value > (1ULL << 32)) { >> -error_set(&error, ERROR_CLASS_GENERIC_ERROR, >> +error_setg(&error, >>"Machine option 'max-ram-below-4g=%"PRIu64 >>"' expects size less than or equal to 4G", value); > > Indentation is now off. Can tidy up in my tree. > >> error_propagate(errp, error); > [Rest snipped, it looks good] > There's also the question if we want to address the ErrorClass name munging of 19/28 by adding your idea of an aliasing typedef in error.h; if so, should I prepare a smaller patch series of both of those changes for consideration for 2.5? -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH v11 18/28] qerror: more error_setg() usage
Eric Blake writes: > On 11/11/2015 07:21 AM, Markus Armbruster wrote: >> Eric Blake writes: >> >>> A few uses of error_set(ERROR_CLASS_GENERIC_ERROR) have snuck in >>> since c6bd8c706. Nuke them. >> >> Doesn't really belong to this series, but that's okay. > > If you're going to modify this for 2.5 inclusion through your qerror > tree, you may want to change the description to: I'll try to get this into 2.5 mostly for the documentation update. > A few uses of error_set(ERROR_CLASS_GENERIC_ERROR) were missed in > c6bd8c706, or have snuck in since. Nuke them. Done. >>> +++ b/hw/i386/pc.c >>> @@ -1795,7 +1795,7 @@ static void pc_machine_set_max_ram_below_4g(Object >>> *obj, Visitor *v, >>> return; >>> } >>> if (value > (1ULL << 32)) { >>> -error_set(&error, ERROR_CLASS_GENERIC_ERROR, >>> +error_setg(&error, >>>"Machine option 'max-ram-below-4g=%"PRIu64 >>>"' expects size less than or equal to 4G", value); >> >> Indentation is now off. Can tidy up in my tree. >> >>> error_propagate(errp, error); >> [Rest snipped, it looks good] >> > > There's also the question if we want to address the ErrorClass name > munging of 19/28 by adding your idea of an aliasing typedef in error.h; > if so, should I prepare a smaller patch series of both of those changes > for consideration for 2.5? Can safely wait for 2.6, can't it?
Re: [Qemu-devel] [PATCH v11 18/28] qerror: more error_setg() usage
On 11/11/2015 10:31 AM, Markus Armbruster wrote: > Eric Blake writes: > >> On 11/11/2015 07:21 AM, Markus Armbruster wrote: >>> Eric Blake writes: >>> A few uses of error_set(ERROR_CLASS_GENERIC_ERROR) have snuck in since c6bd8c706. Nuke them. >>> >>> Doesn't really belong to this series, but that's okay. >> >> If you're going to modify this for 2.5 inclusion through your qerror >> tree, you may want to change the description to: > > I'll try to get this into 2.5 mostly for the documentation update. > >> A few uses of error_set(ERROR_CLASS_GENERIC_ERROR) were missed in >> c6bd8c706, or have snuck in since. Nuke them. > > Done. Oh, and I missed your capitalization preferences in the title: qerror: More error_setg() usage >> There's also the question if we want to address the ErrorClass name >> munging of 19/28 by adding your idea of an aliasing typedef in error.h; >> if so, should I prepare a smaller patch series of both of those changes >> for consideration for 2.5? > > Can safely wait for 2.6, can't it? Yes. Just trying to make sure I'm focusing on 2.5 patches first. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature