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

[Corey Bryant]

On 06/06/2012 01:50 PM, Luiz Capitulino wrote:
> On Wed, 06 Jun 2012 10:04:23 -0400
> Corey Bryant<address@hidden>  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