Re: [libvirt] [PATCHv2 2/4] Add wrapper macros around nla_nest*/nla_put* to make code more readable

This patch adds wrapper macros around nla_nest*/nla_put* which apply to virNetlinkNewLink and virNetDevSetVfConfig and virNetDevVPortProfileOpSetLink. Signed-off-by: Shi Lei <shi_lei(a)massclouds.com&gt; --- src/util/virnetdev.c | 54 ++++++-------- src/util/virnetdevvportprofile.c | 117 ++++++++++--------------------- src/util/virnetlink.c | 74 +++++++------------ src/util/virnetlink.h | 52 ++++++++++++++ 4 files changed, 134 insertions(+), 163 deletions(-) diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c index 8eac419..892a147 100644 --- a/src/util/virnetdev.c +++ b/src/util/virnetdev.c @@ -1657,11 +1657,11 @@ virNetDevSetVfConfig(const char *ifname, int vf, { int rc = -1; char macstr[VIR_MAC_STRING_BUFLEN]; - struct nlmsghdr *resp = NULL; struct nlmsgerr *err; unsigned int recvbuflen = 0; - struct nl_msg *nl_msg; struct nlattr *vfinfolist, *vfinfo; + VIR_AUTOPTR(virNetlinkMsg) nl_msg = NULL; + VIR_AUTOFREE(struct nlmsghdr *) resp = NULL;

struct ifinfomsg ifinfo = { .ifi_family = AF_UNSPEC, .ifi_index = -1, @@ -1676,47 +1676,41 @@ virNetDevSetVfConfig(const char *ifname, int vf, return rc; } - if (nlmsg_append(nl_msg, &ifinfo, sizeof(ifinfo), NLMSG_ALIGNTO) < 0) - goto buffer_too_small; - - if (ifname && - nla_put(nl_msg, IFLA_IFNAME, strlen(ifname)+1, ifname) < 0)

- goto buffer_too_small; - + if (nlmsg_append(nl_msg, &ifinfo, sizeof(ifinfo), NLMSG_ALIGNTO) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("nlmsg_append error")); + return rc; + } - if (!(vfinfolist = nla_nest_start(nl_msg, IFLA_VFINFO_LIST))) - goto buffer_too_small; + NETLINK_MSG_PUT_STRING(nl_msg, IFLA_IFNAME, ifname); - if (!(vfinfo = nla_nest_start(nl_msg, IFLA_VF_INFO))) - goto buffer_too_small; + NETLINK_MSG_NEST_START(nl_msg, vfinfolist, IFLA_VFINFO_LIST); + NETLINK_MSG_NEST_START(nl_msg, vfinfo, IFLA_VF_INFO); if (macaddr) { struct ifla_vf_mac ifla_vf_mac = { - .vf = vf, - .mac = { 0, }, + .vf = vf, + .mac = { 0, }, }; virMacAddrGetRaw(macaddr, ifla_vf_mac.mac); - if (nla_put(nl_msg, IFLA_VF_MAC, sizeof(ifla_vf_mac), - &ifla_vf_mac) < 0) - goto buffer_too_small; + NETLINK_MSG_PUT(nl_msg, IFLA_VF_MAC, + sizeof(ifla_vf_mac), &ifla_vf_mac); } if (vlanid >= 0) { struct ifla_vf_vlan ifla_vf_vlan = { - .vf = vf, - .vlan = vlanid, - .qos = 0, + .vf = vf, + .vlan = vlanid, + .qos = 0, }; - if (nla_put(nl_msg, IFLA_VF_VLAN, sizeof(ifla_vf_vlan), - &ifla_vf_vlan) < 0) - goto buffer_too_small; + NETLINK_MSG_PUT(nl_msg, IFLA_VF_VLAN, + sizeof(ifla_vf_vlan), &ifla_vf_vlan); } - nla_nest_end(nl_msg, vfinfo); - nla_nest_end(nl_msg, vfinfolist); + NETLINK_MSG_NEST_END(nl_msg, vfinfo); + NETLINK_MSG_NEST_END(nl_msg, vfinfolist); if (virNetlinkCommand(nl_msg, &resp, &recvbuflen, 0, 0, NETLINK_ROUTE, 0) < 0) @@ -1767,20 +1761,12 @@ virNetDevSetVfConfig(const char *ifname, int vf, ifname, vf, macaddr ? virMacAddrFormat(macaddr, macstr) : "(unchanged)", vlanid, rc < 0 ? "Fail" : "Success"); - - nlmsg_free(nl_msg); - VIR_FREE(resp); 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; } static int diff --git a/src/util/virnetdevvportprofile.c b/src/util/virnetdevvportprofile.c index 3ebf757..8574571 100644 --- a/src/util/virnetdevvportprofile.c +++ b/src/util/virnetdevvportprofile.c @@ -641,8 +641,6 @@ virNetDevVPortProfileOpSetLink(const char *ifname, int ifindex, int32_t vf, uint8_t op) { - int rc = -1; - struct nlmsghdr *resp = NULL; struct nlmsgerr *err; struct ifinfomsg ifinfo = { .ifi_family = AF_UNSPEC, @@ -651,12 +649,14 @@ virNetDevVPortProfileOpSetLink(const char *ifname, int ifindex, unsigned int recvbuflen = 0; int src_pid = 0; uint32_t dst_pid = 0; - struct nl_msg *nl_msg; struct nlattr *vfports = NULL, *vfport; char macStr[VIR_MAC_STRING_BUFLEN]; char hostUUIDStr[VIR_UUID_STRING_BUFLEN]; char instanceUUIDStr[VIR_UUID_STRING_BUFLEN]; const char *opName; + int vfport_type = IFLA_PORT_SELF; + VIR_AUTOPTR(virNetlinkMsg) nl_msg = NULL; + VIR_AUTOFREE(struct nlmsghdr *) resp = NULL;

switch (op) { case PORT_REQUEST_PREASSOCIATE: @@ -691,24 +691,21 @@ virNetDevVPortProfileOpSetLink(const char *ifname, int ifindex, nl_msg = nlmsg_alloc_simple(RTM_SETLINK, NLM_F_REQUEST); if (!nl_msg) { virReportOOMError(); - return rc; + return -1; } - if (nlmsg_append(nl_msg, &ifinfo, sizeof(ifinfo), NLMSG_ALIGNTO) < 0) - goto buffer_too_small; + if (nlmsg_append(nl_msg, &ifinfo, sizeof(ifinfo), NLMSG_ALIGNTO) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("nlmsg_append error")); + return -1; + } - if (ifname && - nla_put(nl_msg, IFLA_IFNAME, strlen(ifname)+1, ifname) < 0) - goto buffer_too_small; + NETLINK_MSG_PUT_STRING(nl_msg, IFLA_IFNAME, ifname); if (macaddr || vlanid >= 0) { struct nlattr *vfinfolist, *vfinfo; - if (!(vfinfolist = nla_nest_start(nl_msg, IFLA_VFINFO_LIST))) - goto buffer_too_small; - - if (!(vfinfo = nla_nest_start(nl_msg, IFLA_VF_INFO))) - goto buffer_too_small; + NETLINK_MSG_NEST_START(nl_msg, vfinfolist, IFLA_VFINFO_LIST); + NETLINK_MSG_NEST_START(nl_msg, vfinfo, IFLA_VF_INFO); if (macaddr) { struct ifla_vf_mac ifla_vf_mac = { @@ -718,9 +715,8 @@ virNetDevVPortProfileOpSetLink(const char *ifname, int ifindex, virMacAddrGetRaw(macaddr, ifla_vf_mac.mac); - if (nla_put(nl_msg, IFLA_VF_MAC, sizeof(ifla_vf_mac), - &ifla_vf_mac) < 0) - goto buffer_too_small; + NETLINK_MSG_PUT(nl_msg, IFLA_VF_MAC, + sizeof(ifla_vf_mac), &ifla_vf_mac); } if (vlanid >= 0) { @@ -730,77 +726,49 @@ virNetDevVPortProfileOpSetLink(const char *ifname, int ifindex, .qos = 0, }; - if (nla_put(nl_msg, IFLA_VF_VLAN, sizeof(ifla_vf_vlan), - &ifla_vf_vlan) < 0) - goto buffer_too_small; + NETLINK_MSG_PUT(nl_msg, IFLA_VF_VLAN, + sizeof(ifla_vf_vlan), &ifla_vf_vlan); } - nla_nest_end(nl_msg, vfinfo); - nla_nest_end(nl_msg, vfinfolist); - } - - if (vf == PORT_SELF_VF && nltarget_kernel) { - if (!(vfport = nla_nest_start(nl_msg, IFLA_PORT_SELF))) - goto buffer_too_small; - } else { - if (!(vfports = nla_nest_start(nl_msg, IFLA_VF_PORTS))) - goto buffer_too_small; - - /* begin nesting vfports */ - if (!(vfport = nla_nest_start(nl_msg, IFLA_VF_PORT))) - goto buffer_too_small; - }

/** diff --git a/src/util/virnetlink.h b/src/util/virnetlink.h index 8163a81..2e64570 100644 --- a/src/util/virnetlink.h +++ b/src/util/virnetlink.h @@ -48,6 +48,58 @@ struct nlmsghdr; # endif /* __linux__ */ + +# define NETLINK_MSG_NEST_START(msg, container, attrtype) \ +do { \ + container = nla_nest_start(msg, attrtype); \ + if (!container) { \ + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", \ + _("nla_nest_start error [" #attrtype "]")); \ + return -1; \ + } \ +} while(0) + +# define NETLINK_MSG_NEST_END(msg, container) \ +do { nla_nest_end(msg, container); } while(0) + +/* + * When data is an rvalue, compiler will report error as below: + * "the address of [...] will always evaluate as 'true' [-Werror=address]" + * Add a dummy(as an lvalue) to solve it.

+ */ +# define NETLINK_MSG_PUT(msg, attrtype, datalen, data) \ +do { \

+ const void *dummy = data; \

+ if (dummy && nla_put(msg, attrtype, datalen, data) < 0) { \ + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", \ + _("nla_put error [" #attrtype "]")); \ + return -1; \ + } \ +} while(0) + +# define NETLINK_MSG_PUT_STRING(msg, attrtype, str) \ +do { \ + const void *dummy = str; \ + if (dummy && nla_put_string(msg, attrtype, str) < 0) { \ + virReportError(VIR_ERR_INTERNAL_ERROR, \ + _("nla_put_string error [" #attrtype "]: '%s'"), \ + str); \

+ return -1; \ + } \ +} while(0)

+ return -1; \ + } \ +} while(0)