[PATCH 0/1] runqemu: fix handling of SIGTERM and the problem of line wrapping

[PATCH 0/1] runqemu: fix handling of SIGTERM and the problem of line wrapping

[Chen Qi]

*** BLURB HERE ***
The following changes since commit 18b90a0fd9bdc00d82140fbd55761e9cea308fb1:

  yocto-uninative: Add aarch64 uninative tarball checksum (2018-09-20 09:06:00 -0700)

are available in the git repository at:

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

Chen Qi (1):
  runqemu: fix handling of SIGTERM and the problem of line wrapping

 scripts/runqemu | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

-- 
1.9.1

[PATCH 1/1] runqemu: fix handling of SIGTERM and the problem of line wrapping

[Chen Qi]

The current handling of SIGTERM is incorrect as the process pid returned
by Popen call with shell setting to True is actualy the shell instead of
the qemu binary. So fix to send SIGTERM to the process group of runqemu.
This ensures that all processes in the same process group, including the
shell and the qemu process, will receive SIGTERM.

Also, as we install a SIGTERM handler, we need handle the situation of
qemu terminated by SIGTERM, otherwise we will get ERROR message in such
case.

Besides, we have a problem that after running qemu, the terminal's behavior
is incorrect regarding long lines or long commands. Long commands or long
outputs should appear in multiple lines, but they appear in the same line,
overriding previous output. Use `tput smam' to fix this problem.

Signed-off-by: Chen Qi <Qi.Chen@windriver.com>
---
 scripts/runqemu | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/scripts/runqemu b/scripts/runqemu
index 409d17c..bc2aba5 100755
--- a/scripts/runqemu
+++ b/scripts/runqemu
@@ -1213,9 +1213,12 @@ 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())
+        retcode = process.wait()
+        if retcode:
+            if retcode == -signal.SIGTERM:
+                logger.info("Qemu terminated by SIGTERM")
+            else:
+                logger.error("Failed to run qemu: %s", process.stderr.read().decode())
 
     def cleanup(self):
         if self.cleaned:
@@ -1308,8 +1311,10 @@ def main():
 
         def sigterm_handler(signum, frame):
             logger.info("SIGTERM received")
-            os.kill(config.qemupid, signal.SIGTERM)
+            signal.signal(signal.SIGTERM, signal.SIG_IGN)
+            os.kill(0, signal.SIGTERM)
             config.cleanup()
+            subprocess.run(["tput", "smam"])
         signal.signal(signal.SIGTERM, sigterm_handler)
 
         config.check_args()
@@ -1331,6 +1336,7 @@ def main():
         return 1
     finally:
         config.cleanup()
+        subprocess.run(["tput", "smam"])
 
 if __name__ == "__main__":
     sys.exit(main())
-- 
1.9.1

Re: [PATCH 1/1] runqemu: fix handling of SIGTERM and the problem of line wrapping

[Burton, Ross]

Is there a good reason why shell=True is used in the first place?
Just change the cmd to a list, stop passing shell=True, and you don't
need to dance around process groups.

Ross
On Fri, 21 Sep 2018 at 03:30, Chen Qi <Qi.Chen@windriver.com> wrote:
>
> The current handling of SIGTERM is incorrect as the process pid returned
> by Popen call with shell setting to True is actualy the shell instead of
> the qemu binary. So fix to send SIGTERM to the process group of runqemu.
> This ensures that all processes in the same process group, including the
> shell and the qemu process, will receive SIGTERM.
>
> Also, as we install a SIGTERM handler, we need handle the situation of
> qemu terminated by SIGTERM, otherwise we will get ERROR message in such
> case.
>
> Besides, we have a problem that after running qemu, the terminal's behavior
> is incorrect regarding long lines or long commands. Long commands or long
> outputs should appear in multiple lines, but they appear in the same line,
> overriding previous output. Use `tput smam' to fix this problem.
>
> Signed-off-by: Chen Qi <Qi.Chen@windriver.com>
> ---
>  scripts/runqemu | 14 ++++++++++----
>  1 file changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/scripts/runqemu b/scripts/runqemu
> index 409d17c..bc2aba5 100755
> --- a/scripts/runqemu
> +++ b/scripts/runqemu
> @@ -1213,9 +1213,12 @@ 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())
> +        retcode = process.wait()
> +        if retcode:
> +            if retcode == -signal.SIGTERM:
> +                logger.info("Qemu terminated by SIGTERM")
> +            else:
> +                logger.error("Failed to run qemu: %s", process.stderr.read().decode())
>
>      def cleanup(self):
>          if self.cleaned:
> @@ -1308,8 +1311,10 @@ def main():
>
>          def sigterm_handler(signum, frame):
>              logger.info("SIGTERM received")
> -            os.kill(config.qemupid, signal.SIGTERM)
> +            signal.signal(signal.SIGTERM, signal.SIG_IGN)
> +            os.kill(0, signal.SIGTERM)
>              config.cleanup()
> +            subprocess.run(["tput", "smam"])
>          signal.signal(signal.SIGTERM, sigterm_handler)
>
>          config.check_args()
> @@ -1331,6 +1336,7 @@ def main():
>          return 1
>      finally:
>          config.cleanup()
> +        subprocess.run(["tput", "smam"])
>
>  if __name__ == "__main__":
>      sys.exit(main())
> --
> 1.9.1
>
> --
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.openembedded.org/mailman/listinfo/openembedded-core

Re: [PATCH 1/1] runqemu: fix handling of SIGTERM and the problem of line wrapping

[ChenQi]

On 09/21/2018 11:05 PM, Burton, Ross wrote:
> Is there a good reason why shell=True is used in the first place?
> Just change the cmd to a list, stop passing shell=True, and you don't
> need to dance around process groups.

Thanks for your suggestion. I'll send out V2 if all things go well with 
my local testings.

Best Regards,
Chen Qi

> Ross
> On Fri, 21 Sep 2018 at 03:30, Chen Qi <Qi.Chen@windriver.com> wrote:
>> The current handling of SIGTERM is incorrect as the process pid returned
>> by Popen call with shell setting to True is actualy the shell instead of
>> the qemu binary. So fix to send SIGTERM to the process group of runqemu.
>> This ensures that all processes in the same process group, including the
>> shell and the qemu process, will receive SIGTERM.
>>
>> Also, as we install a SIGTERM handler, we need handle the situation of
>> qemu terminated by SIGTERM, otherwise we will get ERROR message in such
>> case.
>>
>> Besides, we have a problem that after running qemu, the terminal's behavior
>> is incorrect regarding long lines or long commands. Long commands or long
>> outputs should appear in multiple lines, but they appear in the same line,
>> overriding previous output. Use `tput smam' to fix this problem.
>>
>> Signed-off-by: Chen Qi <Qi.Chen@windriver.com>
>> ---
>>   scripts/runqemu | 14 ++++++++++----
>>   1 file changed, 10 insertions(+), 4 deletions(-)
>>
>> diff --git a/scripts/runqemu b/scripts/runqemu
>> index 409d17c..bc2aba5 100755
>> --- a/scripts/runqemu
>> +++ b/scripts/runqemu
>> @@ -1213,9 +1213,12 @@ 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())
>> +        retcode = process.wait()
>> +        if retcode:
>> +            if retcode == -signal.SIGTERM:
>> +                logger.info("Qemu terminated by SIGTERM")
>> +            else:
>> +                logger.error("Failed to run qemu: %s", process.stderr.read().decode())
>>
>>       def cleanup(self):
>>           if self.cleaned:
>> @@ -1308,8 +1311,10 @@ def main():
>>
>>           def sigterm_handler(signum, frame):
>>               logger.info("SIGTERM received")
>> -            os.kill(config.qemupid, signal.SIGTERM)
>> +            signal.signal(signal.SIGTERM, signal.SIG_IGN)
>> +            os.kill(0, signal.SIGTERM)
>>               config.cleanup()
>> +            subprocess.run(["tput", "smam"])
>>           signal.signal(signal.SIGTERM, sigterm_handler)
>>
>>           config.check_args()
>> @@ -1331,6 +1336,7 @@ def main():
>>           return 1
>>       finally:
>>           config.cleanup()
>> +        subprocess.run(["tput", "smam"])
>>
>>   if __name__ == "__main__":
>>       sys.exit(main())
>> --
>> 1.9.1
>>
>> --
>> _______________________________________________
>> Openembedded-core mailing list
>> Openembedded-core@lists.openembedded.org
>> http://lists.openembedded.org/mailman/listinfo/openembedded-core