Re: [Qemu-devel] [PATCH] qmp-shell: add persistent command history

[Kashyap Chamarthy]

On Wed, Mar 01, 2017 at 02:44:33PM -0500, John Snow wrote:
> Use the existing readline history function we are utilizing
> to provide persistent command history across instances of qmp-shell.

Thanks for adding it to 'qmp-shell' this is useful.  I normally use the
READLINE functionality using 'qmp_history' file in conjunction with
`socat` (thanks: Markus Armbruster).

    $ socat UNIX:./qmp-sock READLINE,history=$HOME/.qmp_history,prompt='QMP> '

As I often shuffle between 'qmp-shell' and `socat`, so having the
history file conveniently track history is very handy.

> This assists entering debug commands across sessions that may be
> interrupted by QEMU sessions terminating, where the qmp-shell has
> to be relaunched.

> Signed-off-by: John Snow <address@hidden>
> ---
>  scripts/qmp/qmp-shell | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)

I just tested this patch.  And the bare-bones functionality works fine
for me: if the 'qmp_history' file already exists, it'll just append the
history to it; otherwise, it'll create one.

And I have the same question as you, to Nir (especially, I do care
about the history if the 'qmp-shell' process is killed).

For now, I'm abivalent about handling ENOSPC. 

So, with or without the error handling changes that you're discussing,
I'm fine with this initial version of the change, as it is additive.

Just for posterity, my test 'evidence'

- Apply the patch to current Git, and compile:

  $ git log --oneline | head -1
  b8c1c7b qmp-shell: add persistent command history
  
  $ ./qemu-system-x86_64 -version
  QEMU emulator version 2.8.50 (v2.8.0-1813-gb8c1c7b-dirty)

- Run a minimal QEMU instance with '-qmp unix:./qmp-sock,server,nowait']

- Run a few commands from the QMP shell:

    $ ./qmp-shell -v -p /export/qmp-sock 

- Observe the history file:

    $ tail ~/.qmp_history 
    }
    {  
      "execute":"nbd-server-stop"
    }
    { "execute": "query-block" }
    query-block
    query-name
    query-named-block-nodes
    query-kvm
    quit

You see both the raw QMP JSON and 'qmp-shell' history there (as I
mentioned I shuffle between both).

So, FWIW:

  Tested-by: Kashyap Chamarthy <address@hidden>

> diff --git a/scripts/qmp/qmp-shell b/scripts/qmp/qmp-shell
> index 0373b24..b19f44b 100755
> --- a/scripts/qmp/qmp-shell
> +++ b/scripts/qmp/qmp-shell
> @@ -70,6 +70,7 @@ import json
>  import ast
>  import readline
>  import sys
> +import os
>  
>  class QMPCompleter(list):
>      def complete(self, text, state):
> @@ -109,6 +110,8 @@ class QMPShell(qmp.QEMUMonitorProtocol):
>          self._pretty = pretty
>          self._transmode = False
>          self._actions = list()
> +        self._histfile = os.path.join(os.path.expanduser('~'),
> +                                      '.qmp_history')
>  
>      def __get_address(self, arg):
>          """
> @@ -137,6 +140,16 @@ class QMPShell(qmp.QEMUMonitorProtocol):
>          # XXX: default delimiters conflict with some command names (eg. 
> query-),
>          # clearing everything as it doesn't seem to matter
>          readline.set_completer_delims('')
> +        try:
> +            readline.read_history_file(self._histfile)
> +        except:
> +            pass
> +
> +    def __save_history(self):
> +        try:
> +            readline.write_history_file(self._histfile)
> +        except:
> +            pass
>  
>      def __parse_value(self, val):
>          try:
> @@ -244,6 +257,7 @@ class QMPShell(qmp.QEMUMonitorProtocol):
>              print 'command format: <command-name> ',
>              print '[arg-name1=arg1] ... [arg-nameN=argN]'
>              return True
> +        self.__save_history()
>          # For transaction mode, we may have just cached the action:
>          if qmpcmd is None:
>              return True
> -- 
> 2.9.3
> 

-- 
/kashyap