Hi~
On 08/21/2012 11:04 PM, Doug Goldstein wrote:
On Tue, Aug 21, 2012 at 5:12 AM, Tang Chen<tangc...@cn.fujitsu.com> wrote:
This patch improve all the API in virnetlink.c to support
all kinds of netlink protocols, and make all netlink sockets
be able to join in groups.
SOL_NETLINK is not defined on every system, so this patch defines
SOL_NETLINK as 270.
Signed-off-by: Tang Chen<tangc...@cn.fujitsu.com>
---
The commit message is a bit weak on what exactly is being added. The
most details are about a 3 line change.
OK, I will fix this. :)
-static virNetlinkEventSrvPrivatePtr server = NULL;
+static virNetlinkEventSrvPrivatePtr server[MAX_LINKS] = {NULL};
static virNetlinkHandle *placeholder_nlhandle = NULL;
Maybe a little code comment as to why MAX_LINKS. e.g. The Linux kernel
supports up to MAX_LINKS (32 at the time) individual netlink
protocols.
Sure, I will add some comments. :)
/* Function definitions */
@@ -158,6 +163,7 @@ virNetlinkShutdown(void)
* @respbuflen: pointer to integer holding the size of the response buffer
* on return of the function.
* @nl_pid: the pid of the process to talk to, i.e., pid = 0 for kernel
+ * @protocol: netlink protocol
*
* Send the given message to the netlink layer and receive response.
* Returns 0 on success, -1 on error. In case of error, no response
Function documentation doesn't match your implementation below.
I will fix it. :)
@@ -165,7 +171,8 @@ virNetlinkShutdown(void)
*/
int virNetlinkCommand(struct nl_msg *nl_msg,
unsigned char **respbuf, unsigned int *respbuflen,
- uint32_t src_pid, uint32_t dst_pid)
+ uint32_t src_pid, uint32_t dst_pid,
+ unsigned int protocol, unsigned int groups)
{
int rc = 0;
struct sockaddr_nl nladdr = {
@@ -181,17 +188,46 @@ int virNetlinkCommand(struct nl_msg *nl_msg,
int fd;
int n;
struct nlmsghdr *nlmsg = nlmsg_hdr(nl_msg);
- virNetlinkHandle *nlhandle = virNetlinkAlloc();
+ virNetlinkHandle *nlhandle = NULL;
+
+ if (protocol>= MAX_LINKS) {
+ virReportSystemError(EINVAL,
+ _("invalid protocol argument: %d"), protocol);
+ return -EINVAL;
+ }
+
+ if (groups>= 32) {
I believe there is a define for this in the headers so it would be
better to use that then hardcoding a number without any code comments
to what it means. If there's not a define, I would at least document
what the 32 is and where it derives from the kernel code so that if
things change in the future it will be easier to fix.
I found that the document of nl_socket_add_membership() said the following:
"Joins the specified group using the modern socket option which
is available since kernel version 2.6.14.
It allows joining an almost arbitary number of groups without
limitation."
So maybe a check for "groups" is not necessary. Can we just replace
setsockopt()
with nl_socket_add_membership() and leave the checking and error
handling in
nl_socket_add_membership() ?
+ virReportSystemError(EINVAL,
+ _("invalid group argument: %d"), groups);
+ return -EINVAL;
+ }
+ nlhandle = virNetlinkAlloc();
if (!nlhandle) {
virReportSystemError(errno,
"%s", _("cannot allocate nlhandle for netlink"));
return -1;
}
- if (nl_connect(nlhandle, NETLINK_ROUTE)< 0) {
+ if (nl_connect(nlhandle, protocol)< 0) {
virReportSystemError(errno,
- "%s", _("cannot connect to netlink socket"));
+ _("cannot connect to netlink socket with protocol
%d"), protocol);
+ rc = -1;
+ goto error;
+ }
+
+ fd = nl_socket_get_fd(nlhandle);
+ if (fd< 0) {
+ virReportSystemError(errno,
+ "%s", _("cannot get netlink socket fd"));
+ rc = -1;
+ goto error;
+ }
+
+ if (groups&& setsockopt(fd, SOL_NETLINK, NETLINK_ADD_MEMBERSHIP,
+&groups, sizeof(groups))< 0) {
+ virReportSystemError(errno,
+ "%s", _("cannot add netlink membership"));
rc = -1;
goto error;
}
@@ -208,8 +244,6 @@ int virNetlinkCommand(struct nl_msg *nl_msg,
goto error;
}
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
