hmp: Allow for error message hints on HMP

Commits 7216ae3d and d2828429 disabled some error message hints,
all because a change to use modern error reporting meant that the
hint would be output prior to the actual error.  Fix this by making
hints a first-class member of Error.

For example, we are now back to the pleasant:

 $ qemu-system-x86_64 --nodefaults -S --vnc :0 --chardev null,id=,
 qemu-system-x86_64: --chardev null,id=,: Parameter 'id' expects an identifier
 Identifiers consist of letters, digits, '-', '.', '_', starting with a letter.

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Message-Id: <1441901956-21991-1-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
This commit is contained in:
Eric Blake 2015-09-10 10:19:16 -06:00 committed by Markus Armbruster
parent 615cf669b5
commit 50b7b000c9
4 changed files with 57 additions and 35 deletions

View File

@ -155,6 +155,13 @@ void error_setg_win32_internal(Error **errp,
*/ */
void error_propagate(Error **dst_errp, Error *local_err); void error_propagate(Error **dst_errp, Error *local_err);
/**
* Append a printf-style human-readable explanation to an existing error.
* May be called multiple times, and safe if @errp is NULL.
*/
void error_append_hint(Error **errp, const char *fmt, ...)
GCC_FMT_ATTR(2, 3);
/* /*
* Convenience function to report open() failure. * Convenience function to report open() failure.
*/ */

View File

@ -289,37 +289,35 @@ static Object *qdev_get_peripheral_anon(void)
return dev; return dev;
} }
#if 0 /* conversion from qerror_report() to error_set() broke their use */ static void qbus_list_bus(DeviceState *dev, Error **errp)
static void qbus_list_bus(DeviceState *dev)
{ {
BusState *child; BusState *child;
const char *sep = " "; const char *sep = " ";
error_printf("child buses at \"%s\":", error_append_hint(errp, "child buses at \"%s\":",
dev->id ? dev->id : object_get_typename(OBJECT(dev))); dev->id ? dev->id : object_get_typename(OBJECT(dev)));
QLIST_FOREACH(child, &dev->child_bus, sibling) { QLIST_FOREACH(child, &dev->child_bus, sibling) {
error_printf("%s\"%s\"", sep, child->name); error_append_hint(errp, "%s\"%s\"", sep, child->name);
sep = ", "; sep = ", ";
} }
error_printf("\n");
} }
static void qbus_list_dev(BusState *bus) static void qbus_list_dev(BusState *bus, Error **errp)
{ {
BusChild *kid; BusChild *kid;
const char *sep = " "; const char *sep = " ";
error_printf("devices at \"%s\":", bus->name); error_append_hint(errp, "devices at \"%s\":", bus->name);
QTAILQ_FOREACH(kid, &bus->children, sibling) { QTAILQ_FOREACH(kid, &bus->children, sibling) {
DeviceState *dev = kid->child; DeviceState *dev = kid->child;
error_printf("%s\"%s\"", sep, object_get_typename(OBJECT(dev))); error_append_hint(errp, "%s\"%s\"", sep,
if (dev->id) object_get_typename(OBJECT(dev)));
error_printf("/\"%s\"", dev->id); if (dev->id) {
error_append_hint(errp, "/\"%s\"", dev->id);
}
sep = ", "; sep = ", ";
} }
error_printf("\n");
} }
#endif
static BusState *qbus_find_bus(DeviceState *dev, char *elem) static BusState *qbus_find_bus(DeviceState *dev, char *elem)
{ {
@ -461,11 +459,7 @@ static BusState *qbus_find(const char *path, Error **errp)
if (!dev) { if (!dev) {
error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND, error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND,
"Device '%s' not found", elem); "Device '%s' not found", elem);
#if 0 /* conversion from qerror_report() to error_set() broke this: */ qbus_list_dev(bus, errp);
if (!monitor_cur_is_qmp()) {
qbus_list_dev(bus);
}
#endif
return NULL; return NULL;
} }
@ -483,11 +477,7 @@ static BusState *qbus_find(const char *path, Error **errp)
if (dev->num_child_bus) { if (dev->num_child_bus) {
error_setg(errp, "Device '%s' has multiple child buses", error_setg(errp, "Device '%s' has multiple child buses",
elem); elem);
#if 0 /* conversion from qerror_report() to error_set() broke this: */ qbus_list_bus(dev, errp);
if (!monitor_cur_is_qmp()) {
qbus_list_bus(dev);
}
#endif
} else { } else {
error_setg(errp, "Device '%s' has no child bus", elem); error_setg(errp, "Device '%s' has no child bus", elem);
} }
@ -503,11 +493,7 @@ static BusState *qbus_find(const char *path, Error **errp)
bus = qbus_find_bus(dev, elem); bus = qbus_find_bus(dev, elem);
if (!bus) { if (!bus) {
error_setg(errp, "Bus '%s' not found", elem); error_setg(errp, "Bus '%s' not found", elem);
#if 0 /* conversion from qerror_report() to error_set() broke this: */ qbus_list_bus(dev, errp);
if (!monitor_cur_is_qmp()) {
qbus_list_bus(dev);
}
#endif
return NULL; return NULL;
} }
} }

