Attempting to add an object through QMP with an id that is
already used leads to a qemu abort. This is a regression since
d2623129a7de ("qom: Drop parameter @errp of object_property_add()
& friends").

The first patch fixes the issue and the second patch adds a test
to check the error is gracefully returned to the QMP client.

The last patch can be considered independently. It merges all the
object-add tests into a single test function and cover new failure
cases.

Best Regards

Eric

This series can be found at:
https://github.com/eauger/qemu/tree/qom-graceful-v4

History:
- v3 -> v4:
  - addressed style comment from Markus
  - added patch 3

- v2 -> v3:
  - don't take the object reference on failure in
    object_property_try_add_child
  - add g_assert_nonnull(resp) in 2/2 while keeping
    Thomas A-b

- v1 -> v2:
  - use the try terminology.
  - turn object_property_try_add() into a non-static function
  - add the test


Eric Auger (3):
  qom: Introduce object_property_try_add_child()
  tests/qmp-cmd-test: Add qmp/object-add-duplicate-id
  tests/qmp-cmd-test: Add qmp/object-add-failure-modes

 include/qom/object.h       | 26 +++++++++-
 qom/object.c               | 21 ++++++--
 qom/object_interfaces.c    |  7 ++-
 tests/qtest/qmp-cmd-test.c | 99 ++++++++++++++++++++++++++++++++++++--
 4 files changed, 140 insertions(+), 13 deletions(-)

-- 
2.20.1

Patchew URL: https://patchew.org/QEMU/20200629112329.27611-1-eric.auger@redhat.com/



Hi,

This series failed the docker-quick@centos7 build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#!/bin/bash
make docker-image-centos7 V=1 NETWORK=1
time make docker-test-quick@centos7 SHOW_ENV=1 J=14 NETWORK=1
=== TEST SCRIPT END ===

qemu-system-x86_64: falling back to tcg
  TEST    iotest-qcow2: 154
**
ERROR:/tmp/qemu-test/src/tests/qtest/qmp-cmd-test.c:231:test_object_add_failure_modes: assertion failed: (qdict_haskey(resp, "return"))
ERROR - Bail out! ERROR:/tmp/qemu-test/src/tests/qtest/qmp-cmd-test.c:231:test_object_add_failure_modes: assertion failed: (qdict_haskey(resp, "return"))
make: *** [check-qtest-aarch64] Error 1
make: *** Waiting for unfinished jobs....
  TEST    iotest-qcow2: 156
  TEST    iotest-qcow2: 158
---
  TEST    iotest-qcow2: 177
  TEST    iotest-qcow2: 179
**
ERROR:/tmp/qemu-test/src/tests/qtest/qmp-cmd-test.c:231:test_object_add_failure_modes: assertion failed: (qdict_haskey(resp, "return"))
ERROR - Bail out! ERROR:/tmp/qemu-test/src/tests/qtest/qmp-cmd-test.c:231:test_object_add_failure_modes: assertion failed: (qdict_haskey(resp, "return"))
make: *** [check-qtest-x86_64] Error 1
  TEST    iotest-qcow2: 181
  TEST    iotest-qcow2: 184
  TEST    iotest-qcow2: 186
---
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--label', 'com.qemu.instance.uuid=8d9c643afafb4baa8ee26c2263f14e74', '-u', '1001', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=1', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-ang__6uf/src/docker-src.2020-06-29-07.40.57.28166:/var/tmp/qemu:z,ro', 'qemu:centos7', '/var/tmp/qemu/run', 'test-quick']' returned non-zero exit status 2.
filter=--filter=label=com.qemu.instance.uuid=8d9c643afafb4baa8ee26c2263f14e74
make[1]: *** [docker-run] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-ang__6uf/src'
make: *** [docker-run-test-quick@centos7] Error 2

real    17m6.183s
user    0m9.411s


The full log is available at
http://patchew.org/logs/20200629112329.27611-1-eric.auger@redhat.com/testing.docker-quick@centos7/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

On 29/06/20 13:23, Eric Auger wrote:
> Attempting to add an object through QMP with an id that is
> already used leads to a qemu abort. This is a regression since
> d2623129a7de ("qom: Drop parameter @errp of object_property_add()
> & friends").
> 
> The first patch fixes the issue and the second patch adds a test
> to check the error is gracefully returned to the QMP client.
> 
> The last patch can be considered independently. It merges all the
> object-add tests into a single test function and cover new failure
> cases.
> 
> Best Regards
> 
> Eric
> 
> This series can be found at:
> https://github.com/eauger/qemu/tree/qom-graceful-v4
> 
> History:
> - v3 -> v4:
>   - addressed style comment from Markus
>   - added patch 3
> 
> - v2 -> v3:
>   - don't take the object reference on failure in
>     object_property_try_add_child
>   - add g_assert_nonnull(resp) in 2/2 while keeping
>     Thomas A-b
> 
> - v1 -> v2:
>   - use the try terminology.
>   - turn object_property_try_add() into a non-static function
>   - add the test
> 
> 
> Eric Auger (3):
>   qom: Introduce object_property_try_add_child()
>   tests/qmp-cmd-test: Add qmp/object-add-duplicate-id
>   tests/qmp-cmd-test: Add qmp/object-add-failure-modes
> 
>  include/qom/object.h       | 26 +++++++++-
>  qom/object.c               | 21 ++++++--
>  qom/object_interfaces.c    |  7 ++-
>  tests/qtest/qmp-cmd-test.c | 99 ++++++++++++++++++++++++++++++++++++--
>  4 files changed, 140 insertions(+), 13 deletions(-)
> 

Very nice.  I see a failure reported by Patchew, I'll look into it as
well if you don't have time.

Paolo

Hi Paolo,

On 6/29/20 5:30 PM, Paolo Bonzini wrote:
> On 29/06/20 13:23, Eric Auger wrote:
>> Attempting to add an object through QMP with an id that is
>> already used leads to a qemu abort. This is a regression since
>> d2623129a7de ("qom: Drop parameter @errp of object_property_add()
>> & friends").
>>
>> The first patch fixes the issue and the second patch adds a test
>> to check the error is gracefully returned to the QMP client.
>>
>> The last patch can be considered independently. It merges all the
>> object-add tests into a single test function and cover new failure
>> cases.
>>
>> Best Regards
>>
>> Eric
>>
>> This series can be found at:
>> https://github.com/eauger/qemu/tree/qom-graceful-v4
>>
>> History:
>> - v3 -> v4:
>>   - addressed style comment from Markus
>>   - added patch 3
>>
>> - v2 -> v3:
>>   - don't take the object reference on failure in
>>     object_property_try_add_child
>>   - add g_assert_nonnull(resp) in 2/2 while keeping
>>     Thomas A-b
>>
>> - v1 -> v2:
>>   - use the try terminology.
>>   - turn object_property_try_add() into a non-static function
>>   - add the test
>>
>>
>> Eric Auger (3):
>>   qom: Introduce object_property_try_add_child()
>>   tests/qmp-cmd-test: Add qmp/object-add-duplicate-id
>>   tests/qmp-cmd-test: Add qmp/object-add-failure-modes
>>
>>  include/qom/object.h       | 26 +++++++++-
>>  qom/object.c               | 21 ++++++--
>>  qom/object_interfaces.c    |  7 ++-
>>  tests/qtest/qmp-cmd-test.c | 99 ++++++++++++++++++++++++++++++++++++--
>>  4 files changed, 140 insertions(+), 13 deletions(-)
>>
> 
> Very nice.  I see a failure reported by Patchew, I'll look into it as
> well if you don't have time.

As I reported earlier, I am no able to reproduce on my end with both

sudo time make docker-test-mingw@fedora SHOW_ENV=1 J=14 NETWORK=1
sudo time make docker-test-quick@centos7 SHOW_ENV=1 J=14 NETWORK=1

It fails on the first memory-backend-ram object-add that should be valid.

resp = qtest_qmp(qts, "{'execute': 'object-add', 'arguments':"
                    " {'qom-type': 'memory-backend-ram', 'id': 'ram1',
'props': {'size': 4294967296 } } }");

Could it be that the size is too large (4GB)?

Thanks

Eric



> 
> Paolo
> 
> 

_______________________________________________
Patchew-devel mailing list
Patchew-devel@redhat.com
https://www.redhat.com/mailman/listinfo/patchew-devel

Hi Paolo,

On 6/29/20 5:30 PM, Paolo Bonzini wrote:
> On 29/06/20 13:23, Eric Auger wrote:
>> Attempting to add an object through QMP with an id that is
>> already used leads to a qemu abort. This is a regression since
>> d2623129a7de ("qom: Drop parameter @errp of object_property_add()
>> & friends").
>>
>> The first patch fixes the issue and the second patch adds a test
>> to check the error is gracefully returned to the QMP client.
>>
>> The last patch can be considered independently. It merges all the
>> object-add tests into a single test function and cover new failure
>> cases.
>>
>> Best Regards
>>
>> Eric
>>
>> This series can be found at:
>> https://github.com/eauger/qemu/tree/qom-graceful-v4
>>
>> History:
>> - v3 -> v4:
>>   - addressed style comment from Markus
>>   - added patch 3
>>
>> - v2 -> v3:
>>   - don't take the object reference on failure in
>>     object_property_try_add_child
>>   - add g_assert_nonnull(resp) in 2/2 while keeping
>>     Thomas A-b
>>
>> - v1 -> v2:
>>   - use the try terminology.
>>   - turn object_property_try_add() into a non-static function
>>   - add the test
>>
>>
>> Eric Auger (3):
>>   qom: Introduce object_property_try_add_child()
>>   tests/qmp-cmd-test: Add qmp/object-add-duplicate-id
>>   tests/qmp-cmd-test: Add qmp/object-add-failure-modes
>>
>>  include/qom/object.h       | 26 +++++++++-
>>  qom/object.c               | 21 ++++++--
>>  qom/object_interfaces.c    |  7 ++-
>>  tests/qtest/qmp-cmd-test.c | 99 ++++++++++++++++++++++++++++++++++++--
>>  4 files changed, 140 insertions(+), 13 deletions(-)
>>
> 
> Very nice.  I see a failure reported by Patchew, I'll look into it as
> well if you don't have time.

So "[PATCH v5 0/3] Avoid abort on QMP attempt to add an object with
duplicate id" seems to have fixed the Patchew error (I decreased the
size of the memory object downto 1MB).

Thanks

Eric
> 
> Paolo
> 

On 01/07/20 08:51, Auger Eric wrote:
>> Very nice.  I see a failure reported by Patchew, I'll look into it as
>> well if you don't have time.
> 
> So "[PATCH v5 0/3] Avoid abort on QMP attempt to add an object with
> duplicate id" seems to have fixed the Patchew error (I decreased the
> size of the memory object downto 1MB).

Great, I queued that one.  Thanks.

Paolo