This patch simplifies virNetDevBridgeCreate and virNetDevMacVLanCreate
by using virNetlinkNewLink.

Signed-off-by: Shi Lei <shi_lei@massclouds.com>
---
 src/util/virnetdevbridge.c  |  73 +++---------------------
 src/util/virnetdevmacvlan.c | 110 +++++-------------------------------
 2 files changed, 24 insertions(+), 159 deletions(-)

diff --git a/src/util/virnetdevbridge.c b/src/util/virnetdevbridge.c
index bc377b5..ed2db27 100644
--- a/src/util/virnetdevbridge.c
+++ b/src/util/virnetdevbridge.c
@@ -416,78 +416,23 @@ int
 virNetDevBridgeCreate(const char *brname)
 {
     /* use a netlink RTM_NEWLINK message to create the bridge */
-    const char *type = "bridge";
-    struct nlmsgerr *err;
-    struct ifinfomsg ifinfo = { .ifi_family = AF_UNSPEC };
-    unsigned int recvbuflen;
-    struct nlattr *linkinfo;
-    VIR_AUTOPTR(virNetlinkMsg) nl_msg = NULL;
-    VIR_AUTOFREE(struct nlmsghdr *) resp = NULL;
-
-    nl_msg = nlmsg_alloc_simple(RTM_NEWLINK,
-                                NLM_F_REQUEST | NLM_F_CREATE | NLM_F_EXCL);
-    if (!nl_msg) {
-        virReportOOMError();
-        return -1;
-    }
-
-    if (nlmsg_append(nl_msg,  &ifinfo, sizeof(ifinfo), NLMSG_ALIGNTO) < 0)
-        goto buffer_too_small;
-    if (nla_put(nl_msg, IFLA_IFNAME, strlen(brname)+1, brname) < 0)
-        goto buffer_too_small;
-    if (!(linkinfo = nla_nest_start(nl_msg, IFLA_LINKINFO)))
-        goto buffer_too_small;
-    if (nla_put(nl_msg, IFLA_INFO_KIND, strlen(type), type) < 0)
-        goto buffer_too_small;
-    nla_nest_end(nl_msg, linkinfo);
-
-    if (virNetlinkCommand(nl_msg, &resp, &recvbuflen, 0, 0,
-                          NETLINK_ROUTE, 0) < 0) {
-        return -1;
-    }
-
-    if (recvbuflen < NLMSG_LENGTH(0) || resp == NULL)
-        goto malformed_resp;
-
-    switch (resp->nlmsg_type) {
-    case NLMSG_ERROR:
-        err = (struct nlmsgerr *)NLMSG_DATA(resp);
-        if (resp->nlmsg_len < NLMSG_LENGTH(sizeof(*err)))
-            goto malformed_resp;
+    int error = 0;
 
-        if (err->error < 0) {
+    if (virNetlinkNewLink(brname, "bridge", NULL, &error) < 0) {
 # if defined(HAVE_STRUCT_IFREQ) && defined(SIOCBRADDBR)
-            if (err->error == -EOPNOTSUPP) {
-                /* fallback to ioctl if netlink doesn't support creating
-                 * bridges
-                 */
-                return virNetDevBridgeCreateWithIoctl(brname);
-            }
+        if (error == -EOPNOTSUPP) {
+            /* fallback to ioctl if netlink doesn't support creating bridges */
+            return virNetDevBridgeCreateWithIoctl(brname);
+        }
 # endif
-
-            virReportSystemError(-err->error,
-                                 _("error creating bridge interface %s"),
+        if (error < 0)
+            virReportSystemError(-error, _("error creating bridge interface %s"),
                                  brname);
-            return -1;
-        }
-        break;
 
-    case NLMSG_DONE:
-        break;
-    default:
-        goto malformed_resp;
+        return -1;
     }
 
     return 0;
-
- malformed_resp:
-    virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-                   _("malformed netlink response message"));
-    return -1;
- buffer_too_small:
-    virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-                   _("allocated netlink buffer is too small"));
-    return -1;
 }
 
 
