On Fri, Jul 25, 2025 at 4:20 PM Arun Menon <[email protected]> wrote:

> The documentation of qemu_file_get_return_path() states that it can
> return NULL on failure. However, a review of the current implementation
> reveals that it is guaranteed that it will always succeed and will never
> return NULL.
>
> As a result, the NULL checks post calling the function become redundant.
> This commit updates the documentation for the function and removes all
> NULL checks throughout the migration code.
>
> Reviewed-by: Daniel P. Berrangé <[email protected]>
> Signed-off-by: Arun Menon <[email protected]>
>

Reviewed-by: Marc-André Lureau <[email protected]>


> ---
>  migration/colo.c      |  4 ----
>  migration/migration.c | 12 ++----------
>  migration/qemu-file.c |  1 -
>  migration/savevm.c    |  4 ----
>  4 files changed, 2 insertions(+), 19 deletions(-)
>
> diff --git a/migration/colo.c b/migration/colo.c
> index
> e0f713c837f5da25d67afbd02ceb6c54024ca3af..981bd4bf9ced8b45b4c5d494acae119a174ee974
> 100644
> --- a/migration/colo.c
> +++ b/migration/colo.c
> @@ -849,10 +849,6 @@ static void *colo_process_incoming_thread(void
> *opaque)
>      failover_init_state();
>
>      mis->to_src_file = qemu_file_get_return_path(mis->from_src_file);
> -    if (!mis->to_src_file) {
> -        error_report("COLO incoming thread: Open QEMUFile to_src_file
> failed");
> -        goto out;
> -    }
>      /*
>       * Note: the communication between Primary side and Secondary side
>       * should be sequential, we set the fd to unblocked in migration
> incoming
> diff --git a/migration/migration.c b/migration/migration.c
> index
> 10c216d25dec01f206eacad2edd24d21f00e614c..b3bccaeaee806abd01595863f6475057049b0688
> 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -2646,12 +2646,9 @@ out:
>      return NULL;
>  }
>
> -static int open_return_path_on_source(MigrationState *ms)
> +static void open_return_path_on_source(MigrationState *ms)
>  {
>      ms->rp_state.from_dst_file =
> qemu_file_get_return_path(ms->to_dst_file);
> -    if (!ms->rp_state.from_dst_file) {
> -        return -1;
> -    }
>
>      trace_open_return_path_on_source();
>
> @@ -2660,8 +2657,6 @@ static int open_return_path_on_source(MigrationState
> *ms)
>      ms->rp_state.rp_thread_created = true;
>
>      trace_open_return_path_on_source_continue();
> -
> -    return 0;
>  }
>
>  /* Return true if error detected, or false otherwise */
> @@ -4010,10 +4005,7 @@ void migration_connect(MigrationState *s, Error
> *error_in)
>       * QEMU uses the return path.
>       */
>      if (migrate_postcopy_ram() || migrate_return_path()) {
> -        if (open_return_path_on_source(s)) {
> -            error_setg(&local_err, "Unable to open return-path for
> postcopy");
> -            goto fail;
> -        }
> +        open_return_path_on_source(s);
>      }
>
>      /*
> diff --git a/migration/qemu-file.c b/migration/qemu-file.c
> index
> b6ac190034f777dbde0da1598483a892089d7538..f9ccee9a1091ecbd37e6b7d2081a4446442b544d
> 100644
> --- a/migration/qemu-file.c
> +++ b/migration/qemu-file.c
> @@ -125,7 +125,6 @@ static QEMUFile *qemu_file_new_impl(QIOChannel *ioc,
> bool is_writable)
>
>  /*
>   * Result: QEMUFile* for a 'return path' for comms in the opposite
> direction
> - *         NULL if not available
>   */
>  QEMUFile *qemu_file_get_return_path(QEMUFile *f)
>  {
> diff --git a/migration/savevm.c b/migration/savevm.c
> index
> 21899e6beee3d2661c9a694379039e82cefbee4f..d8f5f1966fda831899596173f20fbef25d78829d
> 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -2583,10 +2583,6 @@ static int loadvm_process_command(QEMUFile *f)
>              return 0;
>          }
>          mis->to_src_file = qemu_file_get_return_path(f);
> -        if (!mis->to_src_file) {
> -            error_report("CMD_OPEN_RETURN_PATH failed");
> -            return -1;
> -        }
>
>          /*
>           * Switchover ack is enabled but no device uses it, so send an
> ACK to
>
> --
> 2.50.0
>
>

Reply via email to