Re: [PATCH] qmp-shell: Suppress banner and prompt when stdin is not a TTY

[John Snow <jsnow@redhat.com>]

On 1/17/21 2:27 AM, Dov Murik wrote:
> Detect whether qmp-shell's standard input is not a TTY; in such case,
> assume a non-interactive mode, which suppresses the welcome banner and
> the "(QEMU)" prompt.  This allows for easier consumption of qmp-shell's
> output in scripts.
> 
> Example usage before this change:
> 
>      $ printf "query-status\nquery-kvm\n" | sudo scripts/qmp/qmp-shell qmp-unix-sock
>      Welcome to the QMP low-level shell!
>      Connected to QEMU 5.1.50
> 
>      (QEMU) {"return": {"status": "running", "singlestep": false, "running": true}}
>      (QEMU) {"return": {"enabled": true, "present": true}}
>      (QEMU)
> 
> Example usage after this change:
> 
>      $ printf "query-status\nquery-kvm\n" | sudo scripts/qmp/qmp-shell qmp-unix-sock
>      {"return": {"status": "running", "singlestep": false, "running": true}}
>      {"return": {"enabled": true, "present": true}}
> 
> Signed-off-by: Dov Murik <dovmurik@linux.vnet.ibm.com>

Hiya! I've been taking lead on modernizing a lot of our python 
infrastructure, including our QMP library and qmp-shell.

(Sorry, not in MAINTAINERS yet; but I am in the process of moving these 
scripts and tools over to ./python/qemu/qmp.)

This change makes me nervous, because qmp-shell is not traditionally a 
component we've thought of as needing to preserve backwards-compatible 
behavior. Using it as a script meant to be consumed in a headless 
fashion runs a bit counter to that assumption.

I'd be less nervous if the syntax of qmp-shell was something that was 
well thought-out and rigorously tested, but it's a hodge-podge of 
whatever people needed at the moment. I am *very* reluctant to cement it.

Are you trying to leverage the qmp.py library from bash?

--js

> ---
> 
> Notes:
>      Note that this might be considered a breaking change; if users have
>      automated scripts which assume that qmp-shell prints 3 lines of banner,
>      this change will break their scripts.  If there are special
>      considerations/procedures for breaking changes, please let me know.
>      
>      The rationale behaind the TTY check is to imitate python's behaviour:
>      
>          $ python3
>          Python 3.7.5 (default, Apr 19 2020, 20:18:17)
>          [GCC 9.2.1 20191008] on linux
>          Type "help", "copyright", "credits" or "license" for more information.
>          >>> print(19+23)
>          42
>          >>>
>      
>          $ echo 'print(19+23)' | python3
>          42
> 
>   scripts/qmp/qmp-shell | 19 +++++++++++++++++--
>   1 file changed, 17 insertions(+), 2 deletions(-)
> 
> diff --git a/scripts/qmp/qmp-shell b/scripts/qmp/qmp-shell
> index b4d06096ab..9336066fa8 100755
> --- a/scripts/qmp/qmp-shell
> +++ b/scripts/qmp/qmp-shell
> @@ -288,6 +288,8 @@ class QMPShell(qmp.QEMUMonitorProtocol):
>           self.__completer_setup()
>   
>       def show_banner(self, msg='Welcome to the QMP low-level shell!'):
> +        if not self._interactive:
> +            return
>           print(msg)
>           if not self._greeting:
>               print('Connected')
> @@ -300,6 +302,15 @@ class QMPShell(qmp.QEMUMonitorProtocol):
>               return "TRANS> "
>           return "(QEMU) "
>   
> +    def read_command(self, prompt):
> +        if self._interactive:
> +            return input(prompt)
> +        else:
> +            line = sys.stdin.readline()
> +            if not line:
> +                raise EOFError
> +            return line
> +
>       def read_exec_command(self, prompt):
>           """
>           Read and execute a command.
> @@ -307,7 +318,7 @@ class QMPShell(qmp.QEMUMonitorProtocol):
>           @return True if execution was ok, return False if disconnected.
>           """
>           try:
> -            cmdline = input(prompt)
> +            cmdline = self.read_command(prompt)
>           except EOFError:
>               print()
>               return False
> @@ -322,6 +333,9 @@ class QMPShell(qmp.QEMUMonitorProtocol):
>       def set_verbosity(self, verbose):
>           self._verbose = verbose
>   
> +    def set_interactive(self, interactive):
> +        self._interactive = interactive
> +
>   class HMPShell(QMPShell):
>       def __init__(self, address):
>           QMPShell.__init__(self, address)
> @@ -449,8 +463,9 @@ def main():
>       except qemu.error:
>           die('Could not connect to %s' % addr)
>   
> -    qemu.show_banner()
>       qemu.set_verbosity(verbose)
> +    qemu.set_interactive(sys.stdin.isatty())
> +    qemu.show_banner()
>       while qemu.read_exec_command(qemu.get_prompt()):
>           pass
>       qemu.close()
>