| Message ID | 20211223110756.699148-2-vsementsov@virtuozzo.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | trace qmp commands | expand |
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
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
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(-)