From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48827) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Y3Gm4-0001HU-6A for qemu-devel@nongnu.org; Mon, 22 Dec 2014 23:08:45 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Y3Gm2-0002uc-AA for qemu-devel@nongnu.org; Mon, 22 Dec 2014 23:08:44 -0500 Date: Tue, 23 Dec 2014 14:56:55 +1100 From: David Gibson Message-ID: <20141223035655.GC4576@voom.redhat.com> 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> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="VywGB/WGlW4DM4P8" Content-Disposition: inline In-Reply-To: <5498B793.2070206@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 --VywGB/WGlW4DM4P8 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable 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 globa= l. > >=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. = In > > order to handle incoming streams from older versions, we also need to > > retain the rtc_offset field in the sPAPREnvironment structure, so that = 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 opportun= ity > > to change the rtc offset from a value in seconds to a value in nanoseco= nds, > > allowing nanosecond offsets between host and guest rtc time, if desired. > >=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 sPAPREnvironment. > > + * So when migrating from those versions, poke the incoming offset > > + * value into the RTC device */ > > + if (version_id < 3) { > > + err =3D spapr_rtc_import_offset(spapr->rtc, spapr->rtc_offset); >=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? 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 > > + } > > + > > + return err; > > +} > > + > > static const VMStateDescription vmstate_spapr =3D { > > .name =3D "spapr", > > - .version_id =3D 2, > > + .version_id =3D 3, > > .minimum_version_id =3D 1, > > + .post_load =3D spapr_post_load, > > .fields =3D (VMStateField[]) { > > VMSTATE_UNUSED(4), /* used to be @next_irq */ >=20 > Shouldn't we make the rtc_offset field conditional on version < 3 as > well here? In fact, you could do the same for the legacy next_irq. That > way we don't need to move data over the wire that doesn't get used anymor= e. Uh, yeah, I guess we can do that. It's just a bit awkward, because the vmstatedescription includes built in handling for "earliest version" but not "last version", so I'll need to write a custom test function. --=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 --VywGB/WGlW4DM4P8 Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJUmOgGAAoJEGw4ysog2bOScbgP/jq4+JmmgwLxbMQx0hle1/rq zAz1Sz09RwTHjKoGPjd2nEq8/bV9jSQtm1CGxBRfOmqOYoVoyH6rVcIZVP0Vpkaf clMMHdCeI33wPAV9/QnN10w2Y/eSpB/GkzH1snULJg656805Jw1xfLFAfLkbjXxQ p40aGIayLlle+vhxd6gvUwlxyIO3xH3E6A5X3+pOvwKL3kEWHxGxPDzfAqojNsoL JTGGY9deZE7bYIMHus9KDQkM90D5PTy6bOTQT45j/fEpuuZwnVW4NzmpnRq1bxfl BcSWc0GX7rExEpBU9XdIhb93Nxx3yirQj8qbFanc4+AR16VSEr38JTu8hPny8XrU jkxLz32ijboYlEsVRoHvMWcp9RX+P9sCSxdde80WLMvW4xYVVr4wTp26H2YhiODc FakJsa3Ubofjtbxkjf5hDVu+PlrFpeK/IMsao5f5OytgpFXcK1uE3kwYCb07Vain h4ELFD3h267RfxU/Ij1SBrr0HGURJBFvaIGHePSAdwNVgchNpmNPkO07u0VSc4DM fiqgeB9+iwSjf9WwmlAe4rEFPicylmOSbqD+rkbyDYowLUIjBjiSzXTWlvs/Y/92 INVr9khVZukDDC96HN60wLfSmCmz6jN70sCLTc8qHfgzApN9WpNocdxM9FQhTnMP LTCbob76RPMhh6XUC+u8 =Cqw9 -----END PGP SIGNATURE----- --VywGB/WGlW4DM4P8--