On 2018-12-18 19:33, Markus Armbruster wrote:
> Thomas Huth <th...@redhat.com> writes:
>
>> The last user of blk_attach_dev_legacy() is the code in xen_disk.c.
>> It passes a pointer to a XenBlkDev as second parameter. XenBlkDev
>> is derived from XenDevice which in turn is derived from DeviceState
>> since commit 3a6c9172ac5951e ("xen: create qdev for each backend device").
>> Thus the code can also simply use blk_attach_dev() with a pointer
>> to the DeviceState instead.
>> So we can finally remove all code related to the "legacy_dev" flag, too,
>> and turn the related "void *" in block-backend.c into "DeviceState *"
>> to fix some of the remaining TODOs there.
>>
>> Signed-off-by: Thomas Huth <th...@redhat.com>
>> ---
>> Note: I haven't tested the Xen code since I don't have a working Xen
>> installation at hand. I'd appreciate if someone could check it...
[...]
>> diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c
>> index 36eff94..9605caf 100644
>> --- a/hw/block/xen_disk.c
>> +++ b/hw/block/xen_disk.c
>> @@ -801,7 +801,9 @@ static int blk_connect(struct XenDevice *xendev)
>> * so we can blk_unref() unconditionally */
>> blk_ref(blkdev->blk);
>> }
>> - blk_attach_dev_legacy(blkdev->blk, blkdev);
>> + if (blk_attach_dev(blkdev->blk, DEVICE(blkdev)) < 0) {
>> + return -1;
>> + }
>
> Other error returns in this function call xen_pv_printf() first. Should
> this one, too?
Only some of them do a xen_pv_printf() first, there are also many that
don't. blk_attach_dev() currently only returns an error if a device has
already been attach - which should simply never happen here, so a printf
currently does not seem to be justified to me. Alternatively, we could
also abort() here instead, I think. I'll leave that decision up to the
Xen maintainers...
Thomas
