From: Eric Blake <eblake@redhat.com>
To: John Snow <jsnow@redhat.com>, Kevin Wolf <kwolf@redhat.com>
Cc: vsementsov@virtuozzo.com, famz@redhat.com, qemu-block@nongnu.org,
jcody@redhat.com, qemu-devel@nongnu.org, stefanha@redhat.com,
Wen Congyang <wency@cn.fujitsu.com>
Subject: Re: [Qemu-devel] [PATCH v2 02/11] blockjob: centralize QMP event emissions
Date: Wed, 5 Oct 2016 14:24:05 -0500 [thread overview]
Message-ID: <c486244e-3d0c-c591-09ce-d7519f802d3c@redhat.com> (raw)
In-Reply-To: <0e9d9739-b32b-8cff-3e34-ccc8e971b4c3@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 2315 bytes --]
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?)
>
> 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.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
next prev parent reply other threads:[~2016-10-05 19:24 UTC|newest]
Thread overview: 51+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-09-30 22:00 [Qemu-devel] [PATCH v2 00/11] blockjobs: Fix transactional race condition John Snow
2016-09-30 22:00 ` [Qemu-devel] [PATCH v2 01/11] blockjob: fix dead pointer in txn list John Snow
2016-10-05 13:43 ` Kevin Wolf
2016-09-30 22:00 ` [Qemu-devel] [PATCH v2 02/11] blockjob: centralize QMP event emissions John Snow
2016-10-05 13:43 ` Kevin Wolf
2016-10-05 18:49 ` John Snow
2016-10-05 19:24 ` Eric Blake [this message]
2016-10-05 21:00 ` John Snow
2016-10-10 16:45 ` Kashyap Chamarthy
2016-10-10 18:36 ` John Snow
2016-10-10 19:28 ` Eric Blake
2016-10-11 13:32 ` Kashyap Chamarthy
2016-10-06 7:44 ` Kevin Wolf
2016-10-06 16:57 ` John Snow
2016-10-06 18:16 ` Eric Blake
2016-10-06 18:19 ` John Snow
2016-10-11 9:50 ` Markus Armbruster
2016-09-30 22:00 ` [Qemu-devel] [PATCH v2 03/11] Blockjobs: Internalize user_pause logic John Snow
2016-10-04 0:57 ` Jeff Cody
2016-10-04 2:46 ` John Snow
2016-10-04 18:35 ` John Snow
2016-09-30 22:00 ` [Qemu-devel] [PATCH v2 04/11] blockjobs: Always use block_job_get_aio_context John Snow
2016-10-05 14:02 ` Kevin Wolf
2016-10-06 20:22 ` John Snow
2016-10-07 7:49 ` Paolo Bonzini
2016-10-13 0:49 ` John Snow
2016-10-13 9:03 ` Paolo Bonzini
2016-09-30 22:00 ` [Qemu-devel] [PATCH v2 05/11] blockjobs: split interface into public/private John Snow
2016-10-05 14:17 ` Kevin Wolf
2016-10-05 16:20 ` John Snow
2016-09-30 22:00 ` [Qemu-devel] [PATCH v2 06/11] blockjobs: fix documentation John Snow
2016-10-05 15:03 ` Kevin Wolf
2016-10-05 16:22 ` John Snow
2016-09-30 22:00 ` [Qemu-devel] [PATCH v2 07/11] blockjob: add .clean property John Snow
2016-10-12 11:11 ` Vladimir Sementsov-Ogievskiy
2016-09-30 22:00 ` [Qemu-devel] [PATCH v2 08/11] blockjob: add .start field John Snow
2016-09-30 22:00 ` [Qemu-devel] [PATCH v2 09/11] blockjob: add block_job_start John Snow
2016-10-05 15:17 ` Kevin Wolf
2016-10-06 22:44 ` John Snow
2016-10-17 18:00 ` John Snow
2016-09-30 22:00 ` [Qemu-devel] [PATCH v2 10/11] blockjob: refactor backup_start as backup_job_create John Snow
2016-10-07 18:39 ` John Snow
2016-10-10 8:57 ` Kevin Wolf
2016-10-10 22:51 ` John Snow
2016-10-11 8:56 ` Paolo Bonzini
2016-10-11 9:35 ` Kevin Wolf
2016-10-17 8:59 ` Fam Zheng
2016-09-30 22:00 ` [Qemu-devel] [PATCH v2 11/11] iotests: add transactional failure race test John Snow
2016-10-12 11:26 ` Vladimir Sementsov-Ogievskiy
2016-10-12 16:09 ` John Snow
2016-09-30 22:22 ` [Qemu-devel] [PATCH v2 00/11] blockjobs: Fix transactional race condition no-reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=c486244e-3d0c-c591-09ce-d7519f802d3c@redhat.com \
--to=eblake@redhat.com \
--cc=famz@redhat.com \
--cc=jcody@redhat.com \
--cc=jsnow@redhat.com \
--cc=kwolf@redhat.com \
--cc=qemu-block@nongnu.org \
--cc=qemu-devel@nongnu.org \
--cc=stefanha@redhat.com \
--cc=vsementsov@virtuozzo.com \
--cc=wency@cn.fujitsu.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).