Allow to reuse as much as possible from virNetlinkCommand(). This
comment prepares for the introduction of virNetlindDumpCommand()
only differing by how it handles the responses.
---
 src/util/virnetlink.c | 90 +++++++++++++++++++++++++++++++--------------------
 1 file changed, 55 insertions(+), 35 deletions(-)

diff --git a/src/util/virnetlink.c b/src/util/virnetlink.c
index a5d10fa8e..5fb49251c 100644
--- a/src/util/virnetlink.c
+++ b/src/util/virnetlink.c
@@ -209,61 +209,38 @@ virNetlinkCreateSocket(int protocol)
     goto cleanup;
 }
 
-
-/**
- * virNetlinkCommand:
- * @nlmsg: pointer to netlink message
- * @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.
- * @src_pid: the pid of the process to send a message
- * @dst_pid: the pid of the process to talk to, i.e., pid = 0 for kernel
- * @protocol: netlink protocol
- * @groups: the group identifier
- *
- * Send the given message to the netlink layer and receive response.
- * Returns 0 on success, -1 on error. In case of error, no response
- * buffer will be returned.
- */
-int virNetlinkCommand(struct nl_msg *nl_msg,
-                      struct nlmsghdr **resp, unsigned int *respbuflen,
-                      uint32_t src_pid, uint32_t dst_pid,
-                      unsigned int protocol, unsigned int groups)
+static virNetlinkHandle *
+virNetlinkDoCommand(struct nl_msg *nl_msg, uint32_t src_pid,
+                    struct sockaddr_nl nladdr,
+                    unsigned int protocol, unsigned int groups)
 {
-    int ret = -1;
-    struct sockaddr_nl nladdr = {
-            .nl_family = AF_NETLINK,
-            .nl_pid    = dst_pid,
-            .nl_groups = 0,
-    };
     ssize_t nbytes;
-    struct pollfd fds[1];
     int fd;
     int n;
-    struct nlmsghdr *nlmsg = nlmsg_hdr(nl_msg);
     virNetlinkHandle *nlhandle = NULL;
-    int len = 0;
+    struct pollfd fds[1];
+    struct nlmsghdr *nlmsg = nlmsg_hdr(nl_msg);
 
     if (protocol >= MAX_LINKS) {
         virReportSystemError(EINVAL,
                              _("invalid protocol argument: %d"), protocol);
-        goto cleanup;
+        goto error;
     }
 
     if (!(nlhandle = virNetlinkCreateSocket(protocol)))
-        goto cleanup;
+        goto error;
 
     fd = nl_socket_get_fd(nlhandle);
     if (fd < 0) {
         virReportSystemError(errno,
                              "%s", _("cannot get netlink socket fd"));
-        goto cleanup;
+        goto error;
     }
 
     if (groups && nl_socket_add_membership(nlhandle, groups) < 0) {
         virReportSystemError(errno,
                              "%s", _("cannot add netlink membership"));
-        goto cleanup;
+        goto error;
     }
 
     nlmsg_set_dst(nl_msg, &nladdr);
@@ -274,10 +251,11 @@ int virNetlinkCommand(struct nl_msg *nl_msg,
     if (nbytes < 0) {
         virReportSystemError(errno,
                              "%s", _("cannot send to netlink socket"));
-        goto cleanup;
+        goto error;
     }
 
     memset(fds, 0, sizeof(fds));
+
     fds[0].fd = fd;
     fds[0].events = POLLIN;
 
@@ -289,9 +267,51 @@ int virNetlinkCommand(struct nl_msg *nl_msg,
         if (n == 0)
             virReportSystemError(ETIMEDOUT, "%s",
                                  _("no valid netlink response was received"));
-        goto cleanup;
     }
 
+    return nlhandle;
+
+ error:
+    virNetlinkFree(nlhandle);
+    return NULL;
+}
+
+/**
+ * virNetlinkCommand:
+ * @nlmsg: pointer to netlink message
+ * @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.
+ * @src_pid: the pid of the process to send a message
+ * @dst_pid: the pid of the process to talk to, i.e., pid = 0 for kernel
+ * @protocol: netlink protocol
+ * @groups: the group identifier
+ *
+ * Send the given message to the netlink layer and receive response.
+ * Returns 0 on success, -1 on error. In case of error, no response
+ * buffer will be returned.
+ */
+int virNetlinkCommand(struct nl_msg *nl_msg,
+                      struct nlmsghdr **resp, unsigned int *respbuflen,
+                      uint32_t src_pid, uint32_t dst_pid,
+                      unsigned int protocol, unsigned int groups)
+{
+    int ret = -1;
+    struct sockaddr_nl nladdr = {
+            .nl_family = AF_NETLINK,
+            .nl_pid    = dst_pid,
+            .nl_groups = 0,
+    };
+    struct pollfd fds[1];
+    virNetlinkHandle *nlhandle = NULL;
+    int len = 0;
+
+    memset(fds, 0, sizeof(fds));
+
+    if (!(nlhandle = virNetlinkDoCommand(nl_msg, src_pid, nladdr,
+                                         protocol, groups)))
+        goto cleanup;
+
     len = nl_recv(nlhandle, &nladdr, (unsigned char **)resp, NULL);
     if (len == 0) {
         virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-- 
2.11.0

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

On 03/03/2017 10:00 AM, Cédric Bosdonnat wrote:
> Allow to reuse as much as possible from virNetlinkCommand(). This
> comment prepares for the introduction of virNetlindDumpCommand()
> only differing by how it handles the responses.
> ---
>  src/util/virnetlink.c | 90 +++++++++++++++++++++++++++++++--------------------
>  1 file changed, 55 insertions(+), 35 deletions(-)
>
> diff --git a/src/util/virnetlink.c b/src/util/virnetlink.c
> index a5d10fa8e..5fb49251c 100644
> --- a/src/util/virnetlink.c
> +++ b/src/util/virnetlink.c
> @@ -209,61 +209,38 @@ virNetlinkCreateSocket(int protocol)
>      goto cleanup;
>  }
>  
> -
> -/**
> - * virNetlinkCommand:
> - * @nlmsg: pointer to netlink message
> - * @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.
> - * @src_pid: the pid of the process to send a message
> - * @dst_pid: the pid of the process to talk to, i.e., pid = 0 for kernel
> - * @protocol: netlink protocol
> - * @groups: the group identifier
> - *
> - * Send the given message to the netlink layer and receive response.
> - * Returns 0 on success, -1 on error. In case of error, no response
> - * buffer will be returned.
> - */
> -int virNetlinkCommand(struct nl_msg *nl_msg,
> -                      struct nlmsghdr **resp, unsigned int *respbuflen,
> -                      uint32_t src_pid, uint32_t dst_pid,
> -                      unsigned int protocol, unsigned int groups)
> +static virNetlinkHandle *
> +virNetlinkDoCommand(struct nl_msg *nl_msg, uint32_t src_pid,

Instead of virNetlinkDoCommand(), how about virNetlinkSendRequest() (or virNetlinkSendMessage())?

Aside from that, it looks good. Possibly it could be tweaked a bit to make it usable in virNetlinkDumpLink() as well. Similarly another related cleanup is that all the callers to virNetlinkCommand() end up doing some version of virNetlinkGetErrorCode() - they could be merged.

But all that (aside from renaming the function) is independent of this patch - ACK to this with a name change.


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