[libvirt] [PATCH v2] bhyve: add e1000 nic support

Laine Stump laine at laine.org
Sun Nov 6 20:56:20 UTC 2016


On 11/06/2016 01:09 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
> ---

This patch won't apply to master with "git am -3". It doesn't even 
partially apply and leave merge conflict markers in the source, it does 
exactly nothing :-/.

Can you rebase and resend it? (Is there maybe a prerequisite patch that 
isn't upstream yet?)


>   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
>
> diff --git a/src/bhyve/bhyve_capabilities.c b/src/bhyve/bhyve_capabilities.c
> index be68e51..3012788 100644
> --- a/src/bhyve/bhyve_capabilities.c
> +++ b/src/bhyve/bhyve_capabilities.c
> @@ -192,6 +192,30 @@ bhyveProbeCapsRTC_UTC(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\"") == 0)
> +        *caps |= BHYVE_CAP_NET_E1000;
> +
> + out:
> +    VIR_FREE(error);
> +    virCommandFree(cmd);
> +    return ret;
> +}
> +
>   int
>   virBhyveProbeCaps(unsigned int *caps)
>   {
> @@ -207,6 +231,9 @@ virBhyveProbeCaps(unsigned int *caps)
>       if ((ret = bhyveProbeCapsRTC_UTC(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 8e7646d..d6c7bb0 100644
> --- a/src/bhyve/bhyve_capabilities.h
> +++ b/src/bhyve/bhyve_capabilities.h
> @@ -38,6 +38,7 @@ typedef enum {
>   
>   typedef enum {
>       BHYVE_CAP_RTC_UTC = 1,
> +    BHYVE_CAP_NET_E1000 = 2,
>   } virBhyveCapsFlags;
>   
>   int virBhyveProbeGrubCaps(virBhyveGrubCapsFlags *caps);
> diff --git a/src/bhyve/bhyve_command.c b/src/bhyve/bhyve_command.c
> index 022b03b..cfb8b45 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,
> +                    const virDomainDef *def,
>                       virDomainNetDefPtr net,
>                       virCommandPtr cmd,
>                       bool dryRun)
> @@ -52,26 +53,46 @@ 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)
> -            return -1;
> +            goto out;
>       } else {
>           virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>                          _("Network type %d is not supported"),
>                          virDomainNetGetActualType(net));
> -        return -1;
> +        goto out;
>       }
>   
>       if (!net->ifname ||
>           STRPREFIX(net->ifname, VIR_NET_GENERATED_PREFIX) ||
>           strchr(net->ifname, '%')) {
>           VIR_FREE(net->ifname);
> -        if (VIR_STRDUP(net->ifname, VIR_NET_GENERATED_PREFIX "%d") < 0) {
> -            VIR_FREE(brname);
> -            return -1;
> -        }
> +        if (VIR_STRDUP(net->ifname, VIR_NET_GENERATED_PREFIX "%d") < 0)
> +            goto out;
>       }
>   
>       if (!dryRun) {
> @@ -79,44 +100,41 @@ bhyveBuildNetArgStr(const virDomainDef *def,
>                                              def->uuid, NULL, NULL, 0,
>                                              virDomainNetGetActualVirtPortProfile(net),
>                                              virDomainNetGetActualVlan(net),
> -                                           VIR_NETDEV_TAP_CREATE_IFUP | VIR_NETDEV_TAP_CREATE_PERSIST) < 0) {
> -            VIR_FREE(net->ifname);
> -            VIR_FREE(brname);
> -            return -1;
> -        }
> +                                           VIR_NETDEV_TAP_CREATE_IFUP | VIR_NETDEV_TAP_CREATE_PERSIST) < 0)
> +            goto out;
>   
>           realifname = virNetDevTapGetRealDeviceName(net->ifname);
>   
> -        if (realifname == NULL) {
> -            VIR_FREE(net->ifname);
> -            VIR_FREE(brname);
> -            return -1;
> -        }
> +        if (realifname == NULL)
> +            goto out;
>   
>           VIR_DEBUG("%s -> %s", net->ifname, realifname);
>           /* hack on top of other hack: we need to set
>            * interface to 'UP' again after re-opening to find its
>            * name
>            */
> -        if (virNetDevSetOnline(net->ifname, true) != 0) {
> -            VIR_FREE(realifname);
> -            VIR_FREE(net->ifname);
> -            VIR_FREE(brname);
> -            return -1;
> -        }
> +        if (virNetDevSetOnline(net->ifname, true) != 0)
> +            goto out;
>       } else {
>           if (VIR_STRDUP(realifname, "tap0") < 0)
> -            return -1;
> +            goto out;
>       }
>   
>   
>       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;
> + out:
> +    VIR_FREE(net->ifname);
> +    VIR_FREE(realifname);
> +    VIR_FREE(brname);
>       VIR_FREE(realifname);
> +    VIR_FREE(nic_model);
>   
> -    return 0;
> +    return ret;
>   }
>   
>   static int
> @@ -284,7 +302,7 @@ virBhyveProcessBuildBhyveCmd(virConnectPtr conn,
>       /* Devices */
>       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 0ae7a83..deb3d96 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] */
> @@ -510,6 +511,8 @@ bhyveParsePCINet(virDomainDefPtr def,
>       /* Let's just assume it is VIR_DOMAIN_NET_TYPE_ETHERNET, it could also be
>        * a bridge, but this is the most generic option. */
>       net->type = VIR_DOMAIN_NET_TYPE_ETHERNET;
> +    if (VIR_STRDUP(net->model, model) < 0)
> +        goto error;
>   
>       net->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI;
>       net->info.addr.pci.slot = pcislot;
> @@ -617,7 +620,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 0000000..aa568fe
> --- /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 0000000..cd1ec5d
> --- /dev/null
> +++ b/tests/bhyveargv2xmldata/bhyveargv2xml-e1000.xml
> @@ -0,0 +1,28 @@
> +<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='ethernet'>
> +      <mac address='52:54:00:00:00:00'/>
> +      <target dev='tap0'/>
> +      <model type='e1000'/>
> +      <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/>
> +    </interface>
> +    <interface type='ethernet'>
> +      <mac address='fe:ed:ad:ea:df:15'/>
> +      <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 09cc79b..d920a09 100644
> --- a/tests/bhyveargv2xmldata/bhyveargv2xml-virtio-net.xml
> +++ b/tests/bhyveargv2xmldata/bhyveargv2xml-virtio-net.xml
> @@ -15,11 +15,13 @@
>       <interface type='ethernet'>
>         <mac address='52:54:00:00:00:00'/>
>         <target dev='tap0'/>
> +      <model type='virtio'/>
>         <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/>
>       </interface>
>       <interface type='ethernet'>
>         <mac address='fe:ed:ad:ea:df:15'/>
>         <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 e1bda46..6fbb3d2 100644
> --- a/tests/bhyveargv2xmldata/bhyveargv2xml-virtio-net4.xml
> +++ b/tests/bhyveargv2xmldata/bhyveargv2xml-virtio-net4.xml
> @@ -15,6 +15,7 @@
>       <interface type='ethernet'>
>         <mac address='00:00:00:00:00:00'/>
>         <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 0995f69..e759e4f 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 0000000..cd0e896
> --- /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 3:0,e1000,faketapdev,mac=52:54:00:00:00:00 \
> +-s 2:0,ahci-hd,/tmp/freebsd.img bhyve
> diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-net-e1000.ldargs b/tests/bhyvexml2argvdata/bhyvexml2argv-net-e1000.ldargs
> new file mode 100644
> index 0000000..32538b5
> --- /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 0000000..4b3148c
> --- /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='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/>
> +    </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 b85439b..a92b0cf 100644
> --- a/tests/bhyvexml2argvtest.c
> +++ b/tests/bhyvexml2argvtest.c
> @@ -151,7 +151,7 @@ mymain(void)
>       DO_TEST_FULL(name, FLAG_EXPECT_PARSE_ERROR)
>   
>       driver.grubcaps = BHYVE_GRUB_CAP_CONSDEV;
> -    driver.bhyvecaps = BHYVE_CAP_RTC_UTC;
> +    driver.bhyvecaps = BHYVE_CAP_RTC_UTC | BHYVE_CAP_NET_E1000;
>   
>       DO_TEST("base");
>       DO_TEST("acpiapic");
> @@ -174,11 +174,16 @@ mymain(void)
>       DO_TEST("disk-cdrom-grub");
>       DO_TEST("serial-grub");
>       DO_TEST("localtime");
> +    DO_TEST("net-e1000");
>   
>       driver.grubcaps = 0;
>   
>       DO_TEST("serial-grub-nocons");
>   
> +    driver.bhyvecaps &= ~BHYVE_CAP_NET_E1000;
> +
> +    DO_TEST_FAILURE("net-e1000");
> +
>       virObjectUnref(driver.caps);
>       virObjectUnref(driver.xmlopt);
>   





More information about the libvir-list mailing list