On Tue, 22 May 2012 18:34:19 -0400
Corey Bryant <coreyb(a)linux.vnet.ibm.com> wrote:
On 05/22/2012 04:26 PM, Luiz Capitulino wrote:
> On Tue, 22 May 2012 16:02:19 -0400
> Corey Bryant<coreyb(a)linux.vnet.ibm.com> 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.
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).