From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:33130) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1brtIu-0001Tk-FU for qemu-devel@nongnu.org; Wed, 05 Oct 2016 17:00:41 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1brtIs-0005h8-Fm for qemu-devel@nongnu.org; Wed, 05 Oct 2016 17:00:39 -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> From: John Snow Message-ID: <2a33be0f-1f42-dd3c-93c2-2306811ea757@redhat.com> Date: Wed, 5 Oct 2016 17:00:29 -0400 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; 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: Eric Blake , Kevin Wolf Cc: vsementsov@virtuozzo.com, famz@redhat.com, qemu-block@nongnu.org, jcody@redhat.com, qemu-devel@nongnu.org, stefanha@redhat.com, Wen Congyang On 10/05/2016 03:24 PM, Eric Blake wrote: > On 10/05/2016 01:49 PM, John Snow wrote: > >>> Here we have an additional caller in block/replication.c and qemu-img, >>> so the parameters must stay. For qemu-img, nothing changes. For >>> replication, the block job events are added as a side effect. >>> >>> Not sure if we want to emit such events for an internal block job, but >>> if we do want the change, it should be explicit. >>> >> >> 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. >> >> 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. >> >> 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.)