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

[Nir Soffer]

On Mon, Mar 13, 2017 at 8:04 AM Markus Armbruster  wrote:

> Nir Soffer  writes:
>
> > On Wed, Mar 8, 2017 at 8:29 AM, Markus Armbruster 
> wrote:
> >> John Snow  writes:
> >>
> >>> On 03/07/2017 03:16 AM, Markus Armbruster wrote:
>  John Snow  writes:
> 
> > On 03/06/2017 03:18 AM, Markus Armbruster wrote:
> >> Nir Soffer  writes:
> >>
> >>> On Fri, Mar 3, 2017 at 9:29 PM, John Snow 
> wrote:
> 
> 
>  On 03/03/2017 02:26 PM, Nir Soffer wrote:
> > On Fri, Mar 3, 2017 at 8:54 PM, John Snow 
> wrote:
> >> Use the existing readline history function we are utilizing
> >> to provide persistent command history across instances of
> qmp-shell.
> >>
> >> 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 
> >> ---
> >>
> >> v2: Adjusted the errors to whine about non-ENOENT errors, but
> still
> >> intercept all errors as non-fatal.
> >> Save history atexit() to match bash standard behavior
> >>
> >>  scripts/qmp/qmp-shell | 19 +++
> >>  1 file changed, 19 insertions(+)
> >>
> >> diff --git a/scripts/qmp/qmp-shell b/scripts/qmp/qmp-shell
> >> index 0373b24..55a8285 100755
> >> --- a/scripts/qmp/qmp-shell
> >> +++ b/scripts/qmp/qmp-shell
> >> @@ -70,6 +70,9 @@ import json
> >>  import ast
> >>  import readline
> >>  import sys
> >> +import os
> >> +import errno
> >> +import atexit
> >>
> >>  class QMPCompleter(list):
> >>  def complete(self, text, state):
> >> @@ -109,6 +112,7 @@ class QMPShell(qmp.QEMUMonitorProtocol):
> >>  self._pretty = pretty
> >>  self._transmode = False
> >>  self._actions = list()
> >> +self._histfile = os.path.join(os.path.expanduser('~'),
> '.qmp_history')
> >>
> >> I selfishly object to this filename, because I'm using it with
> >>
> >> $ socat UNIX:/work/armbru/images/test-qmp
> READLINE,history=$HOME/.qmp_history,prompt='QMP> '
> >>
> >> Just kidding.  But seriously, shouldn't this be named after the
> >> *application* (qmp-shell) rather than the protocol (qmp)?
> >>
> >
> > Seeing as the history itself is the qmp-shell syntax, you are
> correct.
> >
> > (Hey, it would be interesting to store the generated QMP into the
> > qmp_history, though...!)
> 
>  Hah!  Saving it would be easy enough, but reloading it...  okay, call
> it
>  a "backup" and declare victory when saving works.
> 
> >>
> >>  def __get_address(self, arg):
> >>  """
> >> @@ -137,6 +141,21 @@ 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 Exception as e:
> >> +if isinstance(e, IOError) and e.errno ==
> errno.ENOENT:
> >> +# File not found. No problem.
> >> +pass
> >> +else:
> >> +print "Failed to read history '%s'; %s" %
> (self._histfile, e)
> >
> > I would handle only IOError, since any other error means a bug
> in this code
> > or in the underlying readline library, and the best way to
> handle this is to
> > let it fail loudly.
> >
> 
>  Disagree. No reason to stop the shell from working because a QOL
> feature
>  didn't initialize correctly.
> 
>  The warning will be enough to solicit reports and fixes if
> necessary.
> >>>
> >>> I agree, the current solution is good tradeoff.
> >>
> >> For what it's worth, bash seems to silently ignore a history file it
> >> can't read.  Tested by running "HISTFILE=xxx bash", then chmod 0
> xxx, da
> >> capo.
> >>
> >
> > Right, done by example.
> >
> >>> One thing missing, is a call to readline.set_history_length,
> without
> >>> it the history
> >>> will grow without limit, see:
> >>>
> https://docs.python.org/2/library/readline.html#readline.set_history_length
> >>
> >> Should this be addressed for 2.9?
> >>
> >
> > You can add a limit if you want to; I don't have 

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

[Markus Armbruster]

Nir Soffer  writes:

> On Wed, Mar 8, 2017 at 8:29 AM, Markus Armbruster  wrote:
>> John Snow  writes:
>>
>>> On 03/07/2017 03:16 AM, Markus Armbruster wrote:
 John Snow  writes:

> On 03/06/2017 03:18 AM, Markus Armbruster wrote:
>> Nir Soffer  writes:
>>
>>> On Fri, Mar 3, 2017 at 9:29 PM, John Snow  wrote:


 On 03/03/2017 02:26 PM, Nir Soffer wrote:
> On Fri, Mar 3, 2017 at 8:54 PM, John Snow  wrote:
>> Use the existing readline history function we are utilizing
>> to provide persistent command history across instances of qmp-shell.
>>
>> 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 
>> ---
>>
>> v2: Adjusted the errors to whine about non-ENOENT errors, but still
>> intercept all errors as non-fatal.
>> Save history atexit() to match bash standard behavior
>>
>>  scripts/qmp/qmp-shell | 19 +++
>>  1 file changed, 19 insertions(+)
>>
>> diff --git a/scripts/qmp/qmp-shell b/scripts/qmp/qmp-shell
>> index 0373b24..55a8285 100755
>> --- a/scripts/qmp/qmp-shell
>> +++ b/scripts/qmp/qmp-shell
>> @@ -70,6 +70,9 @@ import json
>>  import ast
>>  import readline
>>  import sys
>> +import os
>> +import errno
>> +import atexit
>>
>>  class QMPCompleter(list):
>>  def complete(self, text, state):
>> @@ -109,6 +112,7 @@ class QMPShell(qmp.QEMUMonitorProtocol):
>>  self._pretty = pretty
>>  self._transmode = False
>>  self._actions = list()
>> +self._histfile = os.path.join(os.path.expanduser('~'), 
>> '.qmp_history')
>>
>> I selfishly object to this filename, because I'm using it with
>>
>> $ socat UNIX:/work/armbru/images/test-qmp 
>> READLINE,history=$HOME/.qmp_history,prompt='QMP> '
>>
>> Just kidding.  But seriously, shouldn't this be named after the
>> *application* (qmp-shell) rather than the protocol (qmp)?
>>
>
> Seeing as the history itself is the qmp-shell syntax, you are correct.
>
> (Hey, it would be interesting to store the generated QMP into the
> qmp_history, though...!)

 Hah!  Saving it would be easy enough, but reloading it...  okay, call it
 a "backup" and declare victory when saving works.

>>
>>  def __get_address(self, arg):
>>  """
>> @@ -137,6 +141,21 @@ 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 Exception as e:
>> +if isinstance(e, IOError) and e.errno == errno.ENOENT:
>> +# File not found. No problem.
>> +pass
>> +else:
>> +print "Failed to read history '%s'; %s" % 
>> (self._histfile, e)
>
> I would handle only IOError, since any other error means a bug in 
> this code
> or in the underlying readline library, and the best way to handle 
> this is to
> let it fail loudly.
>

 Disagree. No reason to stop the shell from working because a QOL 
 feature
 didn't initialize correctly.

 The warning will be enough to solicit reports and fixes if necessary.
