Skip to contentSkip to conversation

runqemu: clean up correctly on receiving SIGTERM

PATCH In review Chen Qi 2 participants 4 messages Jun 7, 2018

lore
Share & tools
lore

Chen Qi

author
#0 lore raw

cover runqemu: clean up correctly on receiving SIGTERM

The following changes since commit 1c7ad49bfd3e60c44281a8f49d69f4b96c359703:

bitbake: bitbake: Update version to post release 1.39 (2018-06-06 13:35:15 +0100)

are available in the git repository at:

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

Chen Qi (2):
runqemu: add SIGTERM handler to make sure things are cleaned up
runqemu-ifdown: ensure to clean up TAP

 scripts/runqemu        | 23 ++++++++++++++++++++++-
 scripts/runqemu-ifdown | 10 ++++++++++
 2 files changed, 32 insertions(+), 1 deletion(-)
Signature · 1 line
1.9.1
author
#1 lore raw

PATCH 1/2 runqemu: add SIGTERM handler to make sure things are cleaned up

Add SIGTERM handler so that runqemu could clean things up correctly when receving such signal.

This problem was originally observed when running testimage. On some hosts, after running testimage task, the user has to manually operate on the tap interface (e.g. `sudo ip link del tap0') in order for the next runqemu command to launch successfully.

The problem is about runqemu, SIGTERM and network manager on the host.

In testimage task, the runqemu process will receive SIGTERM. In such situation, its cleanup() function is not run, resulting in tap interface not cleaned up. On some hosts, the network manager will bring down the tap interface automatically, thus this problem. I saw this problem on Fedora21.

I think we'd better just clean up the tap interface ourselves.

So this patch adds to runqemu a SIGTERM handler, in which the actual qemu process is terminated and other things cleaned up.

Signed-off-by Chen Qi <[email protected]>
---
 scripts/runqemu | 23 ++++++++++++++++++++++-
 1 file changed, 22 insertions(+), 1 deletion(-)
scripts/runqemu +22 -1
index d998494..de42d0f 100755
@@ -27,6 +27,7 @@ import fcntl
 import shutil
 import glob
 import configparser
+import signal
 
 class RunQemuError(Exception):
     """Custom exception to raise on known errors."""

@@ -233,6 +234,10 @@ class BaseConfig(object):
         # slirp qemus are running.
         self.mac_tap = "52:54:00:12:34:"
         self.mac_slirp = "52:54:00:12:35:"
+        # pid of the actual qemu process
+        self.qemupid = None
+        # avoid cleanup twice
+        self.cleaned = False
 
     def acquire_lock(self, error=True):
         logger.debug("Acquiring lockfile %s..." % self.lock)

@@ -1200,10 +1205,18 @@ class BaseConfig(object):
         cmd = "%s %s" % (self.qemu_opt, kernel_opts)
         logger.info('Running %s\n' % cmd)
         process = subprocess.Popen(cmd, shell=True, stderr=subprocess.PIPE)
+        self.qemupid = process.pid
         if process.wait():
             logger.error("Failed to run qemu: %s", process.stderr.read().decode())
 
     def cleanup(self):
+        if self.cleaned:
+            return
+
+        # avoid dealing with SIGTERM when cleanup function is running
+        signal.signal(signal.SIGTERM, signal.SIG_IGN)
+
+        logger.info("Cleaning up")
         if self.cleantap:
             cmd = 'sudo %s %s %s' % (self.qemuifdown, self.tap, self.bindir_native)
             logger.debug('Running %s' % cmd)

@@ -1227,6 +1240,8 @@ class BaseConfig(object):
             shutil.rmtree(self.rootfs)
             shutil.rmtree('%s.pseudo_state' % self.rootfs)
 
+        self.cleaned = True
+
     def load_bitbake_env(self, mach=None):
         if self.bitbake_e:
             return

@@ -1282,6 +1297,13 @@ def main():
         return 0
     try:
         config = BaseConfig()
+
+        def sigterm_handler(signum, frame):
+            logger.info("SIGTERM received")
+            os.kill(config.qemupid, signal.SIGTERM)
+            config.cleanup()
+        signal.signal(signal.SIGTERM, sigterm_handler)
+
         config.check_args()
         config.read_qemuboot()
         config.check_and_set()

@@ -1300,7 +1322,6 @@ def main():
         traceback.print_exc()
         return 1
     finally:
-        print("Cleanup")
         config.cleanup()
 
 if __name__ == "__main__":
Signature · 1 line
1.9.1
author
#2 lore raw

PATCH 2/2 runqemu-ifdown: ensure to clean up TAP

In reply to Chen Qi

In runqemu-ifup, ip command is used to add TAP; in runqemu-ifdown, we should do the reversed logic, using ip command to delete TAP, to make sure TAP is cleaned up by ourselves.

I can see that in runqemu-ifdown script, 'tunctl -d' and 'iptables' commands are used to deal with TAP, but these two commands cannot make sure that the TAP is cleaned up.

runqemu-ifup uses 'ip' to set up TAP, we really need to do the opposite in runqemu-ifdown.

Signed-off-by Chen Qi <[email protected]>
---
 scripts/runqemu-ifdown | 10 ++++++++++
 1 file changed, 10 insertions(+)
scripts/runqemu-ifdown +10 -0
index ffbc9de..2486968 100755
@@ -51,6 +51,16 @@ fi
 
 $TUNCTL -d $TAP
 
+IFCONFIG=`which ip 2> /dev/null`
+if [ "x$IFCONFIG" = "x" ]; then
+	# better than nothing...
+	IFCONFIG=/sbin/ip
+fi
+if [ -x "$IFCONFIG" ]; then
+	if `$IFCONFIG link show $TAP > /dev/null 2>&1`; then
+		$IFCONFIG link del $TAP
+	fi
+fi
 # cleanup the remaining iptables rules
 IPTABLES=`which iptables 2> /dev/null`
 if [ "x$IPTABLES" = "x" ]; then
Signature · 1 line
1.9.1
#3 lore raw

Re: [PATCH 2/2] runqemu-ifdown: ensure to clean up TAP

Quoted 24 lines · Chen Qi

In runqemu-ifup, ip command is used to add TAP; in runqemu-ifdown, we should do the reversed logic, using ip command to delete TAP, to make sure TAP is cleaned up by ourselves.

I can see that in runqemu-ifdown script, 'tunctl -d' and 'iptables' commands are used to deal with TAP, but these two commands cannot make sure that the TAP is cleaned up.

runqemu-ifup uses 'ip' to set up TAP, we really need to do the opposite in runqemu-ifdown.

Signed-off-by Chen Qi <[email protected]>
---
 scripts/runqemu-ifdown | 10 ++++++++++
 1 file changed, 10 insertions(+)
scripts/runqemu-ifdown +0 -0
index ffbc9de..2486968 100755
@@ -51,6 +51,16 @@ fi
 
 $TUNCTL -d $TAP

scripts/runqemu-ifdown fi · line 63

Quoted 10 lines
scripts/runqemu-ifdown +10 -0
+IFCONFIG=`which ip 2> /dev/null`
+if [ "x$IFCONFIG" = "x" ]; then
+       # better than nothing...
+       IFCONFIG=/sbin/ip
+fi
+if [ -x "$IFCONFIG" ]; then
+       if `$IFCONFIG link show $TAP > /dev/null 2>&1`; then
+               $IFCONFIG link del $TAP
+       fi
+fi

Ah this fixes my problem, Now I dont have to reboot the build box :)