Re: [Qemu-devel] [PATCH 0/6] qapi: Convert migrate

[Jan Kiszka]

On 2012-02-15 13:49, Luiz Capitulino wrote:
> On Wed, 15 Feb 2012 09:59:07 +0100
> Jan Kiszka <address@hidden> wrote:
> 
>> On 2012-02-10 20:31, Luiz Capitulino wrote:
>>> This is a rebase of Anthony's conversion, from his glib branch; and this is
>>> also the beginning of the conversion of complex commands to the qapi.
>>>
>>> There are two important changes that should be observed:
>>>
>>>  1. patch 5/6 purges the 'mon' object from migration code. One of the
>>>     consequences is that we lose the ability to print progress status to
>>>     the HMP user (esp. in block migration)
>>
>> This smells extremely fishy. You have some common "monitor" context in
>> both cases, means something that decides where suspend/resume takes
>> effect or where to pick up file descriptors from. If the exiting Monitor
>> object is not generic enough, introduce some super-class and use that in
>> common services. Or make sure that a variant of Monitor is also valid
>> over QMP. But don't remove the dependency from the API, while
>> reintroducing it via the backdoor of cur_mon.
> 
> What we really want to do here is to untangle HMP and QMP. Unfortunately,
> the migrate command is one of those commands where the two are deeply
> tangled and the split won't be perfect.
> 
> However, the two cases you mention above are solvable:
> 
>  1. suspend/resume: this is *really* a HMP feature and shouldn't be in any
>     QMP code path. This is correctly addressed in this series by moving it
>     to hmp_migrate()

Almost correctly. ;)

> 
>  2. file descriptor passing: the new QMP server will support sessions and
>     we'll move statefull commands (like getfd) to it. When we do it, we'll
>     introduce a new API to get fds that won't depend on the monitor. However,
>     this requires all commands to be converted to the qapi first. Meanwhile
>     we use the qemu_get_fd() API.
> 
>     Note: qemu_get_fd() is temporary, it shouldn't be a problem to use it
>     (if it's not incorrect, of course, I honestly haven't fully tested it 
> yet).

So there will be a common super-class of Monitor and that new QMP
session that also manages the file descriptors? That would make sense.
Still, there would be monitor_get_fd and qmp_get_fd then not
qemu_get_fd. I think that should be done already. BTW, where do you get
the FDs from now in QMP mode? Is there currently a Monitor instance
associated?

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux