From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:36744) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1W3mQv-0006jP-1l for qemu-devel@nongnu.org; Thu, 16 Jan 2014 07:52:34 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1W3mQn-0002xD-Ji for qemu-devel@nongnu.org; Thu, 16 Jan 2014 07:52:28 -0500 Received: from mail-we0-f173.google.com ([74.125.82.173]:58749) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1W3mQn-0002wP-Ar for qemu-devel@nongnu.org; Thu, 16 Jan 2014 07:52:21 -0500 Received: by mail-we0-f173.google.com with SMTP id t60so3159848wes.32 for ; Thu, 16 Jan 2014 04:52:20 -0800 (PST) MIME-Version: 1.0 Sender: peter.crosthwaite@petalogix.com In-Reply-To: <20140116024511.GA11535@stefanha-thinkpad> References: <1389435213-4378-1-git-send-email-b.galvani@gmail.com> <1389435213-4378-2-git-send-email-b.galvani@gmail.com> <20140114051938.GC4803@stefanha-thinkpad.redhat.com> <20140116024511.GA11535@stefanha-thinkpad> Date: Thu, 16 Jan 2014 22:52:19 +1000 Message-ID: From: Peter Crosthwaite Content-Type: text/plain; charset=ISO-8859-1 Subject: Re: [Qemu-devel] [PATCH v2 1/2] hw/net: add support for Allwinner EMAC Fast Ethernet controller List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Hajnoczi Cc: Edgar Iglesias , Peter Maydell , Juan Quintela , "qemu-devel@nongnu.org Developers" , Beniamino Galvani , Li Guang On Thu, Jan 16, 2014 at 12:45 PM, Stefan Hajnoczi wrote: > On Wed, Jan 15, 2014 at 06:24:24PM +1000, Peter Crosthwaite wrote: >> On Tue, Jan 14, 2014 at 3:19 PM, Stefan Hajnoczi wrote: >> > On Mon, Jan 13, 2014 at 11:16:37PM +1000, Peter Crosthwaite wrote: >> >> On Mon, Jan 13, 2014 at 11:15 PM, Peter Crosthwaite >> >> wrote: >> >> > On Sat, Jan 11, 2014 at 8:13 PM, Beniamino Galvani wrote: >> >> >> +static const VMStateDescription vmstate_mii = { >> >> >> + .name = "allwinner_emac_mii", >> >> >> + .version_id = 1, >> >> >> + .minimum_version_id = 1, >> >> >> + .fields = (VMStateField[]) { >> >> >> + VMSTATE_UINT16(bmcr, AwEmacMii), >> >> >> + VMSTATE_UINT16(bmsr, AwEmacMii), >> >> >> + VMSTATE_UINT16(anar, AwEmacMii), >> >> >> + VMSTATE_UINT16(anlpar, AwEmacMii), >> >> >> + VMSTATE_BOOL(link_ok, AwEmacMii), >> >> > >> >> > The net layer should probably properly set link_ok on realize, so I >> >> > doubt it's migratable state. >> > >> > The net layer is not part of live migration. It's up to the emulated >> > device to migrate link state and restore it after migration. In other >> > words, link state should be migrated as part of PHY vmstate. >> > >> >> But is the saved link state meaningful? The fact that net is not >> migrating means that this link_ok migrated variable is potentially >> incoherent with the net layer. E.G. What happens in the following >> case: >> >> Run QEMU >> Link is Good >> Save >> >> Run Again >> Link is bad >> Load >> >> The loaded emulation will have link_ok due to the vmsd load despite >> the link being down in the new net layer environment. It will not >> correct until net-layer activity causes a ->link_state_changed >> callback. > > I would agrue that this example is not valid. NetClient->link_down is > influenced by two things: > 1. User actions that disable the link (e.g. setting link status) > 2. Backends may take the link down, e.g. if the net/socket.c connection > breaks. > > In either case, the guest should migrate with its original link status. > Then, when it resumes on the destination host the link state may change > or an interrupt can be raised. > Will you reliably get your link_state_changed callback post_load though? >> Can this be just solved cleanly by calling the mii_set_link fn (added >> in this patch) always on post load and removing this link_ok state? >> This will mean that guest will see a link state transition immediately >> on load if the link state changes from the saved-machine state to the >> loaded-machine state. > > Is the link_ok field even necessary? No. Thats my theory underpinning my proposed change anyway. > Perhaps we should just rely on > NetClient->link_ok. > I think we are in agreement then? There is no need for phy.link_ok as same information is captured in NetClient. This means there is no link_ok field to migrate. Just migrate the phy registers themselves. For completeness you just need to poll NetClient state post_load which will nicely handle my degenerate case of link activity between save and load (same handler as link_state_changed). In the more usual case of the link state not changing between save and load, this function call will just be a nop, leading to no change of state on the freshly migrated phy registers. Regards Peter