On Fri, Feb 01, 2013 at 08:29:48AM -0600, mdroth wrote: > On Fri, Feb 01, 2013 at 03:20:59PM +0800, Amos Kong wrote: > > On Thu, Jan 31, 2013 at 05:43:51PM -0600, Michael Roth wrote: > > > This reverts commit 84dd2120247a7d25ff1bb337de21c0e76816ad2d. > > > > > > I'm not sure what issue the original commit was meant to fix, or if > > > the logic is actually wrong, but it causes e1000 to stop working > > > after a guest issues a reset. > > > > Hi Michael, > > > > What's your test scenario? > > Nothing special, I just started a guest with > > -net nic,model=e1000 -net user > > or > > -net nic,model=e1000 -net tap > > and networking stopped working (could not lease an IP, no outbound > traffic) after I rebooted. I can reproduce this problem sometimes.
> > I tried this test with current qemu code, link status is not reseted > > to 'up' after step 3. Is it the problem you said? > > I think it's related, but I'm not so much concerned with the qmp-visible > link status changing as I am with the guest. > > > This problem also exists with current virtio (existed in the past) / > > rtl8139 (introduced in 83f58e570f21c3e7227e7fbef1fc0e18b5ed7ea9) > > > > 1) boot a guest with e1000 nic > > 2) set link down in monitor > > (hmp) set_link e1000.0 down > > 3) reset guest by 'system_reset' in monitor > > (hmp) system_reset > > > > > > My original patch is used to restore the link status after guest > > reboot(execute 'reboot' insider guest system). > > The link status should always be up after virtual 'hardware' reset > > (execute 'system_reset' in monitor). > > You sure you don't have that backwards? It seems to me that your > original patch was meant to *prevent* the link status from changing > after a system reset, which makes sense from the perspective of a > qmp-issued "set_link down" meaning "unplug the cable". I was confused after see your revert patch, now I know the real problem, and it's right to prevent down link status in 'system_reset'.