Re: [PULL 37/40] monitor: Tidy up find_device_state()

[Markus Armbruster]

Christian Borntraeger <borntraeger@de.ibm.com> writes:

> Am 13.10.21 um 11:07 schrieb Paolo Bonzini:
>> From: Markus Armbruster <armbru@redhat.com>
>> Commit 6287d827d4 "monitor: allow device_del to accept QOM paths"
>> extended find_device_state() to accept QOM paths in addition to qdev
>> IDs.  This added a checked conversion to TYPE_DEVICE at the end, which
>> duplicates the check done for the qdev ID case earlier, except it sets
>> a *different* error: GenericError "ID is not a hotpluggable device"
>> when passed a QOM path, and DeviceNotFound "Device 'ID' not found"
>> when passed a qdev ID.  Fortunately, the latter won't happen as long
>> as we add only devices to /machine/peripheral/.
>> Earlier, commit b6cc36abb2 "qdev: device_del: Search for to be
>> unplugged device in 'peripheral' container" rewrote the lookup by qdev
>> ID to use QOM instead of qdev_find_recursive(), so it can handle
>> buss-less devices.  It does so by constructing an absolute QOM path.
>> Works, but object_resolve_path_component() is easier.  Switching to it
>> also gets rid of the unclean duplication described above.
>> While there, avoid converting to TYPE_DEVICE twice, first to check
>> whether it's possible, and then for real.
>
> This one broke qemu iotest 280 on s390:
>
>
> 280   fail       [13:06:19] [13:06:19]   0.3s   (last: 0.3s)  output mismatch 
> (see 280.out.bad)
> --- /home/cborntra/REPOS/qemu/tests/qemu-iotests/280.out
> +++ 280.out.bad
> @@ -37,14 +37,14 @@
>  === Resume the VM and simulate a write request ===
>  {"execute": "cont", "arguments": {}}
>  {"return": {}}
> -{"return": ""}
> +{"return": "Error: Device 'vda/virtio-backend' not found\r\n"}
>
>  === Commit it to the backing file ===
>  {"execute": "block-commit", "arguments": {"auto-dismiss": false, "device": 
> "top-fmt", "job-id": "job0", "top-node": "top-fmt"}}
>  {"return": {}}
>  {"execute": "job-complete", "arguments": {"id": "job0"}}
>  {"return": {}}
> -{"data": {"device": "job0", "len": 65536, "offset": 65536, "speed": 0, 
> "type": "commit"}, "event": "BLOCK_JOB_READY", "timestamp": {"microseconds": 
> "USECS", "seconds": "SECS"}}
> -{"data": {"device": "job0", "len": 65536, "offset": 65536, "speed": 0, 
> "type": "commit"}, "event": "BLOCK_JOB_COMPLETED", "timestamp": 
> {"microseconds": "USECS", "seconds": "SECS"}}
> +{"data": {"device": "job0", "len": 0, "offset": 0, "speed": 0, "type": 
> "commit"}, "event": "BLOCK_JOB_READY", "timestamp": {"microseconds": "USECS", 
> "seconds": "SECS"}}
> +{"data": {"device": "job0", "len": 0, "offset": 0, "speed": 0, "type": 
> "commit"}, "event": "BLOCK_JOB_COMPLETED", "timestamp": {"microseconds": 
> "USECS", "seconds": "SECS"}}
>  {"execute": "job-dismiss", "arguments": {"id": "job0"}}
>  {"return": {}}
> Failures: 280
> Failed 1 of 1 iotests

Reproduced.  I'll dig deeper.  Thanks!