Skip to contentSkip to conversation

runqemu fixes about iptables rules and preconfigured tap

PATCH Superseded [email protected] 3 participants 9 messages Aug 27, 2013 – Aug 28, 2013

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore
#0 lore raw

cover runqemu fixes about iptables rules and preconfigured tap

From: Chen Qi <[email protected]>

The following changes since commit 44c3f72684c5c920ce8af1da54a2268047342589:

lib/oeqa/runtime: smart: add checks for smart output (2013-08-26 16:29:18 +0100)

are available in the git repository at:

git://git.pokylinux.org/poky-contrib ChenQi/runqemu-fixes
http://git.pokylinux.org/cgit.cgi/poky-contrib/log/?h=ChenQi/runqemu-fixes

Chen Qi (4):
runqemu-ifdown: clean up the remaining iptables rules
runqemu-internal: don't bring down preconfigured tap interface
runqemu-internal: change the lock acquire/release logic
runqemu-internal: provide more info if a preconfigured tap is used

 scripts/runqemu-ifdown   |   14 ++++++++++++++
 scripts/runqemu-internal |   20 ++++++++++----------
 2 files changed, 24 insertions(+), 10 deletions(-)
Signature · 1 line
1.7.9.5
#1 lore raw

PATCH 1/4 runqemu-ifdown: clean up the remaining iptables rules

From: Chen Qi <[email protected]>

The iptables rules for the tap interface are added by runqemu-ifup everytime we use runqemu to start a qemu target. But it's not cleaned up when runqemu exits.

This patch cleans up the remaining iptables rules for the tap interface in runqemu-ifdown.

