src/libvirt_private.syms | 1 + src/util/virnetdevbridge.c | 73 +++-------------------- src/util/virnetdevmacvlan.c | 137 ++++++++++++++------------------------------ src/util/virnetlink.c | 104 +++++++++++++++++++++++++++++++++ src/util/virnetlink.h | 8 +++ 5 files changed, 164 insertions(+), 159 deletions(-)
This patch adds virNetlinkNewLink for simplifying virNetDevMacVLanCreate
and virNetDevBridgeCreate.
Signed-off-by: Shi Lei <shi_lei@massclouds.com>
---
src/libvirt_private.syms | 1 +
src/util/virnetdevbridge.c | 73 +++--------------------
src/util/virnetdevmacvlan.c | 137 ++++++++++++++------------------------------
src/util/virnetlink.c | 104 +++++++++++++++++++++++++++++++++
src/util/virnetlink.h | 8 +++
5 files changed, 164 insertions(+), 159 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 47ea35f..23931ba 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2439,6 +2439,7 @@ virNetlinkEventServiceStop;
virNetlinkEventServiceStopAll;
virNetlinkGetErrorCode;
virNetlinkGetNeighbor;
+virNetlinkNewLink;
virNetlinkShutdown;
virNetlinkStartup;
diff --git a/src/util/virnetdevbridge.c b/src/util/virnetdevbridge.c
index bc377b5..1f5b37e 100644
--- a/src/util/virnetdevbridge.c
+++ b/src/util/virnetdevbridge.c
@@ -417,77 +417,22 @@ 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;
+ int error = 0;
- 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;
-
- if (err->error < 0) {
+ if (virNetlinkNewLink(NULL, brname, NULL, type, NULL, 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);
- }
-# endif
-
- virReportSystemError(-err->error,
- _("error creating bridge interface %s"),
- brname);
- return -1;
+ if (error == -EOPNOTSUPP) {
+ /* fallback to ioctl if netlink doesn't support creating bridges */
+ return virNetDevBridgeCreateWithIoctl(brname);
}
- break;
+# endif
+ virReportSystemError(-error, _("error creating bridge interface %s"),
+ brname);
- 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..1629add 100644
--- a/src/util/virnetdevmacvlan.c
+++ b/src/util/virnetdevmacvlan.c
@@ -283,6 +283,37 @@ virNetDevMacVLanReleaseName(const char *name)
}
+static int
+virNetDevMacVLanCreateCallback(struct nl_msg *nl_msg, const void *opaque)
+{
+ const uint32_t *mode = (const uint32_t *) opaque;
+ if (!nl_msg || !opaque) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("nl_msg %p or opaque %p is NULL"),
+ nl_msg, opaque);
+ return -1;
+ }
+
+ if (*mode > 0) {
+ struct nlattr *info_data;
+ if (!(info_data = nla_nest_start(nl_msg, IFLA_INFO_DATA)))
+ goto buffer_too_small;
+
+ if (nla_put(nl_msg, IFLA_MACVLAN_MODE, sizeof(*mode), mode) < 0)
+ goto buffer_too_small;
+
+ nla_nest_end(nl_msg, info_data);
+ }
+
+ return 0;
+
+ buffer_too_small:
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("allocated netlink buffer is too small"));
+ return -1;
+}
+
+
/**
* virNetDevMacVLanCreate:
*
@@ -307,113 +338,29 @@ 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;
*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:
+ if (virNetlinkNewLink(&ifindex, ifname, macaddress, type,
+ virNetDevMacVLanCreateCallback, &macvlan_mode,
+ &error) < 0) {
+ char macstr[VIR_MAC_STRING_BUFLEN];
+ if (error == -EEXIST)
*retry = 1;
- goto cleanup;
-
- default:
- virReportSystemError(-err->error,
+ else
+ 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;
}
/**
diff --git a/src/util/virnetlink.c b/src/util/virnetlink.c
index 8f06446..817e347 100644
--- a/src/util/virnetlink.c
+++ b/src/util/virnetlink.c
@@ -489,6 +489,110 @@ virNetlinkDumpLink(const char *ifname, int ifindex,
/**
+ * virNetlinkNewLink:
+ *
+ * @ifindex: The index for the 'link' device
+ * @ifname: The name of the link
+ * @mac: The MAC address of the device
+ * @type: The type of device, i.e., "bridge", "macvtap", "macvlan"
+ * @cb: The callback for filling in IFLA_INFO_DATA for this type
+ * @opaque: opaque for the callback
+ * @error: for retrieving error code
+ *
+ * Create a network "link" (aka interface aka device) with the given
+ * args. This works for many different types of network devices,
+ * including macvtap and bridges.
+ *
+ * Returns 0 on success, -1 on fatal error.
+ */
+int
+virNetlinkNewLink(const int *ifindex,
+ const char *ifname,
+ const virMacAddr *mac,
+ const char *type,
+ virNetlinkNewLinkCallback cb,
+ const void *opaque,
+ int *error)
+{
+ struct nlmsgerr *err;
+ struct nlattr *linkinfo;
+ unsigned int buflen;
+ struct ifinfomsg ifinfo = { .ifi_family = AF_UNSPEC };
+ VIR_AUTOPTR(virNetlinkMsg) nl_msg = NULL;
+ VIR_AUTOFREE(struct nlmsghdr *) resp = NULL;
+
+ *error = 0;
+
+ 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 (ifindex && nla_put_u32(nl_msg, IFLA_LINK, *ifindex) < 0)
+ goto buffer_too_small;
+
+ if (mac && nla_put(nl_msg, IFLA_ADDRESS, VIR_MAC_BUFLEN, mac) < 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 (cb && cb(nl_msg, opaque) < 0)
+ return -1;
+
+ nla_nest_end(nl_msg, linkinfo);
+
+ if (virNetlinkCommand(nl_msg, &resp, &buflen, 0, 0, NETLINK_ROUTE, 0) < 0)
+ return -1;
+
+ if (buflen < 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;
+
+ if (err->error < 0) {
+ *error = err->error;
+ return -1;
+ }
+ break;
+
+ case NLMSG_DONE:
+ break;
+
+ default:
+ goto malformed_resp;
+ }
+
+ 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;
+}
+
+
+/**
* virNetlinkDelLink:
*
* @ifname: Name of the link
diff --git a/src/util/virnetlink.h b/src/util/virnetlink.h
index 1e1e616..195c7bb 100644
--- a/src/util/virnetlink.h
+++ b/src/util/virnetlink.h
@@ -65,6 +65,14 @@ int virNetlinkDumpCommand(struct nl_msg *nl_msg,
unsigned int protocol, unsigned int groups,
void *opaque);
+typedef int (*virNetlinkNewLinkCallback)(struct nl_msg *nl_msg,
+ const void *opaque);
+
+int virNetlinkNewLink(const int *ifindex, const char *ifname,
+ const virMacAddr *macaddress, const char *type,
+ virNetlinkNewLinkCallback cb, const void *opaque,
+ int *error);
+
typedef int (*virNetlinkDelLinkFallback)(const char *ifname);
int virNetlinkDelLink(const char *ifname, virNetlinkDelLinkFallback fallback);
--
2.7.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Thu, Aug 23, 2018 at 12:15:08PM +0800, Shi Lei wrote: > This patch adds virNetlinkNewLink for simplifying virNetDevMacVLanCreate > and virNetDevBridgeCreate. > > Signed-off-by: Shi Lei <shi_lei@massclouds.com> > --- There are multiple changes happening in this patch so it will need to be split into several patches, see below... > src/libvirt_private.syms | 1 + > src/util/virnetdevbridge.c | 73 +++-------------------- > src/util/virnetdevmacvlan.c | 137 ++++++++++++++------------------------------ > src/util/virnetlink.c | 104 +++++++++++++++++++++++++++++++++ > src/util/virnetlink.h | 8 +++ > 5 files changed, 164 insertions(+), 159 deletions(-) > > diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms > index 47ea35f..23931ba 100644 > --- a/src/libvirt_private.syms > +++ b/src/libvirt_private.syms > @@ -2439,6 +2439,7 @@ virNetlinkEventServiceStop; > virNetlinkEventServiceStopAll; > virNetlinkGetErrorCode; > virNetlinkGetNeighbor; > +virNetlinkNewLink; > virNetlinkShutdown; > virNetlinkStartup; > > diff --git a/src/util/virnetdevbridge.c b/src/util/virnetdevbridge.c > index bc377b5..1f5b37e 100644 > --- a/src/util/virnetdevbridge.c > +++ b/src/util/virnetdevbridge.c > @@ -417,77 +417,22 @@ 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; > + int error = 0; > > - 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; > - > - if (err->error < 0) { > + if (virNetlinkNewLink(NULL, brname, NULL, type, NULL, 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); > - } > -# endif > - > - virReportSystemError(-err->error, > - _("error creating bridge interface %s"), > - brname); > - return -1; > + if (error == -EOPNOTSUPP) { > + /* fallback to ioctl if netlink doesn't support creating bridges */ > + return virNetDevBridgeCreateWithIoctl(brname); > } > - break; > +# endif > + virReportSystemError(-error, _("error creating bridge interface %s"), > + brname); > > - 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..1629add 100644 > --- a/src/util/virnetdevmacvlan.c > +++ b/src/util/virnetdevmacvlan.c > @@ -283,6 +283,37 @@ virNetDevMacVLanReleaseName(const char *name) > } > > > +static int > +virNetDevMacVLanCreateCallback(struct nl_msg *nl_msg, const void *opaque) > +{ > + const uint32_t *mode = (const uint32_t *) opaque; > + if (!nl_msg || !opaque) { > + virReportError(VIR_ERR_INTERNAL_ERROR, > + _("nl_msg %p or opaque %p is NULL"), > + nl_msg, opaque); > + return -1; > + } > + > + if (*mode > 0) { > + struct nlattr *info_data; > + if (!(info_data = nla_nest_start(nl_msg, IFLA_INFO_DATA))) > + goto buffer_too_small; > + > + if (nla_put(nl_msg, IFLA_MACVLAN_MODE, sizeof(*mode), mode) < 0) > + goto buffer_too_small; > + > + nla_nest_end(nl_msg, info_data); > + } > + > + return 0; > + > + buffer_too_small: > + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", > + _("allocated netlink buffer is too small")); > + return -1; > +} Having a callback just to add a nested data into nl_msg doesn't feel like the right approach, I'd expect a callback to do more specific stuff, this should be special-cased in virNetlinkNewLink. > + > + > /** > * virNetDevMacVLanCreate: > * > @@ -307,113 +338,29 @@ virNetDevMacVLanCreate(const char *ifname, > uint32_t macvlan_mode, > int *retry) > { ^This replacement would be the last patch in the series. > - 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; > *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: > + if (virNetlinkNewLink(&ifindex, ifname, macaddress, type, > + virNetDevMacVLanCreateCallback, &macvlan_mode, > + &error) < 0) { > + char macstr[VIR_MAC_STRING_BUFLEN]; > + if (error == -EEXIST) > *retry = 1; > - goto cleanup; > - > - default: > - virReportSystemError(-err->error, > + else > + 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; > } > > /** > diff --git a/src/util/virnetlink.c b/src/util/virnetlink.c > index 8f06446..817e347 100644 > --- a/src/util/virnetlink.c > +++ b/src/util/virnetlink.c > @@ -489,6 +489,110 @@ virNetlinkDumpLink(const char *ifname, int ifindex, > > > /** > + * virNetlinkNewLink: Introduction of this function should come in a separate patch before you start replacing the existing ones. > + * > + * @ifindex: The index for the 'link' device > + * @ifname: The name of the link > + * @mac: The MAC address of the device Most of ^these attributes could be packed into a virNetlinkNewLinkData structure (or something similarly named). You'll have to test check for VIR_DOMAIN_NET_TYPE_DIRECT and VIR_NETDEV_MACVLAN_MODE_VEPA according to my comments below inside <comment_block>... > + * @type: The type of device, i.e., "bridge", "macvtap", "macvlan" > + * @cb: The callback for filling in IFLA_INFO_DATA for this type I don't think callback will be necessary > + * @opaque: opaque for the callback > + * @error: for retrieving error code ^see below for my comment on return value... > + * > + * Create a network "link" (aka interface aka device) with the given > + * args. This works for many different types of network devices, > + * including macvtap and bridges. > + * > + * Returns 0 on success, -1 on fatal error. Since this is a nice libvirt wrapper around the netlink communication machinery, I think that returning -<netlink_error> in case of an error is reasonable. Then, each of the callers of this API can handle the return values as they please. > + */ > +int > +virNetlinkNewLink(const int *ifindex, > + const char *ifname, > + const virMacAddr *mac, > + const char *type, > + virNetlinkNewLinkCallback cb, > + const void *opaque, > + int *error) > +{ > + struct nlmsgerr *err; > + struct nlattr *linkinfo; > + unsigned int buflen; > + struct ifinfomsg ifinfo = { .ifi_family = AF_UNSPEC }; > + VIR_AUTOPTR(virNetlinkMsg) nl_msg = NULL; > + VIR_AUTOFREE(struct nlmsghdr *) resp = NULL; > + > + *error = 0; > + > + 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; > + <comment_block> > + if (ifindex && nla_put_u32(nl_msg, IFLA_LINK, *ifindex) < 0) pre-existing, but is there a particular reason why ^this has to be u32?? > + goto buffer_too_small; > + > + if (mac && nla_put(nl_msg, IFLA_ADDRESS, VIR_MAC_BUFLEN, mac) < 0) > + goto buffer_too_small; Similarly, is there a reason why ^this is not necessary to be u32? > + > + 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))) Since we're already touching the code, I would find it more readable if ^this was written as linkinfo = nla_nest_start. The reason for that is that it's not immediately visible where the matching counterpart to "nla_nest_end" is. > + goto buffer_too_small; > + > + if (nla_put(nl_msg, IFLA_INFO_KIND, strlen(type), type) < 0) I'm puzzled why ^this doesn't need strlen(foo)+1 instead of plain strlen(foo) > + goto buffer_too_small; </comment_block> We could actually create a wrapper macro around nla_put which would make the block (begin-end) tiny more readable. See my notes above, as I have further questions...Now, the macros would again be a separate patch to make the change more gradual. Depending on the comments to my questions above, the macro would either take the size of the type and always call nla_put or a specialized nla_put_<string|u32|whatever> function. I'm CC'ing Laine to help answering my questions inside <comment_block>. Erik > + > + if (cb && cb(nl_msg, opaque) < 0) > + return -1; > + > + nla_nest_end(nl_msg, linkinfo); > + > + if (virNetlinkCommand(nl_msg, &resp, &buflen, 0, 0, NETLINK_ROUTE, 0) < 0) > + return -1; > + > + if (buflen < 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; > + > + if (err->error < 0) { > + *error = err->error; > + return -1; > + } > + break; > + > + case NLMSG_DONE: > + break; > + > + default: > + goto malformed_resp; > + } > + > + 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; > +} > + > + > +/** > * virNetlinkDelLink: > * > * @ifname: Name of the link > diff --git a/src/util/virnetlink.h b/src/util/virnetlink.h > index 1e1e616..195c7bb 100644 > --- a/src/util/virnetlink.h > +++ b/src/util/virnetlink.h > @@ -65,6 +65,14 @@ int virNetlinkDumpCommand(struct nl_msg *nl_msg, > unsigned int protocol, unsigned int groups, > void *opaque); > > +typedef int (*virNetlinkNewLinkCallback)(struct nl_msg *nl_msg, > + const void *opaque); > + > +int virNetlinkNewLink(const int *ifindex, const char *ifname, > + const virMacAddr *macaddress, const char *type, > + virNetlinkNewLinkCallback cb, const void *opaque, > + int *error); > + > typedef int (*virNetlinkDelLinkFallback)(const char *ifname); > > int virNetlinkDelLink(const char *ifname, virNetlinkDelLinkFallback fallback); > -- > 2.7.4 > > > -- > libvir-list mailing list > libvir-list@redhat.com > https://www.redhat.com/mailman/listinfo/libvir-list -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2024 Red Hat, Inc.