Re: [PATCH v2 25/25] python: rename qemu.aqmp to qemu.qmp

2021-12-17 Thread John Snow 
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 > 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>>
> >
> > 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  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!

>

Re: [PATCH v2 25/25] python: rename qemu.aqmp to qemu.qmp

2021-12-16 Thread Vladimir Sementsov-Ogievskiy 

17.12.2021 00:10, John Snow wrote:



On Thu, Dec 16, 2021 at 6:41 AM Vladimir Sementsov-Ogievskiy 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 Snowmailto: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 
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..



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.


--
Best regards,
Vladimir

Re: [PATCH v2 25/25] python: rename qemu.aqmp to qemu.qmp

2021-12-16 Thread John Snow 
On Thu, Dec 16, 2021 at 6:41 AM Vladimir Sementsov-Ogievskiy <
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
>
> 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 
> 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*.

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.

--js

Re: [PATCH v2 25/25] python: rename qemu.aqmp to qemu.qmp

2021-12-16 Thread Vladimir Sementsov-Ogievskiy 

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


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

--
Best regards,
Vladimir