From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:49940) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Wwwt0-000087-Cg for qemu-devel@nongnu.org; Tue, 17 Jun 2014 13:09:38 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Wwwss-0004Tj-Sv for qemu-devel@nongnu.org; Tue, 17 Jun 2014 13:09:30 -0400 Received: from cantor2.suse.de ([195.135.220.15]:37553 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Wwwss-0004TN-M5 for qemu-devel@nongnu.org; Tue, 17 Jun 2014 13:09:22 -0400 Message-ID: <53A07641.1080906@suse.de> Date: Tue, 17 Jun 2014 19:09:21 +0200 From: =?ISO-8859-15?Q?Andreas_F=E4rber?= MIME-Version: 1.0 References: <1402505369-12526-1-git-send-email-pbonzini@redhat.com> <1402505369-12526-6-git-send-email-pbonzini@redhat.com> In-Reply-To: <1402505369-12526-6-git-send-email-pbonzini@redhat.com> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 5/5] mc146818rtc: add "rtc" link to "/machine" List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini , qemu-devel@nongnu.org Cc: mtosatti@redhat.com, stefanha@redhat.com Am 11.06.2014 18:49, schrieb Paolo Bonzini: > From: Marcelo Tosatti >=20 > Add a link to rtc under /machine providing a stable > location for management apps to query "date" field. >=20 > {"execute":"qom-get","arguments":{"path":"/machine/rtc","property":"dat= e"} } >=20 > Suggested by Paolo Bonzini. >=20 > Signed-off-by: Marcelo Tosatti > Signed-off-by: Paolo Bonzini > Signed-off-by: mtosatti@redhat.com Do those management apps want a generic location for each machine or just for PC? I know several ARM boards that have different ones on SoC and on PMIC (I2C) for instance. In that case it may be safer to implement an interface and to search for that? Or at least give the "rtc" node a more generic property type for ABI stability? I.e. you only want "date" here, not any other properties of the mc146818rtc. > --- > hw/timer/mc146818rtc.c | 9 +++++++++ > 1 file changed, 9 insertions(+) >=20 > diff --git a/hw/timer/mc146818rtc.c b/hw/timer/mc146818rtc.c > index df54546..8c706e1 100644 > --- a/hw/timer/mc146818rtc.c > +++ b/hw/timer/mc146818rtc.c > @@ -893,6 +893,9 @@ static void rtc_realizefn(DeviceState *dev, Error *= *errp) > =20 > object_property_add(OBJECT(s), "date", "struct tm", > rtc_get_date, NULL, NULL, s, NULL); > + > + object_property_add_alias(qdev_get_machine(), "rtc", > + OBJECT(s), NULL, &error_abort); Irrespective of the error_abort discussion, realize feels too late for setting up such a property. My suggestion would be to do it in PC machine init instead - or if not feasible, here in instance_init with NULL errp. > } > =20 > ISADevice *rtc_init(ISABus *bus, int base_year, qemu_irq intercept_irq= ) > @@ -932,11 +935,17 @@ static void rtc_class_initfn(ObjectClass *klass, = void *data) > dc->cannot_instantiate_with_device_add_yet =3D true; > } > =20 > +static void rtc_finalize(Object *obj) > +{ > + object_property_del(qdev_get_machine(), "rtc", NULL); In Peter's multiple-RTC scenario this means the first RTC unparented(?) will remove the property, not necessarily the RTC the property points to. Not really relevant in practice, just pointing out that it's not just about adding. Do we need to clean it up at all? And why not just use an "old-fashioned" link<> property? Regards, Andreas > +} > + > static const TypeInfo mc146818rtc_info =3D { > .name =3D TYPE_MC146818_RTC, > .parent =3D TYPE_ISA_DEVICE, > .instance_size =3D sizeof(RTCState), > .class_init =3D rtc_class_initfn, > + .instance_finalize =3D rtc_finalize, > }; > =20 > static void mc146818rtc_register_types(void) --=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