On Thu, Oct 30, 2025 at 02:08:06PM +0100, Juraj Marcin wrote:
> On 2025-10-29 15:53, Peter Xu wrote:
> > On Mon, Oct 27, 2025 at 04:41:09PM +0100, Juraj Marcin wrote:
> > > From: Juraj Marcin <[email protected]>
> > > 
> > > This patch addresses a TODO about moving postcopy_ram_listen_thread() to
> > > postcopy file. Furthermore, this patch adds a pair of functions,
> > > postcopy_incoming_setup() and postcopy_incoming_cleanup(), which sets up
> > > and cleans the postcopy_ram_incoming state and the listen thread.
> > 
> > It would be great to separate code movements and changes.
> 
> I wanted to get around the need to expose the postcopy listen thread
> function in a header file, hence the postcopy_incoming_setup() function,
> adding postcopy_incoming_cleanup() together then seemed natural to me.
> 
> However, I could split it like this:
> 
> 1. Move postcopy_ram_listen_thread() to postcopy-ram.c and add a simple
>    wrapper for postcopy_thread_create() (something like
>    postcopy_ram_listen_thread_create).
> 2. Rename postcopy_ram_listen_thread_create to postcopy_incoming_setup
>    and move rest of loadvm_postcopy_handle_listen, that is moved by this
>    patch, and lastly introduce postcopy_ram_incoming_cleanup().

I'm not sure I fully get what you described, but it's okay, I'll read what
you'll post. :)

The idea here is when there's major movement of codes, do it in one patch
(or a few) only for the movement but nothing else.  Then try to do anything
on top, either changing existing code or even adding some wrappers.
Normally that'll make review / backport / ... all easier.

It'll be fine if we need to add some entries to headers then remove them
later on.

-- 
Peter Xu


Reply via email to