From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1NAmvn-0006w7-5L for qemu-devel@nongnu.org; Wed, 18 Nov 2009 10:58:55 -0500 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1NAmvi-0006u8-5g for qemu-devel@nongnu.org; Wed, 18 Nov 2009 10:58:54 -0500 Received: from [199.232.76.173] (port=42208 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1NAmvh-0006tr-R0 for qemu-devel@nongnu.org; Wed, 18 Nov 2009 10:58:49 -0500 Received: from e3.ny.us.ibm.com ([32.97.182.143]:38190) by monty-python.gnu.org with esmtps (TLS-1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.60) (envelope-from ) id 1NAmvg-0008I1-UG for qemu-devel@nongnu.org; Wed, 18 Nov 2009 10:58:49 -0500 Received: from d01relay06.pok.ibm.com (d01relay06.pok.ibm.com [9.56.227.116]) by e3.ny.us.ibm.com (8.14.3/8.13.1) with ESMTP id nAIFnxi7014560 for ; Wed, 18 Nov 2009 10:49:59 -0500 Received: from d01av02.pok.ibm.com (d01av02.pok.ibm.com [9.56.224.216]) by d01relay06.pok.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id nAIFwXqi1327342 for ; Wed, 18 Nov 2009 10:58:33 -0500 Received: from d01av02.pok.ibm.com (loopback [127.0.0.1]) by d01av02.pok.ibm.com (8.14.3/8.13.1/NCO v10.0 AVout) with ESMTP id nAIBpskO004472 for ; Wed, 18 Nov 2009 06:51:54 -0500 Message-ID: <4B0419A6.7010901@linux.vnet.ibm.com> Date: Wed, 18 Nov 2009 09:58:30 -0600 From: Anthony Liguori MIME-Version: 1.0 Subject: Re: [Qemu-devel] [PATCH 10/10] monitor: do_info_balloon(): use QError References: <1258487037-24950-1-git-send-email-lcapitulino@redhat.com> <1258487037-24950-11-git-send-email-lcapitulino@redhat.com> In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: kraxel@redhat.com, qemu-devel@nongnu.org, Luiz Capitulino Markus Armbruster wrote: > Luiz Capitulino writes: > > >> Signed-off-by: Luiz Capitulino >> --- >> monitor.c | 7 ++++--- >> 1 files changed, 4 insertions(+), 3 deletions(-) >> >> diff --git a/monitor.c b/monitor.c >> index 74abef9..e42434f 100644 >> --- a/monitor.c >> +++ b/monitor.c >> @@ -1722,10 +1722,11 @@ static void do_info_balloon(Monitor *mon, QObject **ret_data) >> >> actual = qemu_balloon_status(); >> if (kvm_enabled() && !kvm_has_sync_mmu()) >> - monitor_printf(mon, "Using KVM without synchronous MMU, " >> - "ballooning disabled\n"); >> + qemu_error_new(QERR_SERVICE_UNAVAILABLE, >> + "Using KVM without synchronous MMU, ballooning disabled"); >> else if (actual == 0) >> - monitor_printf(mon, "Ballooning not activated in VM\n"); >> + qemu_error_new(QERR_SERVICE_UNAVAILABLE, >> + "Ballooning not activated in VM"); >> else >> *ret_data = QOBJECT(qint_from_int((int)(actual >> 20))); >> } >> > > In PATCH 7/10: > > +#define QERR_SERVICE_UNAVAILABLE \ > + "{ 'class': 'ServiceUnavailable', 'data': { 'reason': %s } }" > + > > and > > + { > + .error_fmt = QERR_SERVICE_UNAVAILABLE, > + .desc = "%(reason)", > + }, > > How to do a ServiceUnavailable error with a description that is not a > compile time literal? Add another macro and error table entry for that? > An error that just contains reason is a good indication that the error is not the right level of abstraction. There are two error conditions here. One is that kvm is in use, but it's missing a capability and therefore we have to disable a feature. The second error is that the guest did not activate a device. So the first error should be something like #define QERR_KVM_MISSING_CAP \ "{'class': 'KVMMissingCap', 'data' : { 'capability': %s, 'feature': %s' }" Where capability is the string form of the missing cap and feature describes the feature being disabled because of the missing cap. The error format would be "Using KVM without %{capability}, %{feature} unavailable". It would get raised with qemu_error_new(QERR_KVM_MISSING_CAP, kvm_get_cap_desc(KVM_CAP_SYNC_MMU), "balloon"); The error text is slightly modified, but that's okay. Likewise, the second error should be something like #define QERR_DEVICE_NOT_ACTIVE \ "{'class': 'DeviceNotActive', 'data' : { 'device': %s }" qemu_error_new(QERR_DEVICE_NOT_ACTIVE, "balloon"); With a description of "The %{device} device has not been activated by the guest" If I was writing a UI, I would respond very differently to these two errors. For the first error, I would grey out the balloon feature because it's not possible to use ballooning with this very of KVM. For the second error, I would prompt the user when they tried to do ballooning to make sure the driver was loaded in the guest. This behavior can not be achieved the ServiceNotAvailable. -- Regards, Anthony Liguori