[PATCH 2/2] virnetlink: Remove stub functions

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



virnetlink provides stub functions when libnl is not available, but this
approach caused a few problems:

- Commit 582f0966f9b9e2148d8887d072364e2a91aed000 broke builds without
  libnl for Linux because virNetlinkBridgeVlanFilterSet() lacked a stub.

- A call to virNetlinkEventServiceStopAll() stub in
  src/remote/remote_daemon.c resulted in extra debug logs.

Instead of providing stub functions, let callers check the availability
of libnl. Callers are also replaced with stub functions in most cases
so this approach requires less code and less error-prone.

Signed-off-by: Akihiko Odaki <akihiko.odaki@xxxxxxxxxx>
---
 src/libvirt_libnl.syms     |  23 ++++++
 src/libvirt_private.syms   |  19 -----
 src/meson.build            |   6 ++
 src/remote/remote_daemon.c |  10 ++-
 src/util/meson.build       |   5 +-
 src/util/virarptable.c     |   2 +-
 src/util/virnetlink.c      | 196 +++------------------------------------------
 src/util/virnetlink.h      |  11 +--
 8 files changed, 55 insertions(+), 217 deletions(-)

diff --git a/src/libvirt_libnl.syms b/src/libvirt_libnl.syms
new file mode 100644
index 0000000000..091169416b
--- /dev/null
+++ b/src/libvirt_libnl.syms
@@ -0,0 +1,23 @@
+# util/virnetlink.h
+virNetlinkCommand;
+virNetlinkDelLink;
+virNetlinkDumpCommand;
+virNetlinkDumpLink;
+virNetlinkEventAddClient;
+virNetlinkEventRemoveClient;
+virNetlinkEventServiceIsRunning;
+virNetlinkEventServiceLocalPid;
+virNetlinkEventServiceStart;
+virNetlinkEventServiceStop;
+virNetlinkEventServiceStopAll;
+virNetlinkGetErrorCode;
+virNetlinkGetNeighbor;
+virNetlinkNewLink;
+virNetlinkShutdown;
+virNetlinkStartup;
+
+
+# Let emacs know we want case-insensitive sorting
+# Local Variables:
+# sort-fold-case: t
+# End:
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 30a9f806f0..0ce20291df 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -3022,25 +3022,6 @@ virNetDevVPortProfileOpTypeFromString;
 virNetDevVPortProfileOpTypeToString;
 
 
-# util/virnetlink.h
-virNetlinkCommand;
-virNetlinkDelLink;
-virNetlinkDumpCommand;
-virNetlinkDumpLink;
-virNetlinkEventAddClient;
-virNetlinkEventRemoveClient;
-virNetlinkEventServiceIsRunning;
-virNetlinkEventServiceLocalPid;
-virNetlinkEventServiceStart;
-virNetlinkEventServiceStop;
-virNetlinkEventServiceStopAll;
-virNetlinkGetErrorCode;
-virNetlinkGetNeighbor;
-virNetlinkNewLink;
-virNetlinkShutdown;
-virNetlinkStartup;
-
-
 # util/virnodesuspend.h
 virNodeSuspend;
 virNodeSuspendGetTargetMask;
diff --git a/src/meson.build b/src/meson.build
index 9413192a55..989d3113d9 100644
--- a/src/meson.build
+++ b/src/meson.build
@@ -131,6 +131,12 @@ else
   sym_files += 'libvirt_hyperv.syms'
 endif
 
+if conf.has('WITH_LIBNL')
+  used_sym_files += 'libvirt_libnl.syms'
+else
+  sym_files += 'libvirt_libnl.syms'
+endif
+
 # variables filled by subdirectories
 
 libvirt_libs = []
diff --git a/src/remote/remote_daemon.c b/src/remote/remote_daemon.c
index d90355c0d2..f62dc0a837 100644
--- a/src/remote/remote_daemon.c
+++ b/src/remote/remote_daemon.c
@@ -1021,10 +1021,12 @@ int main(int argc, char **argv) {
     }
     umask(old_umask);
 
+#ifdef WITH_LIBNL
     if (virNetlinkStartup() < 0) {
         ret = VIR_DAEMON_ERR_INIT;
         goto cleanup;
     }
+#endif
 
     if (!(dmn = virNetDaemonNew())) {
         ret = VIR_DAEMON_ERR_DRIVER;
@@ -1188,7 +1190,7 @@ int main(int argc, char **argv) {
         goto cleanup;
     }
 
-#if defined(__linux__) && defined(NETLINK_ROUTE)
+#if defined(WITH_LIBNL) && defined(NETLINK_ROUTE)
     /* Register the netlink event service for NETLINK_ROUTE */
     if (virNetlinkEventServiceStart(NETLINK_ROUTE, 0) < 0) {
         ret = VIR_DAEMON_ERR_NETWORK;
@@ -1196,7 +1198,7 @@ int main(int argc, char **argv) {
     }
 #endif
 
-#if defined(__linux__) && defined(NETLINK_KOBJECT_UEVENT)
+#if defined(WITH_LIBNL) && defined(NETLINK_KOBJECT_UEVENT)
     /* Register the netlink event service for NETLINK_KOBJECT_UEVENT */
     if (virNetlinkEventServiceStart(NETLINK_KOBJECT_UEVENT, 1) < 0) {
         ret = VIR_DAEMON_ERR_NETWORK;
@@ -1213,7 +1215,9 @@ int main(int argc, char **argv) {
                 0, "shutdown", NULL, NULL);
 
  cleanup:
+#ifdef WITH_LIBNL
     virNetlinkEventServiceStopAll();
+#endif
 
     if (g_atomic_int_compare_and_exchange(&driversInitialized, 1, 0)) {
         /* NB: Possible issue with timing window between driversInitialized
@@ -1229,7 +1233,9 @@ int main(int argc, char **argv) {
     virObjectUnref(srv);
     virObjectUnref(dmn);
 
+#ifdef WITH_LIBNL
     virNetlinkShutdown();
+#endif
 
     if (pid_file_fd != -1)
         virPidFileReleasePath(pid_file, pid_file_fd);
diff --git a/src/util/meson.build b/src/util/meson.build
index 69ef49139a..992b0441c9 100644
--- a/src/util/meson.build
+++ b/src/util/meson.build
@@ -69,7 +69,6 @@ util_sources = [
   'virnetdevveth.c',
   'virnetdevvlan.c',
   'virnetdevvportprofile.c',
-  'virnetlink.c',
   'virnodesuspend.c',
   'virnuma.c',
   'virnvme.c',
@@ -112,6 +111,10 @@ util_sources = [
   'virxml.c',
 ]
 
+if conf.has('WITH_LIBNL')
+  util_sources += 'virnetlink.c'
+endif
+
 util_public_sources = files(
   'virerror.c',
   'virevent.c',
diff --git a/src/util/virarptable.c b/src/util/virarptable.c
index 20d11f97b0..9c132ea0b6 100644
--- a/src/util/virarptable.c
+++ b/src/util/virarptable.c
@@ -35,7 +35,7 @@
 
 VIR_LOG_INIT("util.arptable");
 
-#ifdef __linux__
+#ifdef WITH_LIBNL
 
 # define NDA_RTA(r) \
     ((struct rtattr*)(((char*)(r)) + NLMSG_ALIGN(sizeof(struct ndmsg))))
diff --git a/src/util/virnetlink.c b/src/util/virnetlink.c
index 206646d9d7..d944bdbce1 100644
--- a/src/util/virnetlink.c
+++ b/src/util/virnetlink.c
@@ -37,11 +37,9 @@ VIR_LOG_INIT("util.netlink");
 
 #define NETLINK_ACK_TIMEOUT_S  (2*1000)
 
-#if defined(WITH_LIBNL)
+#include <linux/veth.h>
 
-# include <linux/veth.h>
-
-# define NETLINK_MSG_NEST_START(msg, container, attrtype) \
+#define NETLINK_MSG_NEST_START(msg, container, attrtype) \
 do { \
     container = nla_nest_start(msg, attrtype); \
     if (!container) { \
@@ -51,7 +49,7 @@ do { \
     } \
 } while (0)
 
-# define NETLINK_MSG_NEST_END(msg, container) \
+#define NETLINK_MSG_NEST_END(msg, container) \
 do { nla_nest_end(msg, container); } while (0)
 
 /*
@@ -59,7 +57,7 @@ do { nla_nest_end(msg, container); } while (0)
  * complain about @data not being a pointer type:
  * error: the address of 'foo' will always evaluate as 'true' [-Werror=address]
  */
-# define NETLINK_MSG_PUT(msg, attrtype, datalen, data) \
+#define NETLINK_MSG_PUT(msg, attrtype, datalen, data) \
 do { \
     const void *dataptr = data; \
     if (dataptr && nla_put(msg, attrtype, datalen, dataptr) < 0) { \
@@ -69,7 +67,7 @@ do { \
     } \
 } while (0)
 
-# define NETLINK_MSG_APPEND(msg, datalen, dataptr) \
+#define NETLINK_MSG_APPEND(msg, datalen, dataptr) \
 do { \
     if (nlmsg_append(msg, dataptr, datalen, NLMSG_ALIGNTO) < 0) { \
         virReportError(VIR_ERR_INTERNAL_ERROR, "%s", \
@@ -89,9 +87,9 @@ struct virNetlinkEventHandle {
     int deleted;
 };
 
-# define virNetlinkAlloc nl_socket_alloc
-# define virNetlinkSetBufferSize nl_socket_set_buffer_size
-# define virNetlinkFree nl_socket_free
+#define virNetlinkAlloc nl_socket_alloc
+#define virNetlinkSetBufferSize nl_socket_set_buffer_size
+#define virNetlinkFree nl_socket_free
 typedef struct nl_sock virNetlinkHandle;
 
 G_DEFINE_AUTOPTR_CLEANUP_FUNC(virNetlinkHandle, virNetlinkFree);
@@ -120,7 +118,7 @@ static int nextWatch = 1;
 
 /* Allocate extra slots for virEventPollHandle/virEventPollTimeout
  records in this multiple */
-# define NETLINK_EVENT_ALLOC_EXTENT 10
+#define NETLINK_EVENT_ALLOC_EXTENT 10
 
 /* Linux kernel supports up to MAX_LINKS (32 at the time) individual
  * netlink protocols. */
@@ -519,7 +517,7 @@ virNetlinkDumpLink(const char *ifname, int ifindex,
     if (ifname)
         NETLINK_MSG_PUT(nl_msg, IFLA_IFNAME, (strlen(ifname) + 1), ifname);
 
-# ifdef RTEXT_FILTER_VF
+#ifdef RTEXT_FILTER_VF
     /* if this filter exists in the kernel's netlink implementation,
      * we need to set it, otherwise the response message will not
      * contain the IFLA_VFINFO_LIST that we're looking for.
@@ -530,7 +528,7 @@ virNetlinkDumpLink(const char *ifname, int ifindex,
         NETLINK_MSG_PUT(nl_msg, IFLA_EXT_MASK,
                         sizeof(ifla_ext_mask), &ifla_ext_mask);
     }
-# endif
+#endif
 
     if (virNetlinkTalk(ifname, nl_msg, src_pid, dst_pid,
                        &resp, &resp_len, &error, NULL) < 0) {
@@ -1264,175 +1262,3 @@ virNetlinkEventRemoveClient(int watch, const virMacAddr *macaddr,
 
     return -1;
 }
-
-#else
-
-# if defined(__linux__)
-static const char *unsupported = N_("libnl was not available at build time");
-# else
-static const char *unsupported = N_("not supported on non-linux platforms");
-# endif
-
-int
-virNetlinkStartup(void)
-{
-    return 0;
-}
-
-void
-virNetlinkShutdown(void)
-{
-    return;
-}
-
-int virNetlinkCommand(struct nl_msg *nl_msg G_GNUC_UNUSED,
-                      struct nlmsghdr **resp G_GNUC_UNUSED,
-                      unsigned int *respbuflen G_GNUC_UNUSED,
-                      uint32_t src_pid G_GNUC_UNUSED,
-                      uint32_t dst_pid G_GNUC_UNUSED,
-                      unsigned int protocol G_GNUC_UNUSED,
-                      unsigned int groups G_GNUC_UNUSED)
-{
-    virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _(unsupported));
-    return -1;
-}
-
-int
-virNetlinkDumpCommand(struct nl_msg *nl_msg G_GNUC_UNUSED,
-                      virNetlinkDumpCallback callback G_GNUC_UNUSED,
-                      uint32_t src_pid G_GNUC_UNUSED,
-                      uint32_t dst_pid G_GNUC_UNUSED,
-                      unsigned int protocol G_GNUC_UNUSED,
-                      unsigned int groups G_GNUC_UNUSED,
-                      void *opaque G_GNUC_UNUSED)
-{
-    virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _(unsupported));
-    return -1;
-}
-
-int
-virNetlinkDumpLink(const char *ifname G_GNUC_UNUSED,
-                   int ifindex G_GNUC_UNUSED,
-                   void **nlData G_GNUC_UNUSED,
-                   struct nlattr **tb G_GNUC_UNUSED,
-                   uint32_t src_pid G_GNUC_UNUSED,
-                   uint32_t dst_pid G_GNUC_UNUSED)
-{
-    virReportSystemError(ENOSYS, "%s",
-                         _("Unable to dump link info on this platform"));
-    return -1;
-}
-
-
-int
-virNetlinkDelLink(const char *ifname G_GNUC_UNUSED,
-                  virNetlinkTalkFallback fallback G_GNUC_UNUSED)
-{
-    virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _(unsupported));
-    return -1;
-}
-
-
-int
-virNetlinkNewLink(const char *ifname G_GNUC_UNUSED,
-                  const char *type G_GNUC_UNUSED,
-                  virNetlinkNewLinkData *extra_args G_GNUC_UNUSED,
-                  int *error G_GNUC_UNUSED)
-{
-    virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _(unsupported));
-    return -1;
-}
-
-
-int
-virNetlinkGetNeighbor(void **nlData G_GNUC_UNUSED,
-                      uint32_t src_pid G_GNUC_UNUSED,
-                      uint32_t dst_pid G_GNUC_UNUSED)
-{
-    virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _(unsupported));
-    return -1;
-}
-
-
-/**
- * stopNetlinkEventServer: stop the monitor to receive netlink
- * messages for libvirtd
- */
-int virNetlinkEventServiceStop(unsigned int protocol G_GNUC_UNUSED)
-{
-    VIR_DEBUG("%s", unsupported);
-    return 0;
-}
-
-/**
- * stopNetlinkEventServerAll: stop all the monitors to receive netlink
- * messages for libvirtd
- */
-int virNetlinkEventServiceStopAll(void)
-{
-    VIR_DEBUG("%s", unsupported);
-    return 0;
-}
-
-/**
- * startNetlinkEventServer: start a monitor to receive netlink
- * messages for libvirtd
- */
-int virNetlinkEventServiceStart(unsigned int protocol G_GNUC_UNUSED,
-                                unsigned int groups G_GNUC_UNUSED)
-{
-    VIR_DEBUG("%s", unsupported);
-    return 0;
-}
-
-/**
- * virNetlinkEventServiceIsRunning: returns if the netlink event
- * service is running.
- */
-bool virNetlinkEventServiceIsRunning(unsigned int protocol G_GNUC_UNUSED)
-{
-    virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _(unsupported));
-    return false;
-}
-
-int virNetlinkEventServiceLocalPid(unsigned int protocol G_GNUC_UNUSED)
-{
-    virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _(unsupported));
-    return -1;
-}
-
-/**
- * virNetlinkEventAddClient: register a callback for handling of
- * netlink messages
- */
-int virNetlinkEventAddClient(virNetlinkEventHandleCallback handleCB G_GNUC_UNUSED,
-                             virNetlinkEventRemoveCallback removeCB G_GNUC_UNUSED,
-                             void *opaque G_GNUC_UNUSED,
-                             const virMacAddr *macaddr G_GNUC_UNUSED,
-                             unsigned int protocol G_GNUC_UNUSED)
-{
-    virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _(unsupported));
-    return -1;
-}
-
-/**
- * virNetlinkEventRemoveClient: unregister a callback from a netlink monitor
- */
-int virNetlinkEventRemoveClient(int watch G_GNUC_UNUSED,
-                                const virMacAddr *macaddr G_GNUC_UNUSED,
-                                unsigned int protocol G_GNUC_UNUSED)
-{
-    virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _(unsupported));
-    return -1;
-}
-
-
-int
-virNetlinkGetErrorCode(struct nlmsghdr *resp G_GNUC_UNUSED,
-                       unsigned int recvbuflen G_GNUC_UNUSED)
-{
-    virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _(unsupported));
-    return -EINVAL;
-}
-
-#endif /* WITH_LIBNL */
diff --git a/src/util/virnetlink.h b/src/util/virnetlink.h
index 327fb426a1..1a1bdebaed 100644
--- a/src/util/virnetlink.h
+++ b/src/util/virnetlink.h
@@ -34,15 +34,6 @@ struct nl_msg *
 virNetlinkMsgNew(int nlmsgtype,
                  int nlmsgflags);
 
-#else
-
-struct nl_msg;
-struct sockaddr_nl;
-struct nlattr;
-struct nlmsghdr;
-
-#endif /* WITH_LIBNL */
-
 int virNetlinkStartup(void);
 void virNetlinkShutdown(void);
 
@@ -140,3 +131,5 @@ int virNetlinkEventAddClient(virNetlinkEventHandleCallback handleCB,
  */
 int virNetlinkEventRemoveClient(int watch, const virMacAddr *macaddr,
                                 unsigned int protocol);
+
+#endif /* WITH_LIBNL */

-- 
2.48.1



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux