On Fri, Mar 3, 2017 at 9:29 PM, John Snow <js...@redhat.com> wrote:
>
>
> On 03/03/2017 02:26 PM, Nir Soffer wrote:
>> On Fri, Mar 3, 2017 at 8:54 PM, John Snow <js...@redhat.com> 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 <js...@redhat.com>
>>> ---
>>>
>>> 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 <nir...@gmail.com>
>>
