Philippe Mathieu-Daudé <phi...@redhat.com> writes: > On 6/5/20 4:56 PM, Markus Armbruster wrote: >> qdev_prop_set_drive() can fail. None of the other qdev_prop_set_FOO() >> can; they abort on error. >> >> To clean up this inconsistency, rename qdev_prop_set_drive() to >> qdev_prop_set_drive_err(), and create a qdev_prop_set_drive() that >> aborts on error. >> >> Coccinelle script to update callers: >> >> @ depends on !(file in "hw/core/qdev-properties-system.c")@ >> expression dev, name, value; >> symbol error_abort; >> @@ >> - qdev_prop_set_drive(dev, name, value, &error_abort); >> + qdev_prop_set_drive(dev, name, value); > > Why not open-code qdev_prop_set_drive_err(..., &error_abort)?
Consistency with qdev_prop_set_chr() and qdev_prop_set_netdev(). My starting point was "what makes block backends so different that they need error handling where nothing else does?" After a considerable amount of digging, my answer is "nothing". qdev_prop_set_drive(), qdev_prop_set_chr() and qdev_prop_set_netdev() can all run into errors. On closer examination, all programming errors (thus &error_abort), except for "backend is already in use", and to trigger that one, you have to get creative and steal the backend for another purpose, e.g. with -global. This is the abridged version of a longwinded argument I didn't want to make in this series, so I left the error handling alone. In the longer run, I want qdev_prop_set_drive_err() to die. > >> >> @@ >> expression dev, name, value, errp; >> @@ >> - qdev_prop_set_drive(dev, name, value, errp); >> + qdev_prop_set_drive_err(dev, name, value, errp); >> > [...]