From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:35759) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YzgFW-0006Iu-V8 for qemu-devel@nongnu.org; Tue, 02 Jun 2015 03:04:39 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YzgFR-00015v-VK for qemu-devel@nongnu.org; Tue, 02 Jun 2015 03:04:34 -0400 Received: from mx1.redhat.com ([209.132.183.28]:58167) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YzgFR-00015h-NO for qemu-devel@nongnu.org; Tue, 02 Jun 2015 03:04:29 -0400 Message-ID: <556D5577.1020002@redhat.com> Date: Tue, 02 Jun 2015 09:04:23 +0200 From: Laszlo Ersek MIME-Version: 1.0 References: <20150601141054.GA11304@redhat.com> <20150601141343.GH13155@redhat.com> <20150601153237.GE2120@HEDWIG.INI.CMU.EDU> <20150601174409-mutt-send-email-mst@redhat.com> <20150601180022.GI2120@HEDWIG.INI.CMU.EDU> <20150601203126.GK2120@HEDWIG.INI.CMU.EDU> In-Reply-To: <20150601203126.GK2120@HEDWIG.INI.CMU.EDU> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] fw cfg files cross-version migration races List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Gabriel L. Somlo" , "Gabriel L. Somlo" Cc: qemu-devel@nongnu.org, Paolo Bonzini , kraxel@redhat.com, "Michael S. Tsirkin" (resending, with Paolo's address corrected) On 06/01/15 22:31, Gabriel L. Somlo wrote: > 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 ? :) I thought once fw_cfg was rebased to an (anonymous?) RAMBlock footing, migration became a non-issue. But, admittedly, I haven't thought much about it. Laszlo