From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53811) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WJSSw-00052r-Ms for qemu-devel@nongnu.org; Fri, 28 Feb 2014 13:47:27 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WJSSp-0001Ev-OP for qemu-devel@nongnu.org; Fri, 28 Feb 2014 13:47:22 -0500 Date: Fri, 28 Feb 2014 13:40:15 -0500 From: Luiz Capitulino Message-ID: <20140228134015.048e6d48@redhat.com> In-Reply-To: <20140225084243.GA3806@Inspiron-3521> References: <1393312205-7858-1-git-send-email-kroosec@gmail.com> <87txbnshy9.fsf@blackfin.pond.sub.org> <20140225084243.GA3806@Inspiron-3521> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] qerror: Improve QERR_DEVICE_NOT_ACTIVE message List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Hani Benhabiles Cc: qemu-trivial@nongnu.org, kwolf@redhat.com, peter.crosthwaite@xilinx.com, Markus Armbruster , qemu-devel@nongnu.org On Tue, 25 Feb 2014 09:42:43 +0100 Hani Benhabiles wrote: > On Tue, Feb 25, 2014 at 09:41:02AM +0100, Markus Armbruster wrote: > > Hani Benhabiles 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 > > > Suggested-by: Markus Armbruster > > > --- > > > 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 > > Sure, no problem with dropping them. Thanks for the review, Markus! Done.