Markus Armbruster <arm...@redhat.com> writes:
> Christian Borntraeger <borntrae...@de.ibm.com> writes:
>
>> Am 13.10.21 um 11:07 schrieb Paolo Bonzini:
>>> From: Markus Armbruster <arm...@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!
Classical case of failing to adhere to "read only the code": reading the
documentation lodged "this is a qdev ID" in my brain, blinding me to the
fact that the code actually treats it as a QOM path relative to
/machine/peripheral/.
Sorry!
Please try "[PATCH] monitor: Fix find_device_state() for IDs containing
slashes".