[YOCTO #5047]

---
 scripts/runqemu-ifdown |   14 ++++++++++++++
 1 file changed, 14 insertions(+)
scripts/runqemu-ifdown +14 -0
index 8b8c5a4..8f66cfa2 100755
@@ -50,3 +50,17 @@ if [ ! -e "$TUNCTL" ]; then
 fi
 
 $TUNCTL -d $TAP
+
+# cleanup the remaining iptables rules
+IPTABLES=`which iptables 2> /dev/null`
+if [ "x$IPTABLES" = "x" ]; then
+	IPTABLES=/sbin/iptables
+fi
+if [ ! -x "$IPTABLES" ]; then
+	echo "$IPTABLES cannot be executed"
+	exit 1
+fi
+n=$[ (`echo $TAP | sed 's/tap//'` * 2) + 1 ]
+dest=$[ (`echo $TAP | sed 's/tap//'` * 2) + 2 ]
+$IPTABLES -D POSTROUTING -t nat -j MASQUERADE -s 192.168.7.$n/32
+$IPTABLES -D POSTROUTING -t nat -j MASQUERADE -s 192.168.7.$dest/32
Signature · 1 line
1.7.9.5
#2 lore raw

PATCH 2/4 runqemu-internal: don't bring down preconfigured tap interface

In reply to [email protected]

From: Chen Qi <[email protected]>

runqemu-ifup and runqemu-ifdown should be pairs. If we're using a preconfigured tap interface, the runqemu-ifdown should not be invoked to bring it down.

---
 scripts/runqemu-internal |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
scripts/runqemu-internal +3 -1
index 8a6e551..8165e13 100755
@@ -175,12 +175,14 @@ else
         POSSIBLE=`$IFCONFIG link | grep 'tap' | awk '{print $2}' | sed s/://`
         TAP=""
         LOCKFILE=""
+        USE_PRECONF_TAP="no"
         for tap in $POSSIBLE; do
             LOCKFILE="$LOCKDIR/$tap"
             echo "Acquiring lockfile for $tap..."
             acquire_lock $LOCKFILE
             if [ $? -eq 0 ]; then
                 TAP=$tap
+                USE_PRECONF_TAP="yes"
                 break
             fi
         done
@@ -215,7 +217,7 @@ else
         fi
 
         cleanup() {
-            if [ ! -e "$NOSUDO_FLAG" ]; then
+            if [ ! -e "$NOSUDO_FLAG" -a "$USE_PRECONF_TAP" = "no" ]; then
                 # Redirect stderr since we could see a LD_PRELOAD warning here if pseudo is loaded
                 # but inactive. This looks scary but is harmless
                 sudo $QEMUIFDOWN $TAP $OECORE_NATIVE_SYSROOT 2> /dev/null
Signature · 1 line
1.7.9.5
#3 lore raw

PATCH 3/4 runqemu-internal: change the lock acquire/release logic

In reply to [email protected]

From: Chen Qi <[email protected]>

Checking whether the lock file exists is sufficient for runqemu. The contents of the lock file is not important.

This patch simplifies the lock acquire/release logic by removing the flock mechanism. Also, we give more information to user indicating why a tap interface is skipped.

As a side effect, the user now can easily create a lock file to make runqemu skip a specific tap interface.

[YOCTO #5048]

---
 scripts/runqemu-internal |   14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)
scripts/runqemu-internal +6 -8
index 8165e13..3f8674c 100755
@@ -135,15 +135,12 @@ else
                 return 1
             fi
 
-            touch $lockfile.lock
-            exec 8>$lockfile.lock
-            flock -n -x 8
-            if [ $? -ne 0 ]; then
-                exec 8>&-
+            if [ -e $lockfile.lock ]; then
                 return 1
+            else
+                touch $lockfile.lock
+                return 0
             fi
-
-            return 0
         }
 
         release_lock() {
@@ -154,7 +151,6 @@ else
             fi
 
             rm -f $lockfile.lock
-            exec  8>&-
         }
 
         LOCKDIR="/tmp/qemu-tap-locks"
@@ -184,6 +180,8 @@ else
                 TAP=$tap
                 USE_PRECONF_TAP="yes"
                 break
+            else
+                echo "Skipping $tap as it's locked by $LOCKFILE.lock"
             fi
         done
 
Signature · 1 line
1.7.9.5
#8 lore raw

PATCH 4/4 runqemu-internal: provide more info if a preconfigured tap is used

In reply to [email protected]

From: Chen Qi <[email protected]>

We should provide the user more information if a preconfigured tap is used. This is because the user might have manually set up the tap interface to be used by other qemu binaries.

So at a minimum, we should let the user know how to make runqemu skip that tap interface.

[YOCTO #5047]

---
 scripts/runqemu-internal |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
scripts/runqemu-internal +1 -1
index 3f8674c..d6ceb28 100755
@@ -211,7 +211,7 @@ else
                 TAP=$tap
             fi 
         else
-            echo "Using preconfigured tap device '$TAP'"
+            echo "Using preconfigured tap device '$TAP'. If this is not intended, create $LOCKFILE.lock to make runqemu skip $TAP."
         fi
 
         cleanup() {
Signature · 1 line
1.7.9.5
#4 lore raw

Re: [PATCH 3/4] runqemu-internal: change the lock acquire/release logic

In reply to [email protected]
Quoted 4 lines · On Tue, 2013-08-27 at 15:08 +0800, [email protected] wrote:

From: Chen Qi <[email protected]>

Checking whether the lock file exists is sufficient for runqemu. The contents of the lock file is not important.

What happens if the runqemu script crashes and leaves the lockfile around? Why are we removing perfectly functional code?

Cheers,

Richard

#6 lore raw

Re: [PATCH 3/4] runqemu-internal: change the lock acquire/release logic

In reply to [email protected]
Quoted 2 lines · On Tue, 2013-08-27 at 15:08 +0800, [email protected] wrote:

This patch simplifies the lock acquire/release logic by removing the flock mechanism.

I'm not sure that "simplifies" is an accurate description of the results of removing flock. How about "breaks"?

p.

author
#5 lore raw

Re: [PATCH 3/4] runqemu-internal: change the lock acquire/release logic

In reply to Richard Purdie
Quoted 15 lines · Richard Purdie
Quoted 4 lines · On Tue, 2013-08-27 at 15:08 +0800, [email protected] wrote:

From: Chen Qi <[email protected]>

Checking whether the lock file exists is sufficient for runqemu. The contents of the lock file is not important.

What happens if the runqemu script crashes and leaves the lockfile around? Why are we removing perfectly functional code?

Cheers,

Richard

Yes. You're right.
I'll drop this patch and send out a V2.

Best Regards,
Chen Qi

author
#7 lore raw

Re: [PATCH 3/4] runqemu-internal: change the lock acquire/release logic

In reply to Phil Blundell
Quoted 12 lines · Phil Blundell
Quoted 2 lines · On Tue, 2013-08-27 at 15:08 +0800, [email protected] wrote:

This patch simplifies the lock acquire/release logic by removing the flock mechanism.

I'm not sure that "simplifies" is an accurate description of the results of removing flock. How about "breaks"?

p.

That would be a better description.
I then realized I did break the logic.
I'll drop this patch and send out a v2.

Best Regards,
Chen Qi