|
From: | Eric Blake |
Subject: | Re: [Qemu-block] [PATCH 1/2] qapi: support device id for x-block-latency-histogram-set |
Date: | Tue, 2 Oct 2018 10:21:23 -0500 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.0 |
On 10/2/18 9:58 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.
Hm. It all looks a bit weird. I've just duplicated block_set_io_throttle interface:
But that interface was pre-existing under a stable name, so it had to worry about back-compat. Here, we are adding a new command, so we don't have to be stuck with back-compat ugliness.
# @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?
I'm leaning towards 3. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
[Prev in Thread] | Current Thread | [Next in Thread] |