On Mon, Jun 01, 2015 at 02:00:22PM -0400, Gabriel L. Somlo wrote:
> On Mon, Jun 01, 2015 at 05:44:47PM +0200, Michael S. Tsirkin wrote:
> > > > Shouldn't we migrate the fw cfg data that the source host generates
> > > > originally, rather than trying to play games make sure the way it
> > > > is re-generated on dest doesn't change.
> > >
> > > Right now, in hw/nvram/fw_cfg.c, we have:
> > >
> > > struct FWCfgState {
> > > /*< private >*/
> > > SysBusDevice parent_obj;
> > > /*< public >*/
> > >
> > > FWCfgEntry entries[2][FW_CFG_MAX_ENTRY];
> > > FWCfgFiles *files;
> > > uint16_t cur_entry;
> > > uint32_t cur_offset;
> > > Notifier machine_ready;
> > > };
> > >
> > > and, later:
> > >
> > > static const VMStateDescription vmstate_fw_cfg = {
> > > .name = "fw_cfg",
> > > .version_id = 2,
> > > .minimum_version_id = 1,
> > > .fields = (VMStateField[]) {
> > > VMSTATE_UINT16(cur_entry, FWCfgState),
> > > VMSTATE_UINT16_HACK(cur_offset, FWCfgState, is_version_1),
> > > VMSTATE_UINT32_V(cur_offset, FWCfgState, 2),
> > > VMSTATE_END_OF_LIST()
> > > }
> > > };
> > >
> > > Would this be as simple as adding a VMSTATE_ARRAY* for 'entries'
> > > and something like a VMSTATE_VBUFFER_ALLOC_UINT32 for 'files', which
> > > is dynamically allocated the first time a fwcfg "file" is inserted ?
> > >
> > > The one catch is that the value of the "files" pointer is itself a
> > > fw_cfg entry (FW_CFG_FILE_DIR), so that would need to be "patched"
> > > on the destination side...
> > >
> > > I do like the idea of simply migrating the full content of the fw_cfg
> > > device though, seems like the safest solution.
> > >
> > > Thanks much,
> > > --Gabriel
> >
> > OK but you need to do a bunch of work on load, e.g. some fw cfg
> > entries trigger callbacks on access, etc.
>
> Oh, you mean here:
>
> typedef struct FWCfgEntry {
> uint32_t len;
> uint8_t *data;
> void *callback_opaque;
> FWCfgReadCallback read_callback;
> } FWCfgEntry;
>
> ... I can't just assume that 'read_callback' is a valid function
> pointer in the context of the destination host ?
>
> Ouch, that could get painful really really quickly :)
Actually, it's much worse than that. A lot of the data items stored in
fw_cfg are just pointers to somewhere in the qemu process address
space, and I have no confidence that these pointers are guaranteed to
make sense in the address space of the *destination* qemu process...
I guess the only reason this isn't a problem is that nobody currently
attempts to access fw_cfg after a migration ? :)
--Gabriel
