From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47944) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YGLI5-0003fn-4t for qemu-devel@nongnu.org; Wed, 28 Jan 2015 00:35:50 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YGLI3-0001Vn-DO for qemu-devel@nongnu.org; Wed, 28 Jan 2015 00:35:49 -0500 Date: Wed, 28 Jan 2015 16:31:40 +1100 From: David Gibson Message-ID: <20150128053140.GB14681@voom> References: <1419293824-2654-1-git-send-email-david@gibson.dropbear.id.au> <1419293824-2654-8-git-send-email-david@gibson.dropbear.id.au> <5498B793.2070206@suse.de> <20141223035655.GC4576@voom.redhat.com> <4D0DDA9D-1143-4EA3-89DF-7D9298A5182C@suse.de> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="Pd0ReVV5GZGQvF3a" Content-Disposition: inline In-Reply-To: <4D0DDA9D-1143-4EA3-89DF-7D9298A5182C@suse.de> Subject: Re: [Qemu-devel] [PATCHv2 7/8] pseries: Move rtc_offset into RTC device's state structure List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexander Graf Cc: "aik@ozlabs.ru" , "qemu-devel@nongnu.org" , "qemu-ppc@nongnu.org" , "mdroth@us.ibm.com" , "amit.shah@redhat.com" , "pbonzini@redhat.com" --Pd0ReVV5GZGQvF3a Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Dec 23, 2014 at 07:41:22AM +0100, Alexander Graf wrote: >=20 >=20 >=20 > > Am 23.12.2014 um 04:56 schrieb David Gibson : > >=20 > >> On Tue, Dec 23, 2014 at 01:30:11AM +0100, Alexander Graf wrote: > >>=20 > >>=20 > >>> On 23.12.14 01:17, David Gibson wrote: > >>> The initial creation of the PAPR RTC qdev class left a wart - the rtc= 's > >>> offset was left in the sPAPREnvironment structure, accessed via a glo= bal. > >>>=20 > >>> This patch moves it into the RTC device's own state structure, were it > >>> belongs. This requires a small change to the migration stream format= =2E In > >>> order to handle incoming streams from older versions, we also need to > >>> retain the rtc_offset field in the sPAPREnvironment structure, so tha= t it > >>> can be loaded into via the vmsd, then pushed into the RTC device. > >>>=20 > >>> Since we're changing the migration format, this also takes the opport= unity > >>> to change the rtc offset from a value in seconds to a value in nanose= conds, > >>> allowing nanosecond offsets between host and guest rtc time, if desir= ed. > >>>=20 > >>> Signed-off-by: David Gibson > >>> --- > >>> hw/ppc/spapr.c | 19 ++++++++++++++++++- > >>> hw/ppc/spapr_rtc.c | 41 +++++++++++++++++++++++++++++++++++++---- > >>> include/hw/ppc/spapr.h | 3 ++- > >>> 3 files changed, 57 insertions(+), 6 deletions(-) > >>>=20 > >>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > >>> index 9722b42..3070be0 100644 > >>> --- a/hw/ppc/spapr.c > >>> +++ b/hw/ppc/spapr.c > >>> @@ -980,10 +980,27 @@ static int spapr_vga_init(PCIBus *pci_bus) > >>> } > >>> } > >>>=20 > >>> +static int spapr_post_load(void *opaque, int version_id) > >>> +{ > >>> + sPAPREnvironment *spapr =3D (sPAPREnvironment *)opaque; > >>> + int err =3D 0; > >>> + > >>> + /* In earlier versions, there was no seperate qdev for the PAPR > >>> + * RTC, so the RTC offset was stored directly in sPAPREnvironmen= t. > >>> + * So when migrating from those versions, poke the incoming offs= et > >>> + * value into the RTC device */ > >>> + if (version_id < 3) { > >>> + err =3D spapr_rtc_import_offset(spapr->rtc, spapr->rtc_offse= t); > >>=20 > >> Do you think you could do this via a qom property set rather than an > >> explicit function call into the rtc code base instead? > >=20 > > Hrm. So, I could expose ns_offset as a property, but I'm not sure > > it's a good idea. Apart from this one instance to handle backwards > > compat migration, it's a purely internal value - seeting it from the > > command line is unlikely to have the desired effect, since it will get > > overwritten by spapr_rtc_realize(). >=20 > It's not about setting it on the command line, I realise that, but as I understand it, putting the qom property there inevitably means that it *could* be set from the command line, and that doesn't sound like a good thing to me. > it's a question on > how we separate our interfaces. The way you handle it right now we > need to make the RTAS code link with the RTC code. With QOM > properties this would happen with a by-name convention both sides > use. Well, yes, and my point is that that there *should not* be a general interface to set the offset, because setting the offset never makes sense, except for this backwards compatibility shim. --=20 David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson --Pd0ReVV5GZGQvF3a Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJUyHQ8AAoJEGw4ysog2bOST00P/15wpC0oN7YntFchTYH/8cxw 9pnfZokh9Q9/MIo3HmfO9XRA0jVy0O2YtSxtW9OfidZYTDGq3y6gXEUYof/Rmh8i HwJVjBqbBiQe5PgEsma/taleGvgAPF4W5pbZyv7VFKlarBl94ippJan23HScX0Np wqvUWhVirzgZQLKQ+UF9HDvMWUW8fM6Shp8RSZaV6mhNWdvfBW/qtHC1nZHUQrRb QvhWsdr/WeTDkhA62NFxBA0Oi7szxQ187sr41qGFcsuDAVhNY0Vry7/Nlq39tak5 aqJSe/HVURL7JPaKsbZfUvIjdloLE3/wUl3CuMLlSNauWrkOXMo19YVOm15KG/xO 5bTxgV5FNJhXJdbd+iNZjQR1DgVRtmbqgwf5vAZTXzgORxigwgidV0aV1pMI9gKP NSWtt2uQnjmbIJXvl4bCpxBDwbaPEMjnGnHKV5xzedPAVJKqF+/GrILyfikExV9O yo0rKars6jX178WCttuK6spqlw2qcBM2LwPC24XLgSnBattMta4kd5svqes23VU9 9bAYNdIwQmReyYMN9BmoySkKg5FhlJcyVPXLrngSaGI35WXJawioN0I2SwQBNpQK TD512LHQIBtsl6wCZ868gFsSHwRkL56DMUDXbfd5sIKTGy1V4o9dOc8YSrsIzx4e W5d8hB6CGgIX6K4TwlTk =4NZV -----END PGP SIGNATURE----- --Pd0ReVV5GZGQvF3a--