[Top][All Lists]

[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...


reply via email to

[Prev in Thread] Current Thread [Next in Thread]