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