Re: [libvirt] [PATCH v3 2/2] bhyve: add e1000 nic support

13 Nov 2016


      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
snip
...
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);
^^^^^^^^^^ I noticed that this bit is not needed as we
free  realifname two lines beyond. I won't be sending v4 just because of
this, I'll squash this in if this one gets acked, or include with the
other stuff if there'll be some other issues with this version.
...
+    VIR_FREE(brname);
     VIR_FREE(realifname);
+    VIR_FREE(nic_model);
-    return 0;
+    return ret;
 }
Roman Bogorodskiy

    