[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 1/2] qapi: support device id for x-block-latency
From: |
Vladimir Sementsov-Ogievskiy |
Subject: |
Re: [Qemu-devel] [PATCH 1/2] qapi: support device id for x-block-latency-histogram-set |
Date: |
Tue, 2 Oct 2018 14:58:42 +0000 |
02.10.2018 17:35, Eric Blake wrote:
> On 10/2/18 9:30 AM, Vladimir Sementsov-Ogievskiy wrote:
>
>>>> -# @device: device name to set latency histogram for.
>>>> +# @device: device name to set latency histogram for (better use @id).
>>>> +#
>>>> +# @id: The name or QOM path of the guest device.
>>>
>>> As long as we are renaming the command, there's no need to keep a
>>> legacy parameter around. Just get rid of device and replace it by
>>> id, rather than worrying about both. The introduction of the stable
>>> command does not have to carry any baggage left over from the x-
>>> preliminary version.
>>>
>>
>> Libvirt don't need both for now, for different scenarios?
>
> If you want both to work because libvirt has reasons to use both, then
> the documentation needs to be more precise. "(better use @id)" does
> not tell me why I shouldn't use @device, or what difference in
> behavior I get by using one instead of the other. Furthermore, if @id
> is able to use the name of the guest device as an alternative to the
> QOM path, then how is that different from @device being the name of
> the guest device?
>
Hm. It all looks a bit weird. I've just duplicated block_set_io_throttle
interface:
# @device: Block device name (deprecated, use @id instead)
#
# @id: The name or QOM path of the guest device (since: 2.8)
blk = qmp_get_blk(arg->has_device ? arg->device : NULL,
arg->has_id ? arg->id : NULL,
errp);
if (!blk) {
return;
}
So, looks like "The name or" is wrong part. @id can be only path.
However, I can call blk_by_name on @id, if blk_by_qdev_id failed..
So, variants:
1. both parameters, current code, but fix documentation (will be @id:
QOM path of the guest device.)
2. only @id and only QOM path
3. only @id, but fullback to blk_by_name(@id) if failed to find qdev.
Any option is ok for me, I don't care. What do you think?
--
Best regards,
Vladimir