On Tue, Dec 23, 2014 at 01:30:11AM +0100, Alexander Graf wrote:
>
>
> 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 global.
> >
> > 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.
> >
> > Since we're changing the migration format, this also takes the opportunity
> > to change the rtc offset from a value in seconds to a value in nanoseconds,
> > allowing nanosecond offsets between host and guest rtc time, if desired.
> >
> > Signed-off-by: David Gibson <da...@gibson.dropbear.id.au>
> > ---
> > hw/ppc/spapr.c | 19 ++++++++++++++++++-
> > hw/ppc/spapr_rtc.c | 41 +++++++++++++++++++++++++++++++++++++----
> > include/hw/ppc/spapr.h | 3 ++-
> > 3 files changed, 57 insertions(+), 6 deletions(-)
> >
> > 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)
> > }
> > }
> >
> > +static int spapr_post_load(void *opaque, int version_id)
> > +{
> > + sPAPREnvironment *spapr = (sPAPREnvironment *)opaque;
> > + int err = 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 = spapr_rtc_import_offset(spapr->rtc, spapr->rtc_offset);
>
> 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().
>
> > + }
> > +
> > + return err;
> > +}
> > +
> > static const VMStateDescription vmstate_spapr = {
> > .name = "spapr",
> > - .version_id = 2,
> > + .version_id = 3,
> > .minimum_version_id = 1,
> > + .post_load = spapr_post_load,
> > .fields = (VMStateField[]) {
> > VMSTATE_UNUSED(4), /* used to be @next_irq */
>
> 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 anymore.
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.
--
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
pgp9mZF8yqwtB.pgp
Description: PGP signature
