From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:54032) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TQKpR-0005df-Ks for qemu-devel@nongnu.org; Mon, 22 Oct 2012 12:26:19 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TQKpP-00045Z-RB for qemu-devel@nongnu.org; Mon, 22 Oct 2012 12:26:13 -0400 Received: from mail-ie0-f173.google.com ([209.85.223.173]:56160) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TQKpP-00045K-MO for qemu-devel@nongnu.org; Mon, 22 Oct 2012 12:26:11 -0400 Received: by mail-ie0-f173.google.com with SMTP id 17so3928710iea.4 for ; Mon, 22 Oct 2012 09:26:11 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <20121022133520.5515bd9f@doriath.home> References: <1350667146-26273-1-git-send-email-peter.maydell@linaro.org> <20121022133520.5515bd9f@doriath.home> Date: Mon, 22 Oct 2012 17:26:10 +0100 Message-ID: From: Peter Maydell Content-Type: text/plain; charset=UTF-8 Subject: Re: [Qemu-devel] [PATCH 0/2] qom: detect attempts to add a property that already exists List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Luiz Capitulino Cc: Paolo Bonzini , Anthony Liguori , qemu-devel@nongnu.org, patches@linaro.org On 22 October 2012 16:35, Luiz Capitulino wrote: > Peter Maydell wrote: > >> The aim of this patch series is to make QEMU exit with a helpful >> error message for bugs where multiple properties of the same name >> are accidentally added to a QOM object. > > Does this happen only at build-time or can it happen at command-line > too? What about QMP/HMP? Anything that cares about not abort()ing needs to pass a valid Error** in. >> In order to achieve this >> for static properties whilst still allowing the hotplug case >> to gracefully fail without killing QEMU, we add the concept >> of a 'critical' error. A critical error is one which must be >> handled somehow -- if we encounter a NULL Error** either when >> the error is raised or later when it is propagated, we will >> abort() rather than throwing the error away. > > This gives me the impression that we're fixing it in the wrong layer. > Besides, all code calling error_propagate() today can now abort > (at least in theory), but that's something we really don't want to happen > in QMP. That's why QMP gets to pass in an Error ** and handle the result. I'm open to better ways to handle this. Perhaps all Errors should be critical? :-) Mostly what this patch is trying to do is deal with the fact that huge amounts of code using the Error interface just throws away the error. I'd rather not have lots of boilerplate at the device and board model level that's just sitting there asserting "this will always succeed". So instead we say "if you pass in a NULL Error ** then you are saying that you know this won't fail". > An alternative would be to let users set is_critical, but add a > error_is_critical() function and let the code that wants to abort > to check for it. > > But, how difficult it's to add a flag to QPM objects to allow/disallow > multiple properties? Why would you want to have multiple properties with the same name? It's not meaningful because property lookup is by name so there's no way to distinguish them. It's always an error, either in QEMU itself or an error by the user on the command line. -- PMM