fix: Avoid duplicating dnsmasq arguments (#1113)

.devcontainer/devcontainer.json

@@ -1,6 +1,6 @@
 {
   "name": "Virtual DSM",
-  "service": "vdsm",
+  "service": "dsm",
   "forwardPorts": [5000],
   "portsAttributes": {
     "5000": {
@@ -11,7 +11,5 @@
   "otherPortsAttributes": {
     "onAutoForward": "ignore"
   }, 
-  "dockerComposeFile": "codespaces.yml",
-  "workspaceFolder": "/workspaces/vdsm",
-  "initializeCommand": "docker system prune --all --force"
+  "dockerComposeFile": "codespaces.yml"
 }

.github/workflows/build.yml

@@ -22,7 +22,7 @@ jobs:
     steps:
       -
         name: Checkout
-        uses: actions/checkout@v5
+        uses: actions/checkout@v6
         with:
           fetch-depth: 0
       -

.github/workflows/check.yml

@@ -9,7 +9,7 @@ jobs:
     steps:
       - 
         name: Checkout
-        uses: actions/checkout@v5
+        uses: actions/checkout@v6
       -
         name: Run ShellCheck
         uses: ludeeus/action-shellcheck@master

.github/workflows/hub.yml

@@ -12,13 +12,15 @@ jobs:
   dockerHubDescription:
     runs-on: ubuntu-latest
     steps:
-    - uses: actions/checkout@v5
-    - 
-      name: Docker Hub Description
-      uses: peter-evans/dockerhub-description@v5
-      with:
-        username: ${{ secrets.DOCKERHUB_USERNAME }}
-        password: ${{ secrets.DOCKERHUB_TOKEN }}
-        repository: ${{ secrets.DOCKERHUB_REPO }}
-        short-description: ${{ github.event.repository.description }}
-        readme-filepath: ./readme.md
+      -
+        name: Checkout repo
+        uses: actions/checkout@v6
+      - 
+        name: Docker Hub Description
+        uses: peter-evans/dockerhub-description@v5
+        with:
+          username: ${{ secrets.DOCKERHUB_USERNAME }}
+          password: ${{ secrets.DOCKERHUB_TOKEN }}
+          repository: ${{ secrets.DOCKERHUB_REPO }}
+          short-description: ${{ github.event.repository.description }}
+          readme-filepath: ./readme.md

.github/workflows/review.yml

@@ -15,7 +15,7 @@ jobs:
     steps:
       -
         name: Checkout
-        uses: actions/checkout@v5
+        uses: actions/checkout@v6
       -
         name: Spelling
         uses: reviewdog/action-misspell@v1

src/network.sh

@@ -19,14 +19,16 @@ set -Eeuo pipefail
 : "${VM_NET_HOST:="VirtualDSM"}"
 : "${VM_NET_MASK:="255.255.255.0"}"
 
-: "${PASST:="passt"}"
+: "${PASST:="/run/passt"}"
 : "${PASST_MTU:=""}"
 : "${PASST_OPTS:=""}"
 : "${PASST_DEBUG:=""}"
+: "${PASST_PID:="/var/run/passt.pid"}"
 
 : "${DNSMASQ_OPTS:=""}"
 : "${DNSMASQ_DEBUG:=""}"
 : "${DNSMASQ:="/usr/sbin/dnsmasq"}"
+: "${DNSMASQ_PID:="/var/run/dnsmasq.pid"}"
 : "${DNSMASQ_CONF_DIR:="/etc/dnsmasq.d"}"
 
 ADD_ERR="Please add the following setting to your container:"
@@ -121,14 +123,15 @@ configureDNS() {
   local host="$4"
   local mask="$5"
   local gateway="$6"
+  local arguments="$DNSMASQ_OPTS"
 
   echo "$gateway" > /run/shm/qemu.gw
 
   [[ "${DNSMASQ_DISABLE:-}" == [Yy1]* ]] && return 0
   [[ "$DEBUG" == [Yy1]* ]] && echo "Starting dnsmasq daemon..."
 
-  local log="/var/log/dnsmasq.log"
-  rm -f "$log"
+  [ -s "$DNSMASQ_PID" ] && pKill "$(<"$DNSMASQ_PID")"
+  rm -f "$DNSMASQ_PID"
 
   case "${NETWORK,,}" in
     "tap" | "tun" | "tuntap" | "y" )
@@ -138,40 +141,45 @@ configureDNS() {
       chmod 644 /var/lib/misc/dnsmasq.leases
 
       # dnsmasq configuration:
-      DNSMASQ_OPTS+=" --dhcp-authoritative"
+      arguments+=" --dhcp-authoritative"
 
       # Set DHCP range and host
-      DNSMASQ_OPTS+=" --dhcp-range=$ip,$ip"
-      DNSMASQ_OPTS+=" --dhcp-host=$mac,,$ip,$host,infinite"
+      arguments+=" --dhcp-range=$ip,$ip"
+      arguments+=" --dhcp-host=$mac,,$ip,$host,infinite"
 
       # Set DNS server and gateway
-      DNSMASQ_OPTS+=" --dhcp-option=option:netmask,$mask"
-      DNSMASQ_OPTS+=" --dhcp-option=option:router,$gateway"
-      DNSMASQ_OPTS+=" --dhcp-option=option:dns-server,$gateway"
+      arguments+=" --dhcp-option=option:netmask,$mask"
+      arguments+=" --dhcp-option=option:router,$gateway"
+      arguments+=" --dhcp-option=option:dns-server,$gateway"
 
   esac
 
   # Set interfaces
-  DNSMASQ_OPTS+=" --interface=$if"
-  DNSMASQ_OPTS+=" --bind-interfaces"
+  arguments+=" --interface=$if"
+  arguments+=" --bind-interfaces"
 
   # Add DNS entry for container
-  DNSMASQ_OPTS+=" --address=/host.lan/$gateway"
+  arguments+=" --address=/host.lan/$gateway"
 
   # Set local dns resolver to dnsmasq when needed
-  [ -f /etc/resolv.dnsmasq ] && DNSMASQ_OPTS+=" --resolv-file=/etc/resolv.dnsmasq"
+  [ -f /etc/resolv.dnsmasq ] && arguments+=" --resolv-file=/etc/resolv.dnsmasq"
 
   # Enable logging to file
-  DNSMASQ_OPTS+=" --log-facility=$log"
+  local log="/var/log/dnsmasq.log"
+  rm -f "$log"
+  arguments+=" --log-facility=$log"
 
-  DNSMASQ_OPTS=$(echo "$DNSMASQ_OPTS" | sed 's/\t/ /g' | tr -s ' ' | sed 's/^ *//')
-  [[ "$DEBUG" == [Yy1]* ]] && printf "Dnsmasq arguments:\n\n%s\n\n" "${DNSMASQ_OPTS// -/$'\n-'}"
+  arguments=$(echo "$arguments" | sed 's/\t/ /g' | tr -s ' ' | sed 's/^ *//')
+  [[ "$DEBUG" == [Yy1]* ]] && printf "Dnsmasq arguments:\n\n%s\n\n" "${arguments// -/$'\n-'}"
 
-  if ! $DNSMASQ ${DNSMASQ_OPTS:+ $DNSMASQ_OPTS}; then
+  if ! $DNSMASQ ${arguments:+ $arguments}; then
 
     local msg="Failed to start Dnsmasq, reason: $?"
-    [ -f "$log" ] && cat "$log"
-    error "$msg"
+
+    if [[ "${NETWORK,,}" == "slirp" || "${NETWORK,,}" == "passt" || "$ROOTLESS" != [Yy1]* || "$DEBUG" == [Yy1]* ]]; then
+      [ -f "$log" ] && [ -s "$log" ] && cat "$log"
+      error "$msg"
+    fi
 
     return 1
   fi
@@ -312,9 +320,6 @@ configurePasst() {
   local log="/tmp/passt.log"
   rm -f "$log"
 
-  local pid="/var/run/dnsmasq.pid"
-  [ -s "$pid" ] && pKill "$(<"$pid")"
-
   local ip="$IP"
   [ -n "$VM_NET_IP" ] && ip="$VM_NET_IP"
 
@@ -346,7 +351,7 @@ configurePasst() {
 
   PASST_OPTS+=" -H $VM_NET_HOST"
   PASST_OPTS+=" -M $GATEWAY_MAC"
-  PASST_OPTS+=" -P /tmp/passt.pid"
+  PASST_OPTS+=" -P  $PASST_PID"
   PASST_OPTS+=" -l $log"
   PASST_OPTS+=" -q"
 
@@ -358,6 +363,8 @@ configurePasst() {
   PASST_OPTS=$(echo "$PASST_OPTS" | sed 's/\t/ /g' | tr -s ' ' | sed 's/^ *//')
   [[ "$DEBUG" == [Yy1]* ]] && printf "Passt arguments:\n\n%s\n\n" "${PASST_OPTS// -/$'\n-'}"
 
+  [ ! -f "$PASST" ] && cp /usr/bin/passt* /run
+
   if ! $PASST ${PASST_OPTS:+ $PASST_OPTS} >/dev/null 2>&1; then
 
     rm -f "$log"
@@ -365,7 +372,7 @@ configurePasst() {
     { $PASST ${PASST_OPTS:+ $PASST_OPTS}; rc=$?; } || :
 
     if (( rc != 0 )); then
-      [ -f "$log" ] && cat "$log"
+      [ -f "$log" ] && [ -s "$log" ] && cat "$log"
       warn "failed to start passt ($rc), falling back to slirp networking!"
       configureSlirp && return 0 || return 1
     fi
@@ -376,7 +383,7 @@ configurePasst() {
     tail -fn +0 "$log" --pid=$$ &
   else
     if [[ "$DEBUG" == [Yy1]* ]]; then
-      [ -f "$log" ] && cat "$log" && echo ""
+      [ -f "$log" ] && [ -s "$log" ] && cat "$log" && echo ""
     fi
   fi
 
@@ -495,8 +502,11 @@ configureNAT() {
     fi
   fi
 
-  if ! iptables -t nat -A POSTROUTING -o "$VM_NET_DEV" -j MASQUERADE; then
-    warn "$tables" && return 1
+  if ! iptables -t nat -A POSTROUTING -o "$VM_NET_DEV" -j MASQUERADE > /dev/null 2>&1; then
+    [[ "$ROOTLESS" == [Yy1]* && "$DEBUG" != [Yy1]* ]] && return 1
+    if ! iptables -t nat -A POSTROUTING -o "$VM_NET_DEV" -j MASQUERADE; then
+      warn "$tables" && return 1
+    fi
   fi
 
   # shellcheck disable=SC2086
@@ -530,13 +540,11 @@ configureNAT() {
 
 closeBridge() {
 
-  local pid="/tmp/passt.pid"
-  [ -s "$pid" ] && pKill "$(<"$pid")"
-  rm -f "$pid"
+  [ -s "$PASST_PID" ] && pKill "$(<"$PASST_PID")"
+  rm -f "$PASST_PID"
 
-  pid="/var/run/dnsmasq.pid"
-  [ -s "$pid" ] && pKill "$(<"$pid")"
-  rm -f "$pid"
+  [ -s "$DNSMASQ_PID" ] && pKill "$(<"$DNSMASQ_PID")"
+  rm -f "$DNSMASQ_PID"
 
   case "${NETWORK,,}" in
     "user"* | "passt" | "slirp" ) return 0 ;;
@@ -592,9 +600,9 @@ closeNetwork() {
 cleanUp() {
 
   # Clean up old files
-  rm -f /tmp/passt.pid
+  rm -f "$PASST_PID"
+  rm -f "$DNSMASQ_PID"
   rm -f /etc/resolv.dnsmasq
-  rm -f /var/run/dnsmasq.pid
 
   if [[ -d "/sys/class/net/$VM_NET_TAP" ]]; then
     info "Lingering interface will be removed..."
@@ -788,7 +796,7 @@ else
 
   case "${NETWORK,,}" in
     "passt" | "slirp" | "user"* ) ;;
-    "tap" | "tun" | "tuntap" | "y" )
+    "tap" | "tun" | "tuntap" | "y" | "" )
 
       # Configure tap interface
       if ! configureNAT; then
@@ -807,7 +815,7 @@ else
   esac
 
   case "${NETWORK,,}" in
-    "tap" | "tun" | "tuntap" | "y" ) ;;
+    "tap" | "tun" | "tuntap" | "y" | "" ) ;;
     "passt" | "user"* )
 
       # Configure for user-mode networking (passt)