Re: [Qemu-block] [PATCH 1/2] qapi: support device id for x-block-latency-histogram-set

[Eric Blake]

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