From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60705) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WLxYD-0008KR-Ga for qemu-devel@nongnu.org; Fri, 07 Mar 2014 11:23:15 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WLxY7-00022f-0X for qemu-devel@nongnu.org; Fri, 07 Mar 2014 11:23:09 -0500 Received: from mx1.redhat.com ([209.132.183.28]:38061) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WLxY6-00022b-Og for qemu-devel@nongnu.org; Fri, 07 Mar 2014 11:23:02 -0500 Message-ID: <1394209338.2663.51.camel@localhost.localdomain> From: Marcel Apfelbaum In-Reply-To: <5319AD0C.9060800@suse.de> References: <1394040647-20083-1-git-send-email-marcel.a@redhat.com> <1394040647-20083-4-git-send-email-marcel.a@redhat.com> <5319087A.3090403@suse.de> <1394170376.2663.24.camel@localhost.localdomain> <5319AD0C.9060800@suse.de> Content-Type: text/plain; charset="UTF-8" Date: Fri, 07 Mar 2014 18:22:18 +0200 Mime-Version: 1.0 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v3 3/3] hw/boards: converted current_machine to be an instance of MachineCLass List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Andreas =?ISO-8859-1?Q?F=E4rber?= Cc: qemu-devel@nongnu.org, ehabkost@redhat.com, mst@redhat.com, Alexey Kardashevskiy , armbru@redhat.com, lcapitulino@redhat.com, blauwirbel@gmail.com, aliguori@amazon.com, imammedo@redhat.com, pbonzini@redhat.com On Fri, 2014-03-07 at 12:27 +0100, Andreas F=C3=A4rber wrote: > Am 07.03.2014 06:32, schrieb Marcel Apfelbaum: > > On Fri, 2014-03-07 at 00:44 +0100, Andreas F=C3=A4rber wrote: > >> Am 05.03.2014 18:30, schrieb Marcel Apfelbaum: > >>> In order to allow attaching machine options to a machine instance, > >>> current_machine is converted into MachineState. > >>> As a first step of deprecating QEMUMachine, some of the functions > >>> were modified to return MachineCLass. > >>> > >>> Signed-off-by: Marcel Apfelbaum > >> > >> Looks mostly good, but same issue as Alexey's patch: We are risking > >> qdev_get_machine() creating a Container-typed /machine node. > >> > >> What about the following on top? > > Hi Andreas, > >=20 > > I checked with the debugger and qdev_get_machine is called > > long after we add the machine to the QOM tree. > > However, the race still exists as someone can call qdev_get_machine > > before the machine is added to the tree, not being aware of that. > >=20 > > Your change solves the problem, thank you! > > Do you want me to add this diff and resend, > > or I will send yours separately? >=20 > My preference would be to avoid another round of review on my part by > simply squashing into your 3/3. There is a problem with it: 'make check fails' on test-qdev-global-props. - 'qdev_get_machine()' is called by 'device_set_realized()' because stati= c_prop_type has TYPE_DEVICE as parent. - The machine is added to the QOM tree *only in vl's main* and this test = does not reach it, but assumes that always will be a machine in the QOM tree. This is no longer true. Possible solution would be making existing 'null machine' a subclass of M= achineClass and add it manually to QOM on this test(and other places as necessary). T= he risk here is that there are other places where the machine needs to be added manually = to the QOM tree. (I am trying this option, but make check gets stuck !!!, debugging) Other possible solution is to add some kind of "CONFIG_MACHINE_IS_QOM_OBJ= ECT" define and use this in qdev_get_machine() implementation. (ugly?) Finally, is possible to be aware that may be a race when doing code revie= w. ("dangerous"?) Any thoughts? Thanks,=20 Marcel =20 >=20 > Cheers, > Andreas >=20