[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [QEMU PATCH] block: Remove blk_attach_dev_legacy() / le
From: |
Thomas Huth |
Subject: |
Re: [Qemu-devel] [QEMU PATCH] block: Remove blk_attach_dev_legacy() / legacy_dev code |
Date: |
Wed, 19 Dec 2018 13:00:09 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 |
On 2018-12-18 19:33, Markus Armbruster wrote:
> Thomas Huth <address@hidden> 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 <address@hidden>
>> ---
>> 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