On Fri, Jul 21, 2017 at 08:53:49AM +0200, Lukáš Doktor wrote: > Dne 20.7.2017 v 20:38 Eduardo Habkost napsal(a): > > On Thu, Jul 20, 2017 at 06:28:13PM +0200, Lukáš Doktor wrote: > >> The "id" is a builtin method to get object's identity and should not be > >> overridden. This might bring some issues in case someone was directly > >> calling "cmd(..., id=id)" but I haven't found such usage on brief search > >> for "cmd\(.*id=". > >> > >> Signed-off-by: Lukáš Doktor <ldok...@redhat.com> > >> --- > >> scripts/qmp/qmp.py | 8 ++++---- > >> 1 file changed, 4 insertions(+), 4 deletions(-) > >> > >> diff --git a/scripts/qmp/qmp.py b/scripts/qmp/qmp.py > >> index a14b001..c3e0206 100644 > >> --- a/scripts/qmp/qmp.py > >> +++ b/scripts/qmp/qmp.py > >> @@ -177,19 +177,19 @@ class QEMUMonitorProtocol(object): > >> print >>sys.stderr, "QMP:<<< %s" % resp > >> return resp > >> > >> - def cmd(self, name, args=None, id=None): > >> + def cmd(self, name, args=None, cmd_id=None): > >> """ > >> Build a QMP command and send it to the QMP Monitor. > >> > >> @param name: command name (string) > >> @param args: command arguments (dict) > >> - @param id: command id (dict, list, string or int) > >> + @param cmd_id: command id (dict, list, string or int) > >> """ > >> qmp_cmd = {'execute': name} > >> if args: > >> qmp_cmd['arguments'] = args > >> - if id: > >> - qmp_cmd['id'] = id > >> + if cmd_id: > >> + qmp_cmd['cmd_id'] = cmd_id > > > > The member sent through the monitor should still be called "id". > > i.e.: > > > > qmp_cmd['id'] = cmd_id > > > Oups, sorry, automatic rename changed it and I forgot to fix > this one back. I'll address this in v2. The main problem with > this patch is it could break named arguments (`cmd(..., id=id)` > calls) so I'm not sure it's worth including. But as mentioned > in commit message I grepped full sources so we better fix this > before someone starts using it.
I'm not convinced we need this patch, either. What exactly breaks if we don't apply this patch and somebody tries to use cmd(..., id=id)? -- Eduardo