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
Attachment:
signature.asc
Description: PGP signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list