Laine Stump wrote:
On 11/07/2016 09:20 AM, Roman Bogorodskiy wrote:
Recently e1000 NIC support was added to bhyve; implement that in the bhyve driver:
- Add capability check by analyzing output of the 'bhyve -s 0,e1000' command - Modify bhyveBuildNetArgStr() to support e1000 and also pass virConnectPtr so it could call bhyveDriverGetCaps() to check if this NIC is supported - Modify command parsing code to add support for e1000 and adjust tests - Add net-e1000 test --- src/bhyve/bhyve_capabilities.c | 27 ++++++++ src/bhyve/bhyve_capabilities.h | 1 + src/bhyve/bhyve_command.c | 74 ++++++++++++++-------- src/bhyve/bhyve_parse_command.c | 9 ++- tests/bhyveargv2xmldata/bhyveargv2xml-e1000.args | 8 +++ tests/bhyveargv2xmldata/bhyveargv2xml-e1000.xml | 28 ++++++++ .../bhyveargv2xmldata/bhyveargv2xml-virtio-net.xml | 2 + .../bhyveargv2xml-virtio-net4.xml | 1 + tests/bhyveargv2xmltest.c | 1 + .../bhyvexml2argvdata/bhyvexml2argv-net-e1000.args | 9 +++ .../bhyvexml2argv-net-e1000.ldargs | 3 + .../bhyvexml2argvdata/bhyvexml2argv-net-e1000.xml | 22 +++++++ tests/bhyvexml2argvtest.c | 7 +- 13 files changed, 162 insertions(+), 30 deletions(-) create mode 100644 tests/bhyveargv2xmldata/bhyveargv2xml-e1000.args create mode 100644 tests/bhyveargv2xmldata/bhyveargv2xml-e1000.xml create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-net-e1000.args create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-net-e1000.ldargs create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-net-e1000.xml
Whoa, thanks for the detailed review, much appreciated!
After several of these... Not necessary this time, but for future reference (or if you want extra Brownie points this time :-)) - we've found it much easier to review patches adding new functionality if other necessary reorganization (e.g. changing all the "return -1" into "goto cleanup" and moving all resource-freeing down to cleanup:) is put in a separate prerequisite patch. Then the new functionality is just a simple addition rather than a re-org + addition.
That's actually what I'm planning to do: split that into two series: cosmetic-ish and the rest, it's easier to maintain small series than tackle with a larger patch, considering that I have only little chucks on time to work on that right now... Roman Bogorodskiy