From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:59895) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TLGn7-0005uv-Hx for qemu-devel@nongnu.org; Mon, 08 Oct 2012 13:06:55 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TLGn6-0004du-D4 for qemu-devel@nongnu.org; Mon, 08 Oct 2012 13:06:53 -0400 Received: from e23smtp02.au.ibm.com ([202.81.31.144]:48792) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TLGn5-0004dS-Rf for qemu-devel@nongnu.org; Mon, 08 Oct 2012 13:06:52 -0400 Received: from /spool/local by e23smtp02.au.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Tue, 9 Oct 2012 03:04:27 +1000 Received: from d23av03.au.ibm.com (d23av03.au.ibm.com [9.190.234.97]) by d23relay05.au.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id q98Gudi654853874 for ; Tue, 9 Oct 2012 03:56:39 +1100 Received: from d23av03.au.ibm.com (loopback [127.0.0.1]) by d23av03.au.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id q98H6QpN030355 for ; Tue, 9 Oct 2012 04:06:26 +1100 From: Anthony Liguori In-Reply-To: References: <1347026144-15098-1-git-send-email-peter.maydell@linaro.org> <87haq5rt28.fsf@codemonkey.ws> Date: Mon, 08 Oct 2012 12:06:16 -0500 Message-ID: <87mwzwrj07.fsf@codemonkey.ws> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Subject: Re: [Qemu-devel] [PATCH v2] qom: Reject attempts to add a property that already exists List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Maydell Cc: Paolo Bonzini , patches@linaro.org, qemu-devel@nongnu.org, Markus Armbruster Peter Maydell writes: > On 8 October 2012 14:29, Anthony Liguori wrote: >> This is wrong. >> >> Container properties are added by the user. You will turn a gracefully >> failure (during hotplug) into an abort(). > > No, it's turning a bug into an abort -- we don't handle trying to > create two identically named properties correctly today. Killing a guest because of something a user mistypes is not very friendly. > >> Please limit this to static properties as they are not added by a user. > > Adding two dynamic properties of the same name is also not > going to work and we need to do something with it... Raise an error. > What is the code path for properties being added by a user? qdev_device_add(). > If it's qdev_device_add() then that code presumably doesn't > care about graceful failures because it passes NULL as an > error pointer. Then we should handle the error there gracefully. > container_get() seems to assume that adding the > child property will always succeed and will not do the right > thing if there already exists a child property of the relevant > name but wrong type. > > Basically it seems to me that any code which might actually > be hit by this assert() rather needs examination and rewriting > to handle the error case anyway... There are only two cases that actually matter today: 1) static properties 2) qdev_device_add(). Yes, (2) is not doign error checking today. It should. I would be very happy with an abort() in (1) since that's clearly a programming bug. Regards, Anthony Liguori > > -- PMM