[PATCH v4 00/25] migration: Cleanup early connection code

Fabiano Rosas posted 25 patches 1 day, 18 hours ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20260123141656.6765-1-farosas@suse.de
Maintainers: Peter Xu <peterx@redhat.com>, Fabiano Rosas <farosas@suse.de>, Mark Kanda <mark.kanda@oracle.com>, Ben Chaney <bchaney@akamai.com>, Li Zhijian <lizhijian@fujitsu.com>
include/migration/cpr.h  |   5 +
migration/channel.c      | 374 ++++++++++++++++++++++----
migration/channel.h      |  27 +-
migration/cpr-exec.c     |   2 +-
migration/cpr-transfer.c |  23 ++
migration/exec.c         |  11 +-
migration/exec.h         |   8 +-
migration/fd.c           |  15 +-
migration/fd.h           |   9 +-
migration/file.c         |  20 +-
migration/file.h         |   7 +-
migration/migration.c    | 562 ++++++++++-----------------------------
migration/migration.h    |  15 +-
migration/multifd.c      |  17 +-
migration/multifd.h      |   2 +-
migration/options.c      |   5 +
migration/postcopy-ram.c |   2 +-
migration/rdma.c         |  46 ++--
migration/rdma.h         |   6 +-
migration/socket.c       |  30 +--
migration/socket.h       |   6 +-
migration/tls.c          |  33 +--
migration/tls.h          |   9 +-
migration/trace-events   |  20 +-
24 files changed, 642 insertions(+), 612 deletions(-)
[PATCH v4 00/25] migration: Cleanup early connection code
Posted by Fabiano Rosas 1 day, 18 hours ago
Rebased on master and adjusted a few commit messages. Also a minor
change patch 20.

CI run: https://gitlab.com/farosas/qemu/-/pipelines/2274889063

v3:
https://lore.kernel.org/r/20260109124043.25019-1-farosas@suse.de

v2:
https://lore.kernel.org/r/20260105190644.14072-1-farosas@suse.de

Removed some extra changes that were not adding much to the series,
left multifd_recv_setup() call where it was; stopped merging the
connection code at the end of the series.

Added further cleanup to CPR code in qmp_migrate and moved it to
cpr-transfer.c.

v1 (rfc):
https://lore.kernel.org/r/20251226211930.27565-1-farosas@suse.de

Address some of the issues that make the early connection code a bit
too idiosyncratic. By "early connection" I mean from
qmp_migrate[_incoming] until the start of the migration
thread|coroutine.

(IOW, the whole dance of going into socket code, starting an async
routine, calling back to migration code, checking TLS, going back
again, coming back once more, etc. All while passing an error_in and
hostname string that eventually gets (maybe) ignored in tls code,
along with some is_resume checks along the way)

This series is mostly inspired by the work Markus and Peter did
recently in organizing some of the error handling code. The new
migration_connect_error_propagate() function seems like a good place
to centralize the error handling and call migration_cleanup() during
this early connection phase when everything is still fairly
linear. (apologies if I'm dirtying your design =)

Aside from the initial patches that are a bit disruptive, most of the
series is just refactoring to make the code easier to navigate, names
more consistent and some general cleanups.

- patches 1-8:

General cleanups, could be applied standalone, although they are
prerequisites for the rest of the series.

- patches 9-12:

Changes to allow calling migration_cleanup() from
migration_connect_error_propagate().

The idea here is to make sure error propagation and cleanup happen
when the error is detected, without calling into non-error-path
functions.

This is the more risky change because it will cause cleanup to run in
places where it didn't before.

- patch 13 & 19:

The main change of this series, simplifying the
qmp-migrate --> migration_connect path. Stops calling the connection
functions when an error happens in the transport code, adds
clarification around which paths have asynchronous completion and
makes the synchronous path return to their caller to start the
migration instead of initiating it themselves.

- patches 14-18, 20-22:

Moves code out of migration.c and into channel.c. Now that the code is
more compartmentalized, move it to a more appropriate source file.

- patches 23-25:

BONUS CONTENT, wrap the uri/channels parsing and move it to channel.c
as well.

- future work?

I think we could move all QMP command functions to a QAPI-specific
file, but I don't see any standardization in the tree, there's
block/qapi.c, various instances of foo-qmp-cmds.c and many more just
laying along with the rest of the code. So I left this for another
moment.

CI run: https://gitlab.com/farosas/qemu/-/pipelines/2233810778

Fabiano Rosas (25):
  migration: Remove redundant state change
  migration: Fix state change at migration_channel_process_incoming
  migration/tls: Remove unused parameter
  migration: Cleanup TLS handshake hostname passing
  migration: Move postcopy_try_recover into migration_incoming_process
  migration: Use migrate_mode() to query for cpr-transfer
  migration: Free the error earlier in the resume case
  migration: Move error reporting out of migration_cleanup
  migration: Expand migration_connect_error_propagate to cover
    cancelling
  migration: yank: Move register instance earlier
  migration: Fold migration_cleanup() into
    migration_connect_error_propagate()
  migration: Handle error in the early async paths
  migration: Move setting of QEMUFile into
    migration_outgoing|incoming_setup
  migration/rdma: Use common connection paths
  migration: Start incoming from channel.c
  migration/channel: Rename migration_channel_connect
  migration: Rename instances of start
  migration: Move channel code to channel.c
  migration: Move transport connection code into channel.c
  migration: Move channel parsing to channel.c
  migration: Move URI parsing to channel.c
  migration: Free cpr-transfer MigrationAddress along with gsource
  migration: Move CPR HUP watch to cpr-transfer.c
  migration: Remove qmp_migrate_finish
  migration/channel: Centralize calling
    migration_channel_connect_outgoing

 include/migration/cpr.h  |   5 +
 migration/channel.c      | 374 ++++++++++++++++++++++----
 migration/channel.h      |  27 +-
 migration/cpr-exec.c     |   2 +-
 migration/cpr-transfer.c |  23 ++
 migration/exec.c         |  11 +-
 migration/exec.h         |   8 +-
 migration/fd.c           |  15 +-
 migration/fd.h           |   9 +-
 migration/file.c         |  20 +-
 migration/file.h         |   7 +-
 migration/migration.c    | 562 ++++++++++-----------------------------
 migration/migration.h    |  15 +-
 migration/multifd.c      |  17 +-
 migration/multifd.h      |   2 +-
 migration/options.c      |   5 +
 migration/postcopy-ram.c |   2 +-
 migration/rdma.c         |  46 ++--
 migration/rdma.h         |   6 +-
 migration/socket.c       |  30 +--
 migration/socket.h       |   6 +-
 migration/tls.c          |  33 +--
 migration/tls.h          |   9 +-
 migration/trace-events   |  20 +-
 24 files changed, 642 insertions(+), 612 deletions(-)

-- 
2.51.0
Re: [PATCH v4 00/25] migration: Cleanup early connection code
Posted by Fabiano Rosas 1 day, 17 hours ago
Fabiano Rosas <farosas@suse.de> writes:

> Rebased on master and adjusted a few commit messages. Also a minor
> change patch 20.
>
> CI run: https://gitlab.com/farosas/qemu/-/pipelines/2274889063
>
> v3:
> https://lore.kernel.org/r/20260109124043.25019-1-farosas@suse.de
>
> v2:
> https://lore.kernel.org/r/20260105190644.14072-1-farosas@suse.de
>
> Removed some extra changes that were not adding much to the series,
> left multifd_recv_setup() call where it was; stopped merging the
> connection code at the end of the series.
>
> Added further cleanup to CPR code in qmp_migrate and moved it to
> cpr-transfer.c.
>
> v1 (rfc):
> https://lore.kernel.org/r/20251226211930.27565-1-farosas@suse.de
>
> Address some of the issues that make the early connection code a bit
> too idiosyncratic. By "early connection" I mean from
> qmp_migrate[_incoming] until the start of the migration
> thread|coroutine.
>
> (IOW, the whole dance of going into socket code, starting an async
> routine, calling back to migration code, checking TLS, going back
> again, coming back once more, etc. All while passing an error_in and
> hostname string that eventually gets (maybe) ignored in tls code,
> along with some is_resume checks along the way)
>
> This series is mostly inspired by the work Markus and Peter did
> recently in organizing some of the error handling code. The new
> migration_connect_error_propagate() function seems like a good place
> to centralize the error handling and call migration_cleanup() during
> this early connection phase when everything is still fairly
> linear. (apologies if I'm dirtying your design =)
>
> Aside from the initial patches that are a bit disruptive, most of the
> series is just refactoring to make the code easier to navigate, names
> more consistent and some general cleanups.
>
> - patches 1-8:
>
> General cleanups, could be applied standalone, although they are
> prerequisites for the rest of the series.
>
> - patches 9-12:
>
> Changes to allow calling migration_cleanup() from
> migration_connect_error_propagate().
>
> The idea here is to make sure error propagation and cleanup happen
> when the error is detected, without calling into non-error-path
> functions.
>
> This is the more risky change because it will cause cleanup to run in
> places where it didn't before.
>
> - patch 13 & 19:
>
> The main change of this series, simplifying the
> qmp-migrate --> migration_connect path. Stops calling the connection
> functions when an error happens in the transport code, adds
> clarification around which paths have asynchronous completion and
> makes the synchronous path return to their caller to start the
> migration instead of initiating it themselves.
>
> - patches 14-18, 20-22:
>
> Moves code out of migration.c and into channel.c. Now that the code is
> more compartmentalized, move it to a more appropriate source file.
>
> - patches 23-25:
>
> BONUS CONTENT, wrap the uri/channels parsing and move it to channel.c
> as well.
>
> - future work?
>
> I think we could move all QMP command functions to a QAPI-specific
> file, but I don't see any standardization in the tree, there's
> block/qapi.c, various instances of foo-qmp-cmds.c and many more just
> laying along with the rest of the code. So I left this for another
> moment.
>
> CI run: https://gitlab.com/farosas/qemu/-/pipelines/2233810778
>
> Fabiano Rosas (25):
>   migration: Remove redundant state change
>   migration: Fix state change at migration_channel_process_incoming
>   migration/tls: Remove unused parameter
>   migration: Cleanup TLS handshake hostname passing
>   migration: Move postcopy_try_recover into migration_incoming_process
>   migration: Use migrate_mode() to query for cpr-transfer
>   migration: Free the error earlier in the resume case
>   migration: Move error reporting out of migration_cleanup
>   migration: Expand migration_connect_error_propagate to cover
>     cancelling
>   migration: yank: Move register instance earlier
>   migration: Fold migration_cleanup() into
>     migration_connect_error_propagate()
>   migration: Handle error in the early async paths
>   migration: Move setting of QEMUFile into
>     migration_outgoing|incoming_setup
>   migration/rdma: Use common connection paths
>   migration: Start incoming from channel.c
>   migration/channel: Rename migration_channel_connect
>   migration: Rename instances of start
>   migration: Move channel code to channel.c
>   migration: Move transport connection code into channel.c
>   migration: Move channel parsing to channel.c
>   migration: Move URI parsing to channel.c
>   migration: Free cpr-transfer MigrationAddress along with gsource
>   migration: Move CPR HUP watch to cpr-transfer.c
>   migration: Remove qmp_migrate_finish
>   migration/channel: Centralize calling
>     migration_channel_connect_outgoing
>
>  include/migration/cpr.h  |   5 +
>  migration/channel.c      | 374 ++++++++++++++++++++++----
>  migration/channel.h      |  27 +-
>  migration/cpr-exec.c     |   2 +-
>  migration/cpr-transfer.c |  23 ++
>  migration/exec.c         |  11 +-
>  migration/exec.h         |   8 +-
>  migration/fd.c           |  15 +-
>  migration/fd.h           |   9 +-
>  migration/file.c         |  20 +-
>  migration/file.h         |   7 +-
>  migration/migration.c    | 562 ++++++++++-----------------------------
>  migration/migration.h    |  15 +-
>  migration/multifd.c      |  17 +-
>  migration/multifd.h      |   2 +-
>  migration/options.c      |   5 +
>  migration/postcopy-ram.c |   2 +-
>  migration/rdma.c         |  46 ++--
>  migration/rdma.h         |   6 +-
>  migration/socket.c       |  30 +--
>  migration/socket.h       |   6 +-
>  migration/tls.c          |  33 +--
>  migration/tls.h          |   9 +-
>  migration/trace-events   |  20 +-
>  24 files changed, 642 insertions(+), 612 deletions(-)

Queued, let me know if there's any objections.