Nir Soffer <nir...@gmail.com> writes: > On Wed, Mar 8, 2017 at 8:29 AM, Markus Armbruster <arm...@redhat.com> wrote: >> John Snow <js...@redhat.com> writes: >> >>> On 03/07/2017 03:16 AM, Markus Armbruster wrote: >>>> John Snow <js...@redhat.com> writes: >>>> >>>>> On 03/06/2017 03:18 AM, Markus Armbruster wrote: >>>>>> Nir Soffer <nir...@gmail.com> writes: >>>>>> >>>>>>> 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') >>>>>> >>>>>> 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" ...) >> >> How sad. If https://bugs.python.org/ didn't require a login, I would've >> filed a bug already. >> >> I'm afraid all I can offer meanwhile is INPUTRC: >> >> Any user can customize programs that use Readline by putting >> commands in an "inputrc" file, conventionally in his home directory. >> The name of this file is taken from the value of the environment >> variable `INPUTRC'. If that variable is unset, the default is >> `~/.inputrc'. If that file does not exist or cannot be read, the >> ultimate default is `/etc/inputrc'. >> > > Having a way to limit history globally looks good enough.
Certainly good enough for merging qmp-shell patches. > Note that python readline does not report the readline > limit correctly (get_history_length() returns -1), but saving > history uses the limit defined in .inputrc. > > Playing with this, I found a nice bug - if you set history > size to N, and your history file contains 2 * N items or more, > python segfaults when entering the first input line. > > I'll file a python bug later. Please do. If you could throw in a (separate) request for letting us set rl_readline_name, that would be great. [...]