We are going to implement automatic trace points for qmp commands.
These several trace points are in conflict with upcoming ones. So, drop
them now.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 blockdev.c         | 8 --------
 job-qmp.c          | 6 ------
 block/trace-events | 9 ---------
 trace-events       | 8 --------
 4 files changed, 31 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 0eb2823b1b..10961d81a4 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2586,8 +2586,6 @@ void qmp_block_stream(bool has_job_id, const char *job_id, const char *device,
         goto out;
     }
 
-    trace_qmp_block_stream(bs);
-
 out:
     aio_context_release(aio_context);
 }
@@ -3354,7 +3352,6 @@ void qmp_block_job_cancel(const char *device,
         goto out;
     }
 
-    trace_qmp_block_job_cancel(job);
     job_user_cancel(&job->job, force, errp);
 out:
     aio_context_release(aio_context);
@@ -3369,7 +3366,6 @@ void qmp_block_job_pause(const char *device, Error **errp)
         return;
     }
 
-    trace_qmp_block_job_pause(job);
     job_user_pause(&job->job, errp);
     aio_context_release(aio_context);
 }
@@ -3383,7 +3379,6 @@ void qmp_block_job_resume(const char *device, Error **errp)
         return;
     }
 
-    trace_qmp_block_job_resume(job);
     job_user_resume(&job->job, errp);
     aio_context_release(aio_context);
 }
@@ -3397,7 +3392,6 @@ void qmp_block_job_complete(const char *device, Error **errp)
         return;
     }
 
-    trace_qmp_block_job_complete(job);
     job_complete(&job->job, errp);
     aio_context_release(aio_context);
 }
@@ -3411,7 +3405,6 @@ void qmp_block_job_finalize(const char *id, Error **errp)
         return;
     }
 
-    trace_qmp_block_job_finalize(job);
     job_ref(&job->job);
     job_finalize(&job->job, errp);
 
@@ -3435,7 +3428,6 @@ void qmp_block_job_dismiss(const char *id, Error **errp)
         return;
     }
 
-    trace_qmp_block_job_dismiss(bjob);
     job = &bjob->job;
     job_dismiss(&job, errp);
     aio_context_release(aio_context);
diff --git a/job-qmp.c b/job-qmp.c
index 829a28aa70..cf0cb9d717 100644
--- a/job-qmp.c
+++ b/job-qmp.c
@@ -57,7 +57,6 @@ void qmp_job_cancel(const char *id, Error **errp)
         return;
     }
 
-    trace_qmp_job_cancel(job);
     job_user_cancel(job, true, errp);
     aio_context_release(aio_context);
 }
@@ -71,7 +70,6 @@ void qmp_job_pause(const char *id, Error **errp)
         return;
     }
 
-    trace_qmp_job_pause(job);
     job_user_pause(job, errp);
     aio_context_release(aio_context);
 }
@@ -85,7 +83,6 @@ void qmp_job_resume(const char *id, Error **errp)
         return;
     }
 
-    trace_qmp_job_resume(job);
     job_user_resume(job, errp);
     aio_context_release(aio_context);
 }
@@ -99,7 +96,6 @@ void qmp_job_complete(const char *id, Error **errp)
         return;
     }
 
-    trace_qmp_job_complete(job);
     job_complete(job, errp);
     aio_context_release(aio_context);
 }
@@ -113,7 +109,6 @@ void qmp_job_finalize(const char *id, Error **errp)
         return;
     }
 
-    trace_qmp_job_finalize(job);
     job_ref(job);
     job_finalize(job, errp);
 
@@ -136,7 +131,6 @@ void qmp_job_dismiss(const char *id, Error **errp)
         return;
     }
 
-    trace_qmp_job_dismiss(job);
     job_dismiss(&job, errp);
     aio_context_release(aio_context);
 }
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"
-
 
 ### Guest events, keep at bottom
 
-- 
2.31.1

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

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.

--js

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