Re: [Qemu-devel] [RFC PATCH 2/4] qmp/hmp: Add getfd_file monitor command

[Corey Bryant]

On 05/23/2012 09:33 AM, Luiz Capitulino wrote:
> On Tue, 22 May 2012 18:34:19 -0400
> Corey Bryant<address@hidden>  wrote:
>
> > On 05/22/2012 04:26 PM, Luiz Capitulino wrote:
> > > On Tue, 22 May 2012 16:02:19 -0400
> > > Corey Bryant<address@hidden>   wrote:
> > >
> > > > > But there's a small problem. Today getfd commands are closely tied to the
> > > > > Monitor. In Anthony's development tree, the getfd commands are tied to the
> > > > > new QMP server's session support.
> > > > >
> > > > > Asking you to integrate the new QMP server only to have the getfd command
> > > > > returning a simple integer would be too much, but at the same time I think
> > > > > you'll have to at least to break it from the monitor. This means moving its
> > > > > data structure away from the Monitor object and probably reworking the
> > > > > internal API used to get fds (ie. monitor_get_fd()).
> > > > >
> > > > > Shouldn't be hard, but you should be careful not to break external users.
> > > >
> > > > Just to verify, are you talking about moving the "fds" off the Monitor
> > > > struct?  -->   QLIST_HEAD(,mon_fd_t) fds;
> > >
> > > Yes.
> > >
> > > > Was this already moved away from the Monitor struct in Anthony's
> > > > development tree?  If not do you have a recommendation on where to move it?
> > >
> > > Yes, iirc it moved inside the new QMP server session support in Anthony's tree.
> > >
> > > > I think this would make more sense to me if I took a look at the getfd
> > > > code in Anthony's development tree.  Is this the correct tree?  I had
> > > > some issues cloning it.  https://github.com/aliguori/qemu-next.git
> > >
> > > The 'development' tree I'm referring to is the old glib branch in
> > > git://repo.or.cz/qemu/aliguori.git.
> >
> > Hmm, it looks like fds is still on the Monitor struct in that branch.
>
> Oh, you're right. That code is unfinished. It seems that I kept the finished
> version in my mind.

Oh how I wish I could git clone some people's brains. :)

> Well, I don't think that moving the fd array to another object will buy us
> much, so you can keep it this way. Note that you still have to convert 
> do_getfd()
> to the QAPI as pointed out by Stefan and that the monitor object (*mon pointer)
> won't be passed to it. You'll have to use cur_mon in qmp_getfd() (as Anthony's
> version does).

Alright, thanks for the info.

--
Regards,
Corey