From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:51852) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a1S10-0006hs-8n for qemu-devel@nongnu.org; Tue, 24 Nov 2015 23:49:11 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1a1S0v-0003MD-9K for qemu-devel@nongnu.org; Tue, 24 Nov 2015 23:49:10 -0500 Received: from mx1.redhat.com ([209.132.183.28]:35215) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a1S0v-0003M8-1w for qemu-devel@nongnu.org; Tue, 24 Nov 2015 23:49:05 -0500 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (Postfix) with ESMTPS id 1A638C0A524B for ; Wed, 25 Nov 2015 04:49:04 +0000 (UTC) References: <1448273262-13845-1-git-send-email-peterx@redhat.com> <56533D45.1060108@redhat.com> <20151123175759.GG3606@hawk.localdomain> <5653C422.3040307@redhat.com> <20151124031027.GC26733@ad.usersys.redhat.com> <56544694.3060309@redhat.com> <20151124113723.GA29832@ad.usersys.redhat.com> <56546ADC.5050402@redhat.com> <20151125024631.GA17946@ad.usersys.redhat.com> From: Peter Xu Message-ID: <56553DBA.1070504@redhat.com> Date: Wed, 25 Nov 2015 12:48:58 +0800 MIME-Version: 1.0 In-Reply-To: <20151125024631.GA17946@ad.usersys.redhat.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH REPOST 0/2] Add basic "detach" support for dump-guest-memory List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Fam Zheng , Eric Blake Cc: Andrew Jones , qemu-devel@nongnu.org, armbru@redhat.com, lcapitulino@redhat.com, pbonzini@redhat.com, Laszlo Ersek On 11/25/2015 10:46 AM, Fam Zheng wrote: > On Tue, 11/24 06:49, Eric Blake wrote: >> On 11/24/2015 04:37 AM, Fam Zheng wrote: >> >>>> I think the patch should be dropped, and periodic progress reports >>>> should be emitted from within the dump loops that do the heavy lifting. >>>> >>>> For the ELF format dumps, that loop appears to reside in dump_iterate() >>>> [dump.c]. >>>> >>>> For the compressed format dumps, the loop seems to live in >>>> write_dump_pages() [dump.c]. >>> >>> This is a good idea! >>> >>> What I'm not sure is where to report the progress. Can it be the monitor where >>> the dump-guest-memory command was issued? In other words, do we support raising >>> events before the previous command returns? If yes, can libvirt handle this >>> correctly? (But the worst case is using another channel to communicate the >>> progress, it is ad-hocery but it must be better than all the risk and effort to >>> enable multi-threaded dump.) >>> >>> Eric, Markus, have any idea with the progress reporting? >> >> I'm fairly certain we support raising events prior to completion of a >> synchronous command; what I'm not sure of is whether the event hits the >> wire right away or whether it piles up waiting for the next synchronous >> command completion. If the latter, then we need to rework it (since the >> whole point of this exercise is that we are trying to give progress of a >> long-running synchronous command that hasn't completed yet). > > So in that case we may want some "flush" operation of events. That sounds OK to > me. > >> But we >> only have the one monitor connection for libvirt - the only way to pass >> events through a second channel is to open a second monitor connection, >> but that feels wrong to make libvirt have to track two monitors. > > OK, that's a fair point, but FWIW I was thinking about adding an optional > argument: > > "*progress": "fd:dump-progress" > > into which dump.c talks in a mini-protocol, to send progress information. It's > just an crazily hacky idea, not anything I'm advocating. If query status is necessary, what about adding one command: "query-dump"? Which could be a simplified version of "query-migration": 1. before first dump: -> { "execute": "query-dump" } <- { "return": {} } 2. one background dump in progress: -> { "execute": "query-dump" } <- { "return":{ "status":"active", "percentage": {0..99}, } } 3. after first dump, and not running background dump (substraction of case 1 and 2) -> { "execute": "query-dump" } <- { "return": { "status": "completed|failed", } } All these would be based on the fact that this patch might not be dropped though. :) Thanks. Peter > > Thanks, > > Fam >