On Tue, Jun 05, 2012 at 04:14:44PM +0200, Paolo Bonzini wrote: > Il 05/06/2012 03:00, Michael Roth ha scritto: > > This will eventually be used to serialize device state for the purposes > > of migration/savevm, and is also useful for introspection/testing. > > > > Signed-off-by: Michael Roth <mdr...@linux.vnet.ibm.com> > > --- > > hw/mc146818rtc.c | 12 ++++++++++++ > > 1 files changed, 12 insertions(+), 0 deletions(-) > > > > diff --git a/hw/mc146818rtc.c b/hw/mc146818rtc.c > > index 7490d28..2dfc233 100644 > > --- a/hw/mc146818rtc.c > > +++ b/hw/mc146818rtc.c > > @@ -26,6 +26,7 @@ > > #include "sysemu.h" > > #include "mc146818rtc.h" > > #include "mc146818rtc_state.h" > > +#include "qapi-generated/mc146818rtc-qapi-visit.h" > > > > #ifdef TARGET_I386 > > #include "apic.h" > > @@ -590,6 +591,14 @@ static void rtc_get_date(Object *obj, Visitor *v, void > > *opaque, > > visit_end_struct(v, errp); > > } > > > > +static void rtc_get_state(Object *obj, Visitor *v, void *opaque, > > + const char *name, Error **errp) > > +{ > > + ISADevice *isa = ISA_DEVICE(obj); > > + RTCState *s = DO_UPCAST(RTCState, dev, isa); > > + visit_type_RTCState(v, &s, name, errp); > > +} > > + > > static int rtc_initfn(ISADevice *dev) > > { > > RTCState *s = DO_UPCAST(RTCState, dev, dev); > > @@ -638,6 +647,9 @@ static int rtc_initfn(ISADevice *dev) > > object_property_add(OBJECT(s), "date", "struct tm", > > rtc_get_date, NULL, NULL, s, NULL); > > > > + object_property_add(OBJECT(s), "state", "RTCState", > > + rtc_get_state, NULL, NULL, s, NULL); > > + > > return 0; > > } > > > > Nice! The next obvious step would be to use this from vmstate instead > of hardcoded struct offsets.
Heh, indeed. I actually looked into this quite a bit thinking it was the next logical progression and was hoping to make this part of the RFC. The main issues I ran into were the numerous cases where vmstate descriptions will "flatten" embedded struct fields rather than marking them as VMS_STRUCT, as it creates a hard incompatibility with, for instance, the way "struct tm current_tm" is serialized. We end up needing to do things like parsing VMStateField.name for "." characters so we know additional levels of traversal are needed to fetch the serialized data. There are also areas where we unroll arrays that cause similar pains. I think it's still doable, but it ends up adding a lot of complexity to the last place we need it. The main benefit, in my mind, was decoupling vmstate from the actual device state such that the vmstate field descriptions became purely a stable "wire" protocol of sorts, and the serialized data could be transformed to maintain that compatibility regardless of how much an actual device's structure changed. But the cases where manipulating a QObject would cover more cases than adding/modifying vmstate field descriptors seem like they'd be too rare to warrant the added complexity. Having this capability in the context of a wire protocol that's better seperated from the data representation makes sense, but in the case of vmstate there's too much overlap and too much complexity IMO. > > Paolo >