[Qemu-devel] [PATCH 0/1] monitor: Fix QMP ABI breakage around "id"

[Qemu-devel] [PATCH 0/1] monitor: Fix QMP ABI breakage around "id"

[Markus Armbruster]

First, my apologies for screwing this up, and for my slow reaction
(I've been offline since Wednesday night).

A number of patches for the regression have been posted.  They all
change the code the same way, but either restore a comment that's no
longer correct, or add a new one that isn't quite right.

This patch is again the same code change, but with a better comment.

I credited the authors of the prior patches by adding their S-o-B.
Hope that's alright.

A test case protecting us from such stupid mistakes would be useful.
Left for later, as we need to fix the regression right away.

Peter, can you apply this directly, or do you need me to post a pull
request.

Markus Armbruster (1):
  monitor: Fix QMP ABI breakage around "id"

 monitor.c | 2 ++
 1 file changed, 2 insertions(+)

-- 
1.9.3

[Qemu-devel] [PATCH 1/1] monitor: Fix QMP ABI breakage around "id"

[Markus Armbruster]

Commit 65207c5 accidentally dropped a line of code we need along with
a comment that became wrong then.  This made QMP reject "id":

    {"execute": "system_reset", "id": "1"}
    {"error": {"class": "GenericError", "desc": "QMP input object member 'id' is unexpected"}}

Put the lost line right back, so QMP again accepts and returns "id",
as promised by the ABI:

    {"execute": "system_reset", "id": "1"}
    {"return": {}, "id": "1"}

Reported-by: Fabio Fantoni <fabio.fantoni@m2r.biz>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Don Slutz <dslutz@verizon.com>
Tested-by: Fabio Fantoni <fabio.fantoni@m2r.biz>
Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
Signed-off-by: Pavel Fedin <p.fedin@samsung.com>
Tested-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 monitor.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/monitor.c b/monitor.c
index c7baa91..9afee7b 100644
--- a/monitor.c
+++ b/monitor.c
@@ -4955,6 +4955,8 @@ static QDict *qmp_check_input_obj(QObject *input_obj, Error **errp)
                           "arguments", "object");
                 return NULL;
             }
+        } else if (!strcmp(arg_name, "id")) {
+            /* Any string is acceptable as "id", so nothing to check */
         } else {
             error_set(errp, QERR_QMP_EXTRA_MEMBER, arg_name);
             return NULL;
-- 
1.9.3

Re: [Qemu-devel] [PATCH 1/1] monitor: Fix QMP ABI breakage around "id"

[Peter Maydell]

On 8 June 2015 at 09:44, Markus Armbruster <armbru@redhat.com> wrote:
> Commit 65207c5 accidentally dropped a line of code we need along with
> a comment that became wrong then.  This made QMP reject "id":
>
>     {"execute": "system_reset", "id": "1"}
>     {"error": {"class": "GenericError", "desc": "QMP input object member 'id' is unexpected"}}
>
> Put the lost line right back, so QMP again accepts and returns "id",
> as promised by the ABI:
>
>     {"execute": "system_reset", "id": "1"}
>     {"return": {}, "id": "1"}
>
> Reported-by: Fabio Fantoni <fabio.fantoni@m2r.biz>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> Signed-off-by: Don Slutz <dslutz@verizon.com>
> Tested-by: Fabio Fantoni <fabio.fantoni@m2r.biz>
> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
> Signed-off-by: Pavel Fedin <p.fedin@samsung.com>
> Tested-by: Eric Blake <eblake@redhat.com>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>

That's a lot of signed-off-by lines for a two line patch :-)

> ---
>  monitor.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/monitor.c b/monitor.c
> index c7baa91..9afee7b 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -4955,6 +4955,8 @@ static QDict *qmp_check_input_obj(QObject *input_obj, Error **errp)
>                            "arguments", "object");
>                  return NULL;
>              }
> +        } else if (!strcmp(arg_name, "id")) {
> +            /* Any string is acceptable as "id", so nothing to check */
>          } else {
>              error_set(errp, QERR_QMP_EXTRA_MEMBER, arg_name);
>              return NULL;
> --
> 1.9.3

I'll apply this directly to master later today.

-- PMM

Re: [Qemu-devel] [PATCH 1/1] monitor: Fix QMP ABI breakage around "id"

[Peter Maydell]

On 8 June 2015 at 10:04, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 8 June 2015 at 09:44, Markus Armbruster <armbru@redhat.com> wrote:
>> Commit 65207c5 accidentally dropped a line of code we need along with
>> a comment that became wrong then.  This made QMP reject "id"

> I'll apply this directly to master later today.


Now done.

-- PMM

Re: [Qemu-devel] [PATCH 1/1] monitor: Fix QMP ABI breakage around "id"

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1156 bytes --]

On 06/08/2015 02:44 AM, Markus Armbruster wrote:
> Commit 65207c5 accidentally dropped a line of code we need along with
> a comment that became wrong then.  This made QMP reject "id":
> 
>     {"execute": "system_reset", "id": "1"}
>     {"error": {"class": "GenericError", "desc": "QMP input object member 'id' is unexpected"}}
> 
> Put the lost line right back, so QMP again accepts and returns "id",
> as promised by the ABI:
> 
>     {"execute": "system_reset", "id": "1"}
>     {"return": {}, "id": "1"}
> 
> Reported-by: Fabio Fantoni <fabio.fantoni@m2r.biz>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> Signed-off-by: Don Slutz <dslutz@verizon.com>
> Tested-by: Fabio Fantoni <fabio.fantoni@m2r.biz>
> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
> Signed-off-by: Pavel Fedin <p.fedin@samsung.com>
> Tested-by: Eric Blake <eblake@redhat.com>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>

Double S-o-b by Markus is odd.  But it's already landed in master, I'm
too late for fixing it :)

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]