[ovs-dev] [PATCH] config: Add explicit support for building on ESX.

Ethan Jackson ethan at nicira.com
Tue Oct 9 22:09:51 UTC 2012


Here's an incremental.

---
 configure.ac            |  4 ++--
 lib/socket-util.c       | 12 +++++-------
 lib/timeval.c           | 42 +++++++-----------------------------------
 lib/timeval.h           | 16 ++++++++++++++++
 tests/test-timeval.c    |  4 ++--
 vswitchd/system-stats.c | 21 ++++++++++-----------
 6 files changed, 42 insertions(+), 57 deletions(-)

diff --git a/configure.ac b/configure.ac
index ba1e936..6fb30fd 100644
--- a/configure.ac
+++ b/configure.ac
@@ -113,8 +113,8 @@ AC_CONFIG_COMMANDS([include/openflow/openflow.h.stamp])
 AC_CONFIG_COMMANDS([ovsdb/ovsdbmonitor/dummy], [:])
 AC_CONFIG_COMMANDS([utilities/bugtool/dummy], [:])
 
-AM_CONDITIONAL([LINUX_DATAPATH], [test "$HAVE_NETLINK" = yes -a "$ESX" = no])
-if test "$HAVE_NETLINK" = yes -a "$ESX" = no; then
+AM_CONDITIONAL([LINUX_DATAPATH], [test "$HAVE_NETLINK" = yes && test "$ESX" = no])
+if test "$HAVE_NETLINK" = yes && test "$ESX" = no; then
     AC_DEFINE([LINUX_DATAPATH], [1], [System uses the linux datapath module.])
 fi
 
diff --git a/lib/socket-util.c b/lib/socket-util.c
index ca9e406..a37dfe4 100644
--- a/lib/socket-util.c
+++ b/lib/socket-util.c
@@ -51,11 +51,9 @@ VLOG_DEFINE_THIS_MODULE(socket_util);
 
 /* #ifdefs make it a pain to maintain code: you have to try to build both ways.
  * Thus, this file compiles all of the code regardless of the target, by
- * writing "if (LINUX)" instead of "#ifdef LINUX_DATAPATH". */
-#ifdef LINUX_DATAPATH
-#define LINUX 1
-#else
-#define LINUX 0
+ * writing "if (LINUX_DATAPATH)" instead of "#ifdef __linux__". */
+#ifndef LINUX_DATAPATH
+#define LINUX_DATAPATH 0
 #endif
 
 #ifndef O_DIRECTORY
@@ -266,7 +264,7 @@ drain_rcvbuf(int fd)
          *
          * On other Unix-like OSes, MSG_TRUNC has no effect in the flags
          * argument. */
