[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-trivial] [Qemu-devel] [PATCH] qerror: Improve QERR_DEVICE_NOT_
From: |
Luiz Capitulino |
Subject: |
Re: [Qemu-trivial] [Qemu-devel] [PATCH] qerror: Improve QERR_DEVICE_NOT_ACTIVE message |
Date: |
Fri, 28 Feb 2014 13:40:15 -0500 |
On Tue, 25 Feb 2014 09:42:43 +0100
Hani Benhabiles <address@hidden> wrote:
> On Tue, Feb 25, 2014 at 09:41:02AM +0100, Markus Armbruster wrote:
> > 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>
>
> Sure, no problem with dropping them. Thanks for the review, Markus!
Done.