[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 2/3] migration: Simplify calling of await_return_path_close_o
From: |
Fabiano Rosas |
Subject: |
Re: [PATCH 2/3] migration: Simplify calling of await_return_path_close_on_source |
Date: |
Mon, 31 Jul 2023 09:42:27 -0300 |
Peter Xu <peterx@redhat.com> writes:
> On Fri, Jul 28, 2023 at 09:15:15AM -0300, Fabiano Rosas wrote:
>> We're about to reuse this function so move the 'rp_thread_created'
>> check into it and remove the redundant tracing and comment.
>>
>> Add a new tracepoint akin to what is already done at
>> migration_completion().
>>
>> Signed-off-by: Fabiano Rosas <farosas@suse.de>
>> ---
>> migration/migration.c | 21 +++++++--------------
>> migration/trace-events | 3 +--
>> 2 files changed, 8 insertions(+), 16 deletions(-)
>>
>> diff --git a/migration/migration.c b/migration/migration.c
>> index 051067f8c5..d6f4470265 100644
>> --- a/migration/migration.c
>> +++ b/migration/migration.c
>> @@ -2038,6 +2038,10 @@ static int open_return_path_on_source(MigrationState
>> *ms,
>> /* Returns 0 if the RP was ok, otherwise there was an error on the RP */
>> static int await_return_path_close_on_source(MigrationState *ms)
>> {
>> + if (!ms->rp_state.rp_thread_created) {
>> + return 0;
>> + }
>> +
>> /*
>> * If this is a normal exit then the destination will send a SHUT and
>> the
>> * rp_thread will exit, however if there's an error we need to cause
>> @@ -2350,20 +2354,9 @@ static void migration_completion(MigrationState *s)
>> goto fail;
>> }
>>
>> - /*
>> - * If rp was opened we must clean up the thread before
>> - * cleaning everything else up (since if there are no failures
>> - * it will wait for the destination to send it's status in
>> - * a SHUT command).
>> - */
>> - if (s->rp_state.rp_thread_created) {
>> - int rp_error;
>> - trace_migration_return_path_end_before();
>> - rp_error = await_return_path_close_on_source(s);
>> - trace_migration_return_path_end_after(rp_error);
>> - if (rp_error) {
>> - goto fail;
>> - }
>> + if (await_return_path_close_on_source(s)) {
>> + trace_migration_completion_rp_err();
>> + goto fail;
>> }
>>
>> if (qemu_file_get_error(s->to_dst_file)) {
>> diff --git a/migration/trace-events b/migration/trace-events
>> index 5259c1044b..33a69064ca 100644
>> --- a/migration/trace-events
>> +++ b/migration/trace-events
>> @@ -157,13 +157,12 @@ migrate_pending_estimate(uint64_t size, uint64_t pre,
>> uint64_t post) "estimate p
>> migrate_send_rp_message(int msg_type, uint16_t len) "%d: len %d"
>> migrate_send_rp_recv_bitmap(char *name, int64_t size) "block '%s' size
>> 0x%"PRIi64
>> migration_completion_file_err(void) ""
>> +migration_completion_rp_err(void) ""
>> migration_completion_vm_stop(int ret) "ret %d"
>> migration_completion_postcopy_end(void) ""
>> migration_completion_postcopy_end_after_complete(void) ""
>> migration_rate_limit_pre(int ms) "%d ms"
>> migration_rate_limit_post(int urgent) "urgent: %d"
>> -migration_return_path_end_before(void) ""
>> -migration_return_path_end_after(int rp_error) "%d"
>> migration_thread_after_loop(void) ""
>> migration_thread_file_err(void) ""
>> migration_thread_setup_complete(void) ""
>> --
>> 2.35.3
>>
>
> Just in case someone may still want to see the old
> trace_migration_return_path_end_before() tracepoint, maybe just move them
> all over?
They are made redundant by
trace_await_return_path_close_on_source_joining() and
trace_await_return_path_close_on_source_close() which already exist in
the function.