qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Amos Kong
Subject: Re: [Qemu-devel] [PATCH for-1.4] Revert "e1000: no need auto-negotiation if link was down"
Date: Fri, 1 Feb 2013 23:56:27 +0800
User-agent: Mutt/1.5.21 (2010-09-15)

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'.




reply via email to

[Prev in Thread] Current Thread [Next in Thread]