case-lib: fix pulseaudio restore couldn't work

[Bin-QA]

1. hijack.sh modify sudo function:
   use --preserve-env=PATH instead of env 'PATH=$PATH'
   which can be worked at 20.04 test environment

2. remove 'nohup' for sudo command to avoid reset sudo

3. kill extend 'sudo' process for pulseaudio after restore finish

Signed-off-by: Wu, BinX binx.wu@intel.com

PS: see also #289

[Bin-QA]

verify script:

$ cat test-case/tmp.sh
#!/bin/bash

source $(dirname ${BASH_SOURCE[0]})/../case-lib/lib.sh

ps -ef |grep pulseaudio

func_lib_disable_pulseaudio
dlogi "PULSECMD_LST:"
echo ${PULSECMD_LST[@]}
dlogi "PULSE_PATHS"
echo ${PULSE_PATHS[@]}

if [ ${#PULSECMD_LST} -ne 0 ];then
for i in $(seq 1 10)
do
        dlogc "sleep $i"
        sleep 1s
done
fi

func_lib_restore_pulseaudio
ps -ef |grep pulseaudio

exit 0

[aiChaoSONG]

@Bin-QA I suggest you split this PR into 2 commits.

• the env change in hijack.sh
• other change in lib.sh

[fredoh9]

Looks good. For first commit text, a typo. How about just

Use --preserve-env=PATH instead of env 'PATH=$PATH'
for sudo parameter

[marc-hb]

This commit message is really too short and doesn't really explain anything.

Please add the error messages to the commit message, a Fixes: #NNN for any bug filed about this (I think there was at least one), explain why these error messages were seen and why this gets rid of them.

[marc-hb]

On my Ubuntu 20.04 system:

   1859 ?        Ss     0:00 /lib/systemd/systemd --user
   1870 ?        Ssl    0:00  \_ /usr/bin/pulseaudio --daemonize=no --log-target=journal

You can't re-use --daemonize=no from a command line, what makes sense for systemd does not make sense from a command line.

[Bin-QA]

On my Ubuntu 20.04 system:

You can't re-use --daemonize=no from a command line, what makes sense for systemd does not make sense from a command line.

@marc-hb
here try to restore to process, because we couldn't confirm which user to load this service, so you think to restart service?

[libinyang]

On my Ubuntu 20.04 system:

   1859 ?        Ss     0:00 /lib/systemd/systemd --user
   1870 ?        Ssl    0:00  \_ /usr/bin/pulseaudio --daemonize=no --log-target=journal

You can't re-use --daemonize=no from a command line, what makes sense for systemd does not make sense from a command line.

Mostly we should not use --daemonize=no. But as we are trying to restore the pulseaudio and we have add "&" at the end of the command, I think it should be OK. Which one is better, pulseaudio --daemonize=no --log-target=journal & or pulseaudio -D --log-target=journal? If both doesn't satisfy our purpose, maybe we have to use systemctl to restart the pulseaudio.

[marc-hb]

Each Linux distribution may use pulseaudio slightly differently. We should stick to how how each distribution configures it.

A very quick search didn't find any summary for Ubuntu. I found something interesting for Debian which may or may not apply to Ubuntu https://wiki.debian.org/PulseAudio

The name systemctl status pulseaudio-enable-autospawn seems very promising but I haven't tested it.

If we don't have enough time now to figure out how to disable/re-enable it cleanly on Ubuntu then we should just uninstall it and postpone pulseaudio tests until we have time to do it properly = not using ps. I have run apt remove pulseaudio on Ubuntu 20.04 weeks or maybe months ago and it didn't cause any issue. In fact apt remove/install pulseaudio could be one clean and simple solution with the caveat of uncontrolled pulseaudio upgrades.

[plbossart]

Looks like this PulseAudio issue is polluting quite a few platforms.

https://sof-ci.01.org/linuxpr/PR1984/build4016/devicetest/

2020-06-30 00:24:06 UTC [REMOTE_INFO] checking for general errors after kmod insert with sof-kernel-log-check tool
2020-06-30 00:24:06 UTC [REMOTE_INFO] checking if firmware is loaded successfully
2020-06-30 00:24:06 UTC [REMOTE_INFO] ==== firmware boot complete: 2 of 2 ====
2020-06-30 00:24:07 UTC [REMOTE_INFO] Restoring pulseaudio
usage: sudo -h | -K | -k | -V
usage: sudo -v [-AknS] [-g group] [-h host] [-p prompt] [-u user]
usage: sudo -l [-AknS] [-g group] [-h host] [-p prompt] [-U user] [-u user]
            [command]
usage: sudo [-AbEHknPS] [-r role] [-t type] [-C num] [-g group] [-h host] [-p
            prompt] [-T timeout] [-u user] [VAR=value] [-i|-s] []
usage: sudo -e [-AknS] [-r role] [-t type] [-C num] [-g group] [-h host] [-p
            prompt] [-T timeout] [-u user] file ...
2020-06-30 00:24:18 UTC [REMOTE_INFO] Time out. Pulseaudio not restored in 10 seconds
2020-06-30 00:24:18 UTC [REMOTE_INFO] Test Result: FAIL!

can we please prioritize this? Thanks!

[marc-hb]

OK, I checked the git log -p of hijack.sh (did no one else do this yet?) and I think I found why this is recent regression: in commit c7d2477 merged on 27th May @Bin-QA made this change:

         ;;
-        '1')    # sudo without passwd
-            eval $(echo "$SUDO_CMD env 'PATH=$PATH' $*")
-            return $?
+        '1')    cmd="$SUDO_CMD env 'PATH=$PATH' $*" # sudo without passwd
         ;;