diff --git a/src/util/virnetdevmacvlan.c b/src/util/virnetdevmacvlan.c
index 2035b1f..a66ab59 100644
--- a/src/util/virnetdevmacvlan.c
+++ b/src/util/virnetdevmacvlan.c
@@ -307,113 +307,33 @@ virNetDevMacVLanCreate(const char *ifname,
                        uint32_t macvlan_mode,
                        int *retry)
 {
-    int rc = -1;
-    struct nlmsgerr *err;
-    struct ifinfomsg ifinfo = { .ifi_family = AF_UNSPEC };
-    int ifindex;
-    unsigned int recvbuflen;
-    struct nl_msg *nl_msg;
-    struct nlattr *linkinfo, *info_data;
-    char macstr[VIR_MAC_STRING_BUFLEN];
-    VIR_AUTOFREE(struct nlmsghdr *) resp = NULL;
-
-    if (virNetDevGetIndex(srcdev, &ifindex) < 0)
-        return -1;
+    int error = 0;
+    int ifindex = 0;
+    virNetlinkNewLinkData data = {
+        .macvlan_mode = &macvlan_mode,
+        .mac = macaddress,
+    };
 
     *retry = 0;
 
-    nl_msg = nlmsg_alloc_simple(RTM_NEWLINK,
-                                NLM_F_REQUEST | NLM_F_CREATE | NLM_F_EXCL);
-    if (!nl_msg) {
-        virReportOOMError();
+    if (virNetDevGetIndex(srcdev, &ifindex) < 0)
         return -1;
-    }
-
-    if (nlmsg_append(nl_msg,  &ifinfo, sizeof(ifinfo), NLMSG_ALIGNTO) < 0)
-        goto buffer_too_small;
-
-    if (nla_put_u32(nl_msg, IFLA_LINK, ifindex) < 0)
-        goto buffer_too_small;
-
-    if (nla_put(nl_msg, IFLA_ADDRESS, VIR_MAC_BUFLEN, macaddress) < 0)
-        goto buffer_too_small;
-
-    if (ifname &&
-        nla_put(nl_msg, IFLA_IFNAME, strlen(ifname)+1, ifname) < 0)
-        goto buffer_too_small;
-
-    if (!(linkinfo = nla_nest_start(nl_msg, IFLA_LINKINFO)))
-        goto buffer_too_small;
-
-    if (nla_put(nl_msg, IFLA_INFO_KIND, strlen(type), type) < 0)
-        goto buffer_too_small;
-
-    if (macvlan_mode > 0) {
-        if (!(info_data = nla_nest_start(nl_msg, IFLA_INFO_DATA)))
-            goto buffer_too_small;
-
-        if (nla_put(nl_msg, IFLA_MACVLAN_MODE, sizeof(macvlan_mode),
-                    &macvlan_mode) < 0)
-            goto buffer_too_small;
-
-        nla_nest_end(nl_msg, info_data);
-    }
 
-    nla_nest_end(nl_msg, linkinfo);
-
-    if (virNetlinkCommand(nl_msg, &resp, &recvbuflen, 0, 0,
-                          NETLINK_ROUTE, 0) < 0) {
-        goto cleanup;
-    }
-
-    if (recvbuflen < NLMSG_LENGTH(0) || resp == NULL)
-        goto malformed_resp;
-
-    switch (resp->nlmsg_type) {
-    case NLMSG_ERROR:
-        err = (struct nlmsgerr *)NLMSG_DATA(resp);
-        if (resp->nlmsg_len < NLMSG_LENGTH(sizeof(*err)))
-            goto malformed_resp;
-
-        switch (err->error) {
-
-        case 0:
-            break;
-
-        case -EEXIST:
+    data.ifindex = &ifindex;
+    if (virNetlinkNewLink(ifname, type, &data, &error) < 0) {
+        char macstr[VIR_MAC_STRING_BUFLEN];
+        if (error == -EEXIST)
             *retry = 1;
-            goto cleanup;
-
-        default:
-            virReportSystemError(-err->error,
+        else if (error < 0)
+            virReportSystemError(-error,
                                  _("error creating %s interface %s@%s (%s)"),
                                  type, ifname, srcdev,
                                  virMacAddrFormat(macaddress, macstr));
-            goto cleanup;
-        }
-        break;
-
-    case NLMSG_DONE:
-        break;
 
-    default:
-        goto malformed_resp;
+        return -1;
     }
 
-    rc = 0;
- cleanup:
-    nlmsg_free(nl_msg);
-    return rc;
-
- malformed_resp:
-    virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-                   _("malformed netlink response message"));
-    goto cleanup;
-
- buffer_too_small:
-    virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-                   _("allocated netlink buffer is too small"));
-    goto cleanup;
+    return 0;
 }
 
 /**
-- 
2.17.1


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On Fri, Sep 07, 2018 at 03:17:26PM +0800, Shi Lei wrote:
> This patch simplifies virNetDevBridgeCreate and virNetDevMacVLanCreate
> by using virNetlinkNewLink.
>
> Signed-off-by: Shi Lei <shi_lei@massclouds.com>
> ---

I'll make a tiny cosmetic change to the commit message before merging.
Reviewed-by: Erik Skultety <eskultet@redhat.com>

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list