From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:38165) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RsN34-000253-ID for qemu-devel@nongnu.org; Tue, 31 Jan 2012 18:23:39 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1RsN33-0008KN-1X for qemu-devel@nongnu.org; Tue, 31 Jan 2012 18:23:38 -0500 Received: from e6.ny.us.ibm.com ([32.97.182.146]:60573) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RsN32-0008KE-P1 for qemu-devel@nongnu.org; Tue, 31 Jan 2012 18:23:36 -0500 Received: from /spool/local by e6.ny.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Tue, 31 Jan 2012 18:23:33 -0500 Received: from d01relay04.pok.ibm.com (d01relay04.pok.ibm.com [9.56.227.236]) by d01dlp02.pok.ibm.com (Postfix) with ESMTP id 791D46E804C for ; Tue, 31 Jan 2012 18:23:06 -0500 (EST) Received: from d03av03.boulder.ibm.com (d03av03.boulder.ibm.com [9.17.195.169]) by d01relay04.pok.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id q0VNN6un281314 for ; Tue, 31 Jan 2012 18:23:06 -0500 Received: from d03av03.boulder.ibm.com (loopback [127.0.0.1]) by d03av03.boulder.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id q0VNN4XQ010764 for ; Tue, 31 Jan 2012 16:23:05 -0700 Message-ID: <4F2877D7.4020807@us.ibm.com> Date: Tue, 31 Jan 2012 17:23:03 -0600 From: Anthony Liguori MIME-Version: 1.0 References: <4F2853EA.3030808@codemonkey.ws> <4F286463.4070807@web.de> <4F286DD1.1050406@us.ibm.com> <4F286FA7.7030606@web.de> In-Reply-To: <4F286FA7.7030606@web.de> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v3 6/7] pcspk: Convert to qdev List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jan Kiszka Cc: Blue Swirl , Marcelo Tosatti , qemu-devel , kvm , Avi Kivity On 01/31/2012 04:48 PM, Jan Kiszka wrote: > On 2012-01-31 23:40, Anthony Liguori wrote: >> Why is what's in the tree not usable? >> >> Just don't do pcspk_init as a static inline (which is not that nice to >> do anyway) and you don't need to worry about the availability of an >> accessor. > > The current pattern requested by some reviewers used to be the one I > applied here. I dislike it as well when the device can't be seriously > configured out. But I can switch over, no problem. > >> >> And BTW, there is strict type checking now, which makes it already an >> improvement over property pointers. > > OK, for my slow understanding: I use qdev_property_add_link in the > device init function to create the property, letting it point to a state > field. But what should I call from the outside to actually set its > value? You have a few options: 1) you can add a generic set_child function to qdev... but don't do this, I'll fix it right in my future series 2) You can take advantage of the fact that the only thing that ever sets this is pcspk_init, move pcspk_init to pcspk.c, and then since you have access to the state structure, directly assign it there. 3) Introduce a pcspk_set_pit() function that just does the same thing as (2). 4) Introduce (1) and then have a pcspk_set_pit() that just calls (1). This is what I think we should do moving forward and I plan to do this in my future refactorings. That all said, I think this isn't going to work for you until my next series is merged. Right now, the link properties have strict type checking. You really need a link in order to be able to accept either a KVMPIT or a normal PIT. My series on the list relaxes the type checking to use implements instead of a strict equality check. So maybe the best thing to do is drop the ptr type entirely, make pcspk_init() be in pcspk.c and touch the private state, and then I'll refactor it later to use a link<> property. > And what C type does this value have? PITState *. Regards, Anthony Liguroi A DeviceState * or a char * > (path)? > > Jan