Re: [Qemu-devel] [PATCH RFC 2/6] migration: isolate return path on src

[Juan Quintela <quintela@redhat.com>]

Peter Xu <peterx@redhat.com> wrote:
> There are some places that binded "return path" with postcopy. Let's be
> prepared for its usage even without postcopy. This patch mainly did this
> on source side.
>
> This has no functional change. But it'll simplify further patches.
>
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
>  migration/migration.c  | 50 +++++++++++++++++++++++++++++++++++---------------
>  migration/trace-events |  4 ++--
>  2 files changed, 37 insertions(+), 17 deletions(-)




>  
> +/* Return true if success, otherwise false. */
> +static bool migrate_return_path_create(MigrationState *s)
> +{
> +    /* Whether we should enable return path */
> +    bool enable_return_path = false;
> +    /* Whether we should force its success */
> +    bool force_return_path = false;
> +
> +    if (migrate_postcopy_ram()) {
> +        enable_return_path = true;
> +        force_return_path = true;
> +    }
> +
> +    if (enable_return_path) {
> +        if (open_return_path_on_source(s) && force_return_path) {
> +            error_report("Unable to open return-path");
> +            return false;
> +        }
> +    }
> +
> +    return true;
> +}
> +

what about this 

static bool migrate_return_patch_create(MigrationState *s)
{
    if (open_return_path_on_source(s)) {
        error_report("Unable to open return-path");
        return false;
    }

    return true;
}

>  void migrate_fd_connect(MigrationState *s)
>  {
>      s->expected_downtime = s->parameters.downtime_limit;
> @@ -2057,17 +2081,13 @@ void migrate_fd_connect(MigrationState *s)
>      notifier_list_notify(&migration_state_notifiers, s);
>  
>      /*
> -     * Open the return path; currently for postcopy but other things might
> -     * also want it.
> +     * Open the return path.
>       */
> -    if (migrate_postcopy_ram()) {
> -        if (open_return_path_on_source(s)) {
> -            error_report("Unable to open return-path for postcopy");
> -            migrate_set_state(&s->state, MIGRATION_STATUS_SETUP,
> -                              MIGRATION_STATUS_FAILED);
> -            migrate_fd_cleanup(s);
> -            return;
> -        }
> +    if (!migrate_return_path_create(s)) {
> +        migrate_set_state(&s->state, MIGRATION_STATUS_SETUP,
> +                          MIGRATION_STATUS_FAILED);
> +        migrate_fd_cleanup(s);
> +        return;
>      }


And this?

>      /*
> -     * Open the return path; currently for postcopy but other things might
> -     * also want it.
> +     * Open the return path.
>       */
> -    if (migrate_postcopy_ram()) {
> -        if (open_return_path_on_source(s)) {
> -            error_report("Unable to open return-path for postcopy");
> -            migrate_set_state(&s->state, MIGRATION_STATUS_SETUP,
> -                              MIGRATION_STATUS_FAILED);
> -            migrate_fd_cleanup(s);
> -            return;
> -        }
> +    if (!migrate_return_path_create(s)) {
> +        migrate_set_state(&s->state, MIGRATION_STATUS_SETUP,
> +                          MIGRATION_STATUS_FAILED);
> +        migrate_fd_cleanup(s);
> +        return;
>      }

    /*
     * Open the return path
     */

    if (migrate_postcopy_ram()) {
        if (!migrate_return_path_create(s)) {
            migrate_set_state(&s->state, MIGRATION_STATUS_SETUP,
                              MIGRATION_STATUS_FAILED);
            migrate_fd_cleanup(s);
            return;
        }
    }

Two less booleans and same behaviour.  It is also shorter, but that was
not the idea.

Later, Juan.