Re: [Qemu-devel] [PATCH for-1.4] Revert "e1000: no need auto-negotiation if link was down"

[Amos Kong]

On Fri, Feb 01, 2013 at 08:49:00AM -0600, mdroth wrote:
> On Fri, Feb 01, 2013 at 03:53:22PM +0800, Amos Kong 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?
> > > 
> > > I tried this test with current qemu code, link status is not reseted
> > > to 'up' after step 3. Is it the problem you said?
> > > 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).
> > 
> > Is it expected?
> > 
> > When we reset the virtual system, do we need to reset the status
> > of simulation of network cable?
> 
> I don't so. If we "unplug the cable", it should stay unplugged when we
> reset the machine.

Ok.
 
> When we reset the machine, we should set the NIC's link status in
> accordance with it's NetClient peer. So if that's what your patch was
> attempting to enforce I think we're in agreement there.
> 
> I think the problem with your original patch is that it doesn't check
> the link status of the nic's NetClient peer, but instead checks it's
> own internal NetClient state, so we end up trying to enforce "leave the
> cable unplugged" even when "set_link down" was never issued.
> 
> And if that's the case I think you can re-spin your original patch for
> 1.4 accordingly. But we definitely need to revert the current one ASAP
> since it breaks all QEMU guests that are started using the default nic.

Agree, thanks.