>>>
>>> I agree, the current solution is good tradeoff.
>>
>> For what it's worth, bash seems to silently ignore a history file it
>> can't read.  Tested by running "HISTFILE=xxx bash", then chmod 0 xxx, da
>> capo.
>>
>
> Right, done by example.
>
>>> One thing missing, is a call to readline.set_history_length, without
>>> it the history
>>> will grow without limit, see:
>>> https://docs.python.org/2/library/readline.html#readline.set_history_length
>>
>> Should this be addressed for 2.9?
>>
>
> You can add a limit if you want to; I don't have suggestions for which
> completely arbitrary limit makes sense, so I left it blank intentionally.

 For what it's worth, bash defaults HISTSIZE to 500.

 GNU Readline lets users configure it in ~/.inputrc.  Conditional
 configuration is 

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

[Nir Soffer]

On Wed, Mar 8, 2017 at 8:29 AM, Markus Armbruster  wrote:
> John Snow  writes:
>
>> On 03/07/2017 03:16 AM, Markus Armbruster wrote:
>>> John Snow  writes:
>>>
 On 03/06/2017 03:18 AM, Markus Armbruster wrote:
> Nir Soffer  writes:
>
>> On Fri, Mar 3, 2017 at 9:29 PM, John Snow  wrote:
>>>
>>>
>>> On 03/03/2017 02:26 PM, Nir Soffer wrote:
 On Fri, Mar 3, 2017 at 8:54 PM, John Snow  wrote:
> Use the existing readline history function we are utilizing
> to provide persistent command history across instances of qmp-shell.
>
> 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 
> ---
>
> v2: Adjusted the errors to whine about non-ENOENT errors, but still
> intercept all errors as non-fatal.
> Save history atexit() to match bash standard behavior
>
>  scripts/qmp/qmp-shell | 19 +++
>  1 file changed, 19 insertions(+)
>
> diff --git a/scripts/qmp/qmp-shell b/scripts/qmp/qmp-shell
> index 0373b24..55a8285 100755
> --- a/scripts/qmp/qmp-shell
> +++ b/scripts/qmp/qmp-shell
> @@ -70,6 +70,9 @@ import json
>  import ast
>  import readline
>  import sys
> +import os
> +import errno
> +import atexit
>
>  class QMPCompleter(list):
>  def complete(self, text, state):
> @@ -109,6 +112,7 @@ class QMPShell(qmp.QEMUMonitorProtocol):
>  self._pretty = pretty
>  self._transmode = False
>  self._actions = list()
> +self._histfile = os.path.join(os.path.expanduser('~'), 
> '.qmp_history')
>
> I selfishly object to this filename, because I'm using it with
>
> $ socat UNIX:/work/armbru/images/test-qmp 
> READLINE,history=$HOME/.qmp_history,prompt='QMP> '
>
> Just kidding.  But seriously, shouldn't this be named after the
> *application* (qmp-shell) rather than the protocol (qmp)?
>

 Seeing as the history itself is the qmp-shell syntax, you are correct.

 (Hey, it would be interesting to store the generated QMP into the
 qmp_history, though...!)
