From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47943) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WLybC-0006dY-7f for qemu-devel@nongnu.org; Fri, 07 Mar 2014 12:30:25 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WLyb4-0000sf-9x for qemu-devel@nongnu.org; Fri, 07 Mar 2014 12:30:18 -0500 Received: from cantor2.suse.de ([195.135.220.15]:43517 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WLyb3-0000sI-W7 for qemu-devel@nongnu.org; Fri, 07 Mar 2014 12:30:10 -0500 Message-ID: <531A0220.7030405@suse.de> Date: Fri, 07 Mar 2014 18:30:08 +0100 From: =?UTF-8?B?QW5kcmVhcyBGw6RyYmVy?= MIME-Version: 1.0 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> <1394209338.2663.51.camel@localhost.localdomain> In-Reply-To: <1394209338.2663.51.camel@localhost.localdomain> Content-Type: text/plain; charset=UTF-8 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: Marcel Apfelbaum 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 Am 07.03.2014 17:22, schrieb Marcel Apfelbaum: > 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, >>> >>> 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. >>> >>> Your change solves the problem, thank you! >>> Do you want me to add this diff and resend, >>> or I will send yours separately? >> >> 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-prop= s. > - 'qdev_get_machine()' is called by 'device_set_realized()' because sta= tic_prop_type > has TYPE_DEVICE as parent. > - The machine is added to the QOM tree *only in vl's main* and this tes= t does not > reach it, but assumes that always will be a machine in the QOM tree. > This is no longer true. >=20 > Possible solution would be making existing 'null machine' a subclass of= MachineClass > and add it manually to QOM on this test(and other places as necessary). The following hack fixes this particular failure for me (ran into it while trying to generate the HTML report): diff --git a/tests/test-qdev-global-props.c b/tests/test-qdev-global-prop= s.c index e4ad173..31bac15 100644 --- a/tests/test-qdev-global-props.c +++ b/tests/test-qdev-global-props.c @@ -167,6 +167,9 @@ int main(int argc, char **argv) g_test_init(&argc, &argv, NULL); module_call_init(MODULE_INIT_QOM); + + object_property_add_child(object_get_root(), "machine", object_new("container"), NULL); + type_register_static(&static_prop_type); type_register_static(&dynamic_prop_type); Not yet suitable for squashing obviously. Andreas > The risk here is > that there are other places where the machine needs to be added manuall= y to the QOM tree. > (I am trying this option, but make check gets stuck !!!, debugging) >=20 > Other possible solution is to add some kind of "CONFIG_MACHINE_IS_QOM_O= BJECT" define > and use this in qdev_get_machine() implementation. (ugly?) >=20 > Finally, is possible to be aware that may be a race when doing code rev= iew. ("dangerous"?) >=20 > Any thoughts? > Thanks,=20 > Marcel >=20 > =20 >=20 >> >> Cheers, >> Andreas >> >=20 >=20 >=20 --=20 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=C3=BCrnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imend=C3=B6rffer; HRB 16746 AG N=C3=BC= rnberg