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

[Dov Murik <dovmurik@linux.vnet.ibm.com>]

Hi John,

On 19/01/2021 22:02, John Snow wrote:
> 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.)

Thanks for this effort.

> 
> 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.

Yes, I understand your choice.


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

Yes, I want to send a few QMP commands and record their output.  If I 
use socat to the unix-socket I need to serialize the JSON request 
myself, so using qmp-shell saves me that; also not sure if there's any 
negotiation done at the beginning by qmp-shell.

Is there an easier way to script qmp commands, short of writing my own 
python program which uses the qmp.py library?

-Dov


> 
> --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()
>>
>