virNetlinkCommand() processes only one response message, while some
netlink commands like routes dumping need to process several ones.
Add virNetlinkDumpCommand() as a virNetlinkCommand() sister.
---
 src/libvirt_private.syms |  1 +
 src/util/virnetlink.c    | 55 ++++++++++++++++++++++++++++++++++++++++++++++++
 src/util/virnetlink.h    |  9 ++++++++
 3 files changed, 65 insertions(+)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index bce0487ab..71143851c 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2117,6 +2117,7 @@ virNetDevVPortProfileOpTypeToString;
 # util/virnetlink.h
 virNetlinkCommand;
 virNetlinkDelLink;
+virNetlinkDumpCommand;
 virNetlinkDumpLink;
 virNetlinkEventAddClient;
 virNetlinkEventRemoveClient;
diff --git a/src/util/virnetlink.c b/src/util/virnetlink.c
index 5fb49251c..4747ba5a4 100644
--- a/src/util/virnetlink.c
+++ b/src/util/virnetlink.c
@@ -335,6 +335,49 @@ int virNetlinkCommand(struct nl_msg *nl_msg,
     return ret;
 }
 
+int
+virNetlinkDumpCommand(struct nl_msg *nl_msg,
+                      virNetlinkDumpCallback callback,
+                      uint32_t src_pid, uint32_t dst_pid,
+                      unsigned int protocol, unsigned int groups,
+                      void *opaque)
+{
+    int ret = -1;
+    bool end = false;
+    int len = 0;
+    struct nlmsghdr *resp = NULL;
+    struct nlmsghdr *msg = NULL;
+
+    struct sockaddr_nl nladdr = {
+            .nl_family = AF_NETLINK,
+            .nl_pid    = dst_pid,
+            .nl_groups = 0,
+    };
+    virNetlinkHandle *nlhandle = NULL;
+
+    if (!(nlhandle = virNetlinkDoCommand(nl_msg, src_pid, nladdr,
+                                         protocol, groups)))
+        goto cleanup;
+
+    while (!end) {
+        len = nl_recv(nlhandle, &nladdr, (unsigned char **)&resp, NULL);
+
+        for (msg = resp; NLMSG_OK(msg, len); msg = NLMSG_NEXT(msg, len)) {
+            if (msg->nlmsg_type == NLMSG_DONE)
+                end = true;
+
+            if (callback(msg, opaque) < 0)
+                goto cleanup;
+        }
+    }
+
+    ret = 0;
+
+ cleanup:
+    virNetlinkFree(nlhandle);
+    return ret;
+}
+
 
 /**
  * virNetlinkDumpLink:
@@ -1062,6 +1105,18 @@ int virNetlinkCommand(struct nl_msg *nl_msg ATTRIBUTE_UNUSED,
     return -1;
 }
 
+int
+virNetlinkDumpCommand(struct nl_msg *nl_msg ATTRIBUTE_UNUSED,
+                      virNetlinkDumpCallback callback ATTRIBUTE_UNUSED,
+                      uint32_t src_pid ATTRIBUTE_UNUSED,
+                      uint32_t dst_pid ATTRIBUTE_UNUSED,
+                      unsigned int protocol ATTRIBUTE_UNUSED,
+                      unsigned int groups ATTRIBUTE_UNUSED,
+                      void *opaque ATTRIBUTE_UNUSED)
+{
+    virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _(unsupported));
+    return -1;
+}
 
 int
 virNetlinkDumpLink(const char *ifname ATTRIBUTE_UNUSED,
diff --git a/src/util/virnetlink.h b/src/util/virnetlink.h
index 11e817c82..088b01343 100644
--- a/src/util/virnetlink.h
+++ b/src/util/virnetlink.h
@@ -52,6 +52,15 @@ int virNetlinkCommand(struct nl_msg *nl_msg,
                       uint32_t src_pid, uint32_t dst_pid,
                       unsigned int protocol, unsigned int groups);
 
+typedef int (*virNetlinkDumpCallback)(const struct nlmsghdr *resp,
+                                      void *data);
+
+int virNetlinkDumpCommand(struct nl_msg *nl_msg,
+                          virNetlinkDumpCallback callback,
+                          uint32_t src_pid, uint32_t dst_pid,
+                          unsigned int protocol, unsigned int groups,
+                          void *opaque);
+
 typedef int (*virNetlinkDelLinkFallback)(const char *ifname);
 
 int virNetlinkDelLink(const char *ifname, virNetlinkDelLinkFallback fallback);
-- 
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:
> virNetlinkCommand() processes only one response message, while some
> netlink commands like routes dumping need to process several ones.
> Add virNetlinkDumpCommand() as a virNetlinkCommand() sister.
> ---
>  src/libvirt_private.syms |  1 +
>  src/util/virnetlink.c    | 55 ++++++++++++++++++++++++++++++++++++++++++++++++
>  src/util/virnetlink.h    |  9 ++++++++
>  3 files changed, 65 insertions(+)
>
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index bce0487ab..71143851c 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -2117,6 +2117,7 @@ virNetDevVPortProfileOpTypeToString;
>  # util/virnetlink.h
>  virNetlinkCommand;
>  virNetlinkDelLink;
> +virNetlinkDumpCommand;
>  virNetlinkDumpLink;
>  virNetlinkEventAddClient;
>  virNetlinkEventRemoveClient;
> diff --git a/src/util/virnetlink.c b/src/util/virnetlink.c
> index 5fb49251c..4747ba5a4 100644
> --- a/src/util/virnetlink.c
> +++ b/src/util/virnetlink.c
> @@ -335,6 +335,49 @@ int virNetlinkCommand(struct nl_msg *nl_msg,
>      return ret;
>  }
>  
> +int
> +virNetlinkDumpCommand(struct nl_msg *nl_msg,
> +                      virNetlinkDumpCallback callback,
> +                      uint32_t src_pid, uint32_t dst_pid,
> +                      unsigned int protocol, unsigned int groups,
> +                      void *opaque)
> +{
> +    int ret = -1;
> +    bool end = false;
> +    int len = 0;
> +    struct nlmsghdr *resp = NULL;
> +    struct nlmsghdr *msg = NULL;
> +
> +    struct sockaddr_nl nladdr = {
> +            .nl_family = AF_NETLINK,
> +            .nl_pid    = dst_pid,
> +            .nl_groups = 0,
> +    };
> +    virNetlinkHandle *nlhandle = NULL;
> +
> +    if (!(nlhandle = virNetlinkDoCommand(nl_msg, src_pid, nladdr,
> +                                         protocol, groups)))
> +        goto cleanup;
> +
> +    while (!end) {
> +        len = nl_recv(nlhandle, &nladdr, (unsigned char **)&resp, NULL);
> +
> +        for (msg = resp; NLMSG_OK(msg, len); msg = NLMSG_NEXT(msg, len)) {
> +            if (msg->nlmsg_type == NLMSG_DONE)
> +                end = true;
> +
> +            if (callback(msg, opaque) < 0)
> +                goto cleanup;

This is relying on the callback function to throw an error in order to get us out of the loop (assuming we never get an NLMSG_DONE). I don't see the callback that's written in patch 4 doing anything along the lines of virNetlinkGetErrorCode() though. Maybe you should call that function from here prior to the callback - that would eliminate the need to separately do that level of error checking in all the future consumers of this new function.

Otherwise I think it looks okay (I can't claim to have great knowledge of multipart netlink messages though, so...)

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