Marc-André Lureau <marcandre.lur...@redhat.com> writes:
> This slightly change the error report message from "Invalid event
> name" to "Invalid parameter".
Actually from
qemu-system-x86_64: LOCATION: Invalid event name "VALUE"
to
qemu-system-x86_64: LOCATION: invalid parameter value: VALUE
Let's get that exactly right in the commit message.
Slight degradation, but the message is sub-par even before the patch.
When complaining about a parameter value, both parameter name and value
should be mentioned, as the value may well not be unique. Not this
patch's problem.
Aside: LOCATION points to where the blkdebug driver is configured. It
should point to the error's actual location instead: the configuration
file named with config=... Also not this patch's problem.
>
> Signed-off-by: Marc-André Lureau <marcandre.lur...@redhat.com>
> ---
> block/blkdebug.c | 28 ++++++++--------------------
> 1 file changed, 8 insertions(+), 20 deletions(-)
>
> diff --git a/block/blkdebug.c b/block/blkdebug.c
> index c19ab28f07..50edda2a31 100644
> --- a/block/blkdebug.c
> +++ b/block/blkdebug.c
> @@ -32,6 +32,7 @@
> #include "qapi/qmp/qbool.h"
> #include "qapi/qmp/qdict.h"
> #include "qapi/qmp/qstring.h"
> +#include "qapi/util.h"
> #include "sysemu/qtest.h"
>
> typedef struct BDRVBlkdebugState {
> @@ -149,20 +150,6 @@ static QemuOptsList *config_groups[] = {
> NULL
> };
>
> -static int get_event_by_name(const char *name, BlkdebugEvent *event)
> -{
> - int i;
> -
> - for (i = 0; i < BLKDBG__MAX; i++) {
> - if (!strcmp(BlkdebugEvent_lookup[i], name)) {
> - *event = i;
> - return 0;
> - }
> - }
> -
> - return -1;
> -}
> -
> struct add_rule_data {
> BDRVBlkdebugState *s;
> int action;
> @@ -173,7 +160,7 @@ static int add_rule(void *opaque, QemuOpts *opts, Error
> **errp)
> struct add_rule_data *d = opaque;
> BDRVBlkdebugState *s = d->s;
> const char* event_name;
> - BlkdebugEvent event;
> + int event;
> struct BlkdebugRule *rule;
> int64_t sector;
>
> @@ -182,8 +169,9 @@ static int add_rule(void *opaque, QemuOpts *opts, Error
> **errp)
> if (!event_name) {
> error_setg(errp, "Missing event name for rule");
> return -1;
> - } else if (get_event_by_name(event_name, &event) < 0) {
> - error_setg(errp, "Invalid event name \"%s\"", event_name);
> + }
> + event = qapi_enum_parse(BlkdebugEvent_lookup, event_name, BLKDBG__MAX,
> -1, errp);
Long line.
> + if (event < 0) {
> return -1;
> }
>
> @@ -743,13 +731,13 @@ static int blkdebug_debug_breakpoint(BlockDriverState
> *bs, const char *event,
> {
> BDRVBlkdebugState *s = bs->opaque;
> struct BlkdebugRule *rule;
> - BlkdebugEvent blkdebug_event;
> + int blkdebug_event;
>
> - if (get_event_by_name(event, &blkdebug_event) < 0) {
> + blkdebug_event = qapi_enum_parse(BlkdebugEvent_lookup, event,
> BLKDBG__MAX, -1, NULL);
Long line.
> + if (blkdebug_event < 0) {
> return -ENOENT;
> }
>
> -
> rule = g_malloc(sizeof(*rule));
> *rule = (struct BlkdebugRule) {
> .event = blkdebug_event,
With the nits touched up:
Reviewed-by: Markus Armbruster <arm...@redhat.com>
