From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1N3rEP-00028A-JQ for qemu-devel@nongnu.org; Fri, 30 Oct 2009 09:09:29 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1N3rEK-00025D-6S for qemu-devel@nongnu.org; Fri, 30 Oct 2009 09:09:28 -0400 Received: from [199.232.76.173] (port=41506 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1N3rEK-000252-1E for qemu-devel@nongnu.org; Fri, 30 Oct 2009 09:09:24 -0400 Received: from e34.co.us.ibm.com ([32.97.110.152]:57771) by monty-python.gnu.org with esmtps (TLS-1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.60) (envelope-from ) id 1N3rEJ-0000Zm-L1 for qemu-devel@nongnu.org; Fri, 30 Oct 2009 09:09:23 -0400 Received: from d03relay03.boulder.ibm.com (d03relay03.boulder.ibm.com [9.17.195.228]) by e34.co.us.ibm.com (8.14.3/8.13.1) with ESMTP id n9UD4R6f027214 for ; Fri, 30 Oct 2009 07:04:27 -0600 Received: from d03av02.boulder.ibm.com (d03av02.boulder.ibm.com [9.17.195.168]) by d03relay03.boulder.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id n9UD94oX153638 for ; Fri, 30 Oct 2009 07:09:11 -0600 Received: from d03av02.boulder.ibm.com (loopback [127.0.0.1]) by d03av02.boulder.ibm.com (8.14.3/8.13.1/NCO v10.0 AVout) with ESMTP id n9UD71m6003605 for ; Fri, 30 Oct 2009 07:07:02 -0600 Message-ID: <4AEAE56E.8040309@us.ibm.com> Date: Fri, 30 Oct 2009 08:09:02 -0500 From: Anthony Liguori MIME-Version: 1.0 References: <1256841750-15228-1-git-send-email-lcapitulino@redhat.com> <4AEA133A.8010906@redhat.com> <20091030102809.1c520282@doriath> <4AEAE261.5030908@redhat.com> In-Reply-To: <4AEAE261.5030908@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: [Qemu-devel] Re: [RFC 0/7] QError v1 List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Gerd Hoffmann Cc: Paolo Bonzini , hollisb@linux.vnet.ibm.com, qemu-devel@nongnu.org, Luiz Capitulino Gerd Hoffmann wrote: > On 10/30/09 13:28, Luiz Capitulino wrote: >>>> - qemu_error("Device \"%s\" not found. Try -device '?' for >>>> a list.\n", >>>> - driver); >>>> + qemu_error_structed(QERR_DEV_NFOUND, "{ 'name': %s }", >>>> driver); >>> >>> why not store the "{ 'name': %s }" in the qerror_table? I guess you >>> plan to have different fields in some cases? >> >> The main reason is to have syntax checking, we can declare it in a >> macro though, in case of generic errors which are going to be used in >> other places. > > I still feel the error reporting is too complex. IMHO there should be > no need to edit two places for error reporting, which means I'd go the > opposite direction: Zap qerror_table[], then have: > > qemu_error_structed(QERR_DEV_NFOUND, "device %{name}s not found", > "{ 'name': %s }", driver); > > Also I think the error codes should be more generic, so you don't need > a new one for each and every error. Ideally we'll have a reasonable > and stable set of error codes after the initial conversion, so you > don't have to touch the management apps just to add new codes as qemu > envolves. The error code must help the management app to decide how > to deal with the error, but it shouldn't carry details not needed for > that. Okay, let's get more clever then and do: #define QERR_DEV_NFOUND "{ 'code': 404, 'name': %s}" So we can do: qemu_error_structured(QERR_DEV_NFOUND, driver); Such that we still get printf style parameter checking. > > Picking the balloon errors (other patch in this thread): You have > *two* error codes for ballooning not being available. I think a > generic "service not available" error code would work for both (and > for other error cases too) and would be good enougth. The management > app will figure it can't balloon down the VM. It will not know the > reason from the error code, but does it have to? I doubt it will > react in a different way. And for manual trouble-shooting the text > message which carries more information gets logged. I think the trouble is Luiz is trying to preserve today's error messages. Honestly, if we need to break those, I don't mind so much because I really doubt anyone is depending on the exact text of the error messages. I agree that a bit more generic error messages wouldn't be a bad thing. -- Regards, Anthony Liguori