View File

@ -20,6 +20,7 @@ struct Error
ErrorClass err_class; ErrorClass err_class;
const char *src, *func; const char *src, *func;
int line; int line;
GString *hint;
}; };
Error *error_abort; Error *error_abort;
@ -115,6 +116,28 @@ void error_setg_file_open_internal(Error **errp,
"Could not open '%s'", filename); "Could not open '%s'", filename);
} }
void error_append_hint(Error **errp, const char *fmt, ...)
{
va_list ap;
int saved_errno = errno;
Error *err;
if (!errp) {
return;
}
err = *errp;
assert(err && errp != &error_abort);
if (!err->hint) {
err->hint = g_string_new(NULL);
}
va_start(ap, fmt);
g_string_append_vprintf(err->hint, fmt, ap);
va_end(ap);
errno = saved_errno;
}
#ifdef _WIN32 #ifdef _WIN32
void error_setg_win32_internal(Error **errp, void error_setg_win32_internal(Error **errp,
@ -151,6 +174,9 @@ Error *error_copy(const Error *err)
err_new = g_malloc0(sizeof(*err)); err_new = g_malloc0(sizeof(*err));
err_new->msg = g_strdup(err->msg); err_new->msg = g_strdup(err->msg);
err_new->err_class = err->err_class; err_new->err_class = err->err_class;
if (err->hint) {
err_new->hint = g_string_new(err->hint->str);
}
return err_new; return err_new;
} }
@ -168,6 +194,9 @@ const char *error_get_pretty(Error *err)
void error_report_err(Error *err) void error_report_err(Error *err)
{ {
error_report("%s", error_get_pretty(err)); error_report("%s", error_get_pretty(err));
if (err->hint) {
error_printf_unless_qmp("%s\n", err->hint->str);
}
error_free(err); error_free(err);
} }
@ -175,6 +204,9 @@ void error_free(Error *err)
{ {
if (err) { if (err) {
g_free(err->msg); g_free(err->msg);
if (err->hint) {
g_string_free(err->hint, true);
}
g_free(err); g_free(err);
} }
} }

View File

@ -205,10 +205,8 @@ void parse_option_size(const char *name, const char *value,
break; break;
default: default:
error_setg(errp, QERR_INVALID_PARAMETER_VALUE, name, "a size"); error_setg(errp, QERR_INVALID_PARAMETER_VALUE, name, "a size");
#if 0 /* conversion from qerror_report() to error_set() broke this: */ error_append_hint(errp, "You may use k, M, G or T suffixes for "
error_printf_unless_qmp("You may use k, M, G or T suffixes for " "kilobytes, megabytes, gigabytes and terabytes.");
"kilobytes, megabytes, gigabytes and terabytes.\n");
#endif
return; return;
} }
} else { } else {
@ -648,9 +646,8 @@ QemuOpts *qemu_opts_create(QemuOptsList *list, const char *id,
if (!id_wellformed(id)) { if (!id_wellformed(id)) {
error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "id", error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "id",
"an identifier"); "an identifier");
#if 0 /* conversion from qerror_report() to error_set() broke this: */ error_append_hint(errp, "Identifiers consist of letters, digits, "
error_printf_unless_qmp("Identifiers consist of letters, digits, '-', '.', '_', starting with a letter.\n"); "'-', '.', '_', starting with a letter.");
#endif
return NULL; return NULL;
} }
opts = qemu_opts_find(list, id); opts = qemu_opts_find(list, id);