Re: [PATCH v4 1/4] python/aqmp: Fix negotiation with pre-"oob" QEMU

[John Snow <jsnow@redhat.com>]

On Tue, Feb 1, 2022 at 7:59 AM Kevin Wolf <kwolf@redhat.com> wrote:
>
> Am 01.02.2022 um 05:11 hat John Snow geschrieben:
> > QEMU versions prior to the "oob" capability *also* can't accept the
> > "enable" keyword argument at all. Fix the handshake process with older
> > QEMU versions.
> >
> > Signed-off-by: John Snow <jsnow@redhat.com>
> > Reviewed-by: Hanna Reitz <hreitz@redhat.com>
> > ---
> >  python/qemu/aqmp/qmp_client.py | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/python/qemu/aqmp/qmp_client.py b/python/qemu/aqmp/qmp_client.py
> > index f1a845cc82..90a8737f03 100644
> > --- a/python/qemu/aqmp/qmp_client.py
> > +++ b/python/qemu/aqmp/qmp_client.py
> > @@ -292,9 +292,9 @@ async def _negotiate(self) -> None:
> >          """
> >          self.logger.debug("Negotiating capabilities ...")
> >
> > -        arguments: Dict[str, List[str]] = {'enable': []}
> > +        arguments: Dict[str, List[str]] = {}
> >          if self._greeting and 'oob' in self._greeting.QMP.capabilities:
> > -            arguments['enable'].append('oob')
> > +            arguments.setdefault('enable', []).append('oob')
> >          msg = self.make_execute_msg('qmp_capabilities', arguments=arguments)
>
> In case you have some interest in bike sheds:
>
> As long as we only ever append a single capability, it doesn't really
> make a difference and an explicit setdefault() when adding it is fine.
> But if we had more than one, maybe making arguments a defaultdict(list)
> would be nicer.
>
> Not worth respinning, of course, if you don't for another reason.
>
> Kevin
>

Nope, no reason. I just forget that there's a fancier doodad. I'll add
a patch to a less-important series to shine this up.

--js