Re: [PATCH v2 1/4] jobs: drop qmp_ trace points

From: Stefan Hajnoczi <stefanha@redhat.com>
To: John Snow <jsnow@redhat.com>
Cc: Kevin Wolf <kwolf@redhat.com>,
	Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>,
	Qemu-block <qemu-block@nongnu.org>,
	Michael Roth <michael.roth@amd.com>,
	qemu-devel <qemu-devel@nongnu.org>,
	Markus Armbruster <armbru@redhat.com>,
	Hanna Reitz <hreitz@redhat.com>,
	Paolo Bonzini <pbonzini@redhat.com>,
	Philippe Mathieu Daude <philmd@redhat.com>
Subject: Re: [PATCH v2 1/4] jobs: drop qmp_ trace points
Date: Wed, 12 Jan 2022 10:45:20 +0000	[thread overview]
Message-ID: <Yd6xQLKerw5LE6FO@stefanha-x1.localdomain> (raw)
In-Reply-To: <CAFn=p-Zx5NatsY6qVrm6dQ2jh8=0030_7+-nKSkFhMgpOV=+Fg@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 2499 bytes --]

On Tue, Jan 11, 2022 at 06:44:58PM -0500, John Snow wrote:
> On Mon, Jan 10, 2022 at 11:06 AM Stefan Hajnoczi <stefanha@redhat.com> wrote:
> >
> > On Thu, Dec 23, 2021 at 12:07:53PM +0100, Vladimir Sementsov-Ogievskiy wrote:
> > > diff --git a/block/trace-events b/block/trace-events
> > > index 549090d453..5be3e3913b 100644
> > > --- a/block/trace-events
> > > +++ b/block/trace-events
> > > @@ -49,15 +49,6 @@ block_copy_read_fail(void *bcs, int64_t start, int ret) "bcs %p start %"PRId64"
> > >  block_copy_write_fail(void *bcs, int64_t start, int ret) "bcs %p start %"PRId64" ret %d"
> > >  block_copy_write_zeroes_fail(void *bcs, int64_t start, int ret) "bcs %p start %"PRId64" ret %d"
> > >
> > > -# ../blockdev.c
> > > -qmp_block_job_cancel(void *job) "job %p"
> > > -qmp_block_job_pause(void *job) "job %p"
> > > -qmp_block_job_resume(void *job) "job %p"
> > > -qmp_block_job_complete(void *job) "job %p"
> > > -qmp_block_job_finalize(void *job) "job %p"
> > > -qmp_block_job_dismiss(void *job) "job %p"
> > > -qmp_block_stream(void *bs) "bs %p"
> > > -
> > >  # file-win32.c
> > >  file_paio_submit(void *acb, void *opaque, int64_t offset, int count, int type) "acb %p opaque %p offset %"PRId64" count %d type %d"
> > >
> > > diff --git a/trace-events b/trace-events
> > > index a637a61eba..1265f1e0cc 100644
> > > --- a/trace-events
> > > +++ b/trace-events
> > > @@ -79,14 +79,6 @@ job_state_transition(void *job,  int ret, const char *legal, const char *s0, con
> > >  job_apply_verb(void *job, const char *state, const char *verb, const char *legal) "job %p in state %s; applying verb %s (%s)"
> > >  job_completed(void *job, int ret) "job %p ret %d"
> > >
> > > -# job-qmp.c
> > > -qmp_job_cancel(void *job) "job %p"
> > > -qmp_job_pause(void *job) "job %p"
> > > -qmp_job_resume(void *job) "job %p"
> > > -qmp_job_complete(void *job) "job %p"
> > > -qmp_job_finalize(void *job) "job %p"
> > > -qmp_job_dismiss(void *job) "job %p"
> >
> > The job pointer argument will be lost. That's not ideal but probably
> > worth getting trace events for all QMP commands.
> >
> > Stefan
> 
> We could move the six job-related tracepoints into the implementation
> routines instead; i.e. job_user_cancel, job_user_pause, etc. This
> would cover all 12 QMP interface tracepoints, and that'd let us keep
> the "implementation" trace points.

Good idea. Having the job pointer might be handy so it's worth
preserving these trace events.

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]