Laine Stump wrote:
On 02/01/2017 11:28 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 | 26 ++++++++++++++++++ src/bhyve/bhyve_capabilities.h | 1 + src/bhyve/bhyve_command.c | 31 +++++++++++++++++++--- src/bhyve/bhyve_parse_command.c | 10 ++++++- tests/bhyveargv2xmldata/bhyveargv2xml-e1000.args | 8 ++++++ tests/bhyveargv2xmldata/bhyveargv2xml-e1000.xml | 30 +++++++++++++++++++++ .../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, 145 insertions(+), 6 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
diff --git a/src/bhyve/bhyve_capabilities.c b/src/bhyve/bhyve_capabilities.c index 83e3ae1b4..a647cd19b 100644 --- a/src/bhyve/bhyve_capabilities.c +++ b/src/bhyve/bhyve_capabilities.c @@ -216,6 +216,29 @@ bhyveProbeCapsAHCI32Slot(unsigned int *caps, char *binary) return ret; }
+static int +bhyveProbeCapsNetE1000(unsigned int *caps, char *binary) +{ + char *error; + virCommandPtr cmd = NULL; + int ret = 0, exit; + + cmd = virCommandNew(binary); + virCommandAddArgList(cmd, "-s", "0,e1000", NULL); + virCommandSetErrorBuffer(cmd, &error); + if (virCommandRun(cmd, &exit) < 0) { + ret = -1; + goto out; + } + + if (strstr(error, "pci slot 0:0: unknown device \"e1000\"") == NULL) + *caps |= BHYVE_CAP_NET_E1000; + + out:
Even if you don't switch to using a ret initialized to -1 (see the layout in bhyveBuildNetArgStr()), please name the label "cleanup" instead of "out".
Flipped ret value and renamed out to cleanup. Generally, I just need to add a helper function for these probes as they're very similar and I'm planning to add some more cap checks.
+ VIR_FREE(error); + virCommandFree(cmd); + return ret; +}
int virBhyveProbeCaps(unsigned int *caps) @@ -235,6 +258,9 @@ virBhyveProbeCaps(unsigned int *caps) if ((ret = bhyveProbeCapsAHCI32Slot(caps, binary))) goto out;
+ if ((ret = bhyveProbeCapsNetE1000(caps, binary))) + goto out; + out: VIR_FREE(binary); return ret; diff --git a/src/bhyve/bhyve_capabilities.h b/src/bhyve/bhyve_capabilities.h index 55581bd68..690feadb8 100644 --- a/src/bhyve/bhyve_capabilities.h +++ b/src/bhyve/bhyve_capabilities.h @@ -39,6 +39,7 @@ typedef enum { typedef enum { BHYVE_CAP_RTC_UTC = 1 << 0, BHYVE_CAP_AHCI32SLOT = 1 << 1, + BHYVE_CAP_NET_E1000 = 1 << 2, } virBhyveCapsFlags;
int virBhyveProbeGrubCaps(virBhyveGrubCapsFlags *caps); diff --git a/src/bhyve/bhyve_command.c b/src/bhyve/bhyve_command.c index a50bd1066..5c86c9f1b 100644 --- a/src/bhyve/bhyve_command.c +++ b/src/bhyve/bhyve_command.c @@ -44,7 +44,8 @@ VIR_LOG_INIT("bhyve.bhyve_command");
static int -bhyveBuildNetArgStr(const virDomainDef *def, +bhyveBuildNetArgStr(virConnectPtr conn,
I think it's strange that you need to pass a virConnectPtr around to your command-line builder functions for caps. In the qemu driver, use of virConnectPtr has been eliminated wherever possible (it's completely absent from qemu_command.c). We just get the qemuCaps directly from the driver object.
Noted, added to my TODO list.
ACK (although I'd greatly prefer "out:" changed to "cleanup:")
Pushed, thanks! Roman Bogorodskiy