* [Qemu-devel] [PATCH] qerror: Improve QERR_DEVICE_NOT_ACTIVE message
@ 2014-02-25 7:10 Hani Benhabiles
2014-02-25 8:41 ` Markus Armbruster
0 siblings, 1 reply; 4+ messages in thread
From: Hani Benhabiles @ 2014-02-25 7:10 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-trivial, kwolf, peter.crosthwaite, armbru, lcapitulino
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 <hani@linux.com>
Suggested-by: Markus Armbruster <armbru@redhat.com>
---
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;
}
--
1.8.3.2
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [Qemu-devel] [PATCH] qerror: Improve QERR_DEVICE_NOT_ACTIVE message
2014-02-25 7:10 [Qemu-devel] [PATCH] qerror: Improve QERR_DEVICE_NOT_ACTIVE message Hani Benhabiles
@ 2014-02-25 8:41 ` Markus Armbruster
2014-02-25 8:42 ` Hani Benhabiles
0 siblings, 1 reply; 4+ messages in thread
From: Markus Armbruster @ 2014-02-25 8:41 UTC (permalink / raw)
To: Hani Benhabiles
Cc: qemu-trivial, kwolf, peter.crosthwaite, qemu-devel, lcapitulino
Hani Benhabiles <kroosec@gmail.com> 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 <hani@linux.com>
> Suggested-by: Markus Armbruster <armbru@redhat.com>
> ---
> 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 <armbru@redhat.com>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [Qemu-devel] [PATCH] qerror: Improve QERR_DEVICE_NOT_ACTIVE message
2014-02-25 8:41 ` Markus Armbruster
@ 2014-02-25 8:42 ` Hani Benhabiles
2014-02-28 18:40 ` Luiz Capitulino
0 siblings, 1 reply; 4+ messages in thread
From: Hani Benhabiles @ 2014-02-25 8:42 UTC (permalink / raw)
To: Markus Armbruster
Cc: qemu-trivial, kwolf, peter.crosthwaite, qemu-devel, lcapitulino
On Tue, Feb 25, 2014 at 09:41:02AM +0100, Markus Armbruster wrote:
> Hani Benhabiles <kroosec@gmail.com> 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 <hani@linux.com>
> > Suggested-by: Markus Armbruster <armbru@redhat.com>
> > ---
> > 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 <armbru@redhat.com>
Sure, no problem with dropping them. Thanks for the review, Markus!
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [Qemu-devel] [PATCH] qerror: Improve QERR_DEVICE_NOT_ACTIVE message
2014-02-25 8:42 ` Hani Benhabiles
@ 2014-02-28 18:40 ` Luiz Capitulino
0 siblings, 0 replies; 4+ messages in thread
From: Luiz Capitulino @ 2014-02-28 18:40 UTC (permalink / raw)
To: Hani Benhabiles
Cc: qemu-trivial, kwolf, peter.crosthwaite, Markus Armbruster,
qemu-devel
On Tue, 25 Feb 2014 09:42:43 +0100
Hani Benhabiles <kroosec@gmail.com> wrote:
> On Tue, Feb 25, 2014 at 09:41:02AM +0100, Markus Armbruster wrote:
> > Hani Benhabiles <kroosec@gmail.com> 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 <hani@linux.com>
> > > Suggested-by: Markus Armbruster <armbru@redhat.com>
> > > ---
> > > 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 <armbru@redhat.com>
>
> Sure, no problem with dropping them. Thanks for the review, Markus!
Done.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2014-02-28 18:47 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-02-25 7:10 [Qemu-devel] [PATCH] qerror: Improve QERR_DEVICE_NOT_ACTIVE message Hani Benhabiles
2014-02-25 8:41 ` Markus Armbruster
2014-02-25 8:42 ` Hani Benhabiles
2014-02-28 18:40 ` Luiz Capitulino
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).