>>>
>>> Hah!  Saving it would be easy enough, but reloading it...  okay, call it
>>> a "backup" and declare victory when saving works.
>>>
>
>  def __get_address(self, arg):
>  """
> @@ -137,6 +141,21 @@ 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 Exception as e:
> +if isinstance(e, IOError) and e.errno == errno.ENOENT:
> +# File not found. No problem.
> +pass
> +else:
> +print "Failed to read history '%s'; %s" % 
> (self._histfile, e)

 I would handle only IOError, since any other error means a bug in this 
 code
 or in the underlying readline library, and the best way to handle this 
 is to
 let it fail loudly.

>>>
>>> Disagree. No reason to stop the shell from working because a QOL feature
>>> didn't initialize correctly.
>>>
>>> The warning will be enough to solicit reports and fixes if necessary.
>>
>> I agree, the current solution is good tradeoff.
>
> For what it's worth, bash seems to silently ignore a history file it
> can't read.  Tested by running "HISTFILE=xxx bash", then chmod 0 xxx, da
> capo.
>

 Right, done by example.

>> One thing missing, is a call to readline.set_history_length, without
>> it the history
>> will grow without limit, see:
>> https://docs.python.org/2/library/readline.html#readline.set_history_length
>
> Should this be addressed for 2.9?
>

 You can add a limit if you want to; I don't have suggestions for which
 completely arbitrary limit makes sense, so I left it blank intentionally.
>>>
>>> For what it's worth, bash defaults HISTSIZE to 500.
>>>
>>> GNU Readline lets users configure it in ~/.inputrc.  Conditional
>>> configuration is possible, i.e. something like
>>>
>>> $if Qmp-shell
>>> set history-size 5000
>>> $endif
>>>
>>> should work, provided qmp-shell sets rl_readline_name as it should.

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

[Markus Armbruster]

John Snow  writes:

> On 03/07/2017 03:16 AM, Markus Armbruster wrote:
>> John Snow  writes:
>> 
>>> On 03/06/2017 03:18 AM, Markus Armbruster wrote:
 Nir Soffer  writes:

> On Fri, Mar 3, 2017 at 9:29 PM, John Snow  wrote:
>>
>>
>> On 03/03/2017 02:26 PM, Nir Soffer wrote:
>>> On Fri, Mar 3, 2017 at 8:54 PM, John Snow  wrote:
 Use the existing readline history function we are utilizing
 to provide persistent command history across instances of qmp-shell.

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

 v2: Adjusted the errors to whine about non-ENOENT errors, but still
 intercept all errors as non-fatal.
 Save history atexit() to match bash standard behavior

  scripts/qmp/qmp-shell | 19 +++
  1 file changed, 19 insertions(+)

 diff --git a/scripts/qmp/qmp-shell b/scripts/qmp/qmp-shell
 index 0373b24..55a8285 100755
 --- a/scripts/qmp/qmp-shell
 +++ b/scripts/qmp/qmp-shell
 @@ -70,6 +70,9 @@ import json
  import ast
  import readline
  import sys
 +import os
 +import errno
 +import atexit

  class QMPCompleter(list):
  def complete(self, text, state):
 @@ -109,6 +112,7 @@ class QMPShell(qmp.QEMUMonitorProtocol):
  self._pretty = pretty
  self._transmode = False
  self._actions = list()
 +self._histfile = os.path.join(os.path.expanduser('~'), 
 '.qmp_history')

 I selfishly object to this filename, because I'm using it with

 $ socat UNIX:/work/armbru/images/test-qmp 
 READLINE,history=$HOME/.qmp_history,prompt='QMP> '

 Just kidding.  But seriously, shouldn't this be named after the
 *application* (qmp-shell) rather than the protocol (qmp)?

>>>
>>> Seeing as the history itself is the qmp-shell syntax, you are correct.
>>>
>>> (Hey, it would be interesting to store the generated QMP into the
>>> qmp_history, though...!)
>> 
>> Hah!  Saving it would be easy enough, but reloading it...  okay, call it
>> a "backup" and declare victory when saving works.
>> 

  def __get_address(self, arg):
  """
 @@ -137,6 +141,21 @@ 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 Exception as e:
 +if isinstance(e, IOError) and e.errno == errno.ENOENT:
 +# File not found. No problem.
 +pass
 +else:
 +print "Failed to read history '%s'; %s" % 
 (self._histfile, e)
>>>
>>> I would handle only IOError, since any other error means a bug in this 
>>> code
>>> or in the underlying readline library, and the best way to handle this 
>>> is to
>>> let it fail loudly.
>>>
>>
>> Disagree. No reason to stop the shell from working because a QOL feature
>> didn't initialize correctly.
>>
>> The warning will be enough to solicit reports and fixes if necessary.
>
> I agree, the current solution is good tradeoff.

 For what it's worth, bash seems to silently ignore a history file it
 can't read.  Tested by running "HISTFILE=xxx bash", then chmod 0 xxx, da
 capo.

>>>
>>> Right, done by example.
>>>
> One thing missing, is a call to readline.set_history_length, without
> it the history
> will grow without limit, see:
> https://docs.python.org/2/library/readline.html#readline.set_history_length

 Should this be addressed for 2.9?

