Re: [Qemu-devel] [PATCH 1/3] qmp/hmp: Add QMP getfd command that returns fd

[Corey Bryant <coreyb@linux.vnet.ibm.com>]

On 06/06/2012 01:50 PM, Luiz Capitulino wrote:
> On Wed, 06 Jun 2012 10:04:23 -0400
> Corey Bryant<coreyb@linux.vnet.ibm.com>  wrote:
>
>>> Today, we return a dict on success:
>>>
>>>    { "return": {} }
>>>
>>> But this patch changes it to:
>>>
>>>    { "return": 42 }
>>>
>>> There are two ways to do this without breaking compatibility:
>>>
>>>    1. Add a new command (say get-file-descriptor)
>>
>> What do you think about using getfd2 for the command name?  I'm thinking
>> getfd2 may be more obvious that it corresponds to closefd.
>
> We're going for more descriptive names in QMP. I don't have strong objections
> against get-fd2 if there's consensus that 'fd' is better than 'file-descriptor',
> although 'fd2' is a bit confusing.

I really don't have a strong opinion either so I'll do whatever the 
consensus wants.  Does anyone else have an opinion?

>
>> That assumes
>> we'll use the same array internally to store fds and closefd can be used
>> to close the fd opened by get-file-descriptor/getfd2.
>
> You mean using the same array for getfd and get-file-descriptor? Yes, the
> descriptor list is global.
>

Yes, that's what I meant.

>> I assume this approach would still return an int:  { "return": 42 }
>
> The new command? Yes.
>

Ok

>>>    2. Return a type instead, like:
>>>
>>>        { "return": { "file-descriptor": 42 } }
>>>
>>> I think I prefer item 1, as we could also take the opportunity to fix the
>>> argument type and improve its name. Besides, we don't have a schema to do 2.
>>
>> Is it fdname that you think could be improved?  fdname seems pretty
>> straight forward to me.
>
> What I'm trying to avoid is having too short names when that's not necessary.
> I think I'd just use 'name' or 'file-descriptor-name' for the verbose option,
> but I don't have strong objections against 'fdname'.
>

Unless anyone objects I'll plan on going with more descriptive names, 
since that is the desired direction for QMP.  So we'll have 
get-file-descriptor for the new command name and file-descriptor-name 
for the argument name.

-- 
Regards,
Corey