All of lore.kernel.org
 help / color / mirror / Atom feed
From: Peter Xu <peterx@redhat.com>
To: Juraj Marcin <jmarcin@redhat.com>
Cc: qemu-devel@nongnu.org,
	"Dr. David Alan Gilbert" <dave@treblig.org>,
	Jiri Denemark <jdenemar@redhat.com>,
	Fabiano Rosas <farosas@suse.de>
Subject: Re: [PATCH v2 2/4] migration: Move postcopy_ram_listen_thread() to postcopy-ram.c
Date: Thu, 30 Oct 2025 11:16:01 -0400	[thread overview]
Message-ID: <aQOBMemFJe4f-Nqx@x1.local> (raw)
In-Reply-To: <dqpskxkmn6zxhd7oeuj4mqpyl7vdhjrecimtldx4k4v6wijnl3@ynjeyazm4ai4>

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 <jmarcin@redhat.com>
> > > 
> > > 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	other threads:[~2025-10-30 15:17 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-10-27 15:41 [PATCH v2 0/4] migration: Introduce POSTCOPY_DEVICE state Juraj Marcin
2025-10-27 15:41 ` [PATCH v2 1/4] migration: Do not try to start VM if disk activation fails Juraj Marcin
2025-10-27 15:41 ` [PATCH v2 2/4] migration: Move postcopy_ram_listen_thread() to postcopy-ram.c Juraj Marcin
2025-10-29 19:53   ` Peter Xu
2025-10-30 13:08     ` Juraj Marcin
2025-10-30 15:16       ` Peter Xu [this message]
2025-10-27 15:41 ` [PATCH v2 3/4] migration: Refactor all incoming cleanup into migration_incoming_cleanup() Juraj Marcin
2025-10-29 22:02   ` Peter Xu
2025-10-27 15:41 ` [PATCH v2 4/4] migration: Introduce POSTCOPY_DEVICE state Juraj Marcin
2025-10-29 22:57   ` Peter Xu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aQOBMemFJe4f-Nqx@x1.local \
    --to=peterx@redhat.com \
    --cc=dave@treblig.org \
    --cc=farosas@suse.de \
    --cc=jdenemar@redhat.com \
    --cc=jmarcin@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.