Scott Feldman wrote:
> On 5/25/10 10:24 AM, "Stefan Berger" <stefanb@xxxxxxxxxxxxxxxxxx> wrote:
>
>> Reposting due to malformatted patch.
>
> Thanks for fixing the malformed issue.
>
> My testing is done with this version v7 plus the other patches list below.
> No issues.
>
> ACK.
>
>> [PATCH v10] vepa: parsing for 802.1Qb{g|h} XML
>> [RFC][PATCH 1/3] vepa+vsi: Introduce dependency on libnl
Since this one is already pushed,
>> [PATCH v3] Add host UUID (to libvirt capabilities)
I applied the other two, and then v7.
Once I fixed the unrelated "make check" failure,
I ran "make check" through valgrind and determined
that the "definite" leaks thus exposed are benign.
One bunch applies only to a test and the other is here:
80 bytes in 1 blocks are definitely lost in loss record 19 of 19
calloc (vg_replace_malloc.c:418)
virAlloc (memory.c:100)
virLastErrorObject (virterror.c:277)
virResetLastError (virterror.c:418)
vshDeinit (virsh.c:10285)
main (virsh.c:10482)
ACK.
For reference, I also computed the v6-v7 incremental, reviewed
it and attach it below:
diff --git a/configure.ac b/configure.ac
index 885b0ae..777dddc 100644
--- a/configure.ac
+++ b/configure.ac
@@ -2024,7 +2024,7 @@ dnl netlink library
LIBNL_CFLAGS=""
LIBNL_LIBS=""
-if test "$with_macvtap" = "yes" || "$with_virtualport" = "yes"; then
+if test "$with_macvtap" = "yes" || test "$with_virtualport" = "yes"; then
PKG_CHECK_MODULES([LIBNL], [libnl-1 >= $LIBNL_REQUIRED], [
], [
AC_MSG_ERROR([libnl >= $LIBNL_REQUIRED is required for macvtap support])
diff --git a/src/util/macvtap.c b/src/util/macvtap.c
index 0224c65..ecaa1e6 100644
--- a/src/util/macvtap.c
+++ b/src/util/macvtap.c
@@ -63,6 +63,13 @@
# define MICROSEC_PER_SEC (1000 * 1000)
+# define NLMSGBUF_SIZE 256
+# define RATTBUF_SIZE 64
+
+
+# define STATUS_POLL_TIMEOUT_USEC (10 * MICROSEC_PER_SEC)
+# define STATUS_POLL_INTERVL_USEC (MICROSEC_PER_SEC / 8)
+
static int associatePortProfileId(const char *macvtap_ifname,
const char *linkdev,
@@ -108,7 +115,7 @@ static void nlClose(int fd)
*/
static
int nlComm(struct nlmsghdr *nlmsg,
- char **respbuf, int *respbuflen)
+ char **respbuf, unsigned int *respbuflen)
{
int rc = 0;
struct sockaddr_nl nladdr = {
@@ -180,8 +187,8 @@ err_exit:
* @respbuf: pointer to pointer where response buffer will be allocated
* @respbuflen: pointer to integer holding the size of the response buffer
* on return of the function.
- * @to_usecs: timeout in microseconds to wait for a success message
- * to be returned
+ * @timeout_usecs: timeout in microseconds to wait for a success message
+ * to be returned
*
* Send the given message to the netlink multicast group and receive
* responses. Skip responses indicating an error and keep on receiving
@@ -190,8 +197,9 @@ err_exit:
* buffer will be returned.
*/
static int
-nlCommWaitSuccess(struct nlmsghdr *nlmsg, int nl_groups,
- char **respbuf, int *respbuflen, long to_usecs)
+nlCommWaitSuccess(struct nlmsghdr *nlmsg, uint32_t nl_groups,
+ char **respbuf, unsigned int *respbuflen,
+ unsigned long long timeout_usecs)
{
int rc = 0;
struct sockaddr_nl nladdr = {
@@ -200,15 +208,13 @@ nlCommWaitSuccess(struct nlmsghdr *nlmsg, int nl_groups,
.nl_groups = nl_groups,
};
int rcvChunkSize = 1024; // expecting less than that
- int rcvoffset = 0;
+ size_t rcv_offset = 0;
ssize_t nbytes;
- int n;
struct timeval tv = {
- .tv_sec = to_usecs / MICROSEC_PER_SEC,
- .tv_usec = to_usecs % MICROSEC_PER_SEC,
+ .tv_sec = timeout_usecs / MICROSEC_PER_SEC,
+ .tv_usec = timeout_usecs % MICROSEC_PER_SEC,
};
- fd_set rfds;
- bool gotvalid = false;
+ bool got_valid = false;
int fd = nlOpen();
static uint32_t seq = 0x1234;
uint32_t myseq = seq++;
@@ -230,12 +236,16 @@ nlCommWaitSuccess(struct nlmsghdr *nlmsg, int nl_groups,
goto err_exit;
}
- while (!gotvalid) {
- rcvoffset = 0;
+ while (!got_valid) {
+
+ rcv_offset = 0;
+
while (1) {
+ int n;
+ fd_set rfds;
socklen_t addrlen = sizeof(nladdr);
- if (VIR_REALLOC_N(*respbuf, rcvoffset+rcvChunkSize) < 0) {
+ if (VIR_REALLOC_N(*respbuf, rcv_offset + rcvChunkSize) < 0) {
virReportOOMError();
rc = -1;
goto err_exit;
@@ -245,12 +255,18 @@ nlCommWaitSuccess(struct nlmsghdr *nlmsg, int nl_groups,
FD_SET(fd, &rfds);
n = select(fd + 1, &rfds, NULL, NULL, &tv);
- if (n == 0) {
+ if (n <= 0) {
+ if (n < 0)
+ virReportSystemError(errno, "%s",
+ _("error in select call"));
+ if (n == 0)
+ virReportSystemError(ETIMEDOUT, "%s",
+ _("no valid netlink response was received"));
rc = -1;
goto err_exit;
}
- nbytes = recvfrom(fd, &((*respbuf)[rcvoffset]), rcvChunkSize, 0,
+ nbytes = recvfrom(fd, &((*respbuf)[rcv_offset]), rcvChunkSize, 0,
(struct sockaddr *)&nladdr, &addrlen);
if (nbytes < 0) {
if (errno == EAGAIN || errno == EINTR)
@@ -260,10 +276,10 @@ nlCommWaitSuccess(struct nlmsghdr *nlmsg, int nl_groups,
rc = -1;
goto err_exit;
}
- rcvoffset += nbytes;
+ rcv_offset += nbytes;
break;
}
- *respbuflen = rcvoffset;
+ *respbuflen = rcv_offset;
/* check message for error */
if (*respbuflen > NLMSG_LENGTH(0) && *respbuf != NULL) {
@@ -282,25 +298,23 @@ nlCommWaitSuccess(struct nlmsghdr *nlmsg, int nl_groups,
case NLMSG_ERROR:
err = (struct nlmsgerr *)NLMSG_DATA(resp);
if (resp->nlmsg_len >= NLMSG_LENGTH(sizeof(*err))) {
- if (-err->error != EOPNOTSUPP) {
+ if (err->error != -EOPNOTSUPP) {
/* assuming error msg from daemon */
- gotvalid = true;
+ got_valid = true;
break;
}
}
/* whatever this is, skip it */
VIR_FREE(*respbuf);
- *respbuf = NULL;
*respbuflen = 0;
break;
case NLMSG_DONE:
- gotvalid = true;
+ got_valid = true;
break;
default:
VIR_FREE(*respbuf);
- *respbuf = NULL;
*respbuflen = 0;
break;
}
@@ -310,7 +324,6 @@ nlCommWaitSuccess(struct nlmsghdr *nlmsg, int nl_groups,
err_exit:
if (rc == -1) {
VIR_FREE(*respbuf);
- *respbuf = NULL;
*respbuflen = 0;
}
@@ -376,15 +389,15 @@ link_add(const char *type,
int *retry)
{
int rc = 0;
- char nlmsgbuf[256];
+ char nlmsgbuf[NLMSGBUF_SIZE];
struct nlmsghdr *nlm = (struct nlmsghdr *)nlmsgbuf, *resp;
struct nlmsgerr *err;
- char rtattbuf[64];
+ char rtattbuf[RATTBUF_SIZE];
struct rtattr *rta, *rta1, *li;
- struct ifinfomsg i = { .ifi_family = AF_UNSPEC };
+ struct ifinfomsg ifinfo = { .ifi_family = AF_UNSPEC };
int ifindex;
char *recvbuf = NULL;
- int recvbuflen;
+ unsigned int recvbuflen;
if (ifaceGetIndex(true, srcdev, &ifindex) != 0)
return -1;
@@ -395,65 +408,46 @@ link_add(const char *type,
nlInit(nlm, NLM_F_REQUEST | NLM_F_CREATE | NLM_F_EXCL, RTM_NEWLINK);
- if (!nlAppend(nlm, sizeof(nlmsgbuf), &i, sizeof(i)))
+ if (!nlAppend(nlm, sizeof(nlmsgbuf), &ifinfo, sizeof(ifinfo)))
goto buffer_too_small;
rta = rtattrCreate(rtattbuf, sizeof(rtattbuf), IFLA_LINK,
&ifindex, sizeof(ifindex));
- if (!rta)
- goto buffer_too_small;
-
- if (!nlAppend(nlm, sizeof(nlmsgbuf), rtattbuf, rta->rta_len))
+ if (!rta || !nlAppend(nlm, sizeof(nlmsgbuf), rtattbuf, rta->rta_len))
goto buffer_too_small;
rta = rtattrCreate(rtattbuf, sizeof(rtattbuf), IFLA_ADDRESS,
macaddress, macaddrsize);
- if (!rta)
- goto buffer_too_small;
-
- if (!nlAppend(nlm, sizeof(nlmsgbuf), rtattbuf, rta->rta_len))
+ if (!rta || !nlAppend(nlm, sizeof(nlmsgbuf), rtattbuf, rta->rta_len))
goto buffer_too_small;
if (ifname) {
rta = rtattrCreate(rtattbuf, sizeof(rtattbuf), IFLA_IFNAME,
ifname, strlen(ifname) + 1);
- if (!rta)
- goto buffer_too_small;
-
- if (!nlAppend(nlm, sizeof(nlmsgbuf), rtattbuf, rta->rta_len))
+ if (!rta || !nlAppend(nlm, sizeof(nlmsgbuf), rtattbuf, rta->rta_len))
goto buffer_too_small;
}
rta = rtattrCreate(rtattbuf, sizeof(rtattbuf), IFLA_LINKINFO, NULL, 0);
- if (!rta)
- goto buffer_too_small;
-
- if (!(li = nlAppend(nlm, sizeof(nlmsgbuf), rtattbuf, rta->rta_len)))
+ if (!rta ||
+ !(li = nlAppend(nlm, sizeof(nlmsgbuf), rtattbuf, rta->rta_len)))
goto buffer_too_small;
rta = rtattrCreate(rtattbuf, sizeof(rtattbuf), IFLA_INFO_KIND,
type, strlen(type));
- if (!rta)
- goto buffer_too_small;
-
- if (!nlAppend(nlm, sizeof(nlmsgbuf), rtattbuf, rta->rta_len))
+ if (!rta || !nlAppend(nlm, sizeof(nlmsgbuf), rtattbuf, rta->rta_len))
goto buffer_too_small;
if (macvlan_mode > 0) {
rta = rtattrCreate(rtattbuf, sizeof(rtattbuf), IFLA_INFO_DATA,
NULL, 0);
- if (!rta)
- goto buffer_too_small;
-
- if (!(rta1 = nlAppend(nlm, sizeof(nlmsgbuf), rtattbuf, rta->rta_len)))
+ if (!rta ||
+ !(rta1 = nlAppend(nlm, sizeof(nlmsgbuf), rtattbuf, rta->rta_len)))
goto buffer_too_small;
rta = rtattrCreate(rtattbuf, sizeof(rtattbuf), IFLA_MACVLAN_MODE,
&macvlan_mode, sizeof(macvlan_mode));
- if (!rta)
- goto buffer_too_small;
-
- if (!nlAppend(nlm, sizeof(nlmsgbuf), rtattbuf, rta->rta_len))
+ if (!rta || !nlAppend(nlm, sizeof(nlmsgbuf), rtattbuf, rta->rta_len))
goto buffer_too_small;
rta1->rta_len = (char *)nlm + nlm->nlmsg_len - (char *)rta1;
@@ -475,15 +469,15 @@ link_add(const char *type,
if (resp->nlmsg_len < NLMSG_LENGTH(sizeof(*err)))
goto malformed_resp;
- switch (-err->error) {
+ switch (err->error) {
case 0:
- break;
+ break;
- case EEXIST:
+ case -EEXIST:
*retry = 1;
rc = -1;
- break;
+ break;
default:
virReportSystemError(-err->error,
@@ -491,10 +485,10 @@ link_add(const char *type,
type);
rc = -1;
}
- break;
+ break;
case NLMSG_DONE:
- break;
+ break;
default:
goto malformed_resp;
@@ -521,14 +515,14 @@ static int
link_del(const char *name)
{
int rc = 0;
- char nlmsgbuf[256];
+ char nlmsgbuf[NLMSGBUF_SIZE];
struct nlmsghdr *nlm = (struct nlmsghdr *)nlmsgbuf, *resp;
struct nlmsgerr *err;
- char rtattbuf[64];
+ char rtattbuf[RATTBUF_SIZE];
struct rtattr *rta;
struct ifinfomsg ifinfo = { .ifi_family = AF_UNSPEC };
char *recvbuf = NULL;
- int recvbuflen;
+ unsigned int recvbuflen;
memset(&nlmsgbuf, 0, sizeof(nlmsgbuf));
@@ -539,10 +533,7 @@ link_del(const char *name)
rta = rtattrCreate(rtattbuf, sizeof(rtattbuf), IFLA_IFNAME,
name, strlen(name)+1);
- if (!rta)
- goto buffer_too_small;
-
- if (!nlAppend(nlm, sizeof(nlmsgbuf), rtattbuf, rta->rta_len))
+ if (!rta || !nlAppend(nlm, sizeof(nlmsgbuf), rtattbuf, rta->rta_len))
goto buffer_too_small;
if (nlComm(nlm, &recvbuf, &recvbuflen) < 0)
@@ -559,20 +550,16 @@ link_del(const char *name)
if (resp->nlmsg_len < NLMSG_LENGTH(sizeof(*err)))
goto malformed_resp;
- switch (-err->error) {
- case 0:
- break;
-
- default:
+ if (err->error) {
virReportSystemError(-err->error,
_("error destroying %s interface"),
name);
rc = -1;
}
- break;
+ break;
case NLMSG_DONE:
- break;
+ break;
default:
goto malformed_resp;
@@ -672,11 +659,9 @@ macvtapModeFromInt(enum virDomainNetdevMacvtapType mode)
switch (mode) {
case VIR_DOMAIN_NETDEV_MACVTAP_MODE_PRIVATE:
return MACVLAN_MODE_PRIVATE;
- break;
case VIR_DOMAIN_NETDEV_MACVTAP_MODE_BRIDGE:
return MACVLAN_MODE_BRIDGE;
- break;
case VIR_DOMAIN_NETDEV_MACVTAP_MODE_VEPA:
default:
@@ -910,20 +895,20 @@ static int
link_dump(bool multicast, int ifindex, struct nlattr **tb, char **recvbuf)
{
int rc = 0;
- char nlmsgbuf[256] = { 0, };
+ char nlmsgbuf[NLMSGBUF_SIZE] = { 0, };
struct nlmsghdr *nlm = (struct nlmsghdr *)nlmsgbuf, *resp;
struct nlmsgerr *err;
- struct ifinfomsg i = {
+ struct ifinfomsg ifinfo = {
.ifi_family = AF_UNSPEC,
.ifi_index = ifindex
};
- int recvbuflen;
+ unsigned int recvbuflen;
*recvbuf = NULL;
nlInit(nlm, NLM_F_REQUEST, RTM_GETLINK);
- if (!nlAppend(nlm, sizeof(nlmsgbuf), &i, sizeof(i)))
+ if (!nlAppend(nlm, sizeof(nlmsgbuf), &ifinfo, sizeof(ifinfo)))
goto buffer_too_small;
if (!multicast) {
@@ -946,17 +931,13 @@ link_dump(bool multicast, int ifindex, struct nlattr **tb, char **recvbuf)
if (resp->nlmsg_len < NLMSG_LENGTH(sizeof(*err)))
goto malformed_resp;
- switch (-err->error) {
- case 0:
- break;
-
- default:
+ if (err->error) {
virReportSystemError(-err->error,
_("error dumping %d interface"),
ifindex);
rc = -1;
}
- break;
+ break;
case GENL_ID_CTRL:
case NLMSG_DONE:
@@ -964,7 +945,7 @@ link_dump(bool multicast, int ifindex, struct nlattr **tb, char **recvbuf)
tb, IFLA_MAX, ifla_policy)) {
goto malformed_resp;
}
- break;
+ break;
default:
goto malformed_resp;
@@ -1048,17 +1029,17 @@ doPortProfileOpSetLink(bool multicast,
uint8_t op)
{
int rc = 0;
- char nlmsgbuf[256];
+ char nlmsgbuf[NLMSGBUF_SIZE];
struct nlmsghdr *nlm = (struct nlmsghdr *)nlmsgbuf, *resp;
struct nlmsgerr *err;
- char rtattbuf[64];
+ char rtattbuf[RATTBUF_SIZE];
struct rtattr *rta, *vfports = NULL, *vfport;
struct ifinfomsg ifinfo = {
.ifi_family = AF_UNSPEC,
.ifi_index = ifindex,
};
char *recvbuf = NULL;
- int recvbuflen = 0;
+ unsigned int recvbuflen = 0;
memset(&nlmsgbuf, 0, sizeof(nlmsgbuf));
@@ -1071,79 +1052,57 @@ doPortProfileOpSetLink(bool multicast,
rta = rtattrCreate(rtattbuf, sizeof(rtattbuf), IFLA_PORT_SELF, NULL, 0);
} else {
rta = rtattrCreate(rtattbuf, sizeof(rtattbuf), IFLA_VF_PORTS, NULL, 0);
- if (!rta)
- goto buffer_too_small;
-
- if (!(vfports = nlAppend(nlm, sizeof(nlmsgbuf),
+ if (!rta ||
+ !(vfports = nlAppend(nlm, sizeof(nlmsgbuf),
rtattbuf, rta->rta_len)))
goto buffer_too_small;
- /* beging nesting vfports */
+ /* begin nesting vfports */
rta = rtattrCreate(rtattbuf, sizeof(rtattbuf), IFLA_VF_PORT, NULL, 0);
}
- if (!rta)
- goto buffer_too_small;
-
- if (!(vfport = nlAppend(nlm, sizeof(nlmsgbuf), rtattbuf, rta->rta_len)))
+ if (!rta ||
+ !(vfport = nlAppend(nlm, sizeof(nlmsgbuf), rtattbuf, rta->rta_len)))
goto buffer_too_small;
if (profileId) {
rta = rtattrCreate(rtattbuf, sizeof(rtattbuf), IFLA_PORT_PROFILE,
profileId, strlen(profileId) + 1);
- if (!rta)
- goto buffer_too_small;
-
- if (!nlAppend(nlm, sizeof(nlmsgbuf), rtattbuf, rta->rta_len))
+ if (!rta || !nlAppend(nlm, sizeof(nlmsgbuf), rtattbuf, rta->rta_len))
goto buffer_too_small;
}
if (portVsi) {
rta = rtattrCreate(rtattbuf, sizeof(rtattbuf), IFLA_PORT_VSI_TYPE,
portVsi, sizeof(*portVsi));
- if (!rta)
- goto buffer_too_small;
-
- if (!nlAppend(nlm, sizeof(nlmsgbuf), rtattbuf, rta->rta_len))
+ if (!rta || !nlAppend(nlm, sizeof(nlmsgbuf), rtattbuf, rta->rta_len))
goto buffer_too_small;
}
if (instanceId) {
rta = rtattrCreate(rtattbuf, sizeof(rtattbuf), IFLA_PORT_INSTANCE_UUID,
instanceId, VIR_UUID_BUFLEN);
- if (!rta)
- goto buffer_too_small;
-
- if (!nlAppend(nlm, sizeof(nlmsgbuf), rtattbuf, rta->rta_len))
+ if (!rta || !nlAppend(nlm, sizeof(nlmsgbuf), rtattbuf, rta->rta_len))
goto buffer_too_small;
}
if (hostUUID) {
rta = rtattrCreate(rtattbuf, sizeof(rtattbuf), IFLA_PORT_HOST_UUID,
hostUUID, VIR_UUID_BUFLEN);
- if (!rta)
- goto buffer_too_small;
-
- if (!nlAppend(nlm, sizeof(nlmsgbuf), rtattbuf, rta->rta_len))
+ if (!rta || !nlAppend(nlm, sizeof(nlmsgbuf), rtattbuf, rta->rta_len))
goto buffer_too_small;
}
if (vf != PORT_SELF_VF) {
rta = rtattrCreate(rtattbuf, sizeof(rtattbuf), IFLA_PORT_VF,
&vf, sizeof(vf));
- if (!rta)
- goto buffer_too_small;
-
- if (!nlAppend(nlm, sizeof(nlmsgbuf), rtattbuf, rta->rta_len))
+ if (!rta || !nlAppend(nlm, sizeof(nlmsgbuf), rtattbuf, rta->rta_len))
goto buffer_too_small;
}
rta = rtattrCreate(rtattbuf, sizeof(rtattbuf), IFLA_PORT_REQUEST,
&op, sizeof(op));
- if (!rta)
- goto buffer_too_small;
-
- if (!nlAppend(nlm, sizeof(nlmsgbuf), rtattbuf, rta->rta_len))
+ if (!rta || !nlAppend(nlm, sizeof(nlmsgbuf), rtattbuf, rta->rta_len))
goto buffer_too_small;
/* end nesting of vport */
@@ -1174,20 +1133,16 @@ doPortProfileOpSetLink(bool multicast,
if (resp->nlmsg_len < NLMSG_LENGTH(sizeof(*err)))
goto malformed_resp;
- switch (-err->error) {
- case 0:
- break;
-
- default:
+ if (err->error) {
virReportSystemError(-err->error,
_("error during virtual port configuration of ifindex %d"),
ifindex);
rc = -1;
}
- break;
+ break;
case NLMSG_DONE:
- break;
+ break;
default:
goto malformed_resp;
@@ -1223,7 +1178,7 @@ doPortProfileOpCommon(bool multicast,
int rc;
char *recvbuf = NULL;
struct nlattr *tb[IFLA_MAX + 1];
- int repeats = 80;
+ int repeats = STATUS_POLL_TIMEOUT_USEC / STATUS_POLL_INTERVL_USEC;
uint16_t status = 0;
rc = doPortProfileOpSetLink(multicast,
@@ -1241,7 +1196,7 @@ doPortProfileOpCommon(bool multicast,
return rc;
}
- while (--repeats) {
+ while (--repeats >= 0) {
rc = link_dump(multicast, ifindex, tb, &recvbuf);
if (rc)
goto err_exit;
@@ -1259,8 +1214,10 @@ doPortProfileOpCommon(bool multicast,
rc = 1;
break;
}
- }
- usleep(125000);
+ } else
+ goto err_exit;
+
+ usleep(STATUS_POLL_INTERVL_USEC);
VIR_FREE(recvbuf);
}
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list