diff mbox series

[ovs-dev,v1] ovs container build: Make kernel module configurable

Message ID 20191112084758.3663-1-amginwal@gmail.com
State Superseded
Headers show
Series [ovs-dev,v1] ovs container build: Make kernel module configurable | expand

Commit Message

aginwala aginwala Nov. 12, 2019, 8:47 a.m. UTC 
From: Aliasgar Ginwala <aginwala@ebay.com>

--with-linux can be made configurable while building containers
for leveraging kernel modules installed on host.
KERNEL_VERSION=host should be used in env variable for the same.

Signed-off-by: Aliasgar Ginwala <aginwala@ebay.com>
---
 utilities/docker/Makefile                       |  2 +-
 utilities/docker/debian/build-kernel-modules.sh | 16 +++++++++++++---
 utilities/docker/start-ovs                      |  9 ++++++++-
 3 files changed, 22 insertions(+), 5 deletions(-)

Comments

Ben Pfaff Nov. 22, 2019, 12:53 a.m. UTC | #1 
On Tue, Nov 12, 2019 at 12:47:58AM -0800, amginwal@gmail.com wrote:
> -./configure --localstatedir="/var" --sysconfdir="/etc" --prefix="/usr" \
> +if [ $KERNEL_VERSION == "host" ]; then

Supporting == in [] or "test" is a GNUism.  Use = for wider support
(including BSD).

> +   ./configure --localstatedir="/var" --sysconfdir="/etc" --prefix="/usr" \
> +--enable-ssl
> +else
> +    ./configure --localstatedir="/var" --sysconfdir="/etc" --prefix="/usr" \
>  --with-linux=/lib/modules/$KERNEL_VERSION/build --enable-ssl
> +fi

There's a lot of duplication above, I'd suggest using variables to
reduce the duplication.
aginwala Dec. 20, 2019, 12:52 a.m. UTC | #2 
Thanks Ben for the review. Sorry for the delay!

On Thu, Nov 21, 2019 at 4:54 PM Ben Pfaff <blp@ovn.org> wrote:

> On Tue, Nov 12, 2019 at 12:47:58AM -0800, amginwal@gmail.com wrote:
> > -./configure --localstatedir="/var" --sysconfdir="/etc" --prefix="/usr" \
> > +if [ $KERNEL_VERSION == "host" ]; then
>
> Supporting == in [] or "test" is a GNUism.  Use = for wider support
> (including BSD).
>
Sure. Sent v2 https://patchwork.ozlabs.org/patch/1213844/ to address both
the comments . PTAL.

>
> > +   ./configure --localstatedir="/var" --sysconfdir="/etc"
> --prefix="/usr" \
> > +--enable-ssl
> > +else
> > +    ./configure --localstatedir="/var" --sysconfdir="/etc"
> --prefix="/usr" \
> >  --with-linux=/lib/modules/$KERNEL_VERSION/build --enable-ssl
> > +fi
>
> There's a lot of duplication above, I'd suggest using variables to
> reduce the duplication.
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
diff mbox series

Patch

diff --git a/utilities/docker/Makefile b/utilities/docker/Makefile
index 8c2f7810e..d8b08a3c9 100644
--- a/utilities/docker/Makefile
+++ b/utilities/docker/Makefile
@@ -10,7 +10,7 @@ 
 #   make push
 
 REPO = ${DOCKER_REPO}
-tag = ${OVS_VERSION}_${KERNEL_VERSION}
+tag = ${OVS_VERSION}_${DISTRO}_${KERNEL_VERSION}
 
 build: ;docker build -t ${REPO}:${tag} --build-arg DISTRO=${DISTRO} \
 --build-arg OVS_BRANCH=${OVS_BRANCH} \
diff --git a/utilities/docker/debian/build-kernel-modules.sh b/utilities/docker/debian/build-kernel-modules.sh
index 18ac35764..17a67bfcb 100755
--- a/utilities/docker/debian/build-kernel-modules.sh
+++ b/utilities/docker/debian/build-kernel-modules.sh
@@ -17,13 +17,17 @@  OVS_BRANCH=$2
 GITHUB_SRC=$3
 
 # Install deps
-linux="linux-image-$KERNEL_VERSION linux-headers-$KERNEL_VERSION"
 build_deps="apt-utils libelf-dev build-essential libssl-dev python3 \
 python3-six wget gdb autoconf libtool git automake bzip2 debhelper \
 dh-autoreconf openssl"
 
 apt-get update
-apt-get install -y ${linux} ${build_deps}
+
+if [ $KERNEL_VERSION != "host" ]; then
+    linux="linux-image-$KERNEL_VERSION linux-headers-$KERNEL_VERSION"
+    apt-get install -y ${linux}
+fi
+apt-get install -y ${build_deps}
 
 # get the source
 mkdir /build; cd /build
@@ -32,8 +36,14 @@  cd ovs
 
 # build and install
 ./boot.sh
-./configure --localstatedir="/var" --sysconfdir="/etc" --prefix="/usr" \
+if [ $KERNEL_VERSION == "host" ]; then
+   ./configure --localstatedir="/var" --sysconfdir="/etc" --prefix="/usr" \
+--enable-ssl
+else
+    ./configure --localstatedir="/var" --sysconfdir="/etc" --prefix="/usr" \
 --with-linux=/lib/modules/$KERNEL_VERSION/build --enable-ssl
+fi
+
 make -j8; make install; make modules_install
 
 # remove deps to make the container light weight.
diff --git a/utilities/docker/start-ovs b/utilities/docker/start-ovs
index 4a1a16cd1..c99380519 100755
--- a/utilities/docker/start-ovs
+++ b/utilities/docker/start-ovs
@@ -38,5 +38,12 @@  case $1 in
                         -vfile:info --mlockall --no-chdir \
                         --log-file=/var/log/openvswitch/ovs-vswitchd.log
         ;;
-        *) echo "$0 [ovsdb-server|ovs-vswitchd]"
+        "ovs-vswitchd-host") /usr/share/openvswitch/scripts/ovs-ctl \
+                             --no-ovsdb-server start
+                             /usr/share/openvswitch/scripts/ovs-ctl stop
+                             ovs-vswitchd --pidfile -vconsole:emer \
+                             -vsyslog:err -vfile:info --mlockall --no-chdir \
+                             --log-file=/var/log/openvswitch/ovs-vswitchd.log
+        ;;
+        *) echo "$0 [ovsdb-server|ovs-vswitchd|ovs-vswitchd-host]"
 esac