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

[Kevin Wolf]

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