>>>
>>> You can add a limit if you want to; I don't have suggestions for which
>>> completely arbitrary limit makes sense, so I left it blank intentionally.
>> 
>> For what it's worth, bash defaults HISTSIZE to 500.
>> 
>> GNU Readline lets users configure it in ~/.inputrc.  Conditional
>> configuration is possible, i.e. something like
>> 
>> $if Qmp-shell
>> set history-size 5000
>> $endif
>> 
>> should work, provided qmp-shell sets rl_readline_name as it should.
>> 
>
> Spoke too soon. I don't see a way to control this in python's readline
> library... I'm not very familiar with readline, is there some
> environment variable or something perhaps?
> (It looks 

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

[John Snow]

On 03/07/2017 03:16 AM, Markus Armbruster wrote:
> John Snow  writes:
> 
>> On 03/06/2017 03:18 AM, Markus Armbruster wrote:
>>> Nir Soffer  writes:
>>>
 On Fri, Mar 3, 2017 at 9:29 PM, John Snow  wrote:
>
>
> On 03/03/2017 02:26 PM, Nir Soffer wrote:
>> On Fri, Mar 3, 2017 at 8:54 PM, John Snow  wrote:
>>> Use the existing readline history function we are utilizing
>>> to provide persistent command history across instances of qmp-shell.
>>>
>>> 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 
>>> ---
>>>
>>> v2: Adjusted the errors to whine about non-ENOENT errors, but still
>>> intercept all errors as non-fatal.
>>> Save history atexit() to match bash standard behavior
>>>
>>>  scripts/qmp/qmp-shell | 19 +++
>>>  1 file changed, 19 insertions(+)
>>>
>>> diff --git a/scripts/qmp/qmp-shell b/scripts/qmp/qmp-shell
>>> index 0373b24..55a8285 100755
>>> --- a/scripts/qmp/qmp-shell
>>> +++ b/scripts/qmp/qmp-shell
>>> @@ -70,6 +70,9 @@ import json
>>>  import ast
>>>  import readline
>>>  import sys
>>> +import os
>>> +import errno
>>> +import atexit
>>>
>>>  class QMPCompleter(list):
>>>  def complete(self, text, state):
>>> @@ -109,6 +112,7 @@ class QMPShell(qmp.QEMUMonitorProtocol):
>>>  self._pretty = pretty
>>>  self._transmode = False
>>>  self._actions = list()
>>> +self._histfile = os.path.join(os.path.expanduser('~'), 
>>> '.qmp_history')
>>>
>>> I selfishly object to this filename, because I'm using it with
>>>
>>> $ socat UNIX:/work/armbru/images/test-qmp 
>>> READLINE,history=$HOME/.qmp_history,prompt='QMP> '
>>>
>>> Just kidding.  But seriously, shouldn't this be named after the
>>> *application* (qmp-shell) rather than the protocol (qmp)?
>>>
>>
>> Seeing as the history itself is the qmp-shell syntax, you are correct.
>>
>> (Hey, it would be interesting to store the generated QMP into the
>> qmp_history, though...!)
> 
> Hah!  Saving it would be easy enough, but reloading it...  okay, call it
> a "backup" and declare victory when saving works.
> 
>>>
>>>  def __get_address(self, arg):
>>>  """
>>> @@ -137,6 +141,21 @@ 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 Exception as e:
>>> +if isinstance(e, IOError) and e.errno == errno.ENOENT:
>>> +# File not found. No problem.
>>> +pass
>>> +else:
>>> +print "Failed to read history '%s'; %s" % 
>>> (self._histfile, e)
>>
>> I would handle only IOError, since any other error means a bug in this 
>> code
>> or in the underlying readline library, and the best way to handle this 
>> is to
>> let it fail loudly.
>>
>
> Disagree. No reason to stop the shell from working because a QOL feature
> didn't initialize correctly.
>
> The warning will be enough to solicit reports and fixes if necessary.

 I agree, the current solution is good tradeoff.
>>>
>>> For what it's worth, bash seems to silently ignore a history file it
>>> can't read.  Tested by running "HISTFILE=xxx bash", then chmod 0 xxx, da
>>> capo.
>>>
>>
>> Right, done by example.
>>
 One thing missing, is a call to readline.set_history_length, without
 it the history
 will grow without limit, see:
 https://docs.python.org/2/library/readline.html#readline.set_history_length
>>>
>>> Should this be addressed for 2.9?
>>>
>>
>> You can add a limit if you want to; I don't have suggestions for which
>> completely arbitrary limit makes sense, so I left it blank intentionally.
> 
> For what it's worth, bash defaults HISTSIZE to 500.
> 
> GNU Readline lets users configure it in ~/.inputrc.  Conditional
> configuration is possible, i.e. something like
> 
> $if Qmp-shell
> set history-size 5000
> $endif
> 
> should work, provided qmp-shell sets rl_readline_name as it should.
> 

Spoke too soon. I don't see a way to control this in python's readline
library... I'm not very familiar with readline, is there some
environment variable or something perhaps?

(It looks like python's code just hard-sets it to "python" ...)

>>> +atexit.register(self.__save_history)
>>> +
>>> +def __save_history(self):
>>> +   

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

[John Snow]

On 03/07/2017 03:16 AM, Markus Armbruster wrote:
> John Snow  writes:
> 
>> On 03/06/2017 03:18 AM, Markus Armbruster wrote:
>>> Nir Soffer  writes:
>>>
 On Fri, Mar 3, 2017 at 9:29 PM, John Snow  wrote:
>
>
> On 03/03/2017 02:26 PM, Nir Soffer wrote:
>> On Fri, Mar 3, 2017 at 8:54 PM, John Snow  wrote:
>>> Use the existing readline history function we are utilizing
>>> to provide persistent command history across instances of qmp-shell.
>>>
>>> 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 
>>> ---
>>>
>>> v2: Adjusted the errors to whine about non-ENOENT errors, but still
>>> intercept all errors as non-fatal.
>>> Save history atexit() to match bash standard behavior
>>>
>>>  scripts/qmp/qmp-shell | 19 +++
>>>  1 file changed, 19 insertions(+)
>>>
>>> diff --git a/scripts/qmp/qmp-shell b/scripts/qmp/qmp-shell
>>> index 0373b24..55a8285 100755
>>> --- a/scripts/qmp/qmp-shell
>>> +++ b/scripts/qmp/qmp-shell
>>> @@ -70,6 +70,9 @@ import json
>>>  import ast
>>>  import readline
>>>  import sys
>>> +import os
>>> +import errno
>>> +import atexit
>>>
>>>  class QMPCompleter(list):
>>>  def complete(self, text, state):
>>> @@ -109,6 +112,7 @@ class QMPShell(qmp.QEMUMonitorProtocol):
>>>  self._pretty = pretty
>>>  self._transmode = False
>>>  self._actions = list()
>>> +self._histfile = os.path.join(os.path.expanduser('~'), 
>>> '.qmp_history')
>>>
>>> I selfishly object to this filename, because I'm using it with
>>>
>>> $ socat UNIX:/work/armbru/images/test-qmp 
>>> READLINE,history=$HOME/.qmp_history,prompt='QMP> '
>>>
>>> Just kidding.  But seriously, shouldn't this be named after the
>>> *application* (qmp-shell) rather than the protocol (qmp)?
>>>
>>
>> Seeing as the history itself is the qmp-shell syntax, you are correct.
>>
>> (Hey, it would be interesting to store the generated QMP into the
>> qmp_history, though...!)
> 
> Hah!  Saving it would be easy enough, but reloading it...  okay, call it
> a "backup" and declare victory when saving works.
> 
>>>
>>>  def __get_address(self, arg):
>>>  """
>>> @@ -137,6 +141,21 @@ 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 Exception as e:
>>> +if isinstance(e, IOError) and e.errno == errno.ENOENT:
>>> +# File not found. No problem.
>>> +pass
>>> +else:
>>> +print "Failed to read history '%s'; %s" % 
>>> (self._histfile, e)
>>
>> I would handle only IOError, since any other error means a bug in this 
>> code
>> or in the underlying readline library, and the best way to handle this 
>> is to
>> let it fail loudly.
>>
>
> Disagree. No reason to stop the shell from working because a QOL feature
> didn't initialize correctly.
>
> The warning will be enough to solicit reports and fixes if necessary.

 I agree, the current solution is good tradeoff.
>>>
>>> For what it's worth, bash seems to silently ignore a history file it
>>> can't read.  Tested by running "HISTFILE=xxx bash", then chmod 0 xxx, da
>>> capo.
>>>
>>
>> Right, done by example.
>>
 One thing missing, is a call to readline.set_history_length, without
 it the history
 will grow without limit, see:
 https://docs.python.org/2/library/readline.html#readline.set_history_length
>>>
>>> Should this be addressed for 2.9?
>>>
>>
>> You can add a limit if you want to; I don't have suggestions for which
>> completely arbitrary limit makes sense, so I left it blank intentionally.
> 
> For what it's worth, bash defaults HISTSIZE to 500.
> 
> GNU Readline lets users configure it in ~/.inputrc.  Conditional
> configuration is possible, i.e. something like
> 
> $if Qmp-shell
> set history-size 5000
> $endif
> 
> should work, provided qmp-shell sets rl_readline_name as it should.
> 

There's the winning ticket!

>>> +atexit.register(self.__save_history)
>>> +
>>> +def __save_history(self):
>>> +try:
>>> +readline.write_history_file(self._histfile)
>>> +except Exception as e:
>>> +print "Failed to save history file '%s'; %s" % 
>>> (self._histfile, e)

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

[Markus Armbruster]

John Snow  writes:

> On 03/06/2017 03:18 AM, Markus Armbruster wrote:
>> Nir Soffer  writes:
>> 
>>> On Fri, Mar 3, 2017 at 9:29 PM, John Snow  wrote:


 On 03/03/2017 02:26 PM, Nir Soffer wrote:
> On Fri, Mar 3, 2017 at 8:54 PM, John Snow  wrote:
>> Use the existing readline history function we are utilizing
>> to provide persistent command history across instances of qmp-shell.
>>
>> 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 
>> ---
>>
>> v2: Adjusted the errors to whine about non-ENOENT errors, but still
>> intercept all errors as non-fatal.
>> Save history atexit() to match bash standard behavior
>>
>>  scripts/qmp/qmp-shell | 19 +++
>>  1 file changed, 19 insertions(+)
>>
>> diff --git a/scripts/qmp/qmp-shell b/scripts/qmp/qmp-shell
>> index 0373b24..55a8285 100755
>> --- a/scripts/qmp/qmp-shell
>> +++ b/scripts/qmp/qmp-shell
>> @@ -70,6 +70,9 @@ import json
>>  import ast
>>  import readline
>>  import sys
>> +import os
>> +import errno
>> +import atexit
>>
>>  class QMPCompleter(list):
>>  def complete(self, text, state):
>> @@ -109,6 +112,7 @@ class QMPShell(qmp.QEMUMonitorProtocol):
>>  self._pretty = pretty
>>  self._transmode = False
>>  self._actions = list()
>> +self._histfile = os.path.join(os.path.expanduser('~'), 
>> '.qmp_history')
>> 
>> I selfishly object to this filename, because I'm using it with
>> 
>> $ socat UNIX:/work/armbru/images/test-qmp 
>> READLINE,history=$HOME/.qmp_history,prompt='QMP> '
>> 
>> Just kidding.  But seriously, shouldn't this be named after the
>> *application* (qmp-shell) rather than the protocol (qmp)?
>> 
>
> Seeing as the history itself is the qmp-shell syntax, you are correct.
>
> (Hey, it would be interesting to store the generated QMP into the
> qmp_history, though...!)

Hah!  Saving it would be easy enough, but reloading it...  okay, call it
a "backup" and declare victory when saving works.

>>
>>  def __get_address(self, arg):
>>  """
>> @@ -137,6 +141,21 @@ 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 Exception as e:
>> +if isinstance(e, IOError) and e.errno == errno.ENOENT:
>> +# File not found. No problem.
>> +pass
>> +else:
>> +print "Failed to read history '%s'; %s" % 
>> (self._histfile, e)
>
> I would handle only IOError, since any other error means a bug in this 
> code
> or in the underlying readline library, and the best way to handle this is 
> to
> let it fail loudly.
>

 Disagree. No reason to stop the shell from working because a QOL feature
 didn't initialize correctly.

 The warning will be enough to solicit reports and fixes if necessary.
>>>
>>> I agree, the current solution is good tradeoff.
>> 
>> For what it's worth, bash seems to silently ignore a history file it
>> can't read.  Tested by running "HISTFILE=xxx bash", then chmod 0 xxx, da
>> capo.
>> 
>
> Right, done by example.
>
>>> One thing missing, is a call to readline.set_history_length, without
>>> it the history
>>> will grow without limit, see:
>>> https://docs.python.org/2/library/readline.html#readline.set_history_length
>> 
>> Should this be addressed for 2.9?
>> 
>
> You can add a limit if you want to; I don't have suggestions for which
> completely arbitrary limit makes sense, so I left it blank intentionally.

For what it's worth, bash defaults HISTSIZE to 500.

GNU Readline lets users configure it in ~/.inputrc.  Conditional
configuration is possible, i.e. something like

$if Qmp-shell
set history-size 5000
$endif

should work, provided qmp-shell sets rl_readline_name as it should.

>> +atexit.register(self.__save_history)
>> +
>> +def __save_history(self):
>> +try:
>> +readline.write_history_file(self._histfile)
>> +except Exception as e:
>> +print "Failed to save history file '%s'; %s" % 
>> (self._histfile, e)
>>
>>  def __parse_value(self, val):
>>  try:
>
> But I think this is good enough and useful as is.
>
> Reviewed-by: Nir Soffer 
>

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

[John Snow]

On 03/06/2017 03:18 AM, Markus Armbruster wrote:
> Nir Soffer  writes:
> 
>> On Fri, Mar 3, 2017 at 9:29 PM, John Snow  wrote:
>>>
>>>
>>> On 03/03/2017 02:26 PM, Nir Soffer wrote:
 On Fri, Mar 3, 2017 at 8:54 PM, John Snow  wrote:
> Use the existing readline history function we are utilizing
> to provide persistent command history across instances of qmp-shell.
>
> 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 
> ---
>
> v2: Adjusted the errors to whine about non-ENOENT errors, but still
> intercept all errors as non-fatal.
> Save history atexit() to match bash standard behavior
>
>  scripts/qmp/qmp-shell | 19 +++
>  1 file changed, 19 insertions(+)
>
> diff --git a/scripts/qmp/qmp-shell b/scripts/qmp/qmp-shell
> index 0373b24..55a8285 100755
> --- a/scripts/qmp/qmp-shell
> +++ b/scripts/qmp/qmp-shell
> @@ -70,6 +70,9 @@ import json
>  import ast
>  import readline
>  import sys
> +import os
> +import errno
> +import atexit
>
>  class QMPCompleter(list):
>  def complete(self, text, state):
> @@ -109,6 +112,7 @@ class QMPShell(qmp.QEMUMonitorProtocol):
>  self._pretty = pretty
>  self._transmode = False
>  self._actions = list()
> +self._histfile = os.path.join(os.path.expanduser('~'), 
> '.qmp_history')
> 
> I selfishly object to this filename, because I'm using it with
> 
> $ socat UNIX:/work/armbru/images/test-qmp 
> READLINE,history=$HOME/.qmp_history,prompt='QMP> '
> 
> Just kidding.  But seriously, shouldn't this be named after the
> *application* (qmp-shell) rather than the protocol (qmp)?
> 

Seeing as the history itself is the qmp-shell syntax, you are correct.

(Hey, it would be interesting to store the generated QMP into the
qmp_history, though...!)

>
>  def __get_address(self, arg):
>  """
> @@ -137,6 +141,21 @@ 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 Exception as e:
> +if isinstance(e, IOError) and e.errno == errno.ENOENT:
> +# File not found. No problem.
> +pass
> +else:
> +print "Failed to read history '%s'; %s" % 
> (self._histfile, e)

 I would handle only IOError, since any other error means a bug in this code
 or in the underlying readline library, and the best way to handle this is 
 to
 let it fail loudly.

>>>
>>> Disagree. No reason to stop the shell from working because a QOL feature
>>> didn't initialize correctly.
>>>
>>> The warning will be enough to solicit reports and fixes if necessary.
>>
>> I agree, the current solution is good tradeoff.
> 
> For what it's worth, bash seems to silently ignore a history file it
> can't read.  Tested by running "HISTFILE=xxx bash", then chmod 0 xxx, da
> capo.
> 

Right, done by example.

>> One thing missing, is a call to readline.set_history_length, without
>> it the history
>> will grow without limit, see:
>> https://docs.python.org/2/library/readline.html#readline.set_history_length
> 
> Should this be addressed for 2.9?
> 

You can add a limit if you want to; I don't have suggestions for which
completely arbitrary limit makes sense, so I left it blank intentionally.

> +atexit.register(self.__save_history)
> +
> +def __save_history(self):
> +try:
> +readline.write_history_file(self._histfile)
> +except Exception as e:
> +print "Failed to save history file '%s'; %s" % 
> (self._histfile, e)
>
>  def __parse_value(self, val):
>  try:

 But I think this is good enough and useful as is.

 Reviewed-by: Nir Soffer 

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

[Kashyap Chamarthy]

On Mon, Mar 06, 2017 at 09:18:29AM +0100, Markus Armbruster wrote:
> Nir Soffer  writes:
> 
> > On Fri, Mar 3, 2017 at 9:29 PM, John Snow  wrote:

[...]

>   class QMPCompleter(list):
>   def complete(self, text, state):
>  @@ -109,6 +112,7 @@ class QMPShell(qmp.QEMUMonitorProtocol):
>   self._pretty = pretty
>   self._transmode = False
>   self._actions = list()
>  +self._histfile = os.path.join(os.path.expanduser('~'), 
>  '.qmp_history')
> 
> I selfishly object to this filename, because I'm using it with
> 
> $ socat UNIX:/work/armbru/images/test-qmp 
> READLINE,history=$HOME/.qmp_history,prompt='QMP> '
> 
> Just kidding.  But seriously, shouldn't this be named after the
> *application* (qmp-shell) rather than the protocol (qmp)?

FWIW, that sounds reasonable to me.  It's clearer that way -- the raw
QMP input history, when using via `socat` as above and the 'qmp-shell'
history won't get mixed up.

[...]

> >> The warning will be enough to solicit reports and fixes if necessary.
> >
> > I agree, the current solution is good tradeoff.
> 
> For what it's worth, bash seems to silently ignore a history file it
> can't read.  Tested by running "HISTFILE=xxx bash", then chmod 0 xxx, da
> capo.
> 
> > One thing missing, is a call to readline.set_history_length, without
> > it the history
> > will grow without limit, see:
> > https://docs.python.org/2/library/readline.html#readline.set_history_length
> 
> Should this be addressed for 2.9?

Sounds like a nice-to-have for me.

>  +atexit.register(self.__save_history)
>  +
>  +def __save_history(self):
>  +try:
>  +readline.write_history_file(self._histfile)
>  +except Exception as e:
>  +print "Failed to save history file '%s'; %s" % 
>  (self._histfile, e)
> 
>   def __parse_value(self, val):
>   try:
> >>>
> >>> But I think this is good enough and useful as is.
> >>>
> >>> Reviewed-by: Nir Soffer 
> >>>
> 

-- 
/kashyap

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

[Markus Armbruster]

Nir Soffer  writes:

> On Fri, Mar 3, 2017 at 9:29 PM, John Snow  wrote:
>>
>>
>> On 03/03/2017 02:26 PM, Nir Soffer wrote:
>>> On Fri, Mar 3, 2017 at 8:54 PM, John Snow  wrote:
 Use the existing readline history function we are utilizing
 to provide persistent command history across instances of qmp-shell.

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

 v2: Adjusted the errors to whine about non-ENOENT errors, but still
 intercept all errors as non-fatal.
 Save history atexit() to match bash standard behavior

  scripts/qmp/qmp-shell | 19 +++
  1 file changed, 19 insertions(+)

 diff --git a/scripts/qmp/qmp-shell b/scripts/qmp/qmp-shell
 index 0373b24..55a8285 100755
 --- a/scripts/qmp/qmp-shell
 +++ b/scripts/qmp/qmp-shell
 @@ -70,6 +70,9 @@ import json
  import ast
  import readline
  import sys
 +import os
 +import errno
 +import atexit

  class QMPCompleter(list):
  def complete(self, text, state):
 @@ -109,6 +112,7 @@ class QMPShell(qmp.QEMUMonitorProtocol):
  self._pretty = pretty
  self._transmode = False
  self._actions = list()
 +self._histfile = os.path.join(os.path.expanduser('~'), 
 '.qmp_history')

I selfishly object to this filename, because I'm using it with

$ socat UNIX:/work/armbru/images/test-qmp 
READLINE,history=$HOME/.qmp_history,prompt='QMP> '

Just kidding.  But seriously, shouldn't this be named after the
*application* (qmp-shell) rather than the protocol (qmp)?


  def __get_address(self, arg):
  """
 @@ -137,6 +141,21 @@ 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 Exception as e:
 +if isinstance(e, IOError) and e.errno == errno.ENOENT:
 +# File not found. No problem.
 +pass
 +else:
 +print "Failed to read history '%s'; %s" % 
 (self._histfile, e)
>>>
>>> I would handle only IOError, since any other error means a bug in this code
>>> or in the underlying readline library, and the best way to handle this is to
>>> let it fail loudly.
>>>
>>
>> Disagree. No reason to stop the shell from working because a QOL feature
>> didn't initialize correctly.
>>
>> The warning will be enough to solicit reports and fixes if necessary.
>
> I agree, the current solution is good tradeoff.

For what it's worth, bash seems to silently ignore a history file it
can't read.  Tested by running "HISTFILE=xxx bash", then chmod 0 xxx, da
capo.

> One thing missing, is a call to readline.set_history_length, without
> it the history
> will grow without limit, see:
> https://docs.python.org/2/library/readline.html#readline.set_history_length

Should this be addressed for 2.9?

 +atexit.register(self.__save_history)
 +
 +def __save_history(self):
 +try:
 +readline.write_history_file(self._histfile)
 +except Exception as e:
 +print "Failed to save history file '%s'; %s" % 
 (self._histfile, e)

  def __parse_value(self, val):
  try:
>>>
>>> But I think this is good enough and useful as is.
>>>
>>> Reviewed-by: Nir Soffer 
>>>

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

[Nir Soffer]

On Fri, Mar 3, 2017 at 9:29 PM, John Snow  wrote:
>
>
> On 03/03/2017 02:26 PM, Nir Soffer wrote:
>> On Fri, Mar 3, 2017 at 8:54 PM, John Snow  wrote:
>>> Use the existing readline history function we are utilizing
>>> to provide persistent command history across instances of qmp-shell.
>>>
>>> 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 
>>> ---
>>>
>>> v2: Adjusted the errors to whine about non-ENOENT errors, but still
>>> intercept all errors as non-fatal.
>>> Save history atexit() to match bash standard behavior
>>>
>>>  scripts/qmp/qmp-shell | 19 +++
>>>  1 file changed, 19 insertions(+)
>>>
>>> diff --git a/scripts/qmp/qmp-shell b/scripts/qmp/qmp-shell
>>> index 0373b24..55a8285 100755
>>> --- a/scripts/qmp/qmp-shell
>>> +++ b/scripts/qmp/qmp-shell
>>> @@ -70,6 +70,9 @@ import json
>>>  import ast
>>>  import readline
>>>  import sys
>>> +import os
>>> +import errno
>>> +import atexit
>>>
>>>  class QMPCompleter(list):
>>>  def complete(self, text, state):
>>> @@ -109,6 +112,7 @@ 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 +141,21 @@ 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 Exception as e:
>>> +if isinstance(e, IOError) and e.errno == errno.ENOENT:
>>> +# File not found. No problem.
>>> +pass
>>> +else:
>>> +print "Failed to read history '%s'; %s" % (self._histfile, 
>>> e)
>>
>> I would handle only IOError, since any other error means a bug in this code
>> or in the underlying readline library, and the best way to handle this is to
>> let it fail loudly.
>>
>
> Disagree. No reason to stop the shell from working because a QOL feature
> didn't initialize correctly.
>
> The warning will be enough to solicit reports and fixes if necessary.

I agree, the current solution is good tradeoff.

One thing missing, is a call to readline.set_history_length, without
it the history
will grow without limit, see:
https://docs.python.org/2/library/readline.html#readline.set_history_length

>
>>> +atexit.register(self.__save_history)
>>> +
>>> +def __save_history(self):
>>> +try:
>>> +readline.write_history_file(self._histfile)
>>> +except Exception as e:
>>> +print "Failed to save history file '%s'; %s" % 
>>> (self._histfile, e)
>>>
>>>  def __parse_value(self, val):
>>>  try:
>>
>> But I think this is good enough and useful as is.
>>
>> Reviewed-by: Nir Soffer 
>>

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

[John Snow]

On 03/03/2017 02:26 PM, Nir Soffer wrote:
> On Fri, Mar 3, 2017 at 8:54 PM, John Snow  wrote:
>> Use the existing readline history function we are utilizing
>> to provide persistent command history across instances of qmp-shell.
>>
>> 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 
>> ---
>>
>> v2: Adjusted the errors to whine about non-ENOENT errors, but still
>> intercept all errors as non-fatal.
>> Save history atexit() to match bash standard behavior
>>
>>  scripts/qmp/qmp-shell | 19 +++
>>  1 file changed, 19 insertions(+)
>>
>> diff --git a/scripts/qmp/qmp-shell b/scripts/qmp/qmp-shell
>> index 0373b24..55a8285 100755
>> --- a/scripts/qmp/qmp-shell
>> +++ b/scripts/qmp/qmp-shell
>> @@ -70,6 +70,9 @@ import json
>>  import ast
>>  import readline
>>  import sys
>> +import os
>> +import errno
>> +import atexit
>>
>>  class QMPCompleter(list):
>>  def complete(self, text, state):
>> @@ -109,6 +112,7 @@ 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 +141,21 @@ 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 Exception as e:
>> +if isinstance(e, IOError) and e.errno == errno.ENOENT:
>> +# File not found. No problem.
>> +pass
>> +else:
>> +print "Failed to read history '%s'; %s" % (self._histfile, 
>> e)
> 
> I would handle only IOError, since any other error means a bug in this code
> or in the underlying readline library, and the best way to handle this is to
> let it fail loudly.
> 

Disagree. No reason to stop the shell from working because a QOL feature
didn't initialize correctly.

The warning will be enough to solicit reports and fixes if necessary.

>> +atexit.register(self.__save_history)
>> +
>> +def __save_history(self):
>> +try:
>> +readline.write_history_file(self._histfile)
>> +except Exception as e:
>> +print "Failed to save history file '%s'; %s" % (self._histfile, 
>> e)
>>
>>  def __parse_value(self, val):
>>  try:
> 
> But I think this is good enough and useful as is.
> 
> Reviewed-by: Nir Soffer 
> 

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

[Nir Soffer]

On Fri, Mar 3, 2017 at 8:54 PM, John Snow  wrote:
> Use the existing readline history function we are utilizing
> to provide persistent command history across instances of qmp-shell.
>
> 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 
> ---
>
> v2: Adjusted the errors to whine about non-ENOENT errors, but still
> intercept all errors as non-fatal.
> Save history atexit() to match bash standard behavior
>
>  scripts/qmp/qmp-shell | 19 +++
>  1 file changed, 19 insertions(+)
>
> diff --git a/scripts/qmp/qmp-shell b/scripts/qmp/qmp-shell
> index 0373b24..55a8285 100755
> --- a/scripts/qmp/qmp-shell
> +++ b/scripts/qmp/qmp-shell
> @@ -70,6 +70,9 @@ import json
>  import ast
>  import readline
>  import sys
> +import os
> +import errno
> +import atexit
>
>  class QMPCompleter(list):
>  def complete(self, text, state):
> @@ -109,6 +112,7 @@ 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 +141,21 @@ 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 Exception as e:
> +if isinstance(e, IOError) and e.errno == errno.ENOENT:
> +# File not found. No problem.
> +pass
> +else:
> +print "Failed to read history '%s'; %s" % (self._histfile, e)

I would handle only IOError, since any other error means a bug in this code
or in the underlying readline library, and the best way to handle this is to
let it fail loudly.

> +atexit.register(self.__save_history)
> +
> +def __save_history(self):
> +try:
> +readline.write_history_file(self._histfile)
> +except Exception as e:
> +print "Failed to save history file '%s'; %s" % (self._histfile, 
> e)
>
>  def __parse_value(self, val):
>  try:

But I think this is good enough and useful as is.

Reviewed-by: Nir Soffer 

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

[John Snow]

Use the existing readline history function we are utilizing
to provide persistent command history across instances of qmp-shell.

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

v2: Adjusted the errors to whine about non-ENOENT errors, but still
intercept all errors as non-fatal.
Save history atexit() to match bash standard behavior

 scripts/qmp/qmp-shell | 19 +++
 1 file changed, 19 insertions(+)

diff --git a/scripts/qmp/qmp-shell b/scripts/qmp/qmp-shell
index 0373b24..55a8285 100755
--- a/scripts/qmp/qmp-shell
+++ b/scripts/qmp/qmp-shell
@@ -70,6 +70,9 @@ import json
 import ast
 import readline
 import sys
+import os
+import errno
+import atexit
 
 class QMPCompleter(list):
 def complete(self, text, state):
@@ -109,6 +112,7 @@ 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 +141,21 @@ 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 Exception as e:
+if isinstance(e, IOError) and e.errno == errno.ENOENT:
+# File not found. No problem.
+pass
+else:
+print "Failed to read history '%s'; %s" % (self._histfile, e)
+atexit.register(self.__save_history)
+
+def __save_history(self):
+try:
+readline.write_history_file(self._histfile)
+except Exception as e:
+print "Failed to save history file '%s'; %s" % (self._histfile, e)
 
 def __parse_value(self, val):
 try:
-- 
2.9.3