[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] qerror: Improve QERR_DEVICE_NOT_ACTIVE message
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH] qerror: Improve QERR_DEVICE_NOT_ACTIVE message |
Date: |
Tue, 25 Feb 2014 09:41:02 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.2 (gnu/linux) |
Hani Benhabiles <address@hidden> writes:
> The error message as currently used is confusing as there are no "balloon" or
> "spice" devices.
>
> (qemu) balloon 1024
> balloon: Device 'balloon' has not been activated
>
> With this patch:
>
> (qemu) balloon 1024
> balloon: No balloon device has been activated
>
> Signed-off-by: Hani Benhabiles <address@hidden>
> Suggested-by: Markus Armbruster <address@hidden>
> ---
> include/qapi/qmp/qerror.h | 2 +-
> qmp.c | 2 --
> 2 files changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h
> index 73c67b7..25193c9 100644
> --- a/include/qapi/qmp/qerror.h
> +++ b/include/qapi/qmp/qerror.h
> @@ -105,7 +105,7 @@ void qerror_report_err(Error *err);
> ERROR_CLASS_GENERIC_ERROR, "Device '%s' does not support hotplugging"
>
> #define QERR_DEVICE_NOT_ACTIVE \
> - ERROR_CLASS_DEVICE_NOT_ACTIVE, "Device '%s' has not been activated"
> + ERROR_CLASS_DEVICE_NOT_ACTIVE, "No %s device has been activated"
>
> #define QERR_DEVICE_NOT_ENCRYPTED \
> ERROR_CLASS_GENERIC_ERROR, "Device '%s' is not encrypted"
> diff --git a/qmp.c b/qmp.c
> index d0d98e7..ffddd26 100644
> --- a/qmp.c
> +++ b/qmp.c
> @@ -289,7 +289,6 @@ void qmp_set_password(const char *protocol, const char
> *password,
>
> if (strcmp(protocol, "spice") == 0) {
> if (!using_spice) {
> - /* correct one? spice isn't a device ,,, */
> error_set(errp, QERR_DEVICE_NOT_ACTIVE, "spice");
> return;
> }
> @@ -337,7 +336,6 @@ void qmp_expire_password(const char *protocol, const char
> *whenstr,
>
> if (strcmp(protocol, "spice") == 0) {
> if (!using_spice) {
> - /* correct one? spice isn't a device ,,, */
> error_set(errp, QERR_DEVICE_NOT_ACTIVE, "spice");
> return;
> }
The wording of the error message is still suboptimal for the
!using_spice case, but it's an improvement. I'd leave the two comments
alone, though. Hani, if you agree, then Luiz could perhaps merge your
patch with these two hunks dropped.
Reviewed-by: Markus Armbruster <address@hidden>