On 2012-02-15 18:23, Luiz Capitulino wrote: > On Wed, 15 Feb 2012 14:34:52 +0100 > Jan Kiszka <jan.kis...@siemens.com> wrote: > >> On 2012-02-15 13:49, Luiz Capitulino wrote: >>> On Wed, 15 Feb 2012 09:59:07 +0100 >>> Jan Kiszka <jan.kis...@siemens.com> 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. ;) > > Well, it was moved to the right place :)
(see the other thread) > >>> >>> 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. > > Oh, yes. Now I see that you said exactly that earlier. Sorry for more or less > re-stating it. > >> Still, there would be monitor_get_fd and qmp_get_fd then not >> qemu_get_fd. I think that should be done already. > > The problem is that monitor_get_fd() already exists and qmp_get_fd() > doesn't make much sense (as this is not related to QMP right now). So, > I could call it monitor_get_fd_cur() or something like this. What object represent a QMP session now? That object will once hold the reference to the FDs. So some qmp_get_fd will take that session and return the requested fd - so, it does make sense, long-term at least. In any case, as long as everyone can mess with cur_mon, you don't need to introduce wrappers that just link a normal monitor service with that variable. Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux
