[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 13/16] qdev: Make qdev_prop_set_drive() match the other helpe
From: |
Markus Armbruster |
Subject: |
Re: [PATCH 13/16] qdev: Make qdev_prop_set_drive() match the other helpers |
Date: |
Mon, 08 Jun 2020 07:20:40 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/26.3 (gnu/linux) |
Philippe Mathieu-Daudé <philmd@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);
>>
> [...]
- [PATCH 08/16] blockdev: Deprecate -drive with bogus interface type, (continued)
- [PATCH 08/16] blockdev: Deprecate -drive with bogus interface type, Markus Armbruster, 2020/06/05
- [PATCH 06/16] fdc: Deprecate configuring floppies with -global isa-fdc, Markus Armbruster, 2020/06/05
- [PATCH 15/16] sd/pxa2xx_mmci: Don't crash on pxa2xx_mmci_init() error, Markus Armbruster, 2020/06/05
- [PATCH 14/16] arm/aspeed: Drop aspeed_board_init_flashes() parameter @errp, Markus Armbruster, 2020/06/05
- [PATCH 13/16] qdev: Make qdev_prop_set_drive() match the other helpers, Markus Armbruster, 2020/06/05
- [PATCH 09/16] qdev: Eliminate get_pointer(), set_pointer(), Markus Armbruster, 2020/06/05
- [PATCH 02/16] iotests/172: Cover empty filename and multiple use of drives, Markus Armbruster, 2020/06/05
- [PATCH 16/16] sd/milkymist-memcard: Fix error API violation, Markus Armbruster, 2020/06/05
- [PATCH 12/16] qdev: Reject chardev property override, Markus Armbruster, 2020/06/05
- [PATCH 04/16] fdc: Reject clash between -drive if=floppy and -global isa-fdc, Markus Armbruster, 2020/06/05
- Re: [PATCH 00/16] Crazy shit around -global (pardon my french), John Snow, 2020/06/10