From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43648) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1V3ENh-0003vC-TW for qemu-devel@nongnu.org; Sat, 27 Jul 2013 19:58:45 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1V3ENX-00075S-Dm for qemu-devel@nongnu.org; Sat, 27 Jul 2013 19:58:37 -0400 Received: from cantor2.suse.de ([195.135.220.15]:47940 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1V3ENX-00075J-4S for qemu-devel@nongnu.org; Sat, 27 Jul 2013 19:58:27 -0400 Message-ID: <51F45E9B.40704@suse.de> Date: Sun, 28 Jul 2013 01:58:19 +0200 From: =?ISO-8859-1?Q?Andreas_F=E4rber?= MIME-Version: 1.0 References: <1374681580-17439-1-git-send-email-mst@redhat.com> <1374681580-17439-13-git-send-email-mst@redhat.com> <51F0FE20.7050302@redhat.com> <20130725105526.GA28819@redhat.com> <51F10668.5020100@redhat.com> <20130725112247.GA28893@redhat.com> <51F11415.6090906@redhat.com> <20130725122359.GA32751@redhat.com> In-Reply-To: <20130725122359.GA32751@redhat.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v3 12/14] pvpanic: add API to access io port List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Michael S. Tsirkin" Cc: Gerd Hoffmann , Anthony Liguori , qemu-devel@nongnu.org Am 25.07.2013 14:23, schrieb Michael S. Tsirkin: > On Thu, Jul 25, 2013 at 02:03:33PM +0200, Gerd Hoffmann wrote: >> On 07/25/13 13:22, Michael S. Tsirkin wrote: >>> On Thu, Jul 25, 2013 at 01:05:12PM +0200, Gerd Hoffmann wrote: >>>>> I can change the implementation but I don't think it's >>>>> a good idea to copy property names around: >>>>> it's too fragile, compiler won't warn us if we >>>>> change the name or value semantics, >>>> >>>> I'm not worried. Changing the strings will break the command line >>>> interface too (qemu -device pvpanic,ioport=3D...), so that isn't goi= ng to >>>> happen. >>> >>> What will catch this breakage? >>> There are 0 users actually tweaking the port >>> number so I'm sure no one will notice this. >>> >>> In any case, catching errors at compile time >>> is much better than at runtime. >>> >>> What exactly are advantages of duplicating >>> property names in this way? I don't see any. >> >> You don't need access to pvpanic internals then and thus the code can = be >> moved over to the acpi generator. At least in this case where all inf= o >> needed is already available via properties. >=20 > We'll have to disagree here. > There's no access to internals with an API. > I prefer using APIs, since they are compiler-checked. Sorry, I don't understand, is there a typo? A qdev "ioport" property is hardly internal, and QOM has an API to access it. For QOM properties we have ABI stability rules in place, not only for the textual command line: No one is allowed to change the type of a property in an incompatible way. And dropping it would be a command-line-incompatible change in this case. Properties don't magically disappear at runtime, and since you're writing this code you can rely on the property being present and the person attempting to remove it noticing it while testing. You're writing redundant code here, and it's not your device IIRC. I vaguely remember seeing you or Xen guys adding some check that the pvpanic device can only be instantiated once, right? In that case I would consider it best to add an object_property_add_child(qdev_get_machine(), "pvpanic", foo, NULL); call into pvpanic device creation, then you can access it from ACPI code without needing to know the type via obj =3D object_path_resolve_component(qdev_get_machine(), "pvpanic"); if (obj !=3D NULL) { val =3D object_property_get_int(obj, "ioport", &err); assert_no_error(&err); } and pvpanic code remains untouched. Andreas --=20 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=FCrnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imend=F6rffer; HRB 16746 AG N=FCrnbe= rg