[libvirt] [PATCH v4] bhyve: add e1000 nic support
Laine Stump
laine at laine.org
Thu Feb 9 20:22:07 UTC 2017
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".
> + 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.
> + const virDomainDef *def,
> virDomainNetDefPtr net,
> virCommandPtr cmd,
> bool dryRun)
> @@ -52,9 +53,30 @@ bhyveBuildNetArgStr(const virDomainDef *def,
> char macaddr[VIR_MAC_STRING_BUFLEN];
> char *realifname = NULL;
> char *brname = NULL;
> + char *nic_model = NULL;
> int ret = -1;
> virDomainNetType actualType = virDomainNetGetActualType(net);
>
> + if (STREQ(net->model, "virtio")) {
> + if (VIR_STRDUP(nic_model, "virtio-net") < 0)
> + return -1;
> + } else if (STREQ(net->model, "e1000")) {
> + if ((bhyveDriverGetCaps(conn) & BHYVE_CAP_NET_E1000) != 0) {
> + if (VIR_STRDUP(nic_model, "e1000") < 0)
> + return -1;
> + } else {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> + _("NIC model 'e1000' is not supported "
> + "by given bhyve binary"));
> + return -1;
> + }
> + } else {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> + _("NIC model '%s' is not supported"),
> + net->model);
> + return -1;
> + }
> +
> if (actualType == VIR_DOMAIN_NET_TYPE_BRIDGE) {
> if (VIR_STRDUP(brname, virDomainNetGetActualBridgeName(net)) < 0)
> goto cleanup;
> @@ -101,8 +123,8 @@ bhyveBuildNetArgStr(const virDomainDef *def,
>
>
> virCommandAddArg(cmd, "-s");
> - virCommandAddArgFormat(cmd, "%d:0,virtio-net,%s,mac=%s",
> - net->info.addr.pci.slot,
> + virCommandAddArgFormat(cmd, "%d:0,%s,%s,mac=%s",
> + net->info.addr.pci.slot, nic_model,
> realifname, virMacAddrFormat(&net->mac, macaddr));
>
> ret = 0;
> @@ -111,6 +133,7 @@ bhyveBuildNetArgStr(const virDomainDef *def,
> VIR_FREE(net->ifname);
> VIR_FREE(brname);
> VIR_FREE(realifname);
> + VIR_FREE(nic_model);
>
> return ret;
> }
> @@ -345,7 +368,7 @@ virBhyveProcessBuildBhyveCmd(virConnectPtr conn,
> }
> for (i = 0; i < def->nnets; i++) {
> virDomainNetDefPtr net = def->nets[i];
> - if (bhyveBuildNetArgStr(def, net, cmd, dryRun) < 0)
> + if (bhyveBuildNetArgStr(conn, def, net, cmd, dryRun) < 0)
> goto error;
> }
> for (i = 0; i < def->ndisks; i++) {
> diff --git a/src/bhyve/bhyve_parse_command.c b/src/bhyve/bhyve_parse_command.c
> index e31f5fbd1..fcaaed275 100644
> --- a/src/bhyve/bhyve_parse_command.c
> +++ b/src/bhyve/bhyve_parse_command.c
> @@ -496,6 +496,7 @@ bhyveParsePCINet(virDomainDefPtr def,
> unsigned pcislot,
> unsigned pcibus,
> unsigned function,
> + const char *model,
> const char *config)
> {
> /* -s slot,virtio-net,tapN[,mac=xx:xx:xx:xx:xx:xx] */
> @@ -514,6 +515,9 @@ bhyveParsePCINet(virDomainDefPtr def,
> if (VIR_STRDUP(net->data.bridge.brname, "virbr0") < 0)
> goto error;
>
> + if (VIR_STRDUP(net->model, model) < 0)
> + goto error;
> +
> net->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI;
> net->info.addr.pci.slot = pcislot;
> net->info.addr.pci.bus = pcibus;
> @@ -620,7 +624,11 @@ bhyveParseBhyvePCIArg(virDomainDefPtr def,
> nahcidisk,
> conf);
> else if (STREQ(emulation, "virtio-net"))
> - bhyveParsePCINet(def, xmlopt, caps, pcislot, bus, function, conf);
> + bhyveParsePCINet(def, xmlopt, caps, pcislot, bus, function,
> + "virtio", conf);
> + else if (STREQ(emulation, "e1000"))
> + bhyveParsePCINet(def, xmlopt, caps, pcislot, bus, function,
> + "e1000", conf);
>
> VIR_FREE(emulation);
> VIR_FREE(slotdef);
> diff --git a/tests/bhyveargv2xmldata/bhyveargv2xml-e1000.args b/tests/bhyveargv2xmldata/bhyveargv2xml-e1000.args
> new file mode 100644
> index 000000000..aa568fe3a
> --- /dev/null
> +++ b/tests/bhyveargv2xmldata/bhyveargv2xml-e1000.args
> @@ -0,0 +1,8 @@
> +/usr/sbin/bhyve \
> +-c 1 \
> +-m 214 \
> +-H \
> +-P \
> +-s 0:0,hostbridge \
> +-s 1:0,e1000,tap0 \
> +-s 1:1,e1000,tap1,mac=FE:ED:AD:EA:DF:15 bhyve
> diff --git a/tests/bhyveargv2xmldata/bhyveargv2xml-e1000.xml b/tests/bhyveargv2xmldata/bhyveargv2xml-e1000.xml
> new file mode 100644
> index 000000000..c6b6c0ef8
> --- /dev/null
> +++ b/tests/bhyveargv2xmldata/bhyveargv2xml-e1000.xml
> @@ -0,0 +1,30 @@
> +<domain type='bhyve'>
> + <name>bhyve</name>
> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
> + <memory unit='KiB'>219136</memory>
> + <currentMemory unit='KiB'>219136</currentMemory>
> + <vcpu placement='static'>1</vcpu>
> + <os>
> + <type>hvm</type>
> + </os>
> + <clock offset='localtime'/>
> + <on_poweroff>destroy</on_poweroff>
> + <on_reboot>destroy</on_reboot>
> + <on_crash>destroy</on_crash>
> + <devices>
> + <interface type='bridge'>
> + <mac address='52:54:00:00:00:00'/>
> + <source bridge='virbr0'/>
> + <target dev='tap0'/>
> + <model type='e1000'/>
> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/>
> + </interface>
> + <interface type='bridge'>
> + <mac address='fe:ed:ad:ea:df:15'/>
> + <source bridge='virbr0'/>
> + <target dev='tap1'/>
> + <model type='e1000'/>
> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/>
> + </interface>
> + </devices>
> +</domain>
> diff --git a/tests/bhyveargv2xmldata/bhyveargv2xml-virtio-net.xml b/tests/bhyveargv2xmldata/bhyveargv2xml-virtio-net.xml
> index 5895c8c53..ed3279d9d 100644
> --- a/tests/bhyveargv2xmldata/bhyveargv2xml-virtio-net.xml
> +++ b/tests/bhyveargv2xmldata/bhyveargv2xml-virtio-net.xml
> @@ -16,12 +16,14 @@
> <mac address='52:54:00:00:00:00'/>
> <source bridge='virbr0'/>
> <target dev='tap0'/>
> + <model type='virtio'/>
> <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/>
> </interface>
> <interface type='bridge'>
> <mac address='fe:ed:ad:ea:df:15'/>
> <source bridge='virbr0'/>
> <target dev='tap1'/>
> + <model type='virtio'/>
> <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/>
> </interface>
> </devices>
> diff --git a/tests/bhyveargv2xmldata/bhyveargv2xml-virtio-net4.xml b/tests/bhyveargv2xmldata/bhyveargv2xml-virtio-net4.xml
> index 5f1972080..7c5493cbb 100644
> --- a/tests/bhyveargv2xmldata/bhyveargv2xml-virtio-net4.xml
> +++ b/tests/bhyveargv2xmldata/bhyveargv2xml-virtio-net4.xml
> @@ -16,6 +16,7 @@
> <mac address='00:00:00:00:00:00'/>
> <source bridge='virbr0'/>
> <target dev='tap1'/>
> + <model type='virtio'/>
> <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/>
> </interface>
> </devices>
> diff --git a/tests/bhyveargv2xmltest.c b/tests/bhyveargv2xmltest.c
> index 0995f6928..e759e4fa3 100644
> --- a/tests/bhyveargv2xmltest.c
> +++ b/tests/bhyveargv2xmltest.c
> @@ -175,6 +175,7 @@ mymain(void)
> DO_TEST("ahci-hd");
> DO_TEST("virtio-blk");
> DO_TEST("virtio-net");
> + DO_TEST("e1000");
> DO_TEST_WARN("virtio-net2");
> DO_TEST_WARN("virtio-net3");
> DO_TEST_WARN("virtio-net4");
> diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-net-e1000.args b/tests/bhyvexml2argvdata/bhyvexml2argv-net-e1000.args
> new file mode 100644
> index 000000000..09e30db46
> --- /dev/null
> +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-net-e1000.args
> @@ -0,0 +1,9 @@
> +/usr/sbin/bhyve \
> +-c 1 \
> +-m 214 \
> +-u \
> +-H \
> +-P \
> +-s 0:0,hostbridge \
> +-s 2:0,ahci,hd:/tmp/freebsd.img \
> +-s 3:0,e1000,faketapdev,mac=52:54:00:00:00:00 bhyve
> diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-net-e1000.ldargs b/tests/bhyvexml2argvdata/bhyvexml2argv-net-e1000.ldargs
> new file mode 100644
> index 000000000..32538b558
> --- /dev/null
> +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-net-e1000.ldargs
> @@ -0,0 +1,3 @@
> +/usr/sbin/bhyveload \
> +-m 214 \
> +-d /tmp/freebsd.img bhyve
> diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-net-e1000.xml b/tests/bhyvexml2argvdata/bhyvexml2argv-net-e1000.xml
> new file mode 100644
> index 000000000..805efe301
> --- /dev/null
> +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-net-e1000.xml
> @@ -0,0 +1,22 @@
> +<domain type='bhyve'>
> + <name>bhyve</name>
> + <uuid>df3be7e7-a104-11e3-aeb0-50e5492bd3dc</uuid>
> + <memory>219136</memory>
> + <vcpu>1</vcpu>
> + <os>
> + <type>hvm</type>
> + </os>
> + <devices>
> + <disk type='file'>
> + <driver name='file' type='raw'/>
> + <source file='/tmp/freebsd.img'/>
> + <target dev='hda' bus='sata'/>
> + <address type='drive' controller='0' bus='0' target='2' unit='0'/>
> + </disk>
> + <interface type='bridge'>
> + <model type='e1000'/>
> + <source bridge="virbr0"/>
> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/>
> + </interface>
> + </devices>
> +</domain>
> diff --git a/tests/bhyvexml2argvtest.c b/tests/bhyvexml2argvtest.c
> index e80705780..158f9617e 100644
> --- a/tests/bhyvexml2argvtest.c
> +++ b/tests/bhyvexml2argvtest.c
> @@ -162,7 +162,7 @@ mymain(void)
> DO_TEST_FULL(name, FLAG_EXPECT_PARSE_ERROR)
>
> driver.grubcaps = BHYVE_GRUB_CAP_CONSDEV;
> - driver.bhyvecaps = BHYVE_CAP_RTC_UTC | BHYVE_CAP_AHCI32SLOT;
> + driver.bhyvecaps = BHYVE_CAP_RTC_UTC | BHYVE_CAP_AHCI32SLOT | BHYVE_CAP_NET_E1000;
>
> DO_TEST("base");
> DO_TEST("acpiapic");
> @@ -185,6 +185,7 @@ mymain(void)
> DO_TEST("disk-cdrom-grub");
> DO_TEST("serial-grub");
> DO_TEST("localtime");
> + DO_TEST("net-e1000");
>
> /* Address allocation tests */
> DO_TEST("addr-single-sata-disk");
> @@ -201,6 +202,10 @@ mymain(void)
>
> DO_TEST("serial-grub-nocons");
>
> + driver.bhyvecaps &= ~BHYVE_CAP_NET_E1000;
> +
> + DO_TEST_FAILURE("net-e1000");
> +
> virObjectUnref(driver.caps);
> virObjectUnref(driver.xmlopt);
>
ACK (although I'd greatly prefer "out:" changed to "cleanup:")
More information about the libvir-list
mailing list