On Tue, 21 Oct 2025 at 18:05, Peter Xu <[email protected]> wrote: > > On Tue, Oct 21, 2025 at 05:49:51PM +0100, Peter Maydell wrote: > > I suppose so, but that seems like something in practice > > we make a lot of use of. It's really handy to be able to > > say "this is how you obtain the integer you wanted to put > > in the migration stream" -- we do a fair amount of that > > in target/arm/machine.c for instance. But those don't > > need to return a failure, I suppose. > > That's exactly what pre_save() / post_load() should do, IMHO. > > Taking example of the arm's case here: > > static int put_fpscr(QEMUFile *f, void *opaque, size_t size, > const VMStateField *field, JSONWriter *vmdesc) > { > ARMCPU *cpu = opaque; > CPUARMState *env = &cpu->env; > uint32_t fpscr = vfp_fpcr_fpsr_needed(opaque) ? 0 : vfp_get_fpscr(env); > > qemu_put_be32(f, fpscr); > return 0; > } > > The .pre_save() should be exactly the logic that generalize whatever we > have had in QEMU's structs into a pure uint32_t, put that into a temp > u32. Then migration should be able to transfer that using whatever way it > prefers. When using qemufile API, it should use vmstate_info_uint32 so as > to not open-code qemu_put_be32().
I think this is very ugly, because it means that the device state struct ends up with a pile of extra fields whose only purpose is to be temporarily used during migration. Having a function which does the "figure out the value at the point where we want it" is a much nicer API because it keeps the migration related complication in the migration code, and keeps it out of the data structures that all the rest of the code has to work with. > Now, we have a major issue with qemufile and all the APIs that bound to it > when we want to move the IO layers from qemufile to iochannels, exactly > because we lack such abstraction layer, where we mixed up "this is how you > obtain the integer" and "send this integer to the wire" operations in one > API. Yeah, this is unfortunate. But I don't think the solution is to require a lot of extra device state struct fields to carry temporary data for migration. It would be better to have an API similar to the existing get/put functions but which passes the data back to the caller instead of calling qemu_put_be32() itself. thanks -- PMM
