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

Ben Pfaff blp at nicira.com
Tue Oct 9 18:32:38 UTC 2012


On Mon, Oct 08, 2012 at 12:49:21PM -0700, Ethan Jackson wrote:
> The ESX userspace looks quite a bit like linux, but has some key
> differences which need to be specially handled in the build.  To
> distinguish between ESX and systems use the linux datapath module,
> this patch adds two new macros "ESX" and "LINUX_DATAPATH".  It uses
> these macros to disable building code on ESX which only applies to
> a true Linux environment.  In addition, it adds a new
> route-table-stub implementation which is required for the build to
> complete successfully on ESX.
> 
> Signed-off-by: Ethan Jackson <ethan at nicira.com>

Thanks.

"-a" isn't portable, so I'd change each of the tests here to
        test "$HAVE_NETLINK" = yes && test "$ESX" = no
> +AM_CONDITIONAL([LINUX_DATAPATH], [test "$HAVE_NETLINK" = yes -a "$ESX" = no])
> +if test "$HAVE_NETLINK" = yes -a "$ESX" = no; then
> +    AC_DEFINE([LINUX_DATAPATH], [1], [System uses the linux datapath module.])
> +fi

Does the 'argv' code in command-line.c actually malfunction on ESX?
It'd be nice to just leave it in, if not.  It's not really tied to
having the Linux datapath, it's orthogonal.  (It should actually work
on many Unix and Unix-like systems, but we specialize it to Linux
pending testing on other systems.)

The new version of the comment in socket-util.c doesn't make sense.  I
think it should say "if (LINUX_DATAPATH)" instead of "#ifdef
__linux__":

    /* #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". */

It also changes LINUX so that it's equivalent to LINUX_DATAPATH.  That
seems odd; perhaps it should instead just do:
    #ifndef LINUX_DATAPATH
    #define LINUX_DATAPATH 0
    #endif
in case that's the intention?

Similarly in system-stats.c

I see a couple uses of "#if AF_PACKET && LINUX_DATAPATH" in this
file.  Perhaps adding a configure check for linux/if_packet.h and
using HAVE_LINUX_IF_PACKET_H as a test would be better?  I think that
that actually would better describe the need.

Does ESX have /proc/self/fd/* that works like the Linux version?  If
so then __linux__ is the right test for put_fd_filename(), otherwise
LINUX_DATAPATH seems good enough for now.

In timeval.c, if we use LINUX_DATAPATH as the check for cache_time,
then can we make that a compile-time constant again?

Thanks!

Ben



More information about the dev mailing list