-        char buffer[LINUX ? 1 : 2048];
+        char buffer[LINUX_DATAPATH ? 1 : 2048];
         ssize_t n_bytes = recv(fd, buffer, sizeof buffer,
                                MSG_TRUNC | MSG_DONTWAIT);
         if (n_bytes <= 0 || n_bytes >= rcvbuf) {
@@ -335,7 +333,7 @@ make_sockaddr_un(const char *name, struct sockaddr_un *un, socklen_t *un_len,
     if (strlen(name) > MAX_UN_LEN) {
         static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
 
-        if (LINUX) {
+        if (LINUX_DATAPATH) {
             /* 'name' is too long to fit in a sockaddr_un, but we have a
              * workaround for that on Linux: shorten it by opening a file
              * descriptor for the directory part of the name and indirecting
diff --git a/lib/timeval.c b/lib/timeval.c
index 9c9cf0e..d3b6685 100644
--- a/lib/timeval.c
+++ b/lib/timeval.c
@@ -40,15 +40,7 @@ VLOG_DEFINE_THIS_MODULE(timeval);
  * to CLOCK_REALTIME. */
 static clockid_t monotonic_clock;
 
-/* Controls whether or not calls to clock_gettime() are cached.  See
- * time_cached() for a detailed explanation. */
-#if defined __x86_64__ && defined LINUX_DATAPATH
-static bool cache_time = false;
-#else
-static bool cache_time = true;
-#endif
-
-/* Has a timer tick occurred? Only relevant if cache_time is true.
+/* Has a timer tick occurred? Only relevant if CACHE_TIME is true.
  *
  * We initialize these to true to force time_init() to get called on the first
  * call to time_msec() or another function that queries the current time. */
@@ -154,15 +146,12 @@ set_up_timer(void)
     static timer_t timer_id;    /* "static" to avoid apparent memory leak. */
     struct itimerspec itimer;
 
-    if (!cache_time) {
+    if (!CACHE_TIME) {
         return;
     }
 
     if (timer_create(monotonic_clock, NULL, &timer_id)) {
-        VLOG_WARN("timer_create failed (%s), disabling cached timing",
-                  strerror(errno));
-        cache_time = false;
-        return;
+        VLOG_FATAL("timer_create failed (%s)", strerror(errno));
     }
 
     itimer.it_interval.tv_sec = 0;
@@ -215,7 +204,7 @@ refresh_monotonic(void)
 /* Forces a refresh of the current time from the kernel.  It is not usually
  * necessary to call this function, since the time will be refreshed
  * automatically at least every TIME_UPDATE_INTERVAL milliseconds.  If
- * cache_time is false, we will always refresh the current time so this
+ * CACHE_TIME is false, we will always refresh the current time so this
  * function has no effect. */
 void
 time_refresh(void)
@@ -356,7 +345,7 @@ time_poll(struct pollfd *pollfds, int n_pollfds, long long int timeout_when,
             break;
         }
 
-        if (!blocked && !cache_time) {
+        if (!blocked && !CACHE_TIME) {
             block_sigalrm(&oldsigs);
             blocked = true;
         }
@@ -370,23 +359,6 @@ time_poll(struct pollfd *pollfds, int n_pollfds, long long int timeout_when,
     return retval;
 }
 
-/* True on systems (particularly x86-64 Linux) where clock_gettime() is
- * inexpensive.  On these systems, we don't bother caching the current time.
- * Instead, we consult clock_gettime() directly when needed.
- *
- * False on systems where clock_gettime() is relatively expensive.  On these
- * systems, we cache the current time and set up a periodic SIGALRM to remind
- * us to update it.
- *
- * Also false on systems (e.g. ESX) that don't support setting up timers based
- * on a monotonically increasing clock. */
-bool
-time_cached(void)
-{
-    time_init();
-    return cache_time;
-}
-
 static void
 sigalrm_handler(int sig_nr OVS_UNUSED)
 {
@@ -397,7 +369,7 @@ sigalrm_handler(int sig_nr OVS_UNUSED)
 static void
 refresh_wall_if_ticked(void)
 {
-    if (!cache_time || wall_tick) {
+    if (!CACHE_TIME || wall_tick) {
         refresh_wall();
     }
 }
@@ -405,7 +377,7 @@ refresh_wall_if_ticked(void)
 static void
 refresh_monotonic_if_ticked(void)
 {
-    if (!cache_time || monotonic_tick) {
+    if (!CACHE_TIME || monotonic_tick) {
         refresh_monotonic();
     }
 }
diff --git a/lib/timeval.h b/lib/timeval.h
index d9eb3c7..cb5191c 100644
--- a/lib/timeval.h
+++ b/lib/timeval.h
@@ -44,6 +44,22 @@ BUILD_ASSERT_DECL(TYPE_IS_SIGNED(time_t));
  * much time will be wasted in signal handlers and calls to clock_gettime(). */
 #define TIME_UPDATE_INTERVAL 100
 
+/* True on systems (particularly x86-64 Linux) where clock_gettime() is
+ * inexpensive.  On these systems, we don't bother caching the current time.
+ * Instead, we consult clock_gettime() directly when needed.
+ *
+ * False on systems where clock_gettime() is relatively expensive.  On these
+ * systems, we cache the current time and set up a periodic SIGALRM to remind
+ * us to update it.
+ *
+ * Also false on systems (e.g. ESX) that don't support setting up timers based
+ * on a monotonically increasing clock. */
+#if defined ESX || (defined __x86_64__ && defined LINUX_DATAPATH)
+#define CACHE_TIME 0
+#else
+#define CACHE_TIME 1
+#endif
+
 void time_disable_restart(void);
 void time_enable_restart(void);
 void time_postfork(void);
diff --git a/tests/test-timeval.c b/tests/test-timeval.c
index a58c041..9896cf7 100644
--- a/tests/test-timeval.c
+++ b/tests/test-timeval.c
@@ -99,7 +99,7 @@ main(int argc, char *argv[])
     } else if (!strcmp(argv[1], "plain")) {
         /* If we're not caching time there isn't much to test and SIGALRM won't
          * be around to pull us out of the select() call, so just skip out */
-        if (!time_cached()) {
+        if (!CACHE_TIME) {
             exit (77);
         }
 
@@ -110,7 +110,7 @@ main(int argc, char *argv[])
         char cwd[1024], *pidfile;
         FILE *success;
 
-        if (!time_cached()) {
+        if (!CACHE_TIME) {
             exit (77);
         }
 
diff --git a/vswitchd/system-stats.c b/vswitchd/system-stats.c
index 766e846..90446f2 100644
--- a/vswitchd/system-stats.c
+++ b/vswitchd/system-stats.c
@@ -48,13 +48,12 @@ VLOG_DEFINE_THIS_MODULE(system_stats);
 
 /* #ifdefs make it a pain to maintain code: you have to try to build both ways.
  * Thus, this file tries to compile as much of the code as possible regardless
- * of the target, by writing "if (LINUX)" instead of "#ifdef LINUX_DATAPATH"
- * where this is possible. */
+ * of the target, by writing "if (LINUX_DATAPATH)" instead of "#ifdef
+ * __linux__" where this is possible. */
 #ifdef LINUX_DATAPATH
 #include <asm/param.h>
-#define LINUX 1
 #else
-#define LINUX 0
+#define LINUX_DATAPATH 0
 #endif
 
 static void
@@ -97,7 +96,7 @@ get_page_size(void)
 static void
 get_memory_stats(struct smap *stats)
 {
-    if (!LINUX) {
+    if (!LINUX_DATAPATH) {
         unsigned int pagesize = get_page_size();
         long int phys_pages = sysconf(_SC_PHYS_PAGES);
 #ifdef _SC_AVPHYS_PAGES
@@ -170,7 +169,7 @@ get_boot_time(void)
     static long long int cache_expiration = LLONG_MIN;
     static long long int boot_time;
 
-    assert(LINUX);
+    assert(LINUX_DATAPATH);
 
     if (time_msec() >= cache_expiration) {
         static const char stat_file[] = "/proc/stat";
@@ -202,7 +201,7 @@ get_boot_time(void)
 static unsigned long long int
 ticks_to_ms(unsigned long long int ticks)
 {
-    assert(LINUX);
+    assert(LINUX_DATAPATH);
 
 #ifndef USER_HZ
 #define USER_HZ 100
@@ -235,7 +234,7 @@ get_raw_process_info(pid_t pid, struct raw_process_info *raw)
     FILE *stream;
     int n;
 
-    assert(LINUX);
+    assert(LINUX_DATAPATH);
 
     sprintf(file_name, "/proc/%lu/stat", (unsigned long int) pid);
     stream = fopen(file_name, "r");
@@ -320,7 +319,7 @@ count_crashes(pid_t pid)
     int crashes = 0;
     FILE *stream;
 
-    assert(LINUX);
+    assert(LINUX_DATAPATH);
 
     sprintf(file_name, "/proc/%lu/cmdline", (unsigned long int) pid);
     stream = fopen(file_name, "r");
@@ -363,7 +362,7 @@ get_process_info(pid_t pid, struct process_info *pinfo)
 {
     struct raw_process_info child;
 
-    assert(LINUX);
+    assert(LINUX_DATAPATH);
     if (!get_raw_process_info(pid, &child)) {
         return false;
     }
@@ -428,7 +427,7 @@ get_process_stats(struct smap *stats)
         key = xasprintf("process_%.*s",
                         (int) (extension - de->d_name), de->d_name);
         if (!smap_get(stats, key)) {
-            if (LINUX && get_process_info(pid, &pinfo)) {
+            if (LINUX_DATAPATH && get_process_info(pid, &pinfo)) {
                 smap_add_format(stats, key, "%lu,%lu,%lld,%d,%lld,%lld",
                                 pinfo.vsz, pinfo.rss, pinfo.cputime,
                                 pinfo.crashes, pinfo.booted, pinfo.uptime);
-- 
1.7.12




More information about the dev mailing list