On Fri, Dec 17, 2021, 2:40 AM Vladimir Sementsov-Ogievskiy < vsement...@virtuozzo.com> wrote:
> 17.12.2021 00:10, John Snow wrote: > > > > > > On Thu, Dec 16, 2021 at 6:41 AM Vladimir Sementsov-Ogievskiy < > vsement...@virtuozzo.com <mailto:vsement...@virtuozzo.com>> wrote: > > > > 15.12.2021 22:39, John Snow wrote: > > > Now that we are fully switched over to the new QMP library, move > it back > > > over the old namespace. This is being done primarily so that we > may > > > upload this package simply as "qemu.qmp" without introducing > confusion > > > over whether or not "aqmp" is a new protocol or not. > > > > > > The trade-off is increased confusion inside the QEMU developer > > > tree. Sorry! > > > > > > Signed-off-by: John Snow<js...@redhat.com <mailto: > js...@redhat.com>> > > > > Great job! > > > > I looked thorough the patch, changes looks correct. Simply rename > every aqmp / AQMP occurrence.. But: > > > > > > [root@kvm review]# git grep -i aqmp > > python/qemu/qmp/aqmp_tui.py:AQMP TUI > > python/qemu/qmp/aqmp_tui.py:AQMP TUI is an asynchronous interface > built on top the of the AQMP library. > > python/qemu/qmp/aqmp_tui.py:Example Usage: aqmp-tui <SOCKET | TCP > IP:PORT> > > python/qemu/qmp/aqmp_tui.py:Full Usage: aqmp-tui --help > > python/qemu/qmp/aqmp_tui.py: Implements the AQMP TUI. > > python/qemu/qmp/aqmp_tui.py: parser = > argparse.ArgumentParser(description='AQMP TUI') > > python/qemu/qmp/legacy.py: self._aqmp = QMPClient(nickname) > > python/qemu/qmp/legacy.py: if self._aqmp.greeting is not None: > > python/qemu/qmp/legacy.py: return > self._aqmp.greeting._asdict() > > python/qemu/qmp/legacy.py: self._aqmp.await_greeting = > negotiate > > python/qemu/qmp/legacy.py: self._aqmp.negotiate = negotiate > > python/qemu/qmp/legacy.py: > self._aqmp.connect(self._address) > > python/qemu/qmp/legacy.py: self._aqmp.await_greeting = True > > python/qemu/qmp/legacy.py: self._aqmp.negotiate = True > > python/qemu/qmp/legacy.py: > self._aqmp.accept(self._address), > > python/qemu/qmp/legacy.py: self._aqmp._raw(qmp_cmd, > assign_id=False), > > python/qemu/qmp/legacy.py: self._aqmp.execute(cmd, kwds), > > python/qemu/qmp/legacy.py: if self._aqmp.events.empty(): > > python/qemu/qmp/legacy.py: self._aqmp.events.get(), > > python/qemu/qmp/legacy.py: events = [dict(x) for x in > self._aqmp.events.clear()] > > python/qemu/qmp/legacy.py: self._aqmp.events.clear() > > python/qemu/qmp/legacy.py: self._aqmp.disconnect() > > python/qemu/qmp/legacy.py: self._aqmp.send_fd_scm(fd) > > python/qemu/qmp/legacy.py: if self._aqmp.runstate == > Runstate.IDLE: > > python/setup.cfg:# AQMP TUI dependencies > > python/setup.cfg: aqmp-tui = qemu.qmp.aqmp_tui:main [tui] > > python/setup.cfg:[mypy-qemu.qmp.aqmp_tui] > > > > [root@kvm review]# git ls-tree -r --name-only HEAD | grep -i aqmp > > python/qemu/qmp/aqmp_tui.py > > > > > > I think, this all should be renamed too > > > > > > For aqmp_tui.py, sure. The new TUI isn't 100% ready to replace qmp-shell > yet, so I wasn't entirely certain what to name it... qmp-tui? > > > > *shrugs*. > > I don't remember what tui is abbreviating) qmp-tui is OK, and it may be > renamed to qmp-shell when it is ready to replace it.. > "text user interface", by analogy with GUI (graphical UI). > > > > for legacy.py, it's just an internal variable name and I wasn't sure it > was worth the churn just to change a private variable. I could still do it > if you feel strongly about it. > > > > I'd rename everything. > Alright, I'll do so in the respin. > > -- > Best regards, > Vladimir > Thanks for the reviews! >