-        '2')    # sudo need passwd
-            eval $(echo "echo '$SUDO_PASSWD' | $SUDO_CMD -S env 'PATH=$PATH' $*")
-            return $?
+        '2')    cmd="echo '$SUDO_PASSWD' | $SUDO_CMD -S env 'PATH=$PATH' $*" # sudo need passwd
         ;;
         *)      # without sudo permission

Single-quoting $PATH and $SUDO_PASSWD is obviously not going to work once the eval is removed. This particular code change was apparently not tested.

@Bin-QA please submit the minimum, thoroughly tested change that fixes only the sudo usage error and nothing else:

usage: sudo -h | -K | -k | -V
usage: sudo -v [-AknS] [-g group] [-h host] [-p prompt] [-u user]
...

Let's postpone longer pulseaudio discussion and redesigns for another time.

This UNTESTED suggestion may be enough:

     case $SUDO_LEVEL in
         '0')    cmd="$*" # as root
         ;;
-        '1')    cmd="$SUDO_CMD env 'PATH=$PATH' $*" # sudo without passwd
+        '1')    cmd="$SUDO_CMD env PATH=$PATH $*" # sudo without passwd
         ;;
-        '2')    cmd="echo '$SUDO_PASSWD' | $SUDO_CMD -S env 'PATH=$PATH' $*" # sudo need passwd
+        '2')    cmd="echo $SUDO_PASSWD | $SUDO_CMD -S env PATH=$PATH $*" # sudo need passwd
         ;;
         *)      # without sudo permission

It won't support whitespace in PATH but it's simpler and I doubt we ever supported that anyway.

Commit c7d2477 was merged on 27th so I wonder how this error wasn't noticed earlier, any clue? Has anything related to pulseaudio changed recently?

[marc-hb]

so I wonder how this error wasn't noticed earlier, any clue? Has anything related to pulseaudio changed recently?

Is our pulseaudio installation and configuration consistent across DUTs?

[aiChaoSONG]

so I wonder how this error wasn't noticed earlier, any clue? Has anything related to pulseaudio changed recently?

Is our pulseaudio installation and configuration consistent across DUTs?

I remember that @keqiaozhang uninstall pulseaudio from all DUTs, don't know why it is still there.

[keqiaozhang]

so I wonder how this error wasn't noticed earlier, any clue? Has anything related to pulseaudio changed recently?

Is our pulseaudio installation and configuration consistent across DUTs?

I remember that @keqiaozhang uninstall pulseaudio from all DUTs, don't know why it is still there.

Yes, I uninstalled pulseaudio for all our DUTs months ago and added this feature in deploy.sh. I checked and found that those DUTs have been upgraded to Ubuntu 20.04 by someone and forgot to uninstall the pulseaudio.

[marc-hb]

Is the deploy.sh script idempotent? It should be.

Is it compatible with Ubuntu 20.04?

[xiulipan]

Close as new version merged.