From nobody Thu Oct 2 13:00:52 2025 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of gnu.org designates 209.51.188.17 as permitted sender) client-ip=209.51.188.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of gnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org; dmarc=fail(p=none dis=none) header.from=redhat.com Return-Path: Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) by mx.zohomail.com with SMTPS id 1551979590086380.2122828395959; Thu, 7 Mar 2019 09:26:30 -0800 (PST) Received: from localhost ([127.0.0.1]:55930 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1h1wmo-00076c-Vl for importer@patchew.org; Thu, 07 Mar 2019 12:26:27 -0500 Received: from eggs.gnu.org ([209.51.188.92]:37333) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1h1wkh-0005Mq-Jl for qemu-devel@nongnu.org; Thu, 07 Mar 2019 12:24:16 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1h1wkg-0003L4-Gi for qemu-devel@nongnu.org; Thu, 07 Mar 2019 12:24:15 -0500 Received: from mx1.redhat.com ([209.132.183.28]:18009) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1h1wkd-0003Ij-P2; Thu, 07 Mar 2019 12:24:12 -0500 Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 0C226882F0; Thu, 7 Mar 2019 17:24:11 +0000 (UTC) Received: from blackfin.pond.sub.org (ovpn-116-92.ams2.redhat.com [10.36.116.92]) by smtp.corp.redhat.com (Postfix) with ESMTPS id E48ED1001DFE; Thu, 7 Mar 2019 17:24:03 +0000 (UTC) Received: by blackfin.pond.sub.org (Postfix, from userid 1000) id 5AD4A1138661; Thu, 7 Mar 2019 18:24:01 +0100 (CET) From: Markus Armbruster To: qemu-devel@nongnu.org Date: Thu, 7 Mar 2019 18:23:50 +0100 Message-Id: <20190307172401.29451-2-armbru@redhat.com> In-Reply-To: <20190307172401.29451-1-armbru@redhat.com> References: <20190307172401.29451-1-armbru@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.84 on 10.5.11.22 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.28]); Thu, 07 Mar 2019 17:24:11 +0000 (UTC) Content-Transfer-Encoding: quoted-printable X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [PATCH v2 01/12] qdev: Fix latent bug with compat_props and onboard devices X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: kwolf@redhat.com, pkrempa@redhat.com, qemu-block@nongnu.org, mst@redhat.com, philmd@redhat.com, mreitz@redhat.com, marcandre.lureau@redhat.com, pbonzini@redhat.com, lersek@redhat.com Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" Content-Type: text/plain; charset="utf-8" Compatibility properties started life as a qdev property thing: we supported them only for qdev properties, and implemented them with the machinery backing command line option -global. Recent commit fa0cb34d221 put them to use (tacitly) with memory backend objects (subtypes of TYPE_MEMORY_BACKEND). To make that possible, we first moved the work of applying them from the -global machinery into TYPE_DEVICE's .instance_post_init() method device_post_init(), in commits ea9ce8934c5 and b66bbee39f6, then made it available to TYPE_MEMORY_BACKEND's .instance_post_init() method host_memory_backend_post_init() as object_apply_compat_props(), in commit 1c3994f6d2a. Note the code smell: we now have function name starting with object_ in hw/core/qdev.c. It has to be there rather than in qom/, because it calls qdev_get_machine() to find the current accelerator's and machine's compat_props. Turns out calling qdev_get_machine() there is problematic. If we qdev_create() from a machine's .instance_init() method, we call device_post_init() and thus qdev_get_machine() before main() can create "/machine" in QOM. qdev_get_machine() tries to get it with container_get(), which "helpfully" creates it as "container" object, and returns that. object_apply_compat_props() tries to paper over the problem by doing nothing when the value of qdev_get_machine() isn't a TYPE_MACHINE. But the damage is done already: when main() later attempts to create the real "/machine", it fails with "attempt to add duplicate property 'machine' to object (type 'container')", and aborts. Since no machine .instance_init() calls qdev_create() so far, the bug is latent. But since I want to do that, I get to fix the bug first. Observe that object_apply_compat_props() doesn't actually need the MachineState, only its the compat_props member of its MachineClass and AccelClass. This permits a simple fix: register MachineClass and AccelClass compat_props with the object_apply_compat_props() machinery right after these classes get selected. This is actually similar to how things worked before commits ea9ce8934c5 and b66bbee39f6, except we now register much earlier. The old code registered them only after the machine's .instance_init() ran, which would've broken compatibility properties for any devices created there. Cc: Marc-Andr=C3=A9 Lureau Signed-off-by: Markus Armbruster Reviewed-by: Marc-Andr=C3=A9 Lureau --- accel/accel.c | 1 + hw/core/qdev.c | 48 ++++++++++++++++++++++++++++++++---------- include/hw/qdev-core.h | 2 ++ vl.c | 1 + 4 files changed, 41 insertions(+), 11 deletions(-) diff --git a/accel/accel.c b/accel/accel.c index 68b6d56323..4a1670e404 100644 --- a/accel/accel.c +++ b/accel/accel.c @@ -66,6 +66,7 @@ static int accel_init_machine(AccelClass *acc, MachineSta= te *ms) *(acc->allowed) =3D false; object_unref(OBJECT(accel)); } + object_set_accelerator_compat_props(acc->compat_props); return ret; } =20 diff --git a/hw/core/qdev.c b/hw/core/qdev.c index d59071b8ed..1a86c7990a 100644 --- a/hw/core/qdev.c +++ b/hw/core/qdev.c @@ -970,25 +970,51 @@ static void device_initfn(Object *obj) QLIST_INIT(&dev->gpios); } =20 +/* + * Global property defaults + * Slot 0: accelerator's global property defaults + * Slot 1: machine's global property defaults + * Each is a GPtrArray of of GlobalProperty. + * Applied in order, later entries override earlier ones. + */ +static GPtrArray *object_compat_props[2]; + +/* + * Set machine's global property defaults to @compat_props. + * May be called at most once. + */ +void object_set_machine_compat_props(GPtrArray *compat_props) +{ + assert(!object_compat_props[1]); + object_compat_props[1] =3D compat_props; +} + +/* + * Set accelerator's global property defaults to @compat_props. + * May be called at most once. + */ +void object_set_accelerator_compat_props(GPtrArray *compat_props) +{ + assert(!object_compat_props[0]); + object_compat_props[0] =3D compat_props; +} + void object_apply_compat_props(Object *obj) { - if (object_dynamic_cast(qdev_get_machine(), TYPE_MACHINE)) { - MachineState *m =3D MACHINE(qdev_get_machine()); - MachineClass *mc =3D MACHINE_GET_CLASS(m); + int i; =20 - if (m->accelerator) { - AccelClass *ac =3D ACCEL_GET_CLASS(m->accelerator); - - if (ac->compat_props) { - object_apply_global_props(obj, ac->compat_props, &error_ab= ort); - } - } - object_apply_global_props(obj, mc->compat_props, &error_abort); + for (i =3D 0; i < ARRAY_SIZE(object_compat_props); i++) { + object_apply_global_props(obj, object_compat_props[i], + &error_abort); } } =20 static void device_post_init(Object *obj) { + /* + * Note: ordered so that the user's global properties take + * precedence. + */ object_apply_compat_props(obj); qdev_prop_set_globals(DEVICE(obj)); } diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h index e70a4bfa49..c1dc06e9bb 100644 --- a/include/hw/qdev-core.h +++ b/include/hw/qdev-core.h @@ -418,6 +418,8 @@ const char *qdev_fw_name(DeviceState *dev); =20 Object *qdev_get_machine(void); =20 +void object_set_machine_compat_props(GPtrArray *compat_props); +void object_set_accelerator_compat_props(GPtrArray *compat_props); void object_apply_compat_props(Object *obj); =20 /* FIXME: make this a link<> */ diff --git a/vl.c b/vl.c index fd0d51320d..ea58a16a4a 100644 --- a/vl.c +++ b/vl.c @@ -3954,6 +3954,7 @@ int main(int argc, char **argv, char **envp) configure_rtc(qemu_find_opts_singleton("rtc")); =20 machine_class =3D select_machine(); + object_set_machine_compat_props(machine_class->compat_props); =20 set_memory_options(&ram_slots, &maxram_size, machine_class); =20 --=20 2.17.2