From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:39077) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Rxia0-0002nr-GB for qemu-devel@nongnu.org; Wed, 15 Feb 2012 12:23:48 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1RxiZu-0005lO-Ml for qemu-devel@nongnu.org; Wed, 15 Feb 2012 12:23:44 -0500 Received: from mx1.redhat.com ([209.132.183.28]:11326) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RxiZu-0005lI-Fp for qemu-devel@nongnu.org; Wed, 15 Feb 2012 12:23:38 -0500 Date: Wed, 15 Feb 2012 15:23:33 -0200 From: Luiz Capitulino Message-ID: <20120215152333.1beb96b1@doriath.home> In-Reply-To: <4F3BB47C.5090304@siemens.com> References: <1328902266-25308-1-git-send-email-lcapitulino@redhat.com> <4F3B73DB.4010105@siemens.com> <20120215104954.1bce66da@doriath.home> <4F3BB47C.5090304@siemens.com> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 0/6] qapi: Convert migrate List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jan Kiszka Cc: "aliguori@us.ibm.com" , "qemu-devel@nongnu.org" , "quintela@redhat.com" On Wed, 15 Feb 2012 14:34:52 +0100 Jan Kiszka wrote: > On 2012-02-15 13:49, Luiz Capitulino wrote: > > On Wed, 15 Feb 2012 09:59:07 +0100 > > Jan Kiszka 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 :) > > > > 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. > BTW, where do you get > the FDs from now in QMP mode? Is there currently a Monitor instance > associated? Yes. The current QMP server is associated with a Monitor instance and it supports the getfd command.