From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58798) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1btfR5-0005nn-Kv for qemu-devel@nongnu.org; Mon, 10 Oct 2016 14:36:28 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1btfR3-0003VB-Cg for qemu-devel@nongnu.org; Mon, 10 Oct 2016 14:36:26 -0400 References: <1475272849-19990-1-git-send-email-jsnow@redhat.com> <1475272849-19990-3-git-send-email-jsnow@redhat.com> <20161005134312.GA1657@noname.str.redhat.com> <0e9d9739-b32b-8cff-3e34-ccc8e971b4c3@redhat.com> <2a33be0f-1f42-dd3c-93c2-2306811ea757@redhat.com> <20161010164542.xze564cbscx4t2up@eukaryote> From: John Snow Message-ID: Date: Mon, 10 Oct 2016 14:36:17 -0400 MIME-Version: 1.0 In-Reply-To: <20161010164542.xze564cbscx4t2up@eukaryote> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v2 02/11] blockjob: centralize QMP event emissions List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kashyap Chamarthy Cc: Eric Blake , Kevin Wolf , vsementsov@virtuozzo.com, famz@redhat.com, qemu-block@nongnu.org, jcody@redhat.com, qemu-devel@nongnu.org, stefanha@redhat.com On 10/10/2016 12:45 PM, Kashyap Chamarthy wrote: > On Wed, Oct 05, 2016 at 05:00:29PM -0400, John Snow wrote: > > [Arbitrarily chiming here, and still catching up with the details of the > thread.] > >> On 10/05/2016 03:24 PM, Eric Blake wrote: >>> On 10/05/2016 01:49 PM, John Snow wrote: > > [...] > >>>> Hmm, do we want to make it so some jobs are invisible and others are >>>> not? Because as it stands right now, neither case is strictly true. We >>>> only emit cancelled/completed events if it was started via QMP, however >>>> we do emit events for error and ready regardless of who started the job. >>> >>> Libvirt tries to mirror any block job event it receives to upper layers. >>> But if it cannot figure out which upper-layer disk the event is >>> associated with, it just drops the event. So I think that from the >>> libvirt perspective, you are okay if if you always report job events, >>> even for internal jobs. (Do we have a quick and easy way to set up an >>> internal job event, to double-check if I can produce some sort of >>> libvirt scenario to trigger the event and see that it gets safely ignored?) >>> >> >> Not in a QEMU release yet, I think. > > If not from an official QEMU release, it'd still be useful to work out a > a way to reproduce what Eric asked even from upstream Git master. > I'll be honest that I don't know; this is related to Replication which I know reasonably little about overall. It got added in the 2.8 timeframe, so the behavior it currently exhibits is not a good or meaningful reference for maintaining compatibility. We can /change/ the behavior before releases with no love lost. >>>> That didn't seem particularly consistent to me; either all events should >>>> be controlled by the job layer itself or none of them should be. >>>> >>>> I opted for "all." >>>> >>>> For "internal" jobs that did not previously emit any events, is it not >>>> true that these jobs still appear in the block job list and are >>>> effectively public regardless? I'd argue that these messages may be of >>>> value for management utilities who are still blocked by these jobs >>>> whether or not they are 'internal' or not. > > It'd certainly be useful durign debugging (for the said management > utilities), if it's possible to distinguish an event that was triggerred > by an internal block job vs. an event emitted by a job explicitly > triggerred by a user action. > Or, what if you just didn't get events for internal jobs? Are events for un-managed jobs useful in any sense beyond understanding the stateful availability of the drive to participate in a new job? > For example, OpenStack Nova calls libvirt API virDomainBlockRebase(), > which internally calls QMP `drive-mirror` that emits events. An "event > origin classification" system, if were to exist, allows one to pay > attention to only those events that are emitted due to an explicit > action and ignore all the rest ('internal'). > > But I'm not quite sure if it's desirable to have this event > classification for cleanliness reasons as Eric points out below. > >>>> I'll push for keeping it mandatory and explicit. If it becomes a >>>> problem, we can always add a 'silent' job property that silences ALL qmp >>>> events, including all completion, error, and ready notices. >>> >>> Completely silencing an internal job seems a little cleaner than having >>> events for the job but not being able to query it. But if nothing >>> breaks by exposing the internal jobs, that seems even easier than trying >>> to decide which jobs are internal and hidden vs. user-requested and public. >>> >> >> Well, at the moment anything requested directly via blockdev.c is "public." >> Before 2.8, all jobs were public ones, with the exception of those in >> qemu-img which is a bit of a different/special case. >> >> We have this block/replication.c beast now, though, and it uses backup_start >> and commit_active_start as it sees fit without direct user intervention. >> >> As it stands, I believe the jobs that replication creates are user-visible >> via query, will not issue cancellation or completion events, but WILL emit >> error events. It may emit ready events for the mirror job it uses, but I >> haven't traced that. (It could, at least.) > > Thanks, the above is useful to know. >