Re: [Qemu-devel] [PATCH] 82571 emulation

[Stefan Hajnoczi]

On Thu, Nov 07, 2013 at 11:06:06AM -0800, akepner wrote:
> First cut at emulating 82571. I'm sure there are lots of rough
> edges, but it's working for me.
> 
> Pretty obviously I started with a copy of the e1000 code, and
> modified just what was necessary to get it working. I'd like
> to also do a s/e1000_/e1000e/ to uniqify identifiers, but for
> now I've left it as it is to make comparison with e1000 easier.
> 
> Signed-off-by: Arthur Kepner <address@hidden>
> ---

Cool, thanks for sharing!  You've probably seen it already since your
email generally follows the conventions, but here's the link for
submitting patches:

http://qemu-project.org/Contribute/SubmitAPatch

If you stuck to the e1000.c code for the most part then there's probably
not much to comment on yet except a few suggestions for getting the
patch into a mergable state:

 * Sharing code with e1000.c may be reasonable, you decide

 * After renaming remaining code to e1000e, please drop dead-code or
   unused parts

 * Please audit the vmstate (migration data fields) to see if cruft can
   be dropped.  In particular, it's tricky to stay backwards-compatible
   with live migration so things tend to get appended rather than
   replaced.

   Since e1000e is starting from a fresh slate, you can delete any
   duplicated or workaround vmstate fields which were solely used for
   backwards compatibility during migration.

 * Please describe your testing: which guest OSes and versions?

 * Please add an indication of missing features from the hardware spec
   which you haven't implemented.  If this is literally just a hack of
   the e1000 code to make the Linux e1000e driver happy, then it may
   still need more work to be generally usable by other OSes.

 * Please test live migration several times with the NIC up and down

If any of this seems overwhelming or more effort than you were prepared
to go through, please ask questions and maybe people will turn up who
can help.

Thanks,